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

x/build/cmd/release: releaselet can be out of sync with cmd/release #33443

Closed
FiloSottile opened this issue Aug 2, 2019 · 3 comments
Closed

x/build/cmd/release: releaselet can be out of sync with cmd/release #33443

FiloSottile opened this issue Aug 2, 2019 · 3 comments

Comments

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Aug 2, 2019

findReleaselet looks for releaselet.go in the current directory and in the workspace, which however might not be the source cmd/release was built from, leading to errors.

https://github.com/golang/build/blob/db014ec7fdd2ab17189c5b9f5ccde2090169900c/cmd/release/release.go#L112-L132

Specifically, misc/boring/build.release uses go get in module mode to fetch cmd/release, but for some reason the command was then finding the older releaselet.go in my GOPATH, and the release was failing (because the older releaselet would look for the blog). One can imagine even worse outcomes.

releaselet.go should be embedded in the cmd/release source, either as a simple constant, or with go:generate and https://github.com/shurcooL/vfsgen.

/cc @bradfitz @katiehockman @toothrot

@FiloSottile FiloSottile added the NeedsFix label Aug 2, 2019
@gopherbot gopherbot added this to the Unreleased milestone Aug 2, 2019
@gopherbot gopherbot added the Builders label Aug 2, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 3, 2019

Change https://golang.org/cl/188738 mentions this issue: misc/boring: add Go 1.11.12b4 and 1.12.7b4 to RELEASES

@dmitshur dmitshur added the modules label Aug 5, 2019
gopherbot pushed a commit that referenced this issue Aug 5, 2019
Also, workaround #33443.

Change-Id: I10667d99769bec3af8696d895d6b8ce1f9dcd2ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/188738
Reviewed-by: Katie Hockman <katie@golang.org>
@dmitshur dmitshur self-assigned this Aug 10, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 10, 2019

Change https://golang.org/cl/189817 mentions this issue: cmd/release: embed releaselet.go source code statically

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 12, 2019

Change https://golang.org/cl/189942 mentions this issue: [dev.boringcrypto] misc/boring: remove wget releaselet.go step in build.release

gopherbot pushed a commit that referenced this issue Aug 13, 2019
…uild.release

This step was added in CL 188738 to work around the issue
golang.org/issue/33443. That issue has now been resolved,
so this step is no longer needed and can be removed.

Updates #33443

Change-Id: I0c9257ab61d53f3a47556882f7dfc8fc119be849
Reviewed-on: https://go-review.googlesource.com/c/go/+/189942
Reviewed-by: Filippo Valsorda <filippo@golang.org>
codebien added a commit to codebien/build that referenced this issue Nov 13, 2019
Previously, the release binary dynamically looked for the releaselet.go
source code. It first checked in the current working directory, and
then used the go/build package to find the location of source code of
golang.org/x/build/cmd/release package on disk.

The release binary is not generally executed by hand, rather it is used
internally by releasebot command. That makes it hard to know in advance
where the current working directory for release will be (that directory
is $HOME/go-releasebot-work/<release> that releasebot creates itself).

Further, with the proliferation of module mode, it's no longer viable
to be able to find the location of a package by its import path via
go/build.Import. As a result, there's now a higher risk of cmd/release
not finding, or finding a wrong version of releaselet.go.

Change the release binary to instead statically embed the releaselet.go
source code as a constant string, in a static.go file that is generated
via a go:generate directive.

Add a test to ensure the embedded copy of releaselet.go doesn't get out
of sync.

The embedding approach was loosely based on approach taken in package
golang.org/x/tools/godoc/static, but it was simplified for the smaller
needs of cmd/release (i.e., embedding a single text file). We rely on
fmt's %q verb to do the escaping, which has the risk of changing between
Go versions, but it's unlikely to happen often to warrant worrying yet.

Fixes golang/go#33443

Change-Id: Ie7a9481c33a7d9668d696d3827e5681b07b37094
Reviewed-on: https://go-review.googlesource.com/c/build/+/189817
Reviewed-by: Filippo Valsorda <filippo@golang.org>
@golang golang locked and limited conversation to collaborators Aug 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.