Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: diagnose packages listed in lexically-impossible modules in vendor/modules.txt #45649

Open
go101 opened this issue Apr 20, 2021 · 6 comments

Comments

@go101
Copy link

@go101 go101 commented Apr 20, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.3 linux/amd6

Does this issue reproduce with the latest release?

Yes

What did you do?

Moved from https://groups.google.com/u/1/g/golang-nuts/c/04MtZ96TPWo

I checked out the kubernetes project at tag v1.20.4.
Then run "go list -deps -json ./... > json.txt" in the project root.
In the produced json.txt file, I noticed that the module path for
two packages look not right. The path of a package should be
prefixed with its module path. But the two aren't. Bug?

The first:

{
    "Dir": "/home/myname/opensource/it-infrastructure/kubernetes/vendor/github.com/Azure/go-autorest/autorest/azure",
    "ImportPath": "github.com/Azure/go-autorest/autorest/azure",
    "Name": "azure",
    "Doc": "Package azure provides Azure-specific implementations used with AutoRest.",
    "Module": {
        "Path": "github.com/Azure/go-autorest/autorest/adal",
        "Replace": {
            "Path": "github.com/Azure/go-autorest/autorest/adal",
            "Version": "v0.9.5"
        }
    },
    ...

The second:

{
    "Dir": "/home/myname/opensource/it-infrastructure/kubernetes/vendor/cloud.google.com/go/compute/metadata",
    "ImportPath": "cloud.google.com/go/compute/metadata",
    "Name": "metadata",
    "Doc": "Package metadata provides access to Google Compute Engine (GCE) metadata and API service accounts.",
    "Module": {
        "Path": "cloud.google.com/go/bigquery",
        "Replace": {
            "Path": "cloud.google.com/go/bigquery",
            "Version": "v1.4.0"
        },
        "Indirect": true
    },
    ...

What did you expect to see?

The path of a package should be prefixed with the path of its containing module.

What did you see instead?

The paths of two 2 packages are not prefixed with their module paths.

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Apr 20, 2021

this seems to happen only with -mod=vendor (side note: tip reports updates to go.mod needed, disabled by -mod=vendor; to update it: go mod tidy, but tidy doesn't do anything)

cc @bcmills @jayconrod @matloob

Loading

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 21, 2021

If I understand correctly, the kubernetes repo is using something other than the go command to generate their go.mod and vendor/modules.txt files.

The go command maintains a number of invariants on the go.mod and vendor/modules.txt files and expects those invariants to hold. I would not be at all surprised if the k8s hack/update-vendor.sh is generating a malformed file (CC @liggitt).

Note that go1.16.3 mod vendor at that commit produces substantial changes in the vendor/modules.txt file:

~/src/k8s.io/kubernetes$ git status
HEAD detached at v1.20.4
nothing to commit, working tree clean

~/src/k8s.io/kubernetes$ go1.16.3 mod vendor
warning: ignoring symlink /usr/local/google/home/bcmills/src/k8s.io/kubernetes/cluster/gce/cos
warning: ignoring symlink /usr/local/google/home/bcmills/src/k8s.io/kubernetes/cluster/gce/custom
warning: ignoring symlink /usr/local/google/home/bcmills/src/k8s.io/kubernetes/cluster/gce/ubuntu

~/src/k8s.io/kubernetes$ git diff --stat vendor/modules.txt
 vendor/modules.txt | 780 ++++++++++++++++++++++++++++++++++++-------------------------------------
 1 file changed, 390 insertions(+), 390 deletions(-)

Perhaps we should validate that the vendor/modules.txt file obeys certain basic properties, like listing each package within a module that could lexically contain it. But if we detect a violation of those properties, the best we can do in -mod=vendor mode is to fail with a useful diagnostic.

Loading

@bcmills bcmills changed the title cmd/go: the modules of some packages look not right in the output of ""go list -deps -json" cmd/go: diagnose packages listed in lexically-impossible modules in vendor/modules.txt Apr 21, 2021
@bcmills bcmills added this to the Backlog milestone Apr 21, 2021
@liggitt
Copy link
Contributor

@liggitt liggitt commented Apr 21, 2021

Kubernetes uses go mod tidy and go mod vendor to generate the vendor/modules.txt, then sorts modules.txt to be deterministically ordered by module name.

At the time we adopted go modules (~go1.11-go1.12 timeframe), go mod vendor produced inconsistent output for vendor/modules.txt due to import order (different across platforms, and impacted by PRs that added files that changed import order but didn't affect overall content of the vendor directory). Those broke presubmit checks that required zero diff on the vendor folder.

Is the generated vendor/modules.txt file now guaranteed to be identical across platforms and independent of import order?

Loading

@liggitt
Copy link
Contributor

@liggitt liggitt commented Apr 21, 2021

oh, funny... Kubernetes merged support for modules on 4/3/2019 (kubernetes/kubernetes#74877), go started sorting on 4/30/2019 (https://go-review.googlesource.com/c/go/+/174527).

I'll happily drop our post-vendor sorting.

Loading

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 21, 2021

@liggitt, go mod vendor has been sorting packages within each module in vendor/modules.txt since CL 174527, which I believe was included in Go 1.13. As far as I can tell, the order of the modules themselves has always been sorted.

Loading

@liggitt
Copy link
Contributor

@liggitt liggitt commented Apr 21, 2021

As far as I can tell, the order of the modules themselves has always been sorted.

I think you're right, it was probably just package-level, which was still painful. Glad to clean that up.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants