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

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

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 2110 bytes --]

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.

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#35603: Go build system mishandles repetitive import paths

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Maxim Cournoyer]

Hello Leo,

Leo Famulari <leo@famulari.name> writes:

> 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!

> 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.

> 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

bug#35603: Go build system mishandles repetitive import paths

[Maxim Cournoyer]

Hello again,

Leo Famulari <leo@famulari.name> writes:

> 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