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

Fix symlink creation when resulting command has spaces #286

Closed
wants to merge 2 commits into from

Conversation

CodeBlueDev
Copy link

The Go version used for the repository development could not be found
so the latest installation available was used. In doing so, some compile
time errors appeared with regards to filepath and the Unzip function
that had to be corrected.

The Go exec module wraps the os.StartProcess functionality. The initial
argument escaping appears to be correct, however, during the pipe
writing when actually starting the process the command is escaped again
in quotations making the command invalid. This makes the command fail
and no symlink is created.

This was verified by printing the resulting argument string by utilizing
the same logic that makes the command line argument and escapes it. As
this appears to be a Go library issue, a different tactic was attempted.
In case of this failure, a backup mechanism has been put in place to
use the os module to create the symlink directly.

After much deliberation it was decided this backup mechanism should only
be attempted when the initial attempt fails. The reasoning behind this
is because the initial command request attempts to elevate the command
which would give access to create the symbolic link in "Program Files",
which is otherwise a protected folder. The os call could fail here due
to insufficient permissions.

Potentially fixes: #281, #266, #30

CodeBlueDev and others added 2 commits July 13, 2017 08:16
The Go version used for the repository development could not be found
so the latest installation available was used. In doing so, some compile
time errors appeared with regards to filepath and the Unzip function
that had to be corrected.

The Go exec module wraps the os.StartProcess functionality. The initial
argument escaping appears to be correct, however, during the pipe
writing when actually starting the process the command is escaped again
in quotations making the command invalid. This makes the command fail
and no symlink is created.

This was verified by printing the resulting argument string by utilizing
the same logic that makes the command line argument and escapes it. As
this appears to be a Go library issue, a different tactic was attempted.
In case of this failure, a backup mechanism has been put in place to
use the os module to create the symlink directly.

After much deliberation it was decided this backup mechanism should only
be attempted when the initial attempt fails. The reasoning behind this
is because the initial command request attempts to elevate the command
which would give access to create the symbolic link in "Program Files",
which is otherwise a protected folder. The os call could fail here due
to insufficient permissions.

Potentially fixes: coreybutler#281, coreybutler#266, coreybutler#30
@coreybutler
Copy link
Owner

My apologies for the delay, I actually had a few of these changes already completed on my local machine. I haven't had time to go through this yet. I fixed a few merge conflicts, but I need to give this a little more attention before I take action (I'm leaning towards merging it).

@CodeBlueDev
Copy link
Author

Hey Corey,

No worries. Let me know if you need anything from me.

I apologize for the conflicts. The Go version I installed was not happy with the import paths or the Unzip function. I tried to find the version of Go you use to no avail so I went ahead and used latest. Perhaps this could be added to your contributing doc. Those were the only conflicts I recall running into just to get it to compile locally. But I may just be doing something silly as I am not familiar with Go.

I appreciate you taking the time to check it out and respond. Feel free to make whatever changes you need to or cherry pick what you want.

@coreybutler
Copy link
Owner

Never got around to this (sorry!) and Go has changed a lot since this was opened.

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.

nvm-windows not working on Windows 10/64/UAC: popup message but no symlink gets created
2 participants