Go build system mishandles repetitive import paths (was [PATCH] build: go-build-system: Ensure uniform unpacking directory.)

  • Done
  • quality assurance status badge
Details
2 participants
  • Leo Famulari
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Leo Famulari
Severity
normal
L
L
Leo Famulari wrote on 6 May 2019 17:42
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . bug-guix@gnu.org)
20190506154249.GA9060@jasmine.lan
On Sun, Apr 14, 2019 at 12:03:05AM -0400, Maxim Cournoyer wrote:
Toggle quote (15 lines)
> From 1f7535fbe28f7ac96e824b792e9f1a140b8c54cd Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Fri, 5 Apr 2019 00:00:08 -0400
> Subject: [PATCH 3/3] build: go-build-system: Ensure uniform unpacking
> directory.
>
> Depending on whether the source is a directory or an archive, we strip the
> source directory or preserve it, respectively. This change makes it so that
> whether the type of the source, it is unpacked at the expected location given
> by the IMPORT-PATH of the Go build system.
>
> * guix/build/go-build-system.scm: Add the (ice-9 ftw) module.
> (unpack): Add inner procedure to maybe strip the top level directory of an
> archive, document it and use it.

This commit (or patch series) broke the build of Syncthing and maybe
others.

It seems like the the new unpacking code is stripping duplicate
directory names?

It fails like this:

------
starting phase `increase-test-timeout'
Backtrace:
6 (primitive-load "/gnu/store/yfvy06fscz726da5wjvh9jxjsah…")
In ice-9/eval.scm:
191:35 5 (_ _)
In srfi/srfi-1.scm:
863:16 4 (every1 #<procedure 870540 at /gnu/store/zmc0hcmdfg5n4…> …)
In /gnu/store/zmc0hcmdfg5n4kl32vcla4cg9c9bspfg-module-import/guix/build/gnu-build-system.scm:
799:28 3 (_ _)
In ice-9/eval.scm:
619:8 2 (_ #(#(#<directory (guile-user) 5ce140>) (#:inputs # …)))
In /gnu/store/zmc0hcmdfg5n4kl32vcla4cg9c9bspfg-module-import/guix/build/utils.scm:
635:19 1 (with-atomic-file-replacement "src/github.com/syncthin…" …)
In unknown file:
0 (mkstemp! "src/github.com/syncthing/syncthing/build.go…" …)

ERROR: In procedure mkstemp!:
In procedure mkstemp!: No such file or directory
------

And indeed, if you keep the failed build directory, you will see that
the path 'src/github.com/syncthing/syncthing' does not exist, even
though this corresponds to the Go import path specified in the package
definition.

Instead it is like src/github.com/syncthing' which is incorrect.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAlzQVfYACgkQJkb6MLrK
fwgSYA//Tygac1w4WpJpQuHUnQAzCGoWN2g5ZR9kz811zK4Qzz48LNPLq1sB0OQ6
oZnEVPY/fPOmLuajlEzkHJ+3kqcivD9ouYdTJZpyuFWuZT4Ze7E/PRfYKj9z6TpC
HVf3Zj59wNzO70SO928/TPjEEWxhUnAOs1kpBX6kUs7OR6akKHBPFKMsS395V4x0
aH9ucxhrSx3lAZPJ19o92Z8OrHHJ9J/QXxlLhjIBUnQq94rn26N0BxGYifHPSY9e
vMiu6SpeT559zKfdC1JXDHq5mUrRP6Nbqt6O4P8Tnxl2kJWAYH8cw6Yg4ahwrQF2
p+ws1KNpqKj5rl3oMrezzmFzpwgl+hsQTUaMtpvhFFOoOc70Dr38m7YTLZAGOWwq
hLxbWBJEfcqo1tmKunS6rbkMIr70g24qrGHs5KNk6PZql2IFTtiNlpSqEarjifQE
VBK29Q5hK/27QtrWQwfD4x4305ZYnujAL0OQusgyTxmEmL22LkQVA7ZDFOsAAvP+
+h4uokWUIArZ62f8HC9Cy1es/juD5im76TswM7bNjehASeSr5zqst0AOta9+REZh
UlkgMpTgTWVDVsJ8kWLsAuqwZ6VW0yBFUtMueUicr5Ly9qPm8UogcdxXHPQ5Njm6
07DIsquiS2NG+tnOtTnB7JFUb4puDw9u+dXUJ2v3keD4BqtinZk=
=b/W/
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 6 May 2019 17:56
Re: Go build system mishandles repetitive import paths
(address . 35603@debbugs.gnu.org)
20190506155617.GA16846@jasmine.lan
Looks like it affects packages that use tarballs (instead of Git
checkouts) and have a repetitive element in the import path. For
example, 'github.com/restic/restic' is also broken.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAlzQWSEACgkQJkb6MLrK
fwj67w/+JDp4gcgiDi5u16GbXvuFeruv/LO65zoTdSJ+1gFvVpfkEDYXdP4Xe4Qd
+Dk6E3GEOl/wFIDtjWljUXwIuu9cY71aooMTl0Tx4NhtErocOrUPvReok9oZAUDs
EJEAgFEeOt3lBamB+gzY8xL/DORwSdiMBIChhCDw4lEbonC9fqcesRBHOg1KP5fO
Wpxtf1EZGv9fvBsC+EHzwMU8meA5LEiV0KF+3YN1IZ7qplLUMeRpSAcSiCIRcOPh
yRO6fRQE9arlojp1CBBCMZI4Qbz72cuNYmuIIcGjh3lCwvb9n+I6kdEDDTSHUJHB
+CicrjLsE0ECi1ZAKt8Ot1pVxUVNKdEr8J8jrGRDW7nCuMGy9W0xLo4CTkEsWQGN
WoBlPINPPh7QrzYoqyeQjyGgvDzAYYhnp09WV8ma3hOacr7QdLfkeHPVbEoOfnPX
P8mlm88B82u0XpWDtSPelqzIBTcoiapia3rsg/PChXRheA8/ytQRWp39C2+L91Zq
HSUe1Eut4vTI/kuoy33FZXIoX/QPlKsg8cy8yMrARX4lGW2pf32a4q/lCkZrtUOR
FShaXnVtlXwsWGIS+tlBr/gdnIhkCItQKUYLe2K3H1+WsadEpA6B38ZEt50vLSx4
4FkZYwQCSUNjtbibfff/uibnOm8zb8c8A3mdTaZg27cX5WXpnDM=
=jPJK
-----END PGP SIGNATURE-----


M
M
Maxim Cournoyer wrote on 7 May 2019 03:10
Re: Go build system mishandles repetitive import paths (was [PATCH] build: go-build-system: Ensure uniform unpacking directory.)
(name . Leo Famulari)(address . leo@famulari.name)(address . 35603-done@debbugs.gnu.org)
87a7fzcbnm.fsf@gmail.com
Hello Leo,

Leo Famulari <leo@famulari.name> writes:

Toggle quote (19 lines)
> On Sun, Apr 14, 2019 at 12:03:05AM -0400, Maxim Cournoyer wrote:
>> From 1f7535fbe28f7ac96e824b792e9f1a140b8c54cd Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> Date: Fri, 5 Apr 2019 00:00:08 -0400
>> Subject: [PATCH 3/3] build: go-build-system: Ensure uniform unpacking
>> directory.
>>
>> Depending on whether the source is a directory or an archive, we strip the
>> source directory or preserve it, respectively. This change makes it so that
>> whether the type of the source, it is unpacked at the expected location given
>> by the IMPORT-PATH of the Go build system.
>>
>> * guix/build/go-build-system.scm: Add the (ice-9 ftw) module.
>> (unpack): Add inner procedure to maybe strip the top level directory of an
>> archive, document it and use it.
>
> This commit (or patch series) broke the build of Syncthing and maybe
> others.

Thanks for the heads-up!

Toggle quote (4 lines)
> It seems like the the new unpacking code is stripping duplicate
> directory names?
>

It does as documented in the docstring of the unpack phase:

If the SOURCE archive has a single top level directory,
it is stripped so that the sources appear directly under UNPACK-PATH.

This behavior was made possible with commit
f42e4ebb56fe4f16991ca6c6e060c8f3535865cb, that made it so that:

[...] whether the type of the source, it is unpacked at the expected
location given by the IMPORT-PATH of the Go build system.

Toggle quote (30 lines)
> It fails like this:
>
> ------
> starting phase `increase-test-timeout'
> Backtrace:
> 6 (primitive-load "/gnu/store/yfvy06fscz726da5wjvh9jxjsah…")
> In ice-9/eval.scm:
> 191:35 5 (_ _)
> In srfi/srfi-1.scm:
> 863:16 4 (every1 #<procedure 870540 at /gnu/store/zmc0hcmdfg5n4…> …)
> In /gnu/store/zmc0hcmdfg5n4kl32vcla4cg9c9bspfg-module-import/guix/build/gnu-build-system.scm:
> 799:28 3 (_ _)
> In ice-9/eval.scm:
> 619:8 2 (_ #(#(#<directory (guile-user) 5ce140>) (#:inputs # …)))
> In /gnu/store/zmc0hcmdfg5n4kl32vcla4cg9c9bspfg-module-import/guix/build/utils.scm:
> 635:19 1 (with-atomic-file-replacement "src/github.com/syncthin…" …)
> In unknown file:
> 0 (mkstemp! "src/github.com/syncthing/syncthing/build.go…" …)
>
> ERROR: In procedure mkstemp!:
> In procedure mkstemp!: No such file or directory
> ------
>
> And indeed, if you keep the failed build directory, you will see that
> the path 'src/github.com/syncthing/syncthing' does not exist, even
> though this corresponds to the Go import path specified in the package
> definition.
>
> Instead it is like src/github.com/syncthing' which is incorrect.

The fix was to drop the "unpack-pack" argument of the go-build-system for
syncthing, which means we want the sources unpacked at the location
specified by the "import-path".

Done with commit d879fd80c74371120a2cfa30e18a2e28dc02d31d; closing.

Thank you!

Maxim
Closed
M
M
Maxim Cournoyer wrote on 7 May 2019 04:42
Re: bug#35603: Go build system mishandles repetitive import paths
(name . Leo Famulari)(address . leo@famulari.name)(address . 35603@debbugs.gnu.org)
871s1bc7di.fsf@gmail.com
Hello again,

Leo Famulari <leo@famulari.name> writes:

Toggle quote (4 lines)
> Looks like it affects packages that use tarballs (instead of Git
> checkouts) and have a repetitive element in the import path. For
> example, 'github.com/restic/restic' is also broken.

I've fixed restic with commit fb09818277; sorry!

Maxim
?