Message ID | 2195d673159785d305502677d786ae9bcd69ac7c.1661508240.git.tobias.kortkamp@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [bug#57430] gnu: wayland-protocols: Fix cross-compilation | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git-branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
On 26-08-2022 12:14, Tobias Kortkamp wrote: > * gnu/packages/freedesktop.scm (wayland-protocols): Fix cross-compilation > [native-inputs]: Add pkg-config-for-build and wayland. According to 'guix gc --references $(guix build wayland-protocols))' and it only having a 'share' subdirectory, this appears pure, architecture-independent, data. As such, here's a proposal for a simpler solution: do: ;; Pure architecture-independent data, cross-compilation is meaningless. ;; Resolve a cross-compilation failure and save some disk space and compilation ;; time by always compiling natively. As an additional benefit, this avoids some ;; worrying about whether placing this package should be placed in 'inputs' or in ;; 'native-inputs', it can now be placed wherever makes the build succeed. (arguments (list #:target #false)) This appears related to https://issues.guix.gnu.org/50627, so putting the people there in CC. I'm wondering, is whatever the issue addressed by 50627 now addressed by this patch? Greetings, Maxime.
Maxime Devos <maximedevos@telenet.be> writes: > [[PGP Signed Part:Undecided]] > On 26-08-2022 12:14, Tobias Kortkamp wrote: > >> * gnu/packages/freedesktop.scm (wayland-protocols): Fix >> cross-compilation >> [native-inputs]: Add pkg-config-for-build and wayland. > > According to 'guix gc --references $(guix build > wayland-protocols))' > and it only having a 'share' subdirectory, this appears pure, > architecture-independent, data. > > As such, here's a proposal for a simpler solution: do: > > ;; Pure architecture-independent data, cross-compilation is > meaningless. > ;; Resolve a cross-compilation failure and save some disk space > and > compilation > ;; time by always compiling natively. As an additional benefit, > this > avoids some > ;; worrying about whether placing this package should be placed > in > 'inputs' or in > ;; 'native-inputs', it can now be placed wherever makes the > build succeed. > > (arguments (list #:target #false)) > > This appears related to https://issues.guix.gnu.org/50627, so > putting > the people there in CC. > > I'm wondering, is whatever the issue addressed by 50627 now > addressed > by this patch? Purpose of 50627 was to reduce dependency on wayland-protocols from other packages. As far as I understand, this one is to reduce dependencies of wayland-protocols itself. As far as I know, there is no binary output of wayland-protocols, and wayland maybe needed as dependency for testing purposes only. IMHO these tests are targeted for developers producing protocol specifications. Once protocol specification is ready wayland-protocols is released. So running tests on wayland-protocols should be pointless waste of resources, as they don't prove that anything useful, instead dependents should test themselves. If testing causing waste of space and resources I would turn them off or probably use copy-build-system even. > Greetings, > Maxime. > > [2. OpenPGP public key --- application/pgp-keys; > OpenPGP_0x49E3EE22191725EE.asc]... > > [[End of PGP Signed Part]]
On 26-08-2022 18:59, muradm wrote: > As far as I understand, this one is to reduce dependencies of > wayland-protocols itself. This one keeps the dependencies the same (except for pkg-config / pkg-config-for-build) -- it just sorts the dependencies differently. > As far as I know, there is no binary output of wayland-protocols, 'ls -R "$(guix build wayland-protocols)"'? Maybe not 'binary', but it still appears required by some dependents. > and wayland maybe needed as dependency for testing purposes only. This appears confirmed by meson.build. In that case, wayland can be removed from 'inputs' and put in 'native-inputs' unconditionally. Tobias, does unconditionally moving wayland from 'inputs' to 'native-inputs' (and unconditionally using pkg-config-for-build) work? Potential problem: lots of dependents according to "guix refresh -l", making it unconditional would need to be done on core-updates or staging. > IMHO these tests are targeted for developers producing protocol > specifications. Once protocol specification is ready > wayland-protocols is released. So running tests on > wayland-protocols should be pointless waste of resources, as they > don't prove that anything useful, instead dependents should > test themselves. If testing causing waste of space and resources > I would turn them off or probably use copy-build-system even. #:tests? #false should suffice, no need to switch to copy-build-system. I see the argument for a waste of CPU time/energy resources, but not for a waste of space. Also, it is a dangerous assumption to make that developers run tests prior to a release. It seems pretty logical to me to do that, but sometimes errors are encountered in Guix and discussions happen upstream that indicate that this somehow didn't happen. Even if developers are known to always run tests, without fail, they might run tests in a different environment. For example, until recent-ish, GCC did -fcommon as a default, but later switched to -fcommon, breaking some builds. Also, 'developers' is not necessarily 'upstream developers'. Sometimes Guix adds a patch to package definitions, in combinations that upstream might not have tested, in that case running tests is important. Additionally, sometimes I find it more practical to write a patch, add it to the package definition and do a "guix build", than to do whatever's the meson equivalent of 'make && make check'. Greetings, Maxime.
Am Freitag, dem 26.08.2022 um 20:58 +0200 schrieb Maxime Devos: > For example, until recent-ish, GCC did -fcommon as a default, but > later switched to -fcommon, breaking some builds. You probably typo'd there. > Additionally, sometimes I find it more practical to write a patch, > add it to the package definition and do a "guix build", than to do > whatever's the meson equivalent of 'make && make check'. This is good advice for everyone regardless of build system, even if you do also run make && make check manually. guix build addresses several other issues, particular in the graphics toolkit world :)
On 26-08-2022 21:22, Liliana Marie Prikler wrote: > Am Freitag, dem 26.08.2022 um 20:58 +0200 schrieb Maxime Devos: >> For example, until recent-ish, GCC did -fcommon as a default, but >> later switched to -fcommon, breaking some builds. > You probably typo'd there. Yes, -fcommon and -fno-common if I have my GCC flags right. Greetings, Maxime
On Fri, Aug 26, 2022 at 08:58:51PM +0200, Maxime Devos wrote: > Tobias, does unconditionally moving wayland from 'inputs' to 'native-inputs' > (and unconditionally using pkg-config-for-build) work? Potential problem: > lots of dependents according to "guix refresh -l", making it unconditional > would need to be done on core-updates or staging. You mean change the patch like this? - (inputs - (list wayland)) - (native-inputs (cons* pkg-config python - (if (%current-target-system) - (list pkg-config-for-build - wayland) ; for wayland-scanner - '()))) + (native-inputs (list pkg-config pkg-config-for-build wayland python)) No, it doesn't work: Run-time dependency wayland-client found: NO (tried pkgconfig) ../wayland-protocols-1.23/tests/meson.build:4:0: ERROR: Dependency "wayland-client" not found, tried pkgconfig What would work is: + (arguments `(#:configure-flags (list "-Dtests=false"))) + (native-inputs (list pkg-config-for-build wayland)) But this turns off the tests. Even then it still looks for wayland-scanner for some reason.
On 27-08-2022 12:40, Tobias Kortkamp wrote: > On Fri, Aug 26, 2022 at 08:58:51PM +0200, Maxime Devos wrote: >> Tobias, does unconditionally moving wayland from 'inputs' to 'native-inputs' >> (and unconditionally using pkg-config-for-build) work? Potential problem: >> lots of dependents according to "guix refresh -l", making it unconditional >> would need to be done on core-updates or staging. > You mean change the patch like this? > > - (inputs > - (list wayland)) > - (native-inputs (cons* pkg-config python > - (if (%current-target-system) > - (list pkg-config-for-build > - wayland) ; for wayland-scanner > - '()))) > + (native-inputs (list pkg-config pkg-config-for-build wayland python)) > > No, it doesn't work: > > Run-time dependency wayland-client found: NO (tried pkgconfig) > > ../wayland-protocols-1.23/tests/meson.build:4:0: ERROR: Dependency "wayland-client" not found, tried pkgconfig I thought it would work because in the meson.build, wayland-protocols was listed as 'native', but apparently things are different on core-updates and master (1.23 vs. 1.25). Greetings, Maxime.
Hello Tobias, > * gnu/packages/freedesktop.scm (wayland-protocols): Fix cross-compilation > [native-inputs]: Add pkg-config-for-build and wayland. Maxime's idea of setting #target to false is interesting but let's go with your original patch because it's the way we handle things in general. Thanks, Mathieu
diff --git a/gnu/packages/freedesktop.scm b/gnu/packages/freedesktop.scm index 037a247243..c0a94c6623 100644 --- a/gnu/packages/freedesktop.scm +++ b/gnu/packages/freedesktop.scm @@ -1074,8 +1074,11 @@ (define-public wayland-protocols (build-system meson-build-system) (inputs (list wayland)) - (native-inputs - (list pkg-config python)) + (native-inputs (cons* pkg-config python + (if (%current-target-system) + (list pkg-config-for-build + wayland) ; for wayland-scanner + '()))) (synopsis "Wayland protocols") (description "Wayland-Protocols contains Wayland protocols that add functionality not available in the Wayland core protocol. Such protocols either