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

Ignore RPM environment variables #1613

Merged
merged 1 commit into from Feb 22, 2023

Conversation

Firstyear
Copy link
Contributor

RPM sets a number of environment variables by default that interfere with the sccache's ability to cache artefacts. Some of these variables are also used by other applications.

This extends the default ignore list to allow sccache to be used under different environments for caching.

An alternate idea is a way to configure and define a list of environment variables to ignore.

@Xuanwo
Copy link
Collaborator

Xuanwo commented Feb 20, 2023

Thank you for the contribution. But it seems a package related to issues that should be addressed by the providers?

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2023

Codecov Report

Base: 30.80% // Head: 30.86% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (8465029) compared to base (4b53aca).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1613      +/-   ##
==========================================
+ Coverage   30.80%   30.86%   +0.05%     
==========================================
  Files          49       49              
  Lines       16618    16630      +12     
  Branches     8049     8043       -6     
==========================================
+ Hits         5120     5133      +13     
- Misses       6216     6228      +12     
+ Partials     5282     5269      -13     
Impacted Files Coverage Δ
src/cmdline.rs 22.55% <0.00%> (-2.24%) ⬇️
src/jobserver.rs 47.91% <0.00%> (-2.09%) ⬇️
src/dist/pkg.rs 13.88% <0.00%> (-0.56%) ⬇️
src/compiler/args.rs 62.25% <0.00%> (-0.36%) ⬇️
src/compiler/gcc.rs 54.13% <0.00%> (-0.31%) ⬇️
src/config.rs 35.47% <0.00%> (-0.18%) ⬇️
src/compiler/rust.rs 33.95% <0.00%> (ø)
src/compiler/compiler.rs 35.96% <0.00%> (ø)
src/lib.rs 11.35% <0.00%> (+0.02%) ⬆️
src/util.rs 37.50% <0.00%> (+0.34%) ⬆️
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Firstyear
Copy link
Contributor Author

We've been having to apply this patch for the last year or so to maintain sccache in the opensuse build service. However, it would likely be beneficial to others which is why I submitted it.

@Xuanwo
Copy link
Collaborator

Xuanwo commented Feb 21, 2023

Great, please format you code to pass the CI first. And I will invite someone who is more familiar with this part to take a look.

RPM sets a number of environment variables by default that interfere
with the sccache's ability to cache artefacts. Some of these
variables are also used by other applications.

This extends the default ignore list to allow sccache to be used
under different environments for caching.
@Firstyear
Copy link
Contributor Author

Done :)

@drahnr
Copy link
Collaborator

drahnr commented Feb 21, 2023

@Firstyear could you elaborate on each of the added entries how it messes up a compilation? Wouldn't one want to include the RPM_* flags to make sure the behavior is consistent when building as rpm vs standalone when using sccache?

@Firstyear
Copy link
Contributor Author

@drahnr Every single one of these values changes every single time RPM is invoked, especially in an automated build environment like OBS. Since sccache will only match on cache items if the environment flags are identical, then since these values are always change, sccache will always cache miss making it ineffective.

@drahnr
Copy link
Collaborator

drahnr commented Feb 22, 2023

Thank you!

@drahnr drahnr merged commit b0dd958 into mozilla:main Feb 22, 2023
@Firstyear Firstyear deleted the 20230220-ignore-rpm-env-vars branch February 22, 2023 04:27
@Firstyear
Copy link
Contributor Author

Thanks for this! It's really appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants