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

don't use realpath in Sys.which #40233

Merged
merged 7 commits into from
Apr 6, 2021
Merged

don't use realpath in Sys.which #40233

merged 7 commits into from
Apr 6, 2021

Conversation

stevengj
Copy link
Member

As discussed recently on discourse, some programs require a certain executable name to function correctly, and behave badly if you call realpath to expand symbolic links (potentially changing the program name).

#26559 by @staticfloat implemented Sys.which, but there doesn't seem to have been any discussion about whether realpath was a good idea.

@stevengj stevengj added the bugfix This change fixes an existing bug label Mar 27, 2021
@stevengj
Copy link
Member Author

stevengj commented Mar 27, 2021

Actually, @vtjnash noted in #26559 (comment) that realpath would be "divergent from the which program, which does no normalization at all", so I'm not sure why we ended up using realpath.

@giordano
Copy link
Contributor

Test failures look related (I think tests are specifically testing the use of realpath).

@stevengj
Copy link
Member Author

stevengj commented Apr 3, 2021

Updated the tests to use abspath rather than realpath.

test/spawn.jl Outdated Show resolved Hide resolved
@stevengj
Copy link
Member Author

stevengj commented Apr 3, 2021

Tests are green; should be good to merge.

@stevengj
Copy link
Member Author

stevengj commented Apr 4, 2021

Or at least, the tests were green. Now I'm seeing an apparently unrelated error:

Error in testset cmdlineargs:
Error During Test at /Users/julia/buildbot/worker-tabularasa/tester_macos64/build/share/julia/test/testdefs.jl:21
  Got exception outside of a @test
  LoadError: could not load library "libjulia"

@vtjnash
Copy link
Member

vtjnash commented Apr 4, 2021

Looks like this is failing a spawn test on Win64?

@vtjnash
Copy link
Member

vtjnash commented Apr 4, 2021

That macOS failure is fixed on master

@stevengj
Copy link
Member Author

stevengj commented Apr 4, 2021

Whoops, yes, now it is failing that test because of the .exe suffix.

Error in testset spawn:
Test Failed at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\spawn.jl:666
  Expression: p == joinpath(abspath("bin1"), "bar")
   Evaluated: "C:\\Windows\\TEMP\\jl_VxLh9u\\bin1\\bar.exe" == "C:\\Windows\\TEMP\\jl_VxLh9u\\bin1\\bar"

I just pushed a commit to fix that.

test/spawn.jl Outdated Show resolved Hide resolved
test/spawn.jl Outdated Show resolved Hide resolved
test/spawn.jl Outdated Show resolved Hide resolved
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Apr 6, 2021
@vtjnash vtjnash merged commit 70333da into master Apr 6, 2021
@vtjnash vtjnash deleted the stevengj-patch-1 branch April 6, 2021 21:41
@simeonschaub simeonschaub removed the merge me PR is reviewed. Merge when all tests are passing label Apr 28, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
As [discussed recently on discourse](https://discourse.julialang.org/t/weird-sys-which-function/58070), some programs require a certain executable name to function correctly, and behave badly if you call `realpath` to expand symbolic links (potentially changing the program name).

c.f. discussion in JuliaLang#26559, implemented `Sys.which`
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
As [discussed recently on discourse](https://discourse.julialang.org/t/weird-sys-which-function/58070), some programs require a certain executable name to function correctly, and behave badly if you call `realpath` to expand symbolic links (potentially changing the program name).

c.f. discussion in JuliaLang#26559, implemented `Sys.which`
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
As [discussed recently on discourse](https://discourse.julialang.org/t/weird-sys-which-function/58070), some programs require a certain executable name to function correctly, and behave badly if you call `realpath` to expand symbolic links (potentially changing the program name).

c.f. discussion in JuliaLang#26559, implemented `Sys.which`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants