Hi Ludo', (hi everyone,) On Wed Jun 14, 2023 at 11:09 PM CEST, Ludovic Courtès wrote: > Timo Wilken 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 . 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