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

Port to Julia 1.3 #49

Merged
merged 4 commits into from
Nov 29, 2019
Merged

Port to Julia 1.3 #49

merged 4 commits into from
Nov 29, 2019

Conversation

davidanthoff
Copy link
Owner

No description provided.

@codecov
Copy link

codecov bot commented Nov 29, 2019

Codecov Report

Merging #49 into master will increase coverage by 0.78%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
+ Coverage   80.13%   80.92%   +0.78%     
==========================================
  Files           2        2              
  Lines         151      152       +1     
==========================================
+ Hits          121      123       +2     
+ Misses         30       29       -1
Impacted Files Coverage Δ
src/Electron.jl 80.4% <83.33%> (+0.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 780e1dd...2fd18e7. Read the comment docs.

@davidanthoff davidanthoff merged commit 13b88af into master Nov 29, 2019
@davidanthoff davidanthoff deleted the julia-1.3 branch November 29, 2019 04:35
tkoolen added a commit to tkoolen/Blink.jl that referenced this pull request Dec 28, 2019
Really, we shouldn't be using 7z.exe at all since it's an internal
Julia utility (see e.g. JuliaLang/julia#33687
and
JuliaLang/julia#33862 (comment)).

In Julia 1.3+, we should probably switch to the Artifacts system at some
point, as was also done in
davidanthoff/Electron.jl#49 for Electron.jl. But
right now, we need a fix for Julia 1.0.5 as well.

This rather complex logic was introduced in JuliaGizmos#177. If 7z.exe is not in `joinpath(Sys.BINDIR, Base.LIBEXECDIR` (or its equivalent on versions prior to 1.3). Note that e.g. BinDeps, which is widely used, doesn't even go to the trouble of
tkoolen added a commit to tkoolen/Blink.jl that referenced this pull request Dec 28, 2019
JuliaGizmos#177 (comment)

This commit is really just to simplify behavior and avoid another wild goose
chase in the future. It does not address recent failures on 1.0.5, which
are just because the location of the 7z.exe that's shipped with Julia
binary distributions on Windows has also changed from 1.0.4 to 1.0.5.

Really, we shouldn't be using 7z.exe at all since it's an internal Julia utility (see e.g. JuliaLang/julia#33687 and JuliaLang/julia#33862 (comment)).

This rather complex logic was introduced in
JuliaGizmos#177 for the case that 7z.exe is not in `joinpath(Sys.BINDIR, Base.LIBEXECDIR` (or its equivalent on versions prior to 1.3). Note that e.g. BinDeps, which is widely used, doesn't even go to the trouble of doing this (https://github.com/JuliaPackaging/BinDeps.jl/blob/3a871c35ef1b0f45760f48088cabd7915838b0e3/src/BinDeps.jl#L116-L120).

7z.exe should only be absent on Windows in case of a native build; it's
copied over into the build tree in the `win-extras` `make` target, which is
not a dependency of the default `make` target
(see https://github.com/JuliaLang/julia/blob/d562715a8f78627627fd0ccf3197de670efbf810/doc/build/distributing.md#windows).

The previous logic attempted to find 7z.exe in likely install locations
of binary downloads of Julia. I'm personally not convinced by the
arguments given here:
JuliaGizmos#177 (comment).

In Julia 1.3+, we should probably switch to the Artifacts system at some point, as was also done in davidanthoff/Electron.jl#49 for Electron.jl. But right now, we need a fix for Julia 1.0.5 as well.
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.

1 participant