vendor, cmd/vendor: not clean on master #36851

dmitshur · 2020-01-28T21:54:22Z

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

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

bcmills · 2020-01-28T22:07:32Z

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 · 2020-01-29T21:00:09Z

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

dmitshur · 2020-01-30T19:09:34Z

/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 · 2020-01-30T19:30:19Z

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:

Release Go 1.14 with non-clean vendor directories.
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).
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 · 2020-01-30T19:45:53Z

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 · 2020-01-30T19:52:25Z

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 · 2020-01-30T20:04:16Z

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 · 2020-01-30T20:06:21Z

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

ianlancetaylor · 2020-01-30T20:07:22Z

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 · 2020-01-30T20:09:59Z

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 · 2020-01-30T20:12:09Z

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 · 2020-01-30T20:16:18Z

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 · 2020-01-30T20:21:01Z

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

ianlancetaylor · 2020-01-30T20:24:19Z

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 · 2020-01-30T20:29:12Z

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 · 2020-01-30T20:45:36Z

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

gopherbot · 2020-01-30T22:20:50Z

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

go get golang.org/x/[email protected] go mod vendor git checkout -- vendor/golang.org/x/sys/unix/asm_linux_riscv64.s \ vendor/golang.org/x/tools/go/analysis/passes/asmdecl/asmdecl.go Updates #36851 Change-Id: I95c0584ede599f600da927a04f135fe64a85037e Reviewed-on: https://go-review.googlesource.com/c/go/+/217118 Run-TryBot: Filippo Valsorda <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>

4a6f656c · 2020-01-31T03:16:35Z

(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 · 2020-01-31T03:20:19Z

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 5219a1e to avoid having to have multiple branches here...

gopherbot · 2020-01-31T15:59:14Z

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

FiloSottile · 2020-01-31T22:43:23Z

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 5219a1e
x/sys release-branch.go1.14-cmd at a43fa87
x/text release-branch.go1.14 at 4b67af8
x/arch release-branch.go1.14 at 8a70ba74b3a1
x/mod release-branch.go1.14 at ed3ec21bb8e
x/tools release-branch.go1.14 at 298f0cb
x/xerrors release-branch.go1.14 at 1b5146add898

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

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

ianlancetaylor · 2020-01-31T23:16:05Z

Thanks @FiloSottile !

gopherbot · 2020-01-31T23:45:13Z

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

gopherbot · 2020-01-31T23:49:32Z

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

We cherry-picked this file to this branch which doesn't have CL 206859. Rather than cherry-picking that CL too, add the doinit() function to match what's currently in the vendor tree. See also this comment on the go tree CL. https://go-review.googlesource.com/c/go/+/216261/3#message-97b8f6687e7cfebc0403a35af8930a4e8948dff0 Updates golang/go#36851 Change-Id: Iddc40e989e56ea934e0aae203108373d5f90e92c Reviewed-on: https://go-review.googlesource.com/c/sys/+/217306 Run-TryBot: Filippo Valsorda <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>

…es in GOROOT Updates #36851 Fixes #36907 Change-Id: I29627729d916e3b8132d46cf458ba856ffb0beeb Reviewed-on: https://go-review.googlesource.com/c/go/+/217218 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]>

gopherbot · 2020-06-04T23:44:37Z

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

Previously, if there was a non-directory file with the name vendor or testdata in the Go source tree, it was possible for some directories to be skipped by filepath.Walk performed in findGorootModules. As unusual and unlikely as such non-directory files are, it's better to ensure all directories are visited, and all modules in the GOROOT source tree are found. This increases confidence that tests relying on findGorootModule will not have unexpected false negatives. For #36851. For #36907. Change-Id: I468e80d8f57119e2c72d546b3fd1e23c31fd6e6c Reviewed-on: https://go-review.googlesource.com/c/go/+/236600 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>

dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Jan 28, 2020

dmitshur added this to the Go1.14 milestone Jan 28, 2020

dmitshur mentioned this issue Jan 28, 2020

std,cmd: add test to ensure that go.mod and vendor are in sync #36852

Closed

bcmills changed the title ~~src/vendor: not clean on master~~ vendor, cmd/vendor: not clean on master Jan 29, 2020

FiloSottile mentioned this issue Jan 30, 2020

x/build: use new name for release branches in golang.org/x repos starting with Go 1.16 #36882

Closed

bcmills mentioned this issue Jan 30, 2020

all: update standard-library dependencies at the start and end of each development cycle #36905

Open

bcmills mentioned this issue Jan 30, 2020

std,cmd: add a unit test to ensure that src/go.mod and src/cmd/go.mod are in sync #36907

Closed

gopherbot closed this as completed in 866920a Feb 1, 2020

sthagen mentioned this issue Feb 2, 2020

std,cmd: sync go.mod with new release branches sthagen/golang-go#138

Merged

dmitshur mentioned this issue Oct 21, 2020

x/build/cmd/coordinator, x/build/maintner/maintnerd/maintapi: golang.org/x repo trybot on custom release branch should use corresponding Go branch #42127

Closed

golang locked and limited conversation to collaborators Jun 4, 2021

gopherbot added the FrozenDueToAge label Jun 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vendor, cmd/vendor: not clean on master #36851

vendor, cmd/vendor: not clean on master #36851

dmitshur commented Jan 28, 2020

bcmills commented Jan 28, 2020

bcmills commented Jan 29, 2020 •

edited

Loading

dmitshur commented Jan 30, 2020 •

edited

Loading

bcmills commented Jan 30, 2020

dmitshur commented Jan 30, 2020 •

edited

Loading

bcmills commented Jan 30, 2020

FiloSottile commented Jan 30, 2020

gopherbot commented Jan 30, 2020

ianlancetaylor commented Jan 30, 2020

bcmills commented Jan 30, 2020

FiloSottile commented Jan 30, 2020

bcmills commented Jan 30, 2020

gopherbot commented Jan 30, 2020

ianlancetaylor commented Jan 30, 2020 •

edited

Loading

bcmills commented Jan 30, 2020

FiloSottile commented Jan 30, 2020

gopherbot commented Jan 30, 2020

4a6f656c commented Jan 31, 2020

4a6f656c commented Jan 31, 2020 •

edited

Loading

gopherbot commented Jan 31, 2020

FiloSottile commented Jan 31, 2020

ianlancetaylor commented Jan 31, 2020

gopherbot commented Jan 31, 2020

gopherbot commented Jan 31, 2020

gopherbot commented Jun 4, 2020

vendor, cmd/vendor: not clean on master #36851

vendor, cmd/vendor: not clean on master #36851

Comments

dmitshur commented Jan 28, 2020

bcmills commented Jan 28, 2020

bcmills commented Jan 29, 2020 • edited Loading

dmitshur commented Jan 30, 2020 • edited Loading

bcmills commented Jan 30, 2020

dmitshur commented Jan 30, 2020 • edited Loading

bcmills commented Jan 30, 2020

FiloSottile commented Jan 30, 2020

gopherbot commented Jan 30, 2020

ianlancetaylor commented Jan 30, 2020

bcmills commented Jan 30, 2020

FiloSottile commented Jan 30, 2020

bcmills commented Jan 30, 2020

gopherbot commented Jan 30, 2020

ianlancetaylor commented Jan 30, 2020 • edited Loading

bcmills commented Jan 30, 2020

FiloSottile commented Jan 30, 2020

gopherbot commented Jan 30, 2020

4a6f656c commented Jan 31, 2020

4a6f656c commented Jan 31, 2020 • edited Loading

gopherbot commented Jan 31, 2020

FiloSottile commented Jan 31, 2020

ianlancetaylor commented Jan 31, 2020

gopherbot commented Jan 31, 2020

gopherbot commented Jan 31, 2020

gopherbot commented Jun 4, 2020

bcmills commented Jan 29, 2020 •

edited

Loading

dmitshur commented Jan 30, 2020 •

edited

Loading

dmitshur commented Jan 30, 2020 •

edited

Loading

ianlancetaylor commented Jan 30, 2020 •

edited

Loading

4a6f656c commented Jan 31, 2020 •

edited

Loading