Go: vendor, cmd/vendor: not clean on master

[dmitshur]

On latest master branch right now, the src/vendor directory is not clean. Its contents do not correspond 1:1 to the versions specified by the src/go.mod file.

Following the steps in src/README.vendor results in a non-zero diff:

go $ go version
go version devel +b13ce14c4a Tue Jan 28 20:26:36 2020 +0000 darwin/amd64
go $ cd src
src $ go mod tidy
src $ go mod vendor
src $ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    deleted:    vendor/golang.org/x/sys/cpu/cpu_riscv64.go

no changes added to commit (use "git add" and/or "git commit -a")

This is due to recent commits such as CL 216260, CL 216261, CL 216262 that are related to riscv64.

This makes it difficult and error prone to make changes to versions in src/go.mod, because the end result includes unexpected changes. See discussion in CL 216680.

If it's reasonable to resolve this, I don't think we should release Go 1.14 with a vendor directory that isn't clean, so tentatively marking this as release-blocker, but open to discussion.

/cc @4a6f656c @FiloSottile @golang/osp-team

[bcmills]

If there are other x/sys changes that we don't want to fold in at this point, one low-risk option would be to commit an x/sys branch that is the same as the current commit but adds the new risc-v file, then update to that commit.

[bcmills]

I noticed today that src/cmd/vendor is not clean either. (It includes changes in both x/sys and x/tools).

[dmitshur]

/cc @bcmills Do you think we should block Go 1.14 RC 1 on this? This is one of the potential blockers we are reviewing in the release meeting.

[bcmills]

I think it is important to at least decide _how_ we will address this issue before we release RC1.

There appear to have been a large number of changes in x/sys/cpu and x/sys/unix in particular since those dependencies were last updated in the main repo.

The options that I see are:

1. Release Go 1.14 with non-clean vendor directories.

2. Upgrade the dependencies in question to some commit that covers the changed files (golang.org/x/sys v0.0.0-20200106114638-5f8ca72cd632 for CL 213397 and golang.org/x/tools v0.0.0-20200124162111-628e9aa307e6 for CL 216337).

3. Publish minimal patches in an off-master branch to the sys and tools repos and update the std and cmd dependencies to point to the patched commits.

I don't particularly like option (1). Option (2) seems cleanest from a process perspective, but option (3) seems less risky.

I think only option (2) would need to gate an RC.

[dmitshur]

Thanks @bcmills.

If we release Go 1.14 with non-clean vendor directories, it will have an effect on all future Go 1.14.x minor releases (and security releases). It will increase the difficulty and risk when backporting future fixes to the release-branch.go1.14 branch of the main Go repo.

We'll either need to resolve this issue soon after releasing Go 1.14 (before Go 1.14.1, etc.), or bear that cost for the rest of the time that Go 1.14 release is supported (at least 1 year). If we're going to have to fix it anyway, it seems better and easier to do so before releasing Go 1.14.

As a result, I believe we should not go with option 1.

[bcmills]

Thinking about future backports, we will probably want a release-branch.go1.14 for the x/sys and x/tools repos anyway. That would fit pretty naturally with option (3), where the off-master branch is simply release-branch.go1.14.

(It's unfortunate to cut the release branch from such old base commits, but it seems a bit late to correct that particular mistake now.)

[FiloSottile]

I agree that this close to the RC the best option is (2) with a new release branch.

That's what x/ repo release branches are for anyway, IMHO. See #36882.

[gopherbot]

Change https://golang.org/cl/217103 mentions this issue: golang.org/x/sys: update to v0.0.0-20200106114638-5f8ca72cd632

[ianlancetaylor]

Just to see what would happen I tried updating x/sys in the main vendor directory.

I'm also fine with creating special purpose branches in x/sys and friends.

[bcmills]

From what I can tell, updating x/sys in the std module wouldn't be terrible — there are a few potentially-unnecessary changes, but nothing huge.

The x/sys/unix and x/tools changes that would be pulled into the cmd module, on the other hand, are fairly extensive.

[FiloSottile]

Are the std and cmd versions in sync, ora can they at least be brought in sync with little damage? It would be pretty annoying to have two release branches, one for std and one for cmd.

[bcmills]

std doesn't depend on x/tools at all, and it's less than a month ahead on x/sys. So I think it should be fine to upgrade x/sys in cmd to the same baseline as what is already in std.

[gopherbot]

Change https://golang.org/cl/217060 mentions this issue: cmd: update x/sys version to match std

[ianlancetaylor]

I folded the cmd update into my CL.

There are quite a few changes. They are probably fine, but I guess I would also be more comfortable with the branch approach. Does someone want to work on that? I'm not too sure how to do it myself.

[bcmills]

I sent CL 217060 to do the minimal update to get std and cmd on the same baseline. It's still a bigger diff than I expected, though.

[FiloSottile]

I can do the branching, cherry-picking and vendoring, once we resolve how to get std and cmd in sync.

[gopherbot]

Change https://golang.org/cl/217118 mentions this issue: cmd/go.mod: sync x/crypto with std

[4a6f656c]

(1) obviously has more implications/fallout than planned, (2) was avoided since it potentially creates churn/breakage close to release, (3) makes sense - if someone can create the branches and/or provide names for these (we need x/sys for cpu/cpu_riscv64.go and unix/asm_linux_riscv64.s, and x/tools for go/analysis/passes/asmdecl), I can readily handle the code changes (or I can review them).

[4a6f656c]

It's also worth noting that x/sys is not in sync across vendor and cmd/vendor:

golang.org/x/sys v0.0.0-20190529130038-5219a1e1c5f8 // indirect
golang.org/x/sys v0.0.0-20190502175342-a43fa875dd82 // indirect

Hopefully we can update cmd/vendor to 5219a1e1c5f8 to avoid having to have multiple branches here...

[gopherbot]

Change https://golang.org/cl/217218 mentions this issue: cmd/internal/moddeps: check for consistent versioning among all modules in GOROOT

[FiloSottile]

This is the current state of the std and cmd modules. Only the vendored version matter because the other modules don't provide any packages that might need backporting.

$ grep '# golang.org/x/' vendor/modules.txt
# golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d
# golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933
# golang.org/x/sys v0.0.0-20190529130038-5219a1e1c5f8
# golang.org/x/text v0.3.3-0.20191031172631-4b67af870c6f

$ grep '# golang.org/x/' cmd/vendor/modules.txt
# golang.org/x/arch v0.0.0-20190815191158-8a70ba74b3a1
# golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d
# golang.org/x/mod v0.2.0
# golang.org/x/sys v0.0.0-20190502175342-a43fa875dd82
# golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e
# golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898

I created the following branches:

• x/crypto release-branch.go1.14 at 69ecbb4d6d5d
• x/net release-branch.go1.14 at ef20fe5d7933
• x/sys release-branch.go1.14-std at 5219a1e1c5f8
• x/sys release-branch.go1.14-cmd at a43fa875dd82
• x/text release-branch.go1.14 at 4b67af870c6f
• x/arch release-branch.go1.14 at 8a70ba74b3a1
• x/mod release-branch.go1.14 at ed3ec21bb8e
• x/tools release-branch.go1.14 at 298f0cb1881e
• x/xerrors release-branch.go1.14 at 1b5146add898

Next I made the cherry-picks that had been applied manually:

• https://go-review.googlesource.com/c/sys/+/217300
• https://go-review.googlesource.com/c/sys/+/217339
• https://go-review.googlesource.com/c/tools/+/217301

Once those are merged I will update the go.mods and hopefully the vendor will be clean.

[ianlancetaylor]

Thanks @FiloSottile !

[gopherbot]

Change https://golang.org/cl/217305 mentions this issue: std,cmd: sync go.mod with new release branches

[gopherbot]

Change https://golang.org/cl/217306 mentions this issue: [release-branch.go1.14-std] cpu: restore doinit() in cpu_riscv64.go

[gopherbot]

Change https://golang.org/cl/236600 mentions this issue: cmd/internal/moddeps: don't skip directories if there are unusual files