Skip to content

make clean does not enable rebuild of geth #17078

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

Closed
hqueue opened this issue Jun 26, 2018 · 4 comments · Fixed by #17079
Closed

make clean does not enable rebuild of geth #17078

hqueue opened this issue Jun 26, 2018 · 4 comments · Fixed by #17079

Comments

@hqueue
Copy link
Contributor

hqueue commented Jun 26, 2018

System information

Geth version: 1.8.12-unstable
Go Version: go1.10.3
OS & Version: OSX
Commit hash : b0cfd9c

Expected behaviour

make geth should rebuild source code after make clean

Actual behaviour

make clean does not enable full rebuild of go-ethereum as below when using go version 1.10 or later.

$ make geth
build/env.sh go run build/ci.go install ./cmd/geth
>>> /usr/local/go/bin/go install -ldflags -X main.gitCommit=0a22ae55729298dcd9979184d0b3c28f48b272bf -s -v ./cmd/geth
Done building.
Run "/Users/hqueue/work/go-ethereum/build/bin/geth" to launch geth.

Steps to reproduce the behaviour

  1. Install go with version 1.10 or higher.

  2. git clone go-ethereum

  3. make geth This will trigger the first time full build of geth

  4. make clean

  5. make geth does not rebuild geth, just install it as below

$ make geth
build/env.sh go run build/ci.go install ./cmd/geth
>>> /usr/local/go/bin/go install -ldflags -X main.gitCommit=0a22ae55729298dcd9979184d0b3c28f48b272bf -s -v ./cmd/geth
Done building.
Run "/Users/hqueue/work/go-ethereum/build/bin/geth" to launch geth.
$

Backtrace

go build make use of cache from 1.10 (see https://golang.org/doc/go1.10#build) and go does not rebuild source code if there is no change.

We have to clean go build cache using go clean -cache.

$ go clean -cache
$ make geth  // Now it will rebuild code

A PR will be filed to perform go clean -cache when we trigger make clean.

@karalabe
Copy link
Member

But why would you want to do a full rebuild? Go uses hashes of the source files to detect whether something changed or not, so doing a full rebuild shouldn't be something too useful. Am I missing something here?

@hqueue
Copy link
Contributor Author

hqueue commented Jun 26, 2018

But why would you want to do a full rebuild? Go uses hashes of the source files to detect whether something changed or not, so doing a full rebuild shouldn't be something too useful. Am I missing something here?

@karalabe Right in normal cases.
However if a developer used make clean explicitly, then I think the developer may want to trigger a clean full build of go-ethereum for some reason. What do you think of it ?

@karalabe
Copy link
Member

Cleaning Go's cache will clean out everything, not just go-ethereum related things. As for forcing a rebuild, why is that good? Any change + Go version change will rebuild everything anyway. Feels like a hack to mimic some legacy behavior originating from C++ which was not always able to detect changes properly.

@hqueue
Copy link
Contributor Author

hqueue commented Jun 29, 2018

Cleaning Go's cache will clean out everything, not just go-ethereum related things

Right. This is one (critical) downside of using go clean -cache.

As for forcing a rebuild, why is that good? Any change + Go version change will rebuild everything anyway. Feels like a hack to mimic some legacy behavior originating from C++ which was not always able to detect changes properly.

Agree :) If go develpers don't care about this issue, we can just close this issue.

Frankly speaking, I'm still more get used to legacy behavior of C++ make system, that's one reason I raised this issue.

Thanks for sharing your opinion on this cleaning issue :)

I will close this issue and related PR soon, if there is no more discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants