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: Windows zip/installer includes unnecessary pkg/tool/windows_amd64/api.exe #52335

Closed
dagood opened this issue Apr 13, 2022 · 5 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone

Comments

@dagood
Copy link
Contributor

dagood commented Apr 13, 2022

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

https://go.dev/dl/go1.18.1.windows-amd64.zip

What did you expect to see?

I don't think pkg/tool/windows_amd64/api.exe should be included. pkg/tool/linux_amd64/api isn't included in the Linux tar.gz.

The logic behind leaving out api is explained in cmd/release/release.go#L506-L510 and #13030:

	// Users don't need the api checker binary pre-built. It's
	// used by tests, but all.bash builds it first.
	if err := client.RemoveAll(ctx, b.toolDir()+"/api"); err != nil {
		return err
	}

What did you see instead?

pkg/tool/windows_amd64/api.exe is in the zip. It also shows up after using the msi installer.


I think cmd/release is probably just missing the code to remove the .exe because it was originally written with Linux in mind. I spotted this when I was comparing a Go build I did vs. the official Go build--this didn't break something I was working on. It seems pretty minor. (The file is only 5 MB.)

@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Apr 13, 2022
@gopherbot gopherbot added this to the Unreleased milestone Apr 13, 2022
@dmitshur
Copy link
Contributor

dmitshur commented Apr 13, 2022

Thanks for the report. That does look like a simple oversight caused by the ".exe" suffix.

I think it might be convenient for @heschi to incorporate a fix for this issue as part of #51797.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. OS-Windows labels Apr 13, 2022
@heschi
Copy link
Contributor

heschi commented Apr 14, 2022

Indeed. Added to the stack.

@heschi heschi self-assigned this Apr 14, 2022
@dmitshur
Copy link
Contributor

Added to the stack.

Not mailed yet or am I not looking in the right place?

@heschi
Copy link
Contributor

heschi commented Apr 19, 2022

I just haven't mailed it yet to spare everyone the ~20 emails, but it's in my local stack.

@gopherbot
Copy link

Change https://go.dev/cl/399158 mentions this issue: internal/task: fix bugs in release tasks

@rsc rsc unassigned heschi Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
Development

No branches or pull requests

4 participants