* [bug#63631] [PATCH] import: go: Handle subpackage versioning correctly.
@ 2023-05-21 21:18 Timo Wilken
2023-05-21 21:54 ` wolf
2023-06-14 21:09 ` Ludovic Courtès
0 siblings, 2 replies; 4+ messages in thread
From: Timo Wilken @ 2023-05-21 21:18 UTC (permalink / raw)
To: 63631, 54097; +Cc: wolf, Timo Wilken
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@yoctocell.xyz>
;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2023 Timo Wilken <guix@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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [bug#63631] [PATCH] import: go: Handle subpackage versioning correctly.
2023-05-21 21:18 [bug#63631] [PATCH] import: go: Handle subpackage versioning correctly Timo Wilken
@ 2023-05-21 21:54 ` wolf
2023-06-14 21:09 ` Ludovic Courtès
1 sibling, 0 replies; 4+ messages in thread
From: wolf @ 2023-05-21 21:54 UTC (permalink / raw)
To: Timo Wilken; +Cc: 54097, 63631
[-- Attachment #1: Type: text/plain, Size: 9139 bytes --]
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@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@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@yoctocell.xyz>
> ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
> ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
> +;;; Copyright © 2023 Timo Wilken <guix@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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* [bug#63631] [PATCH] import: go: Handle subpackage versioning correctly.
2023-05-21 21:18 [bug#63631] [PATCH] import: go: Handle subpackage versioning correctly Timo Wilken
2023-05-21 21:54 ` wolf
@ 2023-06-14 21:09 ` Ludovic Courtès
[not found] ` <CTF06XBYWPT0.1MV6QA1B2OB98@lap.twilken.net>
1 sibling, 1 reply; 4+ messages in thread
From: Ludovic Courtès @ 2023-06-14 21:09 UTC (permalink / raw)
To: Timo Wilken; +Cc: 63647, 63631, Simon Tournier, 54097, wolf
Hi Timo,
Timo Wilken <guix@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’.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [bug#64035] bug#63001: bug#63631: [PATCH] import: go: Handle subpackage versioning correctly.
[not found] ` <CTF06XBYWPT0.1MV6QA1B2OB98@lap.twilken.net>
@ 2023-08-16 15:59 ` Simon Tournier
0 siblings, 0 replies; 4+ messages in thread
From: Simon Tournier @ 2023-08-16 15:59 UTC (permalink / raw)
To: Timo Wilken, Ludovic Courtès
Cc: 63631, 64036, 63647, 64035, 63001, 54097, wolf
Hi Timo,
On Sat, 17 Jun 2023 at 17:12, "Timo Wilken" <guix@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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-16 17:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-21 21:18 [bug#63631] [PATCH] import: go: Handle subpackage versioning correctly Timo Wilken
2023-05-21 21:54 ` wolf
2023-06-14 21:09 ` Ludovic Courtès
[not found] ` <CTF06XBYWPT0.1MV6QA1B2OB98@lap.twilken.net>
2023-08-16 15:59 ` [bug#64035] bug#63001: bug#63631: " Simon Tournier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).