GNU bug report logs - #49602
[PATCH] import: go: Upgrade go.mod parser.

Previous Next

Full log

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

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 49602 <at> debbugs.gnu.org
Subject: Re: bug#49602: [PATCH] import: go: Upgrade go.mod parser.
Date: Sun, 18 Jul 2021 20:15:03 -0700

Hello,

Thanks for the review :) I just knew I wouldn't get that long commit
message right the first time!

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

>> (%go.mod-require-directive-rx)
>
> The above line can be removed, as it is duplicated below.
>
>> (%go.mod-replace-directive-rx): Remove unused variable.

Those are actually two distinct variables ("...-require-..." vs
"...-replace-..."), though I should've written "variables."

>> +  (define-peg-pattern string-or-ident body
>                                    ^
> Perhaps prefer the fully spelled out 'string-or-identifier' variable name.
>

Agree.

>> +(define (go.mod-requirements go.mod)
>> +  "Compute and return the list of requirements specified by GO.MOD."
>> +  (define (replace directive requirements)
>> +    (define (maybe-replace module-path new-requirement)
>> +      ;; Do not allow version updates for indirect dependencies
>> +      ;; TODO: Is this correct behavior? It's in the go.mod for a
>> reason...
>
> According to [1], it seems that yes: "replace directives only apply in
> the main module's go.mod file and are ignored in other modules.", IIUC.
>
> [1]  https://golang.org/ref/mod#go-mod-file-replace
>

My read of it is that if module A requires module B, replace directives
in B's go.mod are ignored, but A's go.mod can replace any module, direct
or indirect. For example, if module B hasn't been updated, but the
version of module C it depends on has a bug that affects it, module A
can use a replace in it's go.mod without requiring an upstream update of
module B. To be fair, this is usually handled by specifying the indirect
dependency with a specific version as a requirement in module A's
go.mod, but the replace should be valid as well. 

The reason it was skipped before, I think (if it was intentional), is
that since we only have the one version of a module in Guix at a time,
it's not necessary to make the indirect dependency explicitly an input,
so don't include it. On the other hand, if it *was* used to avoid a bug
in a version used by an indirect dependency, wouldn't we want to make
sure the Guix package was the fixed version? This is why I was
questioning whether leaving it out was correct. 

> Pushed with commit 793ba333c6, after fixing up some indents and
> substituting the TODO for an explanatory comment with a reference to the
> replace directive, and a few more cosmetic changes.

Thanks for the final touches.

> Thanks for the continued improvements to the Go importer :-).

Happy to help :) Originally, I was just trying to package a Go program,
and look what happened...

--
Sarah

Previous Next

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