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

go: incorporate compiler and asm tool flags into compile cache key #14664

Open
tdyas opened this issue Mar 1, 2022 · 3 comments
Open

go: incorporate compiler and asm tool flags into compile cache key #14664

tdyas opened this issue Mar 1, 2022 · 3 comments
Labels
backend: Go Go backend-related issues

Comments

@tdyas
Copy link
Contributor

tdyas commented Mar 1, 2022

#14661 did not add compiler options as part of the cache key since some amount of tracing values in go's logic was still necessary. This issue covers doing that work.

@tdyas tdyas added the backend: Go Go backend-related issues label Mar 1, 2022
@stuhood
Copy link
Member

stuhood commented Mar 2, 2022

The go tool version not ending up in the cache key makes sense, because it is provided via a path rather than a digest. But it's not clear how other values would not end up as part of the process's digest: are they not part of the input files?

@tdyas
Copy link
Contributor Author

tdyas commented Mar 2, 2022

The go tool version not ending up in the cache key makes sense, because it is provided via a path rather than a digest. But it's not clear how other values would not end up as part of the process's digest: are they not part of the input files?

GOOS and GOARCH, for example, would not be part of the cache key, unless they had been set explicitly in the environment.

That said, some of the logic related to incorporating command-line options is unnecessary since they are part of the Command proto. But that part of the algorithm was not adopted by #14661 in any event.

@stuhood
Copy link
Member

stuhood commented Mar 3, 2022

GOOS and GOARCH, for example, would not be part of the cache key, unless they had been set explicitly in the environment.

I think that that could be addressed by setting Process.platform_constraint for go executions, since that is included in the cache key here:

{
command
.environment_variables
.push(remexec::command::EnvironmentVariable {
name: CACHE_KEY_TARGET_PLATFORM_ENV_VAR_NAME.to_string(),
value: match req.platform_constraint {
Some(plat) => plat.into(),
None => "none".to_string(),
},
});
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Go Go backend-related issues
Projects
None yet
Development

No branches or pull requests

2 participants