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

installer silently fails? #17

Closed
stevengj opened this issue Oct 5, 2015 · 29 comments
Closed

installer silently fails? #17

stevengj opened this issue Oct 5, 2015 · 29 comments

Comments

@stevengj
Copy link
Member

stevengj commented Oct 5, 2015

I'm trying a 64-bit Windows 8 machine (technically, running on a Mac under VMware fusion) with Julia 0.4.0-rc4, and the miniconda installer seems to silently do nothing:
image
That is, it runs installer.exe and seems to succeed (no error is printed), but nothing seems to be installed (no conda command or anything else in deps/usr except for installer.exe itself).

Anything I should be trying?

@Luthaf
Copy link
Contributor

Luthaf commented Oct 5, 2015

Can you try to run it directly ?

using Conda
installer = joinpath(Conda.SCRIPTDIR, "installer.exe")
PREFIX = Conda.PREFIX
run(`$installer /S  /AddToPath=0 /RegisterPython=0 /D=$PREFIX`)

@stevengj
Copy link
Member Author

stevengj commented Oct 6, 2015

joinpath(Conda.SCRIPTDIR, "installer.exe") doesn't exist, but joinpath(dirname(Conda.SCRIPTDIR), "installer.exe") exists. However, run($installer /S /AddToPath=0 /RegisterPython=0 /D=$PREFIX) produces no output — it succeeds in the sense that success($installer /S /AddToPath=0 /RegisterPython=0 /D=$PREFIX) is true, but it appears to do nothing.

@stevengj
Copy link
Member Author

stevengj commented Oct 6, 2015

Running installer.exe (with the same arguments as above) at the command line also seems to do nothing.

@stevengj
Copy link
Member Author

stevengj commented Oct 6, 2015

Trying to run installer.exe with no arguments brings up the GUI installer, but when I try to install into PREFIX it complains that the directory already exists.

@stevengj
Copy link
Member Author

stevengj commented Oct 6, 2015

If I move installer.exe to the deps directory and delete usr, the silent install still fails, but I was able to succeed with the GUI install.

@stevengj
Copy link
Member Author

stevengj commented Oct 6, 2015

Does it matter that my home directory has two spaces in it? Just wondering if this is one of those "script gets confused by whitespace" problems.

@Luthaf
Copy link
Contributor

Luthaf commented Oct 6, 2015

It might be that kind of issue. Other than that, I have no idea ...

@stevengj
Copy link
Member Author

stevengj commented Oct 6, 2015

Nope, it still fails silently copying installer.exe to a path not containing spaces.

@mlubin
Copy link

mlubin commented Oct 22, 2015

Another report of this happening on Windows 10. The user has a space in their user name/home directory.

CC @juan-pablo-vielma

@Luthaf
Copy link
Contributor

Luthaf commented Oct 22, 2015

As I do not have a windows machine with me, I can not investigate this. If someone can try to dig this bug, I can try to fix it after.

@dhoegh
Copy link
Contributor

dhoegh commented Oct 25, 2015

I have investigated the issue. The core issue is that the quoting of the backticks is wrong and interfere with the argument passing on line: https://github.com/Luthaf/Conda.jl/blob/9dbdbca7e8338a67576bb3e92191b0000cd0d68e/src/Conda.jl#L120
The cmd on that line return a quote around the /D=path and not only the path, as:

julia> `$installer /S  /AddToPath=0 /RegisterPython=0 /D=$PREFIX`
path/Installer.exe /S  /AddToPath=0 /RegisterPython=0 '/D=path'`

This makes the installer fail silently. This issue has also occurred in JuliaLang/IJulia.jl#211 (comment). @ihnorton's workaround from https://gist.github.com/ihnorton/82dc5b41a537de710ab2 segfualt under 0.4. I cannot figure out why it segfault and it is not a good long term solution.

The easiest way to reproduce this failure under windows is:

 ENV["HOME"]="C:\\Users\\test\\My\ test\ home\\"
Pkg.add("Conda")
 using Conda
 Conda.list()

@Luthaf
Copy link
Contributor

Luthaf commented Oct 25, 2015

Thank you for taking the time to do this!

The cmd on that line return a quote around the /D=path and not only the path

Ok, so that is an issue with Julia quoting. Could quoting the path by ourselve fix this ? Like this:

`$installer /S  /AddToPath=0 /RegisterPython=0 /D="$PREFIX"`

@ihnorton's workaround from https://gist.github.com/ihnorton/82dc5b41a537de710ab2 segfualt under 0.4. I cannot figure out why it segfault and it is not a good long term solution.

This is messing directly with Win32 API, which I would prefer not have to do.

@dhoegh
Copy link
Contributor

dhoegh commented Oct 26, 2015

It will still quotes wrong.

julia> `installer /S  /AddToPath=0 /RegisterPython=0 /D="test space"`
`installer /S  /AddToPath=0 /RegisterPython=0 '/D=test space'`

Anyway the path should not be quoted at all according to my experience, supported by conda/conda#1068 (comment).

@stevengj
Copy link
Member Author

@dhoegh, note that no actual quotation marks are passed to the program, this is just how the backtick commands are printed.

run(cmd) in Julia does not involve the shell at all. It directly executes the installer.exe program and passes it the strings /S, /AddToPath=0, /RegisterPython=0, and /D=test space (no quotes). In a C program, these strings are passed via the char **argv argument to main(argc, argv).

So, the installer.exe program is getting the whole string, with no quotes. The issue seems to be that the installer.exe program is getting confused by /D arguments that have spaces — maybe it wants explicit quotation marks? To me, this is a bug in installer.exe.

In any case, you can pass explicit quotation marks if you want by

`$installer /S  /AddToPath=0 /RegisterPython=0 /D=\"$PREFIX\"`

(Note backslashes!)

@dhoegh
Copy link
Contributor

dhoegh commented Oct 26, 2015

As conda/conda#1068 (comment) states it should not be quoted and I have also tested the same in a cmd.exe running installer.exe /S /AddToPath=0 /RegisterPython=0 /D=test space work but installer.exe /S /AddToPath=0 /RegisterPython=0 /D="test space" do not work. Maybe we could workaround it by passing it /S, /AddToPath=0, /RegisterPython=0, /D=test, and space?

@Luthaf
Copy link
Contributor

Luthaf commented Oct 26, 2015

Maybe we could workaround it by passing it /S, /AddToPath=0, /RegisterPython=0, /D=test, and space?

That might actually work, because that should be what the installed is getting as arguments from the shell.

@stevengj
Copy link
Member Author

It would be mildly horrifying if the installer.exe program accepted paths with spaces when they are split over multiple arguments, but not when they are a single argument.

You can do this by

`$installer /S  /AddToPath=0 /RegisterPython=0 $(split("/D=$PREFIX"))`

(If this works, I'm having trouble imagining with what perverse code installer.exe is implemented.)

@stevengj
Copy link
Member Author

(This is especially horrifying if you consider paths with multiple spaces, e.g. foo bar. How would you pass this to installer.exe? installer /Dfoo bar will pass the arguments /Dfoo and bar from the shell, with no indication that there was more than one space between them.)

(Note that the above text does not display correctly because Markdown helpfully strips out the extra spaces, even in quoted code text.)

@dhoegh
Copy link
Contributor

dhoegh commented Oct 26, 2015

I have just tested that $installer /S /AddToPath=0 /RegisterPython=0 $(split("/D=$PREFIX")) works!!!

@stevengj
Copy link
Member Author

Probably someone should file an Anaconda issue. Not accepting paths with spaces as a single argument, but only as multiple arguments, is ridiculous (and cannot handle paths with multiple spaces as noted above).

@dhoegh
Copy link
Contributor

dhoegh commented Oct 26, 2015

I can confirm that $installer /S /AddToPath=0 /RegisterPython=0 /D=\"$PREFIX\" do not work. Our workaround $installer /S /AddToPath=0 /RegisterPython=0 $(split("/D=$PREFIX")) do not work for paths with multiple spaces.

@Luthaf
Copy link
Contributor

Luthaf commented Oct 26, 2015

A workaround for path with single space inside will already be better than the current implementation. I'll fill an issue at anaconda repo, but let's use the workaround for now!

@stevengj
Copy link
Member Author

Agreed.

(This is probably a casualty of the fact that Windows forces programs to do their own command-line parsing.)

@stevengj
Copy link
Member Author

Now I understand what is going on.

What I said above was wrong; I keep forgetting how messed-up Windows is.

It is literally impossible for Julia to send arguments to a spawned process as an array of individual strings. Windows only allows you to pass arguments as a single "command-line" string, and it forces programs to do their own parsing to split this string into individual arguments.

What libuv does when passing arguments containing spaces is to automatically enclose the arguments in double quotes. i.e. installer.exe /Dfoo bar is passed as the command line installer.exe "/Dfoo bar"

However, it is really on a program-by-program basis how the command-line arguments are parsed on Windows. Some programs probably recognize the double quotes and parse accordingly. Some other programs, like installer.exe, don't recognize the double quotes but combine multiple arguments by looking for /. So there is no one consistently right thing.

The right thing in our cases is to attach the UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS to the command, which will tell uv_spawn not to do the quotation marks. Then we can just do $installer /S /AddToPath=0 /RegisterPython=0 /D=$PREFIX and it should work, even for paths with multiple spaces.

Unfortunately, Julia doesn't currently provide an API for attaching this flag. I'll look into it.

@dhoegh
Copy link
Contributor

dhoegh commented Oct 26, 2015

I have made a PR #22 which also adds the test to appveyor.

@dhoegh
Copy link
Contributor

dhoegh commented Oct 27, 2015

This should probably be closed when a new Conda release is tagged, as the workaround should be good for the common user case.

@stevengj
Copy link
Member Author

Agreed.

@Luthaf
Copy link
Contributor

Luthaf commented Nov 3, 2015

@Luthaf Luthaf closed this as completed Nov 3, 2015
@stevengj
Copy link
Member Author

Support for verbatim Windows options has now been merged into Julia 0.5 (JuliaLang/julia@003f415). So, I think you can now do:

if VERSION >= "0.5.0-dev+8873" # Julia PR #13780
    run(Cmd(`$installer /S /AddToPath=0 /RegisterPython=0 /D=$PREFIX`, windows_verbatim=true))
else
    ....workaround from PR #22...
end

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

No branches or pull requests

4 participants