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

use JULIA_DEPOT_PATH as default install path #240

Merged
merged 2 commits into from
Apr 26, 2023
Merged

use JULIA_DEPOT_PATH as default install path #240

merged 2 commits into from
Apr 26, 2023

Conversation

Roger-luo
Copy link
Collaborator

fix #238

@MilesCranmer
Copy link

MilesCranmer commented Apr 25, 2023

Quick comments:

  1. Should use DEPOT_PATH rather than ENV[JULIA_DEPOT_PATH]. The global variable DEPOT_PATH is automatically populated in Julia and always exists.
  2. Note that DEPOT_PATH is a vector of directories. I think it should go through them until it finds one that is writeable.

@Roger-luo
Copy link
Collaborator Author

Note that DEPOT_PATH is a vector of directories. I think it should go through them until it finds one that is writeable.

I'm not sure how to check if it's writable, it seems there is a filemode function but I didn't find any way of checking path permissions unless we want to do a shell call

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b3520ed) 82.84% compared to head (1c3089d) 82.84%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #240   +/-   ##
=======================================
  Coverage   82.84%   82.84%           
=======================================
  Files          21       21           
  Lines        1586     1586           
=======================================
  Hits         1314     1314           
  Misses        272      272           
Impacted Files Coverage Δ
src/configs.jl 76.92% <100.00%> (ø)

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MilesCranmer
Copy link

I think first(DEPOT_PATH) as you wrote it is a good default strategy.

A more robust option, in case people have populated their JULIA_DEPOT_PATH with non-writeable directories (such as system-wide package registries), is to have path::Union{String,Nothing}=nothing. Then, if it's still nothing at the install step, you could loop through DEPOT_PATH, and wrap the writing in a try-catch. Once it succeeds writing you can exit the loop.

@Roger-luo
Copy link
Collaborator Author

Thanks I'm gonna do the simple fix.

@Roger-luo Roger-luo merged commit b352976 into main Apr 26, 2023
@Roger-luo Roger-luo deleted the roger/patch branch April 26, 2023 18:21
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.

[BUG] Custom JULIA_DEPOT_PATH not being respected
2 participants