GNU bug report logs - #54097
go importer does not honor multi-module repositories

Previous Next

Package: guix;

Reported by: Björn Höfling <bjoern.hoefling <at> bjoernhoefling.de>

Date: Mon, 21 Feb 2022 22:45:02 UTC

Severity: normal

To reply to this bug, email your comments to 54097 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Mon, 21 Feb 2022 22:45:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Björn Höfling <bjoern.hoefling <at> bjoernhoefling.de>:
New bug report received and forwarded. Copy sent to bug-guix <at> gnu.org. (Mon, 21 Feb 2022 22:45:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Björn Höfling <bjoern.hoefling <at> bjoernhoefling.de>
To: <bug-guix <at> gnu.org>
Subject: go importer does not honor multi-module repositories
Date: Mon, 21 Feb 2022 23:43:54 +0100
[Message part 1 (text/plain, inline)]
Go usually has the 1 repository=1 module convention. However, it
is also allowed that one repository contains multiple go modules.

If repository "foo" contains only one module, then versions are tagged
"v1.2.3".

However, if the repository "foo" contains modules "bar" and "baz", each
in a sub-directory of "foo", the versions will be tagged with their
respective prefix, i.e.:

foo/v1.2.3
bar/v4.5.6

See here:

https://github.com/golang/go/wiki/Modules#publishing-a-release
https://github.com/golang/go/wiki/Modules#faqs--multi-module-repositories
https://stackoverflow.com/questions/64701064/golang-separate-versioning-of-multiple-modules

However, our go-importer does not honor this. The Google Cloud API
modules are structured into sub-modules, but our importer searches the
wrong tag and raises an exception:

$ ./pre-inst-env guix import go cloud.google.com/go/storage
URL FOR VERSIONS: https://proxy.golang.org/cloud.google.com/go/storage/@v/list
FETCH_GO_MOD: https://proxy.golang.org/cloud.google.com/go/storage/@v/v1.21.0.mod
Backtrace:
In ice-9/boot-9.scm:
  1752:10 17 (with-exception-handler _ _ #:unwind? _ # _)
In unknown file:
          16 (apply-smob/0 #<thunk 7f5b3d24f0c0>)
In ice-9/boot-9.scm:
    724:2 15 (call-with-prompt _ _ #<procedure default-prompt-handle?>)
In ice-9/eval.scm:
    619:8 14 (_ #(#(#<directory (guile-user) 7f5b3d255c80>)))
In guix/ui.scm:
   2209:7 13 (run-guix . _)
  2172:10 12 (run-guix-command _ . _)
In guix/scripts/import.scm:
   124:11 11 (guix-import . _)
In ice-9/boot-9.scm:
  1752:10 10 (with-exception-handler _ _ #:unwind? _ # _)
In guix/scripts/import/go.scm:
   116:29  9 (_)
In ice-9/exceptions.scm:
   406:15  8 (go-module->guix-package* . _)
In ice-9/boot-9.scm:
  1752:10  7 (with-exception-handler _ _ #:unwind? _ # _)
In guix/import/go.scm:
   525:18  6 (go-module->guix-package "cloud.google.com/go/storage" # ?)
In guix/git.scm:
    277:4  5 (update-cached-checkout _ #:ref _ #:recursive? _ # _ # _ ?)
   266:18  4 (resolve _)
In git/reference.scm:
     60:8  3 (_ _ _)
In git/bindings.scm:
     77:2  2 (raise-git-error _)
In ice-9/boot-9.scm:
  1685:16  1 (raise-exception _ #:continuable? _)
  1683:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1683:16: In procedure raise-exception:
Git error: reference 'refs/tags/v1.21.0' not found


The correct git reference to look for is:

refs/tags/storage/v1.21.0

Björn

[Message part 2 (application/pgp-signature, inline)]

Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Tue, 22 Feb 2022 00:40:02 GMT) Full text and rfc822 format available.

Message #8 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: raingloom <raingloom <at> riseup.net>
To: Björn Höfling <bjoern.hoefling <at> bjoernhoefling.de>
Cc: 54097 <at> debbugs.gnu.org
Subject: Re: bug#54097: go importer does not honor multi-module repositories
Date: Tue, 22 Feb 2022 01:38:48 +0100
On Mon, 21 Feb 2022 23:43:54 +0100
Björn Höfling <bjoern.hoefling <at> bjoernhoefling.de> wrote:

> Go usually has the 1 repository=1 module convention. However, it
> is also allowed that one repository contains multiple go modules.
> 
> If repository "foo" contains only one module, then versions are tagged
> "v1.2.3".
> 
> However, if the repository "foo" contains modules "bar" and "baz",
> each in a sub-directory of "foo", the versions will be tagged with
> their respective prefix, i.e.:
> 
> foo/v1.2.3
> bar/v4.5.6
> 
> See here:
> 
> https://github.com/golang/go/wiki/Modules#publishing-a-release
> https://github.com/golang/go/wiki/Modules#faqs--multi-module-repositories
> https://stackoverflow.com/questions/64701064/golang-separate-versioning-of-multiple-modules
> 
> However, our go-importer does not honor this. The Google Cloud API
> modules are structured into sub-modules, but our importer searches the
> wrong tag and raises an exception:
> 
> $ ./pre-inst-env guix import go cloud.google.com/go/storage
> URL FOR VERSIONS:
> https://proxy.golang.org/cloud.google.com/go/storage/@v/list
> FETCH_GO_MOD:
> https://proxy.golang.org/cloud.google.com/go/storage/@v/v1.21.0.mod
> Backtrace: In ice-9/boot-9.scm: 1752:10 17 (with-exception-handler _
> _ #:unwind? _ # _) In unknown file:
>           16 (apply-smob/0 #<thunk 7f5b3d24f0c0>)
> In ice-9/boot-9.scm:
>     724:2 15 (call-with-prompt _ _ #<procedure
> default-prompt-handle?>) In ice-9/eval.scm:
>     619:8 14 (_ #(#(#<directory (guile-user) 7f5b3d255c80>)))
> In guix/ui.scm:
>    2209:7 13 (run-guix . _)
>   2172:10 12 (run-guix-command _ . _)
> In guix/scripts/import.scm:
>    124:11 11 (guix-import . _)
> In ice-9/boot-9.scm:
>   1752:10 10 (with-exception-handler _ _ #:unwind? _ # _)
> In guix/scripts/import/go.scm:
>    116:29  9 (_)
> In ice-9/exceptions.scm:
>    406:15  8 (go-module->guix-package* . _)
> In ice-9/boot-9.scm:
>   1752:10  7 (with-exception-handler _ _ #:unwind? _ # _)
> In guix/import/go.scm:
>    525:18  6 (go-module->guix-package "cloud.google.com/go/storage" #
> ?) In guix/git.scm:
>     277:4  5 (update-cached-checkout _ #:ref _ #:recursive? _ # _ # _
> ?) 266:18  4 (resolve _)
> In git/reference.scm:
>      60:8  3 (_ _ _)
> In git/bindings.scm:
>      77:2  2 (raise-git-error _)
> In ice-9/boot-9.scm:
>   1685:16  1 (raise-exception _ #:continuable? _)
>   1683:16  0 (raise-exception _ #:continuable? _)
> 
> ice-9/boot-9.scm:1683:16: In procedure raise-exception:
> Git error: reference 'refs/tags/v1.21.0' not found
> 
> 
> The correct git reference to look for is:
> 
> refs/tags/storage/v1.21.0
> 
> Björn
> 

I think this has been mentioned before, but I really think we should
use Go's built-in tooling to extract this info, instead of
reimplementing it in Guile. There is already some massive ineffeciency
introduced by the importer cloning whole repos, whereas `go get` is
smart enough to only download `go.mod` files.
Just my 2c.




Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Tue, 22 Feb 2022 13:00:02 GMT) Full text and rfc822 format available.

Message #11 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: Blake Shaw <blake <at> nonconstructivism.com>
To: Björn Höfling <bjoern.hoefling <at> bjoernhoefling.de>
Cc: 54097 <at> debbugs.gnu.org
Subject: Re: bug#54097: go importer does not honor multi-module repositories
Date: Tue, 22 Feb 2022 19:59:26 +0700
great investigation, thanks for sharing!
-- 
“In girum imus nocte et consumimur igni”




Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Sun, 21 May 2023 21:20:02 GMT) Full text and rfc822 format available.

Message #14 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: Timo Wilken <guix <at> twilken.net>
To: guix-patches <at> gnu.org,
	54097 <at> debbugs.gnu.org
Cc: wolf <at> wolfsden.cz, Timo Wilken <guix <at> twilken.net>
Subject: [PATCH] import: go: Handle subpackage versioning correctly.
Date: Sun, 21 May 2023 23:18:08 +0200
Some Go source repositories (notably the Google Cloud SDK) contain multiple
submodules and use a `refs/tags/<submodule>/<version>' tagging scheme.

Fixes <https://bugs.gnu.org/54097>.

* guix/import/go.scm (vcs->origin): Accept a module-path-suffix.
(go-module->guix-package): Use the new parameter.
---
Here's a patch that fixes the reported issue (bug#54097) for me. I've only
tested this on the github.com/googleapis/google-cloud-go/compute package so
far, though it seems to work there. Perhaps others have more testcases?

I don't know enough about Go tooling to use it, so I've just patched the Guile
logic of the importer. (I don't write Go, I just want to package stuff written
in it.) In terms of performance, at least the repo contents are apparently
cached by the first `git-checkout-hash' call, even if it fails, so the second
call doesn't have to redownload them.

 guix/import/go.scm | 56 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/guix/import/go.scm b/guix/import/go.scm
index 0357e6a1eb..652ac58b6f 100644
--- a/guix/import/go.scm
+++ b/guix/import/go.scm
@@ -7,6 +7,7 @@
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune <at> gmail.com>
+;;; Copyright © 2023 Timo Wilken <guix <at> twilken.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -89,6 +90,7 @@ (define-module (guix import go)
 
 ;;; TODO list
 ;;; - get correct hash in vcs->origin for Mercurial and Subversion
+;;; - handle subdir/vX.Y versioning in vcs->origin for Mercurial and Subversion
 
 ;;; Code:
 
@@ -513,29 +515,54 @@ (define* (git-checkout-hash url reference algorithm)
                                           `(tag-or-commit . ,reference)))))
     (file-hash* checkout #:algorithm algorithm #:recursive? #true)))
 
-(define (vcs->origin vcs-type vcs-repo-url version)
+(define (vcs->origin vcs-type vcs-repo-url module-path-suffix version)
   "Generate the `origin' block of a package depending on what type of source
 control system is being used."
   (case vcs-type
     ((git)
-     (let ((plain-version? (string=? version (go-version->git-ref version)))
-           (v-prefixed?    (string-prefix? "v" version)))
+     (let ((v-prefixed? (string-prefix? "v" version))
+           (path-prefixed? #f)
+           (trimmed-path-suffix (string-trim-both module-path-suffix #\/))
+           (checkout-hash (false-if-git-not-found
+                           (git-checkout-hash
+                            vcs-repo-url
+                            (go-version->git-ref version)
+                            (hash-algorithm sha256)))))
+       ;; If `checkout-hash' is false, that must mean that a tag named after
+       ;; the version doesn't exist.  Some repos that contain submodules use a
+       ;; <submodule>/<version> tagging scheme instead, so try that.
+       (unless checkout-hash
+         (when (string=? "" trimmed-path-suffix)
+           ;; If this isn't a submodule, <submodule>/<version> tagging makes no sense.
+           ;; Tell the user we couldn't find the original version.
+           (raise
+            (formatted-message (G_ "could not find git reference '~a' in repository '~a'")
+                               (go-version->git-ref version) vcs-repo-url)))
+         (set! path-prefixed? #t)
+         (set! checkout-hash (git-checkout-hash
+                              vcs-repo-url
+                              (go-version->git-ref
+                               (string-append trimmed-path-suffix "/" version))
+                              (hash-algorithm sha256))))
        `(origin
           (method git-fetch)
           (uri (git-reference
                 (url ,vcs-repo-url)
-                ;; This is done because the version field of the package,
-                ;; which the generated quoted expression refers to, has been
-                ;; stripped of any 'v' prefixed.
-                (commit ,(if (and plain-version? v-prefixed?)
-                             '(string-append "v" version)
-                             '(go-version->git-ref version)))))
+                ;; The 'v' is prepended again because the version field of
+                ;; the package, which the generated quoted expression refers
+                ;; to, has been stripped of any 'v' prefixed.
+                (commit (go-version->git-ref
+                         ,(cond
+                           (path-prefixed?
+                            `(string-append
+                              ,trimmed-path-suffix "/"
+                              ,@(if v-prefixed? '("v" version) '(version))))
+                           (v-prefixed? '(string-append "v" version))
+                           (else 'version))))))
           (file-name (git-file-name name version))
           (sha256
            (base32
-            ,(bytevector->nix-base32-string
-              (git-checkout-hash vcs-repo-url (go-version->git-ref version)
-                                 (hash-algorithm sha256))))))))
+            ,(bytevector->nix-base32-string checkout-hash))))))
     ((hg)
      `(origin
         (method hg-fetch)
@@ -614,6 +641,9 @@ (define* (go-module->guix-package module-path #:key
           (match:prefix (string-match "([\\./]v[0-9]+)?$" module-path)))
          (guix-name (go-module->guix-package-name module-path))
          (root-module-path (module-path->repository-root module-path))
+         (module-path-suffix  ; subdirectory inside the source repo
+          (substring module-path-sans-suffix
+                     (string-prefix-length root-module-path module-path-sans-suffix)))
          ;; The VCS type and URL are not included in goproxy information. For
          ;; this we need to fetch it from the official module page.
          (meta-data (fetch-module-meta-data root-module-path))
@@ -627,7 +657,7 @@ (define* (go-module->guix-package module-path #:key
         (name ,guix-name)
         (version ,(strip-v-prefix version*))
         (source
-         ,(vcs->origin vcs-type vcs-repo-url version*))
+         ,(vcs->origin vcs-type vcs-repo-url module-path-suffix version*))
         (build-system go-build-system)
         (arguments
          '(#:import-path ,module-path

base-commit: e499cb2c12d7f1c6d2f004364c9cc7bdb7e38cd5
-- 
2.40.1





Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Sun, 21 May 2023 21:55:02 GMT) Full text and rfc822 format available.

Message #17 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: wolf <wolf <at> wolfsden.cz>
To: Timo Wilken <guix <at> twilken.net>
Cc: 54097 <at> debbugs.gnu.org, guix-patches <at> gnu.org
Subject: Re: [PATCH] import: go: Handle subpackage versioning correctly.
Date: Sun, 21 May 2023 23:54:33 +0200
[Message part 1 (text/plain, inline)]
Hi,

What a coincidence, this week I happened to take a look at this same issue (in
my case I wanted to build terraform).  I failed to get it properly working, so
I'm happy someone else took a look at it as well.

On 2023-05-21 23:18:08 +0200, Timo Wilken wrote:
> Some Go source repositories (notably the Google Cloud SDK) contain multiple
> submodules and use a `refs/tags/<submodule>/<version>' tagging scheme.
> 
> Fixes <https://bugs.gnu.org/54097>.
> 
> * guix/import/go.scm (vcs->origin): Accept a module-path-suffix.
> (go-module->guix-package): Use the new parameter.
> ---
> Here's a patch that fixes the reported issue (bug#54097) for me. I've only
> tested this on the github.com/googleapis/google-cloud-go/compute package so
> far, though it seems to work there. Perhaps others have more testcases?

Please give the github.com/Azure/go-autorest/tracing <at> v0.6.0 a go.  My code
failed on it, and (assuming I applied the patch correctly) your does as well.
Here are reproduction steps to make it easier for you (please tell me if I did
something wrong):

    $ echo '(use-modules (guix packages) (guix git-download) (guix build-system go) ((guix licenses) #:prefix license:))' >/tmp/x.scm
    $ ./pre-inst-env guix import go -r github.com/Azure/go-autorest/tracing <at> v0.6.0 >>/tmp/x.scm
    $ echo go-github-com-azure-go-autorest-tracing >>/tmp/x.scm
    $ guix build -f /tmp/x.scm
    [..]
    starting phase `unpack'
    `/gnu/store/857z63cfgclsh6g52vj9xnm7iv97yz97-go-github-com-azure-go-autorest-tracing-0.6.0-checkout/.gitignore' -> `/tmp/guix-build-go-github-com-azure-go-autorest-tracing-0.6.0.drv-0/src/github.com/Azure/go-autorest/.gitignore'
    error: in phase 'unpack': uncaught exception:
    system-error "copy-file" "~A" ("Permission denied") (13) 
    phase `unpack' failed after 0.0 seconds
    [..]

I will not pretend to have a full grasp on how (guix build-system go) works,
however my debugging lead me to the observation that it tries to unpack two
dependencies into one file system tree overlayed on top of each other.  I think
the current way (GO111MODULE=off) of building of golang packages does not play
very well with well, go modules.

Either the build system needs to be smarter about unpacking dependencies (and
doing it in a correct order), or we should start using go modules for the builds
(it can still be down offline, just the dependencies are in different paths).
The second approach is what I wanted to explore, but did not get to it yet (and
likely will not for a month or two).

> 
> I don't know enough about Go tooling to use it, so I've just patched the Guile
> logic of the importer. (I don't write Go, I just want to package stuff written
> in it.) In terms of performance, at least the repo contents are apparently
> cached by the first `git-checkout-hash' call, even if it fails, so the second
> call doesn't have to redownload them.
> 
>  guix/import/go.scm | 56 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/guix/import/go.scm b/guix/import/go.scm
> index 0357e6a1eb..652ac58b6f 100644
> --- a/guix/import/go.scm
> +++ b/guix/import/go.scm
> @@ -7,6 +7,7 @@
>  ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
>  ;;; Copyright © 2021 Sarah Morgensen <iskarian <at> mgsn.dev>
>  ;;; Copyright © 2021 Simon Tournier <zimon.toutoune <at> gmail.com>
> +;;; Copyright © 2023 Timo Wilken <guix <at> twilken.net>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -89,6 +90,7 @@ (define-module (guix import go)
>  
>  ;;; TODO list
>  ;;; - get correct hash in vcs->origin for Mercurial and Subversion
> +;;; - handle subdir/vX.Y versioning in vcs->origin for Mercurial and Subversion
>  
>  ;;; Code:
>  
> @@ -513,29 +515,54 @@ (define* (git-checkout-hash url reference algorithm)
>                                            `(tag-or-commit . ,reference)))))
>      (file-hash* checkout #:algorithm algorithm #:recursive? #true)))
>  
> -(define (vcs->origin vcs-type vcs-repo-url version)
> +(define (vcs->origin vcs-type vcs-repo-url module-path-suffix version)
>    "Generate the `origin' block of a package depending on what type of source
>  control system is being used."
>    (case vcs-type
>      ((git)
> -     (let ((plain-version? (string=? version (go-version->git-ref version)))
> -           (v-prefixed?    (string-prefix? "v" version)))
> +     (let ((v-prefixed? (string-prefix? "v" version))
> +           (path-prefixed? #f)
> +           (trimmed-path-suffix (string-trim-both module-path-suffix #\/))
> +           (checkout-hash (false-if-git-not-found
> +                           (git-checkout-hash
> +                            vcs-repo-url
> +                            (go-version->git-ref version)
> +                            (hash-algorithm sha256)))))
> +       ;; If `checkout-hash' is false, that must mean that a tag named after
> +       ;; the version doesn't exist.  Some repos that contain submodules use a
> +       ;; <submodule>/<version> tagging scheme instead, so try that.
> +       (unless checkout-hash
> +         (when (string=? "" trimmed-path-suffix)
> +           ;; If this isn't a submodule, <submodule>/<version> tagging makes no sense.
> +           ;; Tell the user we couldn't find the original version.
> +           (raise
> +            (formatted-message (G_ "could not find git reference '~a' in repository '~a'")
> +                               (go-version->git-ref version) vcs-repo-url)))
> +         (set! path-prefixed? #t)
> +         (set! checkout-hash (git-checkout-hash
> +                              vcs-repo-url
> +                              (go-version->git-ref
> +                               (string-append trimmed-path-suffix "/" version))
> +                              (hash-algorithm sha256))))
>         `(origin
>            (method git-fetch)
>            (uri (git-reference
>                  (url ,vcs-repo-url)
> -                ;; This is done because the version field of the package,
> -                ;; which the generated quoted expression refers to, has been
> -                ;; stripped of any 'v' prefixed.
> -                (commit ,(if (and plain-version? v-prefixed?)
> -                             '(string-append "v" version)
> -                             '(go-version->git-ref version)))))
> +                ;; The 'v' is prepended again because the version field of
> +                ;; the package, which the generated quoted expression refers
> +                ;; to, has been stripped of any 'v' prefixed.
> +                (commit (go-version->git-ref
> +                         ,(cond
> +                           (path-prefixed?
> +                            `(string-append
> +                              ,trimmed-path-suffix "/"
> +                              ,@(if v-prefixed? '("v" version) '(version))))
> +                           (v-prefixed? '(string-append "v" version))
> +                           (else 'version))))))
>            (file-name (git-file-name name version))
>            (sha256
>             (base32
> -            ,(bytevector->nix-base32-string
> -              (git-checkout-hash vcs-repo-url (go-version->git-ref version)
> -                                 (hash-algorithm sha256))))))))
> +            ,(bytevector->nix-base32-string checkout-hash))))))
>      ((hg)
>       `(origin
>          (method hg-fetch)
> @@ -614,6 +641,9 @@ (define* (go-module->guix-package module-path #:key
>            (match:prefix (string-match "([\\./]v[0-9]+)?$" module-path)))
>           (guix-name (go-module->guix-package-name module-path))
>           (root-module-path (module-path->repository-root module-path))
> +         (module-path-suffix  ; subdirectory inside the source repo
> +          (substring module-path-sans-suffix
> +                     (string-prefix-length root-module-path module-path-sans-suffix)))
>           ;; The VCS type and URL are not included in goproxy information. For
>           ;; this we need to fetch it from the official module page.
>           (meta-data (fetch-module-meta-data root-module-path))
> @@ -627,7 +657,7 @@ (define* (go-module->guix-package module-path #:key
>          (name ,guix-name)
>          (version ,(strip-v-prefix version*))
>          (source
> -         ,(vcs->origin vcs-type vcs-repo-url version*))
> +         ,(vcs->origin vcs-type vcs-repo-url module-path-suffix version*))
>          (build-system go-build-system)
>          (arguments
>           '(#:import-path ,module-path
> 
> base-commit: e499cb2c12d7f1c6d2f004364c9cc7bdb7e38cd5
> -- 
> 2.40.1
>

I did not really take a look at the scheme code, I'm still Guix and Scheme
beginner, so I'm very much not up to the task of doing actual code review.
Nevertheless, I hope my mail helps at least a bit.

Have a nice day,
W.

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Mon, 22 May 2023 19:12:01 GMT) Full text and rfc822 format available.

Message #20 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: "Timo Wilken" <guix <at> twilken.net>
To: "wolf" <wolf <at> wolfsden.cz>
Cc: 54097 <at> debbugs.gnu.org
Subject: Re: [PATCH] import: go: Handle subpackage versioning correctly.
Date: Mon, 22 May 2023 21:11:34 +0200
Hi wolf,

On Sun May 21, 2023 at 11:54 PM CEST, wolf wrote:
> Please give the github.com/Azure/go-autorest/tracing <at> v0.6.0 a go.  My code
> failed on it, and (assuming I applied the patch correctly) your does as well.
> Here are reproduction steps to make it easier for you (please tell me if I did
> something wrong):

I don't think you did anything wrong there.

That's an issue I've run into in the past as well, though it seems to be a bug
in the Go build system, not the importer (and in my patch I only touch the
importer).

> I will not pretend to have a full grasp on how (guix build-system go) works,
> however my debugging lead me to the observation that it tries to unpack two
> dependencies into one file system tree overlayed on top of each other.  I think
> the current way (GO111MODULE=off) of building of golang packages does not play
> very well with well, go modules.

Fair enough! I don't know much about Go -- I don't write software in it, I
just want to package some stuff written in it; in my case, that's
Matrix-related programs.

> Either the build system needs to be smarter about unpacking dependencies (and
> doing it in a correct order), or we should start using go modules for the builds
> (it can still be down offline, just the dependencies are in different paths).
> The second approach is what I wanted to explore, but did not get to it yet (and
> likely will not for a month or two).

Your second approach sounds sensible!

If I can find the time and motivation to dig in to this, I might have a go as
well... But if you get anything working, that would be much appreciated! :)

Cheers,
Timo




Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Wed, 14 Jun 2023 21:10:02 GMT) Full text and rfc822 format available.

Message #23 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Timo Wilken <guix <at> twilken.net>
Cc: 63647 <at> debbugs.gnu.org, 63631 <at> debbugs.gnu.org,
 Simon Tournier <zimon.toutoune <at> gmail.com>, 54097 <at> debbugs.gnu.org,
 wolf <at> wolfsden.cz
Subject: Re: bug#63631: [PATCH] import: go: Handle subpackage versioning
 correctly.
Date: Wed, 14 Jun 2023 23:09:35 +0200
Hi Timo,

Timo Wilken <guix <at> twilken.net> skribis:

> Some Go source repositories (notably the Google Cloud SDK) contain multiple
> submodules and use a `refs/tags/<submodule>/<version>' tagging scheme.
>
> Fixes <https://bugs.gnu.org/54097>.
>
> * guix/import/go.scm (vcs->origin): Accept a module-path-suffix.
> (go-module->guix-package): Use the new parameter.
> ---
> Here's a patch that fixes the reported issue (bug#54097) for me. I've only
> tested this on the github.com/googleapis/google-cloud-go/compute package so
> far, though it seems to work there. Perhaps others have more testcases?
>
> I don't know enough about Go tooling to use it, so I've just patched the Guile
> logic of the importer. (I don't write Go, I just want to package stuff written
> in it.) In terms of performance, at least the repo contents are apparently
> cached by the first `git-checkout-hash' call, even if it fails, so the second
> call doesn't have to redownload them.

What you propose looks similar to part of the work Simon Tournier
submitted at <https://issues.guix.gnu.org/63647>.

What would you suggest?  Simon?

Thanks for the patch, Timo!

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Sat, 17 Jun 2023 15:14:02 GMT) Full text and rfc822 format available.

Message #26 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: "Timo Wilken" <guix <at> twilken.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 63631 <at> debbugs.gnu.org, 64036 <at> debbugs.gnu.org,
 Simon Tournier <zimon.toutoune <at> gmail.com>, 63647 <at> debbugs.gnu.org,
 64035 <at> debbugs.gnu.org, 63001 <at> debbugs.gnu.org, 54097 <at> debbugs.gnu.org,
 wolf <at> wolfsden.cz
Subject: Re: bug#63631: [PATCH] import: go: Handle subpackage versioning
 correctly.
Date: Sat, 17 Jun 2023 17:12:58 +0200
[Message part 1 (text/plain, inline)]
Hi Ludo', (hi everyone,)

On Wed Jun 14, 2023 at 11:09 PM CEST, Ludovic Courtès wrote:
> Timo Wilken <guix <at> twilken.net> skribis:
> > Here's a patch that fixes the reported issue (bug#54097) for me. I've only
> > tested this on the github.com/googleapis/google-cloud-go/compute package so
> > far, though it seems to work there. Perhaps others have more testcases?
> >
> > I don't know enough about Go tooling to use it, so I've just patched the Guile
> > logic of the importer. (I don't write Go, I just want to package stuff written
> > in it.) In terms of performance, at least the repo contents are apparently
> > cached by the first `git-checkout-hash' call, even if it fails, so the second
> > call doesn't have to redownload them.

I've been testing my patch further this weekend, and I have a couple more
patches in the pipeline; I suppose I ought to clean those up and submit them.

In particular, I've got fixes for the following queued up locally:

1. Finding the `module-path-subdir' needs another case for e.g.
   cloud.google.com/go/*.

2. My patch sometimes generates an unnecessary `go-version->git-ref' call.

3. Go versions need to be parsed from go.mod, since some packages require a
   newer Go compiler than our default. This I've got a patch for, but this Go
   version also ought to propagate up the dependency tree. I haven't found an
   easy way to do that, since the importer seems to generate top-level
   packages first, before descending the dep tree...

4. `fetch-module-meta-data' ought to ignore 4xx HTTP errors to follow the
   spec; gonum.org/v1/gonum specifically depends on this behaviour.

I've been trying to recursively import github.com/matrix-org/dendrite, which
has a particularly large and hairy dependency tree. While I can now import it
without crashes, I can't build it from the imported package definitions yet --
mainly because of lots of dependency cycles in the generated packages, but
there may be more issues hidden beneath that.

Still, I can recommend it as a test of everyone's importer patches, since
it'll find a lot of edge cases in importing alone!

> What you propose looks similar to part of the work Simon Tournier
> submitted at <https://issues.guix.gnu.org/63647>.

It seems lots of people have been working on the same problem -- in addition
to Simon's patches, I found a patch submitted by Elbek (issues 64035 & 64036;
Cc'd). I also forgot about the issue I submitted months ago (63001)...

> What would you suggest?  Simon?

Here's a brief comparison between Simon's patches and mine -- Simon's seem to
contain fixes for a couple more things than mine currently does:

1. Simon sorts available versions in an error message; this can presumably be
   merged independently since it doesn't conflict with other patches.

2. Simon always prepends a "SUBDIR/" prefix to the tag if found, whereas I try
   to find the plain "vX" tag first, then fall back to "SUBDIR/vX". Judging by
   https://go.dev/ref/mod#vcs-version, Simon's approach seems more correct.
   I'll change my implementation to match and try it out.

3. For detecting the `module-path-subdirectory' in Simon's patches: that's the
   same approach I used initially, but I found I have to try `(substring
   module-path (string-length import-prefix))' first (to handle e.g.
   cloud.google.com/go/*). This is one of the things I haven't submitted
   yet...

> Thanks for the patch, Timo!

Thanks for your work in sorting through all of this, Ludo'!

Cheers,
Timo
[signature.asc (application/pgp-signature, inline)]

Information forwarded to bug-guix <at> gnu.org:
bug#54097; Package guix. (Wed, 16 Aug 2023 17:12:02 GMT) Full text and rfc822 format available.

Message #29 received at 54097 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Timo Wilken <guix <at> twilken.net>, Ludovic Courtès
 <ludo <at> gnu.org>
Cc: 63631 <at> debbugs.gnu.org, 64036 <at> debbugs.gnu.org, 63647 <at> debbugs.gnu.org,
 64035 <at> debbugs.gnu.org, 63001 <at> debbugs.gnu.org, 54097 <at> debbugs.gnu.org,
 wolf <at> wolfsden.cz
Subject: Re: bug#63001: bug#63631: [PATCH] import: go: Handle subpackage
 versioning correctly.
Date: Wed, 16 Aug 2023 17:59:53 +0200
Hi Timo,

On Sat, 17 Jun 2023 at 17:12, "Timo Wilken" <guix <at> twilken.net> wrote:

>> What would you suggest?  Simon?
>
> Here's a brief comparison between Simon's patches and mine -- Simon's seem to
> contain fixes for a couple more things than mine currently does:
>
> 1. Simon sorts available versions in an error message; this can presumably be
>    merged independently since it doesn't conflict with other patches.
>
> 2. Simon always prepends a "SUBDIR/" prefix to the tag if found, whereas I try
>    to find the plain "vX" tag first, then fall back to "SUBDIR/vX". Judging by
>    https://go.dev/ref/mod#vcs-version, Simon's approach seems more correct.
>    I'll change my implementation to match and try it out.
>
> 3. For detecting the `module-path-subdirectory' in Simon's patches: that's the
>    same approach I used initially, but I found I have to try `(substring
>    module-path (string-length import-prefix))' first (to handle e.g.
>    cloud.google.com/go/*). This is one of the things I haven't submitted
>    yet...

Sorry if I have missed some patches or overlooked something.  Do you
plan to send another patch series handling all?


Cheers,
simon




This bug report was last modified 250 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.