-
Notifications
You must be signed in to change notification settings - Fork 63
Quote $cmdargs string to prevent path errors #357
Conversation
Hi @ph1ll, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
The tests are failing for this, but it also seems they were failing prior to the change. I am not familiar with this script so not sure how/where else this is called and what the expected format for |
Closing as causes issues with other dnvm commands |
Ok, following a bit more debugging, the steps to reproduce this problem in PowerShell are:
This results in
As
This fails firstly due to the unescaped/unquoted parenthesis but would also fail due to the unquoted path. It appears that Visual Studio is attempting to install My PowerShell isn't great, but if I come up with a fix I will update the PR. Is it preferable to open this as an issue for now? |
Hi @ph1ll, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
Hi @ph1ll, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
Apologies for the spam while rebasing then. Updated to loop over arguments and check whether they contain whitespace or parenthesis, and if so, ensure they are quoted prior to adding to |
People on Stack Overflow also appear to be affected by this issue. http://stackoverflow.com/questions/31546274/how-to-fix-dnx-dnvm-in-visual-studio-2015/31571324 |
@ph1ll, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
@BrennanConroy please review in early RC1. Thanks. |
@BrennanConroy When you get the time :) |
Could you rebase this on the latest changes, then I'll merge it in |
@BrennanConroy PR has been rebased on the latest dev branch. Let me know if you need anything else. |
Any idea why AppVeyor is failing? |
Just looking into that now, will update when I work it out |
I had missed the initial |
Ok, updated and I also fixed tabs/whitespace in my changes. Hopefully appveryor and travis will be happy now. As a comparison, the behavior of the current dev branch is:
Where as with this PR:
|
Quote $cmdargs string to prevent path errors
b1ae8a9 |
Using the new release of VS 2015 an error is thrown in the DNVM output window when VS attempts to install a runtime when none was found in the user .dnx folder.
This is caused by the script attempting to execute:
Naturally this path should be quoted. This MR resolves this by quoting the $cmdargs variable, resulting in the execution of:
And VS is once again happy.
A side effect of this bug (VS not being able to load DNX) is that all DNX projects are loaded as "Miscellaneous Files" and no intellisense is provided and builds within VS are broken. Restarting VS following copying the patched
dnvm.ps1
toC:\Program Files\Microsoft DNX\Dnvm
resolves this.