Skip to content

Fix: use_julia should not set LD_LIBRARY_PATH #900

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

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

GlenHertz
Copy link
Contributor

I was compiling julia master and it errored out and after talking
to the julia devs they say it is becaues LD_LIBRARY_PATH is set
and it should never be set (julia takes care of it automatically).

So since no upside in setting it and only a downside, submitting
a PR.

I was compiling julia master and it errored out and after talking
to the julia devs they say it is becaues LD_LIBRARY_PATH is set
and it should never be set (julia takes care of it automatically).

So since no upside in setting it and only a downside, submitting
a PR.
@fredrikekre
Copy link
Contributor

It is not set for Julia's sake, but I agree that it is a bit annoying to set since it makes different Julia version clash with each other. How about a more targeted deletion of LD_LIBRARY_PATH? Something like

diff --git a/stdlib.sh b/stdlib.sh
index 372e76d..6b143fe 100755
--- a/stdlib.sh
+++ b/stdlib.sh
@@ -1027,7 +1027,15 @@ use_julia() {
     return 1
   fi
 
+  # Julia is happier without LD_LIBRARY_PATH set: store the value (if set)
+  # and reset the value after load_prefix.
+  if [ -n "${LD_LIBRARY_PATH+x}" ]; then local _LD_LIBRARY_PATH="${LD_LIBRARY_PATH}"; fi
   load_prefix "$julia_prefix"
+  if [ -n "${_LD_LIBRARY_PATH+x}" ]; then
+      LD_LIBRARY_PATH="${_LD_LIBRARY_PATH}"
+  else
+      unset LD_LIBRARY_PATH
+  fi
 
   log_status "Successfully loaded $(julia --version), from prefix ($julia_prefix)"
 }

@GlenHertz
Copy link
Contributor Author

I like the idea but if a user is setting LD_LIBRARY_PATH for some other non-julia purpose then wouldn't this clobber their setup? I think we would need to check if they were pointing to a julia lib and if so then replace it with the new location.

@fredrikekre
Copy link
Contributor

No, my suggestion should leave LD_LIBRARY_PATH identical to what it was before (unset if unset, populated if populated).

@GlenHertz
Copy link
Contributor Author

Ah I see, but when using load_prefix it will also set CPATH which shouldn't be set. It is more (complex) code to call a function (load_prefix) that doesn't do what is needed and then to fix it with more code before and after calling it. If load_prefix changes in the future then it may do something unwanted to use_julia. If PATH (and MANPATH) are the recommended env vars in default circumstances then shouldn't those only be set (instead of setting extra env vars and then returning them to their original state)?

@fredrikekre
Copy link
Contributor

fredrikekre commented Feb 28, 2022

Do the other variables cause any problems?

@GlenHertz
Copy link
Contributor Author

I don't know but I don't see any Julia docs or recommendations to set them.

@GlenHertz
Copy link
Contributor Author

Hi, I ran into this issue again causing julia master to segfault. I don't get why LD_LIBRARY_PATH is set when it causes issues and it is not supposed to be set.

@zimbatm zimbatm merged commit b08f07f into direnv:master Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants