GNU bug report logs - #64035
[PATH] fix a bug on importing go packages.

Previous Next

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

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

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

From: Elbek <el <at> mathematica.uz>
To: "guix-patches" <guix-patches <at> gnu.org>
Subject: [PATH] fix a bug on importing go packages.
Date: Mon, 12 Jun 2023 23:18:23 +0500

Hello.

This patch addresses the issue https://issues.guix.gnu.org/63001

Currently "guix import go ..." looks to the repository depending on the version of a package.

For example, if package version is $1.11.0$, the importer tries to look to the repository's tag named v1.11.0,
but some packages are using tags that are prefixed by the module name.

For example, the package cloud.google.com/go/workflows use the tag workflows/v1.11.0

This patch changes go importer, such that it looks to the module-name prefixed tags if they exists, and behaves as before otherwise.

Message #8 received at 64035 <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)]

Message #11 received at 64035 <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

Previous Next

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