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

cmd/go: build cache not updating when pkg-config files changed #24544

Closed
AlexRouSg opened this issue Mar 26, 2018 · 6 comments
Closed

cmd/go: build cache not updating when pkg-config files changed #24544

AlexRouSg opened this issue Mar 26, 2018 · 6 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@AlexRouSg
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

linux amd64

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

Trying to run tests on a package that uses C libraries using pkg-config to set compile and linker flags when I changed the install location of the C libraries in a script which is how I noticed this.

go test mypkg
go test mypkg

move location of C library and update pkg-config file

go clean -testcache mypkg
go test mypkg

What did you expect to see?

ok mypkg 1.151s
ok mypkg (cached)

nothing from go clean
ok mypkg 1.151s

What did you see instead?

ok mypkg 1.151s
ok mypkg (cached)

nothing from go clean

# mypkg (testmain)
go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/bin/ld: cannot find -lSDL2
collect2: error: ld returned 1 exit status

FAIL	mypkg [build failed]

For some reason it is important to run go test twice to get the cached result before moving the C library for it to fail. To test that it wasn't a bad config, running go clean -cache -testcache mypkg before testing clears the compile error.

/cc @ianlancetaylor

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Mar 26, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Mar 26, 2018
@bcmills
Copy link
Contributor

bcmills commented Aug 2, 2018

In order to invalidate the cache in this case, we would have to keep track of which external packages are linked into each test and rescan those packages whenever we rerun the test.

It's not obvious to me how expensive that would be in order to catch a relatively rare source of cache invalidation from external dependencies. (See also #26366 (comment).) Perhaps @rsc has some insight.

@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 2, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 2, 2018
@bcmills
Copy link
Contributor

bcmills commented Aug 2, 2018

Since this was reported on 1.10, has a workaround (go clean -cache), and unless I am mistaken occurs rarely in practice, I don't think it needs to be a release-blocker for 1.11.

@ianlancetaylor
Copy link
Contributor

In principle we could implement this by running pkg-config while computing the hash, rather than when doing the build.

However, we are currently adopting a general policy that changes to files outside the tree do not affect the build. For example, if Go code using cgo #include's a header file in a different directory, that does not invalidate the cache. And see #26506. It wouldn't make sense to change the specific case of pkg-config without changing the general case of #include.

Based on that I think this is working as designed, so closing.

@AlexRouSg
Copy link
Contributor Author

I think this should at least be pointed out in the cgo docs. It seems non intuitive and nothing obvious hints that it would behave in such a way.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Aug 3, 2018

@AlexRouSg It seems to me that this is already documented in the cmd/go docs, which say

The build cache correctly accounts for changes to Go source files, compilers,
compiler options, and so on: cleaning the cache explicitly should not be necessary
in typical use.  However, the build cache does not detect changes to C libraries imported
with cgo.  If you have made changes to the C libraries on your system, you will need to
clean the cache explicitly or else use the -a build flag (see 'go help build') to force
rebuilding of packages that depend on the updated C libraries.

@AlexRouSg
Copy link
Contributor Author

My bad, haven't checked the docs in a while.

@golang golang locked and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

4 participants