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/tools: rewrite git history to remove 19 MB binary blob #34688

Closed
bradfitz opened this issue Oct 4, 2019 · 4 comments

Comments

@bradfitz
Copy link
Member

commented Oct 4, 2019

https://go-review.googlesource.com/c/tools/+/198743 reverts a large binary that was checked in by mistake in https://golang.org/cl/198441

I filed #34687 to prevent this in the future, but this bug is about tracking rewriting the history away.

Previously:

/cc @andybons @dmitshur @toothrot @stamblerre @ianthehat

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2019

git rebase -i 8d2ce75645885d56cfc03e5c581a06ae7b74e22a^
edit the one, delete the other

Was:

9cd7e18f8ef73a428ee6125634aae52a8d0c94d4 (origin/master, origin/HEAD, master) internal/lsp: invert the diff dependencies so myers depends on diff
b2e299babdd048bbc30a98a0b2627b2bf3f3f464 internal/lsp: allow the diff alorithm to be specified per view
0a61691d5d7577c585cc381b9abbe327930494d5 tools: remove binary that was accidentally checked in
fb78014554ee601778f5f954d48e8cd1a76a75cb go/gcexportdata: use IExportData when writing gcexportdata
c56b4b191e2daed0dce81abe93b3230b594b39b9 internal/lsp: use dependencies in cache keys
9ade4c73f2afdb56d278b4e3587f70b0c0f0cb9d internal/lsp: do not allow diff.ApplyEdits to be replaced
6fe9ea94a73dc611358a83811806b16f9e748c56 internal/lsp: restore "IsIncomplete" completion flag
8d2ce75645885d56cfc03e5c581a06ae7b74e22a internal/lsp: address staticcheck warnings

Now:

b917058f11a9c069117ccc8250e9164691c8aac3 (HEAD -> fix_history) internal/lsp: invert the diff dependencies so myers depends on diff
6e0078a8999d96ba44c5db04c63cfd88dae8556a internal/lsp: allow the diff alorithm to be specified per view
246a69f4f163d1088ce08437583357030ef29a58 go/gcexportdata: use IExportData when writing gcexportdata
9901302c21f4ea129de26085f5829b9221beb590 internal/lsp: use dependencies in cache keys
f7bb6f12f04c2ac2e4d81348554844e038fea161 internal/lsp: do not allow diff.ApplyEdits to be replaced
91543147e31248677f3a92c0ee8f88984fc2bbb7 internal/lsp: restore "IsIncomplete" completion flag
f0a16743a2556d975af0186057d0c0c4c03c51af internal/lsp: address staticcheck warnings
87e6e099c6fe5a0b09988620ac29cd758e123efb internal/lsp: don't overwrite suffix when inserting completion
@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2019

Done.

(Temporarily added myself to may-force-push to git push -f origin HEAD:master it)

@bradfitz bradfitz closed this Oct 4, 2019
@dmitshur

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

This seems reasonable, thanks for doing that Brad.

One new consideration compared to previous times are module versions. I think rewriting history may render pseudo-versions derived from commits that are no longer on master branch unusable when fetched directly from VCS:

$ cd $(mktemp -d)
$ export GOPATH=$(pwd)
$ export GO111MODULE=on
$ export GOPROXY=direct
$ export GOSUMDB=off
$ go list -m golang.org/x/tools@v0.0.0-20191003235154-9cd7e18f8ef7
go: finding golang.org/x/tools v0.0.0-20191003235154-9cd7e18f8ef7
go list -m: golang.org/x/tools@v0.0.0-20191003235154-9cd7e18f8ef7: invalid version: unknown revision 9cd7e18f8ef7
$ echo $?
1

They should still be available via the module mirror or another proxy, if cached in time. E.g.:

$ unset GOPROXY
$ unset GOSUMDB
$ go list -m golang.org/x/tools@v0.0.0-20191003235154-9cd7e18f8ef7
go: finding golang.org/x/tools v0.0.0-20191003235154-9cd7e18f8ef7
golang.org/x/tools v0.0.0-20191003235154-9cd7e18f8ef7
$ echo $?
0

Otherwise people will need to change to a working updated pseudo-version. They'd only be affected if they updated to a very new pseudo-version in the short window of time that the rewrite touched.

Given we're dealing with v0.0.0- pseudo-versions this seems okay in order to prune a large binary from history, but our options would be more limited when dealing with stable v1+ releases. In general, resolving #10658 will help with this.

@dmitshur dmitshur added the modules label Oct 4, 2019
@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

Given we're dealing with v0.0.0- pseudo-versions this seems okay in order to prune a large binary from history, but our options would be more limited when dealing with stable v1+ releases.

Note that for tagged releases, the go command actually doesn't care whether you've changed the underlying commit or commit metadata as long as the contents of the module at that tag remain the same. So as long as the binary blob didn't make it into a tagged release, the impact will remain about as low as for v0.0.0- pseudo-versions — actually even lower since most users will presumably stick to the tagged releases.

Pseudo-versions are different because the commit metadata is embedded in the version string itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.