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

feat: Introduce DFX_CACHE_ROOT environment variable #2112

Merged

Conversation

paulyoung
Copy link
Contributor

@paulyoung paulyoung commented Mar 31, 2022

Description

A new environment variable, DFX_CACHE_ROOT, has been introduced to allow setting the cache root directory to a different location than the configuration root directory. Previously DFX_CONFIG_ROOT was repurposed for this which only allowed one location to be set for both the cache and configuration root directories.

The motivation for this is described in paulyoung/nixpkgs-dfinity-sdk#4, where the cache root directory is necessarily set to a read-only directory. This means that the configuration root directory is also read-only, resulting in the error Cannot create config directory when trying to write .config/dfx

Fixes #2106 and paulyoung/nixpkgs-dfinity-sdk#4

How Has This Been Tested?

Only tested with cargo test locally. Unable to run end-to-end tests locally due to #1955.

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@paulyoung paulyoung requested a review from a team as a code owner March 31, 2022 08:34
@paulyoung
Copy link
Contributor Author

Please advise on the PR title. The check failed but I used the format shown at https://www.conventionalcommits.org/en/v1.0.0/#commit-message-with--to-draw-attention-to-breaking-change

@paulyoung paulyoung changed the title feat!: Introduce DFX_CACHE_ROOT environment variable feat: Introduce DFX_CACHE_ROOT environment variable Mar 31, 2022
@sesi200
Copy link
Contributor

sesi200 commented Mar 31, 2022

Hi Paul

Thank you very much for your PR! We would like add those changes. Annoyingly, Hydra (required part of CI, removing it is WIP) does not properly work on external PRs, so someone from the team will have to re-implement your changes and create a separate PR to merge them. This is planned for the next sprint. We'll keep you posted on the progress.

Severin

@ericswanson-dfinity
Copy link
Member

ericswanson-dfinity commented Mar 31, 2022

@paulyoung Thanks for this! The CI (internal, that you can't see) is failing in this test, when it tries to write to $HOME/.cache: https://github.com/dfinity/sdk/blob/master/src/dfx/src/lib/environment.rs#L490 .

[2022-03-31 08:37:17] thread 'lib::environment::tests::test_passwords' panicked at
 'called `Result::unwrap()` on an `Err` value: Cannot create cache directory at '/homeless-shelter/.cache/dfinity'.',
 src/dfx/src/lib/environment.rs:501:14

If you'd like to try changing this test to alter DFX_CACHE_ROOT rather than DFX_CONFIG_ROOT, it's possible that the tests will pass, and we can get this merged in right away. Otherwise I will take care of it in our next sprint.

@ericswanson-dfinity
Copy link
Member

@paulyoung So sorry, but it looks like there is one more place in the internal CI that needs to be changed from DFX_CONFIG_ROOT to DFX_CACHE_ROOT, here: https://github.com/dfinity/sdk/blob/master/check-binaries.nix#L18

@ericswanson-dfinity ericswanson-dfinity merged commit f4e24bf into dfinity:master Apr 1, 2022
@paulyoung paulyoung deleted the paulyoung/dfx-cache-root branch April 1, 2022 02:14
@paulyoung
Copy link
Contributor Author

Please could we get a release that includes this change?

nomeata added a commit to nomeata/nixpkgs-dfinity-sdk that referenced this pull request Mar 13, 2023
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.

Introduce DFX_CACHE_ROOT instead of using DFX_CONFIG_ROOT for the cache directory
3 participants