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

Fix test_env to use build, fixing analysis caching #568

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Fix test_env to use build, fixing analysis caching #568

merged 1 commit into from
Jun 8, 2021

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Jun 8, 2021

Per https://docs.bazel.build/versions/master/guide.html#option-defaults, test inherits from build, thus why this fixes caching.

The issue can be observed with bazel build :all && bazel test :all, the line:
INFO: Build option --test_env has changed, discarding analysis cache.

@jonmeow jonmeow requested a review from chandlerc June 8, 2021 16:22
@jonmeow jonmeow requested a review from a team as a code owner June 8, 2021 16:22
@google-cla google-cla bot added the cla: yes PR meets CLA requirements according to bot. label Jun 8, 2021
@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 8, 2021

Bazel is a bit extreme when it comes to cache invalidation. In principle you could use --trim_test_configuration to exclude lots of flags (which should only affect test runtime) from the build cache key, but I've seen odd cornercases where tests depend on the trimmed flags, so I no longer have that in my default .bazelrc...

@jonmeow
Copy link
Contributor Author

jonmeow commented Jun 8, 2021

Bazel is a bit extreme when it comes to cache invalidation. In principle you could use --trim_test_configuration to exclude lots of flags (which should only affect test runtime) from the build cache key, but I've seen odd cornercases where tests depend on the trimmed flags, so I no longer have that in my default .bazelrc...

--trim_test_configuration doesn't affect this issue (I checked to make sure), and will will be the default in future releases per bazelbuild/bazel#6842.

Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM given our constraints...

@jonmeow jonmeow merged commit 4a0202d into carbon-language:trunk Jun 8, 2021
@jonmeow jonmeow deleted the testenv branch June 8, 2021 17:58
chandlerc pushed a commit that referenced this pull request Jun 28, 2022
Per https://docs.bazel.build/versions/master/guide.html#option-defaults, test inherits from build, thus why this fixes caching.

The issue can be observed with `bazel build :all && bazel test :all`, the line:
`INFO: Build option --test_env has changed, discarding analysis cache.`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes PR meets CLA requirements according to bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants