Page MenuHomeFreeBSD

Mk/Uses/gnome.mk: Switch to graphics/librsvg2-rust on tier 1 archs
ClosedPublic

Authored by tcberner on Jan 17 2019, 6:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 22, 2:08 AM
Unknown Object (File)
Sat, Dec 21, 6:08 AM
Unknown Object (File)
Sat, Dec 21, 3:22 AM
Unknown Object (File)
Thu, Dec 5, 4:39 AM
Unknown Object (File)
Thu, Dec 5, 4:37 AM
Unknown Object (File)
Thu, Dec 5, 4:31 AM
Unknown Object (File)
Thu, Dec 5, 4:27 AM
Unknown Object (File)
Wed, Dec 4, 3:04 AM

Details

Summary

graphics/librsvg2 is stuck on the 2.40 branch due to newer versions requiring a Rust toolchain which we do not have on all tier 2 archs. I packaged a newer librsvg2 version as graphics/librsvg2-rust. It is now on a stable version 2.46.0 which was just released [1]. Let's switch to it on tier 1 archs. OpenBSD has been doing something similar for over a year now.

2.46.0 is ABI compatible with 2.40. However newer APIs are only implemented in newer librsvg2 versions and I suspect it is only a matter of time until someone runs into trouble porting some applications.

[1] https://download.gnome.org/sources/librsvg/2.46/librsvg-2.46.0.news

Test Plan

exp-run

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 26463
Build 24889: arc lint + arc unit

Event Timeline

Wouldn't it be better to have a USE_GNOME=librsvg2 that would do the right thing depending on the architecture, with some obscure knob that one could switch if they really want the non rust version on rust supported architectures.

In D18878#404051, @mat wrote:

Wouldn't it be better to have a USE_GNOME=librsvg2 that would do the right thing depending on the architecture, with some obscure knob that one could switch if they really want the non rust version on rust supported architectures.

I have no way of knowing if switching all ports to USE_GNOME=librsvg2 instead of having librsvg2 in LIB_DEPENDS is the right thing to do. The way USE_GNOME=librsvg2 it's setup, it adds a couple of extra direct dependencies to those. Probably it would be fine, but I opted to do the dumb, safe thing here first.

As for LIBRSVG2_DEFAULT instead of an obscure knob: Why invent some non-standard knob when we have DEFAULT_VERSIONS? I though the ports framework is reducing its use of obscure knobs or similar not inventing new ones. Switching the default based on the architecture happens with LINUX_DEFAULT already. LINUX_DEFAULT is also only ever used in one place in USES=linux AFAICT.

First, USE_GNOME is standard, already exists, and is not obscure. It says "I need these GNOME components".

Then, of course, USE_GNOME=librsvg2 would need to add the correct dependencies wether it uses the legacy version or the new rust version, so this is not really an argument.

Adding a LIBRSVG2_DEFAULT means that both versions are interchangeable, like php, perl, python... If you do not know if graphics/librsvg2-rust works with the ports currently depending on graphics/librsvg2, then, well, I don't understand why you want to provide a knob that changes it globally, as it will possibly break everything.

First, USE_GNOME is standard, already exists, and is not obscure. It says "I need these GNOME components".

What did you mean by the "obscure knob that one could switch if they really want the non rust version on rust supported architectures" you mentioned previously then?

In the first paragraph of my previous comment I'm only explaining why I did not switch all consumers to USE_GNOME=librsvg2 yet.

Then, of course, USE_GNOME=librsvg2 would need to add the correct dependencies wether it uses the legacy version or the new rust version, so this is not really an argument.

And how would it switch between the legacy and Rust version? You previously mentioned introducing an obscure knob for this, but something like LIBRSVG2_DEFAULT is IMHO a lot better.

Adding a LIBRSVG2_DEFAULT means that both versions are interchangeable, like php, perl, python... If you do not know if graphics/librsvg2-rust works with the ports currently depending on graphics/librsvg2, then, well, I don't understand why you want to provide a knob that changes it globally, as it will possibly break everything.

Since when are the different PHP, Perl, Python, SSL defaults completely interchangeable?

librsvg2-rust should be API and even ABI compatible [1] with librsvg2, so yes they are interchangeable. Even more so than LibreSSL vs. OpenSSL.

I'm not changing the default from graphics/librsvg2 at all here, so this breaks nothing. This is about making it possible to use graphics/librsvg2-rust if users want to opt into it and to make testing easier and nothing else. As mentioned in the summary.

[1] https://abi-laboratory.pro/?view=timeline&l=librsvg

Ok, so if the new is ABI compatible, why not simply add a USE_GNOME=librsvg2 and have gnome.mk choose which one to use depending on the arch? I only talked about an obscure knob because I did not know they were compatible. Is there a reason someone would want to use the legacy version and not the new one?

(I'm not talking about people who complain that jdk has a build depends on cups, or who are still backporting things to work with 8.4, I'm talking people we actually want to listen to.)

I am not against a LIBRSVG2_DEFAULT, I am just saying that the default choice should be done depending on the architecture, and wondering why someone would want to switch back.

Reclaim after rP511743. librsvg2-rust is now at 2.46.0 (a stable version).

  • Adopt approach suggested by @mat: Hook it up in USES=gnome only and use librsvg2-rust on tier 1 archs
  • Add :build and :run support for the librsvg2 component in USES=gnome. A couple of ports only need it at build or runtime.
  • Switch all consumers to USE_GNOME=librsvg2
  • Add librsvg2-rust to qa.sh
tobik retitled this revision from Mk/bsd.default-versions.mk: Add LIBRSVG2_DEFAULT to Mk/Uses/gnome.mk: Switch to graphics/librsvg2-rust on tier 1 archs.Sep 10 2019, 7:48 AM
tobik edited the summary of this revision. (Show Details)
tobik edited the test plan for this revision. (Show Details)
  • Fix qa.sh integration
  • Use run dependency only in deskutils/shutter like before
  • Readd librsvg dep to www/midori
  • Extend to all archs that lang/rust supports

it is only a matter of time until someone runs into trouble porting some applications.

Nowadays affects LIBRSVG2 option in x11/lavalauncher. I wonder if it's possible to work around using the old API.

Run-time dependency librsvg-2.0 found: NO
meson.build:39:0: ERROR: Invalid version of dependency, need 'librsvg-2.0' ['>= 2.45.6'] found '2.40.21'.
[...]
../src/types/image.c:205:3: error: use of undeclared identifier 'RsvgLength'
                RsvgLength rsvg_width, rsvg_height;
                ^
../src/types/image.c:206:3: error: use of undeclared identifier 'RsvgRectangle'
                RsvgRectangle viewbox;
                ^
../src/types/image.c:207:3: error: implicit declaration of function 'rsvg_handle_get_intrinsic_dimensions' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                rsvg_handle_get_intrinsic_dimensions(image->rsvg_handle,
                ^
../src/types/image.c:207:3: note: did you mean 'rsvg_handle_get_dimensions'?
/usr/local/include/librsvg-2.0/librsvg/rsvg.h:146:6: note: 'rsvg_handle_get_dimensions' declared here
void rsvg_handle_get_dimensions (RsvgHandle * handle, RsvgDimensionData * dimension_data);
     ^
../src/types/image.c:208:18: error: use of undeclared identifier 'rsvg_width'
                                &has_width, &rsvg_width, &has_height, &rsvg_height,
                                             ^
../src/types/image.c:208:44: error: use of undeclared identifier 'rsvg_height'
                                &has_width, &rsvg_width, &has_height, &rsvg_height,
                                                                       ^
../src/types/image.c:209:20: error: use of undeclared identifier 'viewbox'
                                &has_viewbox, &viewbox);
                                               ^
../src/types/image.c:212:13: error: use of undeclared identifier 'rsvg_width'
                else if ( rsvg_width.length == 0 || rsvg_height.length == 0 )
                          ^
../src/types/image.c:212:39: error: use of undeclared identifier 'rsvg_height'
                else if ( rsvg_width.length == 0 || rsvg_height.length == 0 )
                                                    ^
../src/types/image.c:214:13: error: use of undeclared identifier 'rsvg_width'
                else if ( rsvg_width.unit != RSVG_UNIT_PX || rsvg_height.unit != RSVG_UNIT_PX )
                          ^
../src/types/image.c:214:32: error: use of undeclared identifier 'RSVG_UNIT_PX'
                else if ( rsvg_width.unit != RSVG_UNIT_PX || rsvg_height.unit != RSVG_UNIT_PX )
                                             ^
../src/types/image.c:214:48: error: use of undeclared identifier 'rsvg_height'
                else if ( rsvg_width.unit != RSVG_UNIT_PX || rsvg_height.unit != RSVG_UNIT_PX )
                                                             ^
../src/types/image.c:214:68: error: use of undeclared identifier 'RSVG_UNIT_PX'
                else if ( rsvg_width.unit != RSVG_UNIT_PX || rsvg_height.unit != RSVG_UNIT_PX )
                                                                                 ^
../src/types/image.c:218:38: error: use of undeclared identifier 'rsvg_width'
                        cairo_scale(cairo, (float)width / rsvg_width.length,
                                                          ^
../src/types/image.c:219:21: error: use of undeclared identifier 'rsvg_height'
                                        (float)width / rsvg_height.length);
                                                       ^
14 errors generated.

This should get an exp-run followed by a commit?

tcberner added a reviewer: tobik.
  • librsvg2: add powerpc64le to the list (submitted by: mikael)
This revision was not accepted when it landed; it landed in state Needs Review.Nov 9 2020, 5:08 PM
This revision was automatically updated to reflect the committed changes.