GNU bug report logs - #63001
Go importer doesn't know MODULE/vX.Y version tags

Previous Next

Package: guix;

Reported by: "Timo Wilken" <guix <at> twilken.net>

Date: Fri, 21 Apr 2023 17:41:01 UTC

Severity: normal

Done: Sharlatan Hellseher <sharlatanus <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


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

This bug report was last modified 115 days ago.

Previous Next


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