#63631 [PATCH] import: go: Handle subpackage versioning correctly.

Message #8 received at submit@debbugs.gnu.org (full text, mbox, reply):

Received: (at submit) by debbugs.gnu.org; 21 May 2023 21:54:46 +0000
From debbugs-submit-bounces@debbugs.gnu.org Sun May 21 17:54:45 2023
Received: from localhost ([127.0.0.1]:32959 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
	id 1q0r0n-0002JO-6Y
	for submit@debbugs.gnu.org; Sun, 21 May 2023 17:54:45 -0400
Received: from lists.gnu.org ([209.51.188.17]:48934)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <ws@wolfsnet.cz>) id 1q0r0l-0002J2-0I
 for submit@debbugs.gnu.org; Sun, 21 May 2023 17:54:43 -0400
Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <ws@wolfsnet.cz>) id 1q0r0k-0002hV-MF
 for guix-patches@gnu.org; Sun, 21 May 2023 17:54:42 -0400
Received: from wolfsden.cz ([37.205.8.62])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <ws@wolfsnet.cz>) id 1q0r0i-0007Hu-Ok
 for guix-patches@gnu.org; Sun, 21 May 2023 17:54:42 -0400
Received: by wolfsden.cz (Postfix, from userid 104)
 id EBF9225A96C; Sun, 21 May 2023 21:54:34 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=wolfsden.cz; s=mail;
 t=1684706074; bh=7jZSQ6kPl4uZBrcrCMjCjeDmk7WiY/3V+1MatjIbtig=;
 h=Date:From:To:Cc:Subject:References:In-Reply-To;
 b=Av1Y3shEepVs1TcQ3YMvzcRZ9vOZQTy4Ib2LT8Vg2a4XBAhSvAJEFaDo+Orga4nbf
 2hEzsxlKbC1UfDirH7Kry2VfN8NrqaFTlY8ujSbp/zhgJiqffKE2ZjLXubOKCVZCW9
 yGFYIKIhI5WpHiAUKNzpKnTTAQpSNycOYKdaZpb3Yao6LuGEi3virJkDDMK06idZMP
 7u0ZEN9fKycaIHnRXF6J2DPbYjMzka/yLyn7tW3wPHhguxImnyp3KU6nzFmEfZSxjv
 y1lhVWkPFEWFwN07HBJuLylQqsUEEey7pyKupxGk0WLFTOwVkBHlCWnxpdIqfM1LHB
 eEmH/7T8nUxR2vmdYrHMLvUnePRWF52NpQjFLoEGaF6FmIDYCX6ODnbR14T7SqDmIe
 ycsUjJK/9spVcP+iJrD3t6G9eJvlNr6VZrpnBUqOYG4VyqDtx1WP3x60zG/0kE7iiq
 Gptw8OBUVZgKVYJi7I/VhL7IP91Y+daMaRW/DI7K56ElPTf7X+mem+hmfbygp0Wg/P
 ipblJjidqrbYsmHRpNZ0o6TEWcNHuy+EnO/PHPVENVBmZKEu0t/c9LGbch7u9UC34e
 8e4Vxc6cN55W7y+VFV6v/2cfTen/aj7JlP7VHcmXw6cN+IlaqspX4qnZayrgKspmw7
 r3TwtDRDdtUzIzVuuTpWsv+c=
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on wolfsden
X-Spam-Level: 
X-Spam-Status: No, score=-0.9 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED,
 DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
 T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
 version=3.4.6
Received: from localhost (unknown [128.0.188.242])
 by wolfsden.cz (Postfix) with ESMTPSA id 0EFE125A9EB;
 Sun, 21 May 2023 21:54:34 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=wolfsden.cz; s=mail;
 t=1684706074; bh=7jZSQ6kPl4uZBrcrCMjCjeDmk7WiY/3V+1MatjIbtig=;
 h=Date:From:To:Cc:Subject:References:In-Reply-To;
 b=Av1Y3shEepVs1TcQ3YMvzcRZ9vOZQTy4Ib2LT8Vg2a4XBAhSvAJEFaDo+Orga4nbf
 2hEzsxlKbC1UfDirH7Kry2VfN8NrqaFTlY8ujSbp/zhgJiqffKE2ZjLXubOKCVZCW9
 yGFYIKIhI5WpHiAUKNzpKnTTAQpSNycOYKdaZpb3Yao6LuGEi3virJkDDMK06idZMP
 7u0ZEN9fKycaIHnRXF6J2DPbYjMzka/yLyn7tW3wPHhguxImnyp3KU6nzFmEfZSxjv
 y1lhVWkPFEWFwN07HBJuLylQqsUEEey7pyKupxGk0WLFTOwVkBHlCWnxpdIqfM1LHB
 eEmH/7T8nUxR2vmdYrHMLvUnePRWF52NpQjFLoEGaF6FmIDYCX6ODnbR14T7SqDmIe
 ycsUjJK/9spVcP+iJrD3t6G9eJvlNr6VZrpnBUqOYG4VyqDtx1WP3x60zG/0kE7iiq
 Gptw8OBUVZgKVYJi7I/VhL7IP91Y+daMaRW/DI7K56ElPTf7X+mem+hmfbygp0Wg/P
 ipblJjidqrbYsmHRpNZ0o6TEWcNHuy+EnO/PHPVENVBmZKEu0t/c9LGbch7u9UC34e
 8e4Vxc6cN55W7y+VFV6v/2cfTen/aj7JlP7VHcmXw6cN+IlaqspX4qnZayrgKspmw7
 r3TwtDRDdtUzIzVuuTpWsv+c=
Received: from localhost (localhost [local])
 by localhost (OpenSMTPD) with ESMTPA id b10323c4;
 Sun, 21 May 2023 21:54:33 +0000 (UTC)
Date: Sun, 21 May 2023 23:54:33 +0200
From: wolf <wolf@wolfsden.cz>
To: Timo Wilken <guix@twilken.net>
Subject: Re: [PATCH] import: go: Handle subpackage versioning correctly.
Message-ID: <ZGqTGVrlJsLi9hxW@ws>
References: <6dd1de3dd4d968876fa55f5126056834c77b0244.1684703258.git.guix@twilken.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
 protocol="application/pgp-signature"; boundary="iw7hpULjExUreuIc"
Content-Disposition: inline
In-Reply-To: <6dd1de3dd4d968876fa55f5126056834c77b0244.1684703258.git.guix@twilken.net>
Received-SPF: none client-ip=37.205.8.62; envelope-from=ws@wolfsnet.cz;
 helo=wolfsden.cz
X-Spam_score_int: -17
X-Spam_score: -1.8
X-Spam_bar: -
X-Spam_report: (-1.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HEADER_FROM_DIFFERENT_DOMAINS=0.25,
 SPF_HELO_PASS=-0.001, SPF_NONE=0.001, T_SCC_BODY_TEXT_LINE=-0.01,
 UNPARSEABLE_RELAY=0.001 autolearn=no autolearn_force=no
X-Spam_action: no action
X-Spam-Score: -2.0 (--)
X-Debbugs-Envelope-To: submit
Cc: 54097@debbugs.gnu.org, guix-patches@gnu.org
X-BeenThere: debbugs-submit@debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request@debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit@debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request@debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request@debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces@debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
X-Spam-Score: -3.0 (---)

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

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

Send a report that this bug log contains spam.