diff mbox series

[bug#69827,1/3] build-system/go: Add subdir parameter to go-version->git-ref.

Message ID bfb6cb47bc6e2a05846653ca0b07470d4ae8ae05.1710584715.git.cdo@mutix.org
State New
Headers show
Series [bug#69827,1/3] build-system/go: Add subdir parameter to go-version->git-ref. | expand

Commit Message

Christina O'Donnell March 16, 2024, 10:26 a.m. UTC
* guix/build-system/go.scm (go-version->git-ref): Add subdir keyword
parameter. This is needed because Go can have mutliple modules at different
versions in a single repo. It distinguishes their releases by using tags
with their subdirectory. See https://go.dev/ref/mod#vcs-version.

Change-Id: I68bc9e785e49877bb0b756de8458308549f4c957
---
 guix/build-system/go.scm | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)


base-commit: efc0ee1d7f2b704d3fc0c8aea0ef0ad1ac2972e1

Comments

Christina O'Donnell March 16, 2024, 10:45 a.m. UTC | #1
Hi,

These three patches above allow the go importer to automatically import 
modules that are located in a monorepo where the submodules are 
versioned independently. See https://go.dev/ref/mod#vcs-version:

 > If a module is defined in a subdirectory within the repository, that 
is, the module subdirectory portion of the module path is not empty, 
then each tag name must be prefixed with the module subdirectory, 
followed by a slash. For example, the module golang.org/x/tools/gopls is 
defined in the gopls subdirectory of the repository with root path 
golang.org/x/tools. The version v0.4.0 of that module must have the tag 
named gopls/v0.4.0 in that repository.

After this change, I am able to import github.com/gohugio/hugo and its 
400 odd dependencies with much fewer failures.

I'll submit those patches separately once I've tested them.

Kind regards,

Christina

PS. This is my first time sending a patch series to debbugs, so I almost 
certainly did something wrong. Feedback on that would be appreciated.
Sharlatan Hellseher April 16, 2024, 7:05 p.m. UTC | #2
Hi,

Just a clarifying question. Do we definitely need to know
 anything about the folder structure where go.mod governs the
 module definition?

There is an API https://pkg.go.dev/  where the most of go projects
 are registered, would it be easier just query it directly?

Thanks,
Oleg
Christina O'Donnell May 11, 2024, 12:04 p.m. UTC | #3
Hi Oleg,

Thanks for having a look at this!

I don't believe you copied me in the last email so I didn't see your 
email when it went out.

 > Just a clarifying question. Do we definitely need to know
 > anything about the folder structure where go.mod governs the
 > module definition?

 > There is an API https://pkg.go.dev/ where the most of go projects
 > are registered, would it be easier just query it directly?

pkg.go.dev gets its sources from proxy.golang.org, and pkg.go.dev 
focuses mainly on building and hosting documentation [1]. It doesn't 
expose an API for fetching version info [2].

We're already querying proxy.golang.org to get the latest version, and 
that response contains information about the sub-directory and git tag, 
so we may as well use it.

Kind regards,

Christina

[1] https://pkg.go.dev/about
[2] https://pkg.go.dev/cmd/api
Sharlatan Hellseher Sept. 4, 2024, 7:18 p.m. UTC | #4
Hi,

It's time to proper review your proposal :-)

I faced with obstacles of monorepo during review  process, especially where
nested path (subdir) has more than one level.

Let's have look at https://github.com/Azure/azure-sdk-for-go
which is required for restic update.

I will apply your patches and play with importing/upgrading projects like
restic to check how it feels.

Related issues:
- https://issues.guix.gnu.org/72963, requires
https://github.com/charmbracelet/x - monorepo
- https://issues.guix.gnu.org/63019, requires
https://github.com/Azure/azure-sdk-for-go - monorepo
- updating aws-sdk-go-v2 https://github.com/aws/aws-sdk-go-v2

Thanks,
Oleg
Sharlatan Hellseher Sept. 7, 2024, 2:28 p.m. UTC | #5
Hi,

Some feed back on the patches: when applied on go-team I could easily
import the full Restic dependencies tree it took about 30min to complete
as Guix missed nearly all packages for google cloud.

Mean while it was handy to use in IPFS/Kubo unbundling process where me
and Artyom in our leisure time try to pack all vendored dependencies.

The import of https://github.com/libp2p/go-libp2p recursivly produced
all 26 missing packages which I about to submit soon!

As a side note, it's not the first proposal of that kind, please see:

- <2021-12-07> guix import go error <https://issues.guix.gnu.org/52362>
  by Stephen Webber <montokapro@gmail.com>

- <2023-04-21> Go importer doesn't know MODULE/vX.Y version tags
  <https://issues.guix.gnu.org/63001> by Timo Wilken <guix@twilken.net>

- <2023-05-22> [PATCH 0/2] Fix annoyance with "guix import go"
  <https://issues.guix.gnu.org/63647> by Simon Tournier
  <zimon.toutoune@gmail.com>

- <2023-06-12> [PATH] fix a bug on importing go packages.
  <https://issues.guix.gnu.org/64035>,
  <https://issues.guix.gnu.org/64036> by Elbek


May we come to some consensus and apply some median patch including all
proposals and fixing the currently issue with import golang modules with
subdirs?

--
Oleg
Simon Tournier Sept. 10, 2024, 12:21 p.m. UTC | #6
Hi Oleg,

Thanks for taking care about the unmerged patches. :-)


On Sat, 07 Sep 2024 at 15:28, Sharlatan Hellseher <sharlatanus@gmail.com> wrote:

> - <2023-05-22> [PATCH 0/2] Fix annoyance with "guix import go"
>   <https://issues.guix.gnu.org/63647> by Simon Tournier
>   <zimon.toutoune@gmail.com>

[...]

> May we come to some consensus and apply some median patch including all
> proposals and fixing the currently issue with import golang modules with
> subdirs?

Well, from my perspective, patch#63647 was ready but since I am not
really using “guix import go”, I do not feel qualified to know what is
better: error out at the first Git issue and continue with a wrong hash.

Somehow, what appears to you the better answer of:

        I thought it would be more appropriate to error out as soon as we have a
        Git problem, rather than print a warning and emit an incorrect hash.

        I understand that, when using ‘--recursive’, this means aborting the
        whole process without producing anything.  But maybe that’s better that
        producing an incorrect (origin …) form?

My intuition is not to abort the whole process and instead get something
where I manually fix these incorrect hashes.  When I had to do it, it
had been far easier and faster than looping over “guix import go”.

In other words, “guix import go foo -r” and paste the output in one
file, then run “guix build foo” and fix the incorrect hash origins or
some other package tweaks.

I have not looked into the other patches; if they fix similar issues,

Cheers,
simon


1: [bug#63647] [PATCH 0/2] Fix annoyance with "guix import go"
Ludovic Courtès <ludo@gnu.org>
Tue, 06 Jun 2023 17:42:00 +0200
id:87zg5c61iv.fsf@gnu.org
https://issues.guix.gnu.org/63647
https://issues.guix.gnu.org/msgid/87zg5c61iv.fsf@gnu.org
https://yhetil.org/guix/87zg5c61iv.fsf@gnu.org
Sharlatan Hellseher Sept. 15, 2024, 8:40 p.m. UTC | #7
Hi,

After a few days of testing with importing, building and updating Golang
packages I can say it feels very stable with patches proposed by
Christiana - there was no single import failure on such a long chain of
imports like for go-github-com-spf13-afero (209 packages+),
go-github-com-ipfs-boxo (231 package+).

I've used it as a current implementation to fix mentioned issues with
guix import go.

Pushed as 45cbf468d2..23dbf8d073 to go-team.

The branch contains the fresh versions of golang-build packages which
might need to rebuild most of the Goalang containing in Guix. But it
would let us to bring Prometheus soon, most of the components a also on
the branch.

--
Oleg
diff mbox series

Patch

diff --git a/guix/build-system/go.scm b/guix/build-system/go.scm
index 0934fded07..94c5439dd1 100644
--- a/guix/build-system/go.scm
+++ b/guix/build-system/go.scm
@@ -56,11 +56,12 @@  (define %go-pseudo-version-rx
                 "([0-9A-Fa-f]{12})"            ;commit hash
                 "(\\+incompatible)?$")))       ;optional +incompatible tag
 
-(define (go-version->git-ref version)
+(define* (go-version->git-ref version #:key subdir)
   "Parse VERSION, a \"pseudo-version\" as defined at
-<https://golang.org/ref/mod#pseudo-versions>, and extract the commit hash from
-it, defaulting to full VERSION (stripped from the \"+incompatible\" suffix if
-present) if a pseudo-version pattern is not recognized."
+<https://golang.org/ref/mod#pseudo-versions>, and extract the commit hash from it,
+defaulting to full VERSION (stripped from the \"+incompatible\" suffix if present) if
+a pseudo-version pattern is not recognized.  If SUBDIR is specified and this is not a
+pseudo-version, then this will prefix SUBDIR/ to the returned tag."
   ;; A module version like v1.2.3 is introduced by tagging a revision in the
   ;; underlying source repository.  Untagged revisions can be referred to
   ;; using a "pseudo-version" like v0.0.0-yyyymmddhhmmss-abcdefabcdef, where
@@ -78,7 +79,9 @@  (define (go-version->git-ref version)
          (match (regexp-exec %go-pseudo-version-rx version)))
     (if match
         (match:substring match 2)
-        version)))
+        (if subdir
+            (string-append subdir "/" version)
+            version))))
 
 (define (go-pseudo-version? version)
   "True if VERSION is a Go pseudo-version, i.e., a version string made of a