Just use BinDeps.unpack_cmd instead of looking for 7zip ourselves #229
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In Julia 1.3.0 7z.exe is moved into the /libexec folder.
Now check for if Julia >= 1.3.0 and select the right location for 7.exe.
`unpack_cmd` is exported from BinDeps. Let them deal with the 7zip stuff, we don't need to do anything special in this package.
twavv
approved these changes
Dec 28, 2019
|
This looks awesome. Is there really no way to tell if something is an error related to a command not being found besides checking the status code (using what's more or less and implementation detail)? I think the current solution is good but that's surprising/unfortunate. |
|
I guess we could extract the executable out of the command using |
|
Eh I don't think so. I think the way you have it now is great. |
|
@tkoolen If you're happy with this, I can merge and release a patch version. |
|
I went ahead and merged. Before release:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
unpack_cmdis exported from BinDeps. Let them deal with the 7zip stuff, we don't need to do anything special in this package. Though we'll still be nice and try to point at a possible solution for Windows users that build from source.This still uses
Base.UV_ENOENT, which isn't exported, but it's a lot better than what we had before. Alternatively, we can useBinDeps.exe7zand directly check for its existence, but that isn't exported either and I expectBase.UV_ENOENTto be less likely to change.