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.

Sign up for GitHub

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

Jump to bottom

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

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

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

go101 opened this issue Apr 20, 2021 · 6 comments
Labels
GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@go101
Copy link

go101 commented Apr 20, 2021 •
edited

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 seankhliao added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 20, 2021
@seankhliao
Copy link
Member

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

@bcmills
Copy link
Contributor

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.

@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 NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Apr 21, 2021
@bcmills bcmills added this to the Backlog milestone Apr 21, 2021
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 21, 2021
@liggitt
Copy link
Contributor

liggitt commented Apr 21, 2021 •
edited

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?

@liggitt
Copy link
Contributor

liggitt commented Apr 21, 2021 •
edited

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.

@bcmills
Copy link
Contributor

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.

@liggitt liggitt mentioned this issue Apr 21, 2021
Merged
@liggitt
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@liggitt @bcmills @gopherbot @seankhliao @go101