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

Don't generate cache key, if not caching builds. #1194

Merged

Conversation

tomprince
Copy link
Contributor

Description

The cache key generation does environment substitution in places that running
the commands doesn't. This causes issues if a command uses complex shell
substitutions. The cache key is generated even if caching isn't enabled. (See #1170)

This disables the cache key generation if caching is not enabled. This doesn't
fix the underlying issue, but limits it to when the cache is being used.

Submitter Checklist

  • Includes unit tests
  • Adds integration tests if needed.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

The cache key generation does environment subsitution in places that running
the commands doesn't. This causes issues if a command uses complex shell
substitutions. The cache key is generated even if caching isn't enabled.

This disables the cache key generation if caching is not enabled. This doesn't
fix the underlying issue, but limits it to when the cache is being used.
@googlebot googlebot added the cla: yes CLA signed by all commit authors label Apr 13, 2020
@tejal29 tejal29 merged commit 6c62764 into GoogleContainerTools:master May 1, 2020
@tomprince tomprince deleted the greedy-cache-substitution branch May 1, 2020 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed by all commit authors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants