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

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

Received: (at 63631) by debbugs.gnu.org; 17 Jun 2023 15:13:55 +0000
From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 17 11:13:55 2023
Received: from localhost ([127.0.0.1]:52277 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
	id 1qAXcg-0000Ei-KO
	for submit@debbugs.gnu.org; Sat, 17 Jun 2023 11:13:55 -0400
Received: from mx2.mythic-beasts.com ([46.235.227.24]:35093)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <timo@twilken.net>)
 id 1qAXcc-0000EB-Cs; Sat, 17 Jun 2023 11:13:52 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=twilken.net
 ; s=mythic-beasts-k1; h=To:From:Subject:Date;
 bh=pLlAjfqsoe+3U3a73MQeHAaM/uSI+UrqGsrVeSKy1QM=; b=zI4HPa8dV46Hjnio0YDducSSwZ
 M29u2+iSJgjnWkngcWfT9SjYDe9ZLcVAF6i1Ndo75UO/OHkpFsGhXUIm1L8Fz/B5Y6Tg3zjvoVFmy
 d2/oqU7XiJvDOnOICpI3cx5UJO/54X/q5T9ko6ub0/iW9+KHQrST+7gIUbsLEFePPVSChBnKxzIj0
 6vl4AibdV6yP/bGdLIvjcfMXs4yCOg/+eCxfY8y0t1VVKYU/YVdRUrSzYPgPYv7nhW0qOEJZKfPL7
 6rRHFmKh51WiRPEO/oUchLlUi9MFuV/F8pPh+ZAd9D/fTIovUzVWesUglqbjMlC1UubueYwa/EPQC
 NdDeVMug==;
Received: by mailhub-hex-d.mythic-beasts.com with esmtpsa (TLS1.3) tls
 TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2)
 (envelope-from <timo@twilken.net>)
 id 1qAXcS-004VRH-W7; Sat, 17 Jun 2023 16:13:41 +0100
Content-Type: multipart/signed;
 boundary=dc3bfcd0ce40fc9a1a65ded3476be064f73442d113438739c89741eeba4a;
 micalg=pgp-sha256; protocol="application/pgp-signature"
Date: Sat, 17 Jun 2023 17:12:58 +0200
Subject: Re: bug#63631: [PATCH] import: go: Handle subpackage versioning
 correctly.
From: "Timo Wilken" <guix@twilken.net>
To: Ludovic Courtès <ludo@gnu.org>
Message-Id: <CTF06XBYWPT0.1MV6QA1B2OB98@lap.twilken.net>
X-Mailer: aerc 0.14.0
References: <6dd1de3dd4d968876fa55f5126056834c77b0244.1684703258.git.guix@twilken.net>
 <87pm5xrbsg.fsf@gnu.org>
In-Reply-To: <87pm5xrbsg.fsf@gnu.org>
X-BlackCat-Spam-Score: 16
X-Spam-Status: No, score=1.6
X-Spam-Score: 0.1 (/)
X-Debbugs-Envelope-To: 63631
Cc: 63631@debbugs.gnu.org, 64036@debbugs.gnu.org,
 Simon Tournier <zimon.toutoune@gmail.com>, 63647@debbugs.gnu.org,
 64035@debbugs.gnu.org, 63001@debbugs.gnu.org, 54097@debbugs.gnu.org,
 wolf@wolfsden.cz
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: -0.9 (/)

[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@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)]

Send a report that this bug log contains spam.