[ 
https://issues.apache.org/jira/browse/CB-14166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16608415#comment-16608415
 ] 

ASF GitHub Bot commented on CB-14166:
-------------------------------------

raphinesse commented on issue #688: CB-14166 distinguish npm.cmd and npm.exe in 
win32 to check escape necessary
URL: https://github.com/apache/cordova-lib/pull/688#issuecomment-419705596
 
 
   Thanks for your initiative on this and the great work debugging this issue, 
@knight9999.
   
   However, I stand by what I said in 
https://github.com/apache/cordova-lib/pull/622#issuecomment-404836911:
   > IMHO this should happen in `superspawn` not in `plugman`. `plugman` should 
have no obligation to worry whether fetch might use its arguments unescaped in 
a shell.
   
   Or as @oliversalzburg puts it in [CB-14166]:
   
   > The solution is still to properly escape when running commands on the 
shell, and to not escape when not running on the shell. Proper abstractions 
will take care of that for you.
   >
   > In this scenario, the only problem is that part of the code (`fetch.js`) 
decides that it already knows how the command is executed in the end, which it 
does not. The additional escaping introduced in this place is a mistake. It 
needs to be removed and `superspawn` needs to be taught how to properly escape, 
or, as I have been saying continuously, it should be replaced with something 
that works (`cross-spawn`, `execa`).
   
   To sum it up:
   - The current code that handles the version escaping is broken and needs to 
be removed, not patched.
   - We need a properly working abstraction for our use case of "I want to run 
a command, without worrying about the platform I'm on". IMHO, the options to 
achieve that, ordered by the amount of work involved (ascending) are:
     - Patch `superspawn` to handle this case properly
     - Change `superspawn` to use `cross-spawn` instead of `child_process.spawn`
     - Get rid of `superspawn` in favor of `cross-spawn` or `execa`
   
   Incidentally, these options are also ordered by my approval for them 
(ascending).
   
   
   [CB-14166]: 
https://issues.apache.org/jira/browse/CB-14166?focusedCommentId=16605736&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16605736

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Cordova on windows fails when adding plugin
> -------------------------------------------
>
>                 Key: CB-14166
>                 URL: https://issues.apache.org/jira/browse/CB-14166
>             Project: Apache Cordova
>          Issue Type: Bug
>          Components: cordova-windows
>            Reporter: takuya
>            Assignee: Jesse MacFadyen
>            Priority: Blocker
>         Attachments: screenshot-1.png, スクリーンショット 2018-09-06 11.36.42.png
>
>
> cordova on windows fails to add the plugin.
> This error happens even when cordova-plugin-whitelist.
> Therefore `cordova platform add windows` also fails.
> For example,
> ```
> > cordova create sample
> > cd sample
> > cordova platform add windows
> ```
> brings following error.
> ```
> ...
> Check your connection and plugin name/version/URL.
> Error: C:\Program Files (x86)\Nodist\bin\npm.exe: Command failed with exit 
> code 1 Error output:
> npm ERR! code EINVALIDTAGNAME
> npm ERR! Invalid tag name ""1"": Tags may not have any characters that 
> encodeURIComponent encodes.
> ```
> This error happens for windows environment only.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org

Reply via email to