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

Install 7z.dll to libexec instead of bin. #33592

Merged
merged 1 commit into from
Oct 19, 2019
Merged

Conversation

staticfloat
Copy link
Member

This is required for 7z.exe to load properly. I believe this was not
caught in previous testing because if the bin folder is on the PATH,
this is not necessary, however we definitely cannot rely on that.

This is required for `7z.exe` to load properly.  I believe this was not
caught in previous testing because if the `bin` folder is on the `PATH`,
this is not necessary, however we definitely cannot rely on that.
@staticfloat staticfloat added priority This should be addressed urgently backport 1.3 labels Oct 17, 2019
@fredrikekre fredrikekre added this to the 1.3 milestone Oct 17, 2019
@ararslan
Copy link
Member

however we definitely cannot rely on that.

Possible to add a test by setting withenv("PATH" => "...")?

@staticfloat staticfloat merged commit 864e6d6 into master Oct 19, 2019
@staticfloat staticfloat deleted the sf/7z_dll_installation branch October 19, 2019 00:41
@ararslan
Copy link
Member

...I guess not then

@StefanKarpinski StefanKarpinski added the needs tests Unit tests are required for this change label Oct 21, 2019
@KristofferC
Copy link
Member

No need to create merge commits for this type of 1-commit PRs imo.

@KristofferC KristofferC mentioned this pull request Oct 22, 2019
19 tasks
@davidanthoff
Copy link
Contributor

Presumably that explains this test failure for Electron.jl on 1.3-rc4.1?

Having said that, shouldn't PkgEval have caught this?

@staticfloat
Copy link
Member Author

Yes, this should be the reason why. You should replace that direct invocation with something like this:

7z_path = withenv("PATH" => string(joinpath(Sys.BINDIR, "..", "libexec"), Sys.iswindows() ? ";" : ":", ENV["PATH"]) do
	Sys.which(`7z`)
end
run(`\$7z_path ...`)

@thudjx
Copy link

thudjx commented May 27, 2020

I am coding in Windows and get the same problem. There do be the 7z.dll and 7z.exe in the libexec folder. So how should I fix this?
image

@staticfloat
Copy link
Member Author

Please open a new issue with the full error message you're seeing, and the version of Julia you're using.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Unit tests are required for this change priority This should be addressed urgently
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants