Remove windows specific juliarc.jl #21540
Conversation
StefanKarpinski
changed the title
|
embedding example is failing for some reason, any ideas? |
StefanKarpinski
changed the title
musm
force-pushed
the
julrc
branch
3 times, most recently
from
fbd9a93
to
58242f2
Compare
|
@tkelman anyideas on the embedding failure, is busybox or 7z somehow needed for that ? Not sure how to test locally last time I tried to build on windows using the readme was not successful . |
You're going to need to be more specific than "not successful" |
|
For the embedding executable to run on windows, libjulia either needs to be copied into the same directory as the embedding executable, or we can do the path appending as part of |
examples/embedding/embedding-test.jl
Outdated
| @@ -3,6 +3,11 @@ | |||
| # tests the output of the embedding example is correct | |||
| using Base.Test | |||
|
|
|||
| if is_windows() | |||
| # libjulia needs to be in the same directory as the embedding executable | |||
There was a problem hiding this comment.
Feel free to update the makefile to also copy libjulia if you feel that it's also useful to do that for external users copying the makefile
|
great tests now pass |
|
not a lot of packages that explicitly use 7z |
StefanKarpinski
changed the title
|
bump, FYI this will need a tag on WinRPM and BinDeps before merging |
|
WinRPM and BinDeps have been tagged, this should be good to go now. Unless someone requests something else. |
|
Updated to include NEWS entry. |
NEWS.md
Outdated
| @@ -175,6 +175,8 @@ Deprecated or removed | |||
| * The forms of `read`, `readstring`, and `eachline` that accepted both a `Cmd` object and an | |||
| input stream are deprecated. Use e.g. `read(pipeline(stdin, cmd))` instead ([#22762]). | |||
|
|
|||
| * The default `juliarc.jl` file on Windows has been removed. Use `joinpath(JULIA_HOME, "7z.exe")` | |||
| if you need access, e.g., to `7z.exe` on Windows ([#21540]). | |||
There was a problem hiding this comment.
there's nothing specific to 7z about this - it's the case for any executable or library in julia's bin dir
There was a problem hiding this comment.
Correct, that is why I put an e.g. in the entry. Will update since it may not be clear.
NEWS.md
Outdated
| @@ -175,6 +175,9 @@ Deprecated or removed | |||
| * The forms of `read`, `readstring`, and `eachline` that accepted both a `Cmd` object and an | |||
| input stream are deprecated. Use e.g. `read(pipeline(stdin, cmd))` instead ([#22762]). | |||
|
|
|||
| * The default `juliarc.jl` file on Windows has been removed. Now must explicitly include the | |||
| file if you need access to executables or libraries in the `JULIA_HOME` directory, e.g. | |||
musm
force-pushed
the
julrc
branch
3 times, most recently
from
675f497
to
7c64576
Compare
|
The FreeBSD CI build error is unrelated (caused by #22566) |
|
Interesting (unrelated) failure on 64 bit windows https://gist.github.com/musm/117d2a64cc8dc874455de45283b230df |
|
The error looks similar to #11083 |
|
restarting for travis |
|
Unrelated errors on Travis. Is this good to go now? |
|
@tkelman can we proceed with this? It would be nice to have the same experience as in linux and since the juliarc file is not needed it makes sense to me to remove it. FWIW I hit the original issue twice (first time to report the issue, which was noted by |
|
Rebased PR. |
|
bump |
|
I don't personally think the issue this fixes is consequential, but the changes it's making could be. Windows shared libraries are searched for in PATH, on other platforms that's handled differently. |
I think all the libs are loaded when julia is started irrespective of whether there are in the .juliarc file. For example I haven't ran into any issues with running julia without this file and calling functions that are linked to the lib in the @vtjnash you made this comment here #18769 (comment) I agree it's not necessary, but I hate stuff that exists or is there without a purpose. I just would like to confirm that this is in fact needed before closing the PR. |
|
It's not pure Julia code I'm concerned about, it's packages or external programs that might be trying to find libraries included in Julia, and may have a harder time doing so with this change. |
|
How about we test this out and if there are any reported issues, we can revert the change? |
|
The trouble is that this won't get widespread testing until after it's too late to revert this (i.e. 1.0). Still, I guess we're going ahead with trying it. |
|
Fair enough, feel free to revert . |
|
If it is determined to be a problem during 0.7 feature freeze or rc's, I don't think it would break anything to revert even fairly late in the 0.7 release cycle. |
close #18769