x/exp/cmd/gorelease: cannot process package _test files #44440

carnott-snap · 2021-02-19T23:26:16Z

When I add a test file like:

package my_test
//...

in a sample module:

module github.com/user/repo
// ...

gorelease now starts failing:

$ make gorelease
github.com/user/repo
------------------------------------
Incompatible changes:
- ErrXxx: removed
- Yyy: removed

Inferred base version: v1.2.1
go.mod: the following requirements are needed
        github.com/user/repo@v1.1.0
Run 'go mod tidy' to add missing requirements.
make: *** [Makefile:104: gorelease] Error 1

I can confirm that I did not remove Yyy or ErrXxx, and that the the rest of the toolchain can find them, e.g. go doc Yyy. This may end up being a bug in apidiff, but I have not dug into the root cause.

The text was updated successfully, but these errors were encountered:

carnott-snap · 2021-03-02T06:49:56Z

if you want a repro; iiuc, the Incompatible changes block from my redacted example is just fallout from the root cause, thus it is not in the repro.

carnott-snap · 2021-03-03T18:45:34Z

cc @jayconrod

jayconrod · 2021-04-06T14:16:10Z

Not sure what the cause is, but I was able to reproduce this at d352d2db2ceb4c0411268ee2de764e3f82e27280.

The message comes from gorelease itself when loading packages, and it happens with -base=none, so I don't think apidiff is involved.

cc @jadekler

jeanbza · 2021-04-07T14:58:26Z

(will also try to debug this friday if nobody gets there before me!)

jeanbza · 2021-04-10T01:59:46Z

I did some debugging and found the cause: cyclic imports are not considered in prepareLoadDir's go.mod / go.sum comparison sections. The following hacky change fixes the issue:

deklerk at deklerk1 in ~/workspace/exp/cmd/gorelease on master*
$ git diff
diff --git a/cmd/gorelease/gorelease.go b/cmd/gorelease/gorelease.go
index 3e09351..54bdac7 100644
--- a/cmd/gorelease/gorelease.go
+++ b/cmd/gorelease/gorelease.go
@@ -80,6 +80,7 @@
 package main

 import (
+       "bufio"
        "bytes"
        "encoding/json"
        "errors"
@@ -1018,6 +1019,9 @@ func prepareLoadDir(modFile *modfile.File, modPath, modRoot, version string, cac
                }
                lines := make([]string, len(modFile.Require))
                for i, req := range modFile.Require {
+                       if req.Mod.Path == modPath {
+                               continue
+                       }
                        lines[i] = req.Mod.String()
                }
                sort.Strings(lines)
@@ -1068,7 +1072,24 @@ func prepareLoadDir(modFile *modfile.File, modPath, modRoot, version string, cac
                        // "empty go.sum".
                }

-               if bytes.Compare(goSumData, newGoSumData) != 0 {
+               var oldSum, newSum string
+               scanner := bufio.NewScanner(bytes.NewReader(goSumData))
+               for scanner.Scan() {
+                       line := scanner.Text()
+                       if strings.Contains(line, modPath) {
+                               continue
+                       }
+                       oldSum += line
+               }
+               scanner = bufio.NewScanner(bytes.NewReader(newGoSumData))
+               for scanner.Scan() {
+                       line := scanner.Text()
+                       if strings.Contains(line, modPath) {
+                               continue
+                       }
+                       newSum += line
+               }
+               if newSum != oldSum {
                        diagnostics = append(diagnostics, "go.sum: one or more sums are missing. Run 'go mod tidy' to add missing sums.")
                }
        }

I actually ran into this independently in https://go-review.googlesource.com/c/exp/+/288032, funnily enough.

Why is this happening:

In the provided repro, there's a module cycle:

$ go mod graph
gist.github.com/c2a4204c76b6ee2956c0f3c855e593d4.git gist.github.com/ac489642a42c01530c15516160048aea.git@v1.0.0
gist.github.com/c2a4204c76b6ee2956c0f3c855e593d4.git golang.org/x/exp@v0.0.0-20210220032938-85be41e4509f
gist.github.com/ac489642a42c01530c15516160048aea.git@v1.0.0 gist.github.com/c2a4204c76b6ee2956c0f3c855e593d4.git@v1.0.0
[...]

So, in copyModuleToTempDir, we create a temp version of module gist.github.com/c2a4204c76b6ee2956c0f3c855e593d4.git with version v0.0.0-gorelease (here.

Then later in prepareLoadDir, we depend on this v0.0.0-gorelease version (here and here).

Well, when we run go get -d ., it upgrades the v0.0.0-gorelease to v1.0.0, presumably because that's the latest version in the mod graph cycle. (maybe different reason? that's my guess) (see here)

So anyways, the code above basically ignores anything with the same modPath as the base module when doing the go.mod and go.sum missing checks.

I'll work on getting a test out for this, and a better solution.

@jayconrod , let me know if you can think of a nicer way to tackle this than my proposal above. It's late on a Friday so I may be missing something obvious. :)

gopherbot · 2021-04-15T03:55:44Z

Change https://golang.org/cl/310369 mentions this issue: cmd/gorelease: skip circular imports when inspect go.sum and go.mod differences

gopherbot · 2021-04-16T16:46:07Z

Change https://golang.org/cl/310809 mentions this issue: cmd/gorelease: skip circular imports when inspect go.sum and go.mod differences

jeanbza · 2021-04-30T22:21:13Z

I appear unable to reproduce this in the latest version of gorelease:

cd $(mktemp -d)
go mod init gist.github.com/ac489642a42c01530c15516160048aea.git
printf "package pkg\nimport \"gist.github.com/c2a4204c76b6ee2956c0f3c855e593d4.git\"\nconst Empty = c2a4204c76b6ee2956c0f3c855e593d4.Empty" > foo.go
go mod tidy
gorelease

(whereas before, this produced a missing go.sum entry diagnostic)

Furthermore, the test cases I introduced in https://go-review.googlesource.com/c/exp/+/310369 no longer fail, after having the CL rebased to head.

edit: On second inspection, the tests-passing might be related to #45892. Should have an answer soon, but for now would trust real world experimentation more.

@carnott-snap , could you confirm that you still see this issue after upgrading to the latest gorelease?

jeanbza · 2021-05-07T17:53:04Z

Another update: after https://go-review.googlesource.com/c/exp/+/318029, I am now able to see test errors again. Will continue working on this.

gopherbot added this to the Unreleased milestone Feb 19, 2021

seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 24, 2021

jayconrod added modules Tools This label describes issues relating to any tools in the x/tools repository. labels Apr 6, 2021

jeanbza self-assigned this Apr 10, 2021

gopherbot closed this as completed in golang/exp@2df949a May 10, 2021

golang locked and limited conversation to collaborators May 10, 2022

gopherbot added the FrozenDueToAge label May 10, 2022

rsc unassigned jeanbza Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/exp/cmd/gorelease: cannot process package _test files #44440

x/exp/cmd/gorelease: cannot process package _test files #44440

carnott-snap commented Feb 19, 2021

carnott-snap commented Mar 2, 2021 •

edited

carnott-snap commented Mar 3, 2021

jayconrod commented Apr 6, 2021

jeanbza commented Apr 7, 2021

jeanbza commented Apr 10, 2021

gopherbot commented Apr 15, 2021

gopherbot commented Apr 16, 2021

jeanbza commented Apr 30, 2021 •

edited

jeanbza commented May 7, 2021

x/exp/cmd/gorelease: cannot process package _test files #44440

x/exp/cmd/gorelease: cannot process package _test files #44440

Comments

carnott-snap commented Feb 19, 2021

carnott-snap commented Mar 2, 2021 • edited

carnott-snap commented Mar 3, 2021

jayconrod commented Apr 6, 2021

jeanbza commented Apr 7, 2021

jeanbza commented Apr 10, 2021

gopherbot commented Apr 15, 2021

gopherbot commented Apr 16, 2021

jeanbza commented Apr 30, 2021 • edited

jeanbza commented May 7, 2021

carnott-snap commented Mar 2, 2021 •

edited

jeanbza commented Apr 30, 2021 •

edited