diff mbox series

[bug#57430] gnu: wayland-protocols: Fix cross-compilation

Message ID 2195d673159785d305502677d786ae9bcd69ac7c.1661508240.git.tobias.kortkamp@gmail.com
State Accepted
Headers show
Series [bug#57430] gnu: wayland-protocols: Fix cross-compilation | expand

Checks

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

Commit Message

Tobias Kortkamp Aug. 26, 2022, 10:14 a.m. UTC
* gnu/packages/freedesktop.scm (wayland-protocols): Fix cross-compilation
[native-inputs]: Add pkg-config-for-build and wayland.
---
 gnu/packages/freedesktop.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

M Aug. 26, 2022, 3:04 p.m. UTC | #1
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.
muradm Aug. 26, 2022, 4:59 p.m. UTC | #2
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]]
M Aug. 26, 2022, 6:58 p.m. UTC | #3
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.
Liliana Marie Prikler Aug. 26, 2022, 7:22 p.m. UTC | #4
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 :)
M Aug. 26, 2022, 8:34 p.m. UTC | #5
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
Tobias Kortkamp Aug. 27, 2022, 10:40 a.m. UTC | #6
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.
M Aug. 27, 2022, 11:02 a.m. UTC | #7
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.
Mathieu Othacehe Aug. 30, 2022, 7:18 a.m. UTC | #8
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 mbox series

Patch

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