cmd/go: go build errors that don't correspond to source file until build cache is cleared #69179
Comments
sipsma
changed the title
|
I think somethhing to do with the module index? Cache verify: With the module index on (default): With module index off, build succeeds: |
seankhliao
added
NeedsInvestigation
|
I'll look into this |
|
Let me know if there's anything I can do to help debug further! |
|
@matloob @seankhliao Quick question pending the investigation on your end, is there any obvious reason it would be a bad idea to run with Asking because this bug is causing lots of problems in our CI test suite specifically, so I'm considering just disabling the mod index there while things get figured out upstream here. In some preliminary runs I didn't notice any performance impact for us when running with it disabled for the whole test suite (and no occurrences of the bug so far 🤞). However, that |
|
I don't think disabling the module index should cause any correctness problems. (edit: correctness, not performance) |
|
Something similar is also happening to us. Our scenario:
I too have created a docker image which shows the reproduction. I can share a link if needed. Commit Diffdiff --git a/core/capabilities/ccip/common/common_test.go b/core/capabilities/ccip/common/common_test.go
index 9b974ec60e..f1695d4187 100644
--- a/core/capabilities/ccip/common/common_test.go
+++ b/core/capabilities/ccip/common/common_test.go
@@ -9,9 +9,9 @@ import (
"github.com/stretchr/testify/require"
"github.com/smartcontractkit/chainlink-integrations/evm/assets"
+ "github.com/smartcontractkit/chainlink-integrations/evm/testutils"
capcommon "github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/common"
kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry_1_1_0"
- "github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
)
func Test_HashedCapabilityId(t *testing.T) {go env |
|
We also started facing this, but unfortunately I don't have a reproduction script. Just in case it helps, this is what we get: |
|
@erikburt I think I understand why this is happening in your case. We use the modification time on each of the files as part of the cache key for the module index file. If a file in your package changes, but the file has the same size and modification time we try to use the same cache entry. Thanks for providing the reference file! It's clear why the size of the file isn't changing since the import is moving in the file, but the value for the time stands out. I was going to say that we should reject time.Unix(0,0) but it seems like the value you're getting is time.Unix(1,0). It would be interesting to know where that value is coming from. But that should be pretty simple to fix once we know- we'll reject using the index in those cases. @agis I'm less clear about what's happening in your case. Though one thing stands out to me: the size of the 'new' index file is significantly smaller than the size of the 'old' file. Do you remember what change you made to the package before you saw this? Could you tell us which operating system you were using and which filesystem the files you're building are stored on? |
|
@matloob - thank you for the response. We are actively setting the modtime for our files to I guess it's technically not valid given the above case, but it doesn't seem like there's any other workaround. |
@matloob that's extremely useful information, I actually wouldn't be entirely surprised if that's what caused this problem for us too despite the fact that we aren't resetting any timestamps to constant values. The reason being that we have actually hit problems in the past caused by two different go source files ending up with the same length and nanosecond mod timestamp despite having different contents.
Surprisingly, it turns out this is actually not an absurdly obscure situation to run into in the real world, mainly due to two factors
I don't know if there's any possible better heuristic for the go index to switch to than "size+modtime" that's also feasible. Content-hashing is an obvious option but comes w/ plenty of performance tradeoffs of course. Personally, I'd be happy if there was just an "officially enshrined" way to disable the go index. We haven't had any problems since disabling it but it feels like somewhat shaky ground since it's essentially an undocumented feature flag (at least last I checked). |
|
@sipsma Thanks for the information. A couple of questions
We wait a little bit before allowing a file to allowing a file to be cached https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/modindex/read.go;drc=665af869920432879629c1d64cf59f129942dcd6;l=100 . My understanding is that after the wait the files should have settled, so that if there were two writes done so close to each other that the get the same mtime, only the contents of the actual later write that won should be cached. I wonder if there's something incorrect in that logic?
In this example the processes are modifying source files, right? Are these things like automated go generate runs?
If we can't solve this we may want to document the GODEBUG but I'd feel better if we could find a way to better detect situations where we should automatically skip indexing a package. |
Due to an issue documented [here](golang/go#69179), this occurs when adding a new import for chainlink-deployments-framework/deployments, i need to set the goindex=0 for godebug. There was a line setting it already but it didn not apply correctly.
Due to an issue documented [here](golang/go#69179), this occurs when adding a new import for chainlink-deployments-framework/deployments, i need to set the goindex=0 for godebug. There was a line setting it already but it didn not apply correctly.
Due to an issue documented [here](golang/go#69179), this occurs when adding a new import for chainlink-deployments-framework/deployments, i need to set the goindex=0 for godebug. There was a line setting it already but it didn not apply correctly.
Due to an issue documented [here](golang/go#69179), this occurs when adding a new import for chainlink-deployments-framework/deployments, i need to set the goindex=0 for godebug. There was a line setting it already but it didn not apply correctly.
Go version
go version go1.23.0 linux/amd64
Output of
go envin your module/workspace:What did you do?
When attempting to build a valid go program, we are getting errors about imports on certain lines of source code that don't match the actual contents of the source file they are reported on. The errors go away and the program builds successfully once the go build cache at
/root/.cache/go-buildis deleted and nothing else changes.The repro is quite complex and takes a long time to trigger, so the best I could do for now was create a docker image with the go toolchain, source code and go build cache in place that reproduces the problem. Including commands for reproducing with that image below.
For more context, we are hitting this in Dagger, which is a container-based DAG execution engine that, among other things, does a lot of containerized building of Go code.
We specifically see this problem arise during integration tests, which will run, over the course of ~20min, many (probably 100+)
go buildexecutions in separate containers. The most relevant details I can think of:/root/.cache/go-build) and the go mod cache (always mounted at/go/pkg/mod)/srcand built with the commandgo build -o /runtime .from within that/srcdirectory/src/internal. They may also have the same go mod name at times.go build,go mod tidy, etc. in containersWhat did you see happen?
As mentioned above, the best I could do for now was capture the state of one of the containers hitting this error in a docker image. I pushed the image to dockerhub at
eriksipsma/corrupt-cache:latest. It's alinux/amd64only image unfortunately since that's what our CI is, which is the only place I can get this to happen consistently.Trigger the go build error:
The errors refer to the source file at
/src/internal/dagger/dagger.gen.go. However, the imports it's erroring on are not the actual imports in the source file:dagger/test/internal/but the actual imports in the source code aredagger/bare/internaldagger/test/internal. So it seems likego buildhere is somehow finding something in the cache from a previous build and incorrectly using it for this one.The error goes away if you first clear the build cache and then run the same
go buildcommand:What did you expect to see?
For
go buildto not report errors that don't correspond to the source contents, and for the go build cache to not need to be cleared in order to get rid of the errors.The text was updated successfully, but these errors were encountered: