Page MenuHomePhabricator

ArcanistPhpcsLinter issue with msdos and cygwin
Closed, ResolvedPublic

Description

Hello, I am using the following in msdos:

phpcs --version

PHP_CodeSniffer version 1.5.1 (stable) by Squiz (http://www.squiz.net)

commit 7c4600052700668f78a6595f9477618ca2c0ed5a
Date: Mon Feb 3 10:01:19 2014 -0800

where arc

D:\Projects\workspace\vendor\facebook\arcanist\bin\arc
D:\Projects\workspace\vendor\facebook\arcanist\bin\arc.bat

.arcconfig:
{

"project_id" : "base-codeigniter",
"conduit_uri" : "http://phabricator.hostname.com:8082",
"lint.engine" : "ArcanistSingleLintEngine",
"lint.engine.single.linter" : "ArcanistPhpcsLinter"

}

D:\Projects\sohyper\workspace\base-codeigniter-scrape>arc lint application\views
\ValidRegexTarget.php --trace
libphutil loaded from 'D:\Projects\sohyper\workspace\vendor\facebook\libphutil\s
rc'.
arcanist loaded from 'D:\Projects\sohyper\workspace\vendor\facebook\arcanist\src
'.
Working Copy: Reading .arcconfig from "D:\Projects\workspace\base-codeigniter/.arcconfig".
Working Copy: Path "D:\Projects\workspace\base-codeigniter" is part of git working copy "D:\Projects\workspace\base-codeigniter"
.
Working Copy: Project root is at "D:\Projects\sohyper\workspace\base-codeigniter
-scrape".

[0] <lint> ArcanistPhpcsLinter <paths = 1>
[1] <exec> $ where "phpcs"

<<< [1] <exec> 220,533 us

[2] <exec> $ "phpcs" --report=xml "D:\Projects\workspace\base-codei

gniter-scrape\application\views\ValidRegexTarget.php"
<<< [0] <lint> 285,608 us

[3] <lint> ArcanistPhpcsLinter

<<< [2] <exec> 28,112 us
<<< [3] <lint> 29,159 us

[2014-02-04 13:32:32] EXCEPTION: (PhutilAggregateException) Some linters failed:

  • ArcanistPhpcsLinter: CommandException: Command failed with error #1! COMMAND "phpcs" --report=xml "D:\Projects\workspace\base-codeigniter-scra

pe\application\views\ValidRegexTarget.php"

STDOUT
(empty)

STDERR
The filename, directory name, or volume label syntax is incorrect.
 at [D:\Projects\sohyper\workspace\vendor\facebook\arcanist\src\lint\engin

e\ArcanistLintEngine.php:370]

#0 ArcanistLintEngine::run() called at [D:\Projects\workspace\vendor\f

acebook\arcanist\src\workflow\ArcanistLintWorkflow.php:359]

#1 ArcanistLintWorkflow::run() called at [D:\Projects\workspace\vendor

\facebook\arcanist\scripts\arcanist.php:321]


for cygwin
CYGWIN_NT-6.2 netbook 1.7.27(0.271/5/3) 2013-12-09 11:54 x86_64 Cygwin

which arc

arc is /cygdrive/d/sohyper/workspace/vendor/facebook/arcanist/bin/arc

arc lint
Could not open input file: /cygdrive/d/Projects/sohyper/workspace/vendor/facebook/arcanist/bin/../scripts/arcanist.php

Any help appreciated

Revisions and Commits

Event Timeline

JoggerTech raised the priority of this task from to Needs Triage.
JoggerTech updated the task description. (Show Details)
JoggerTech added a subscriber: JoggerTech.

Does the command work when run on its own?

phpcs --report=xml D:\Projects\workspace\base-codeigniter-scrape\application\views\ValidRegexTarget.php

Does that path actually exist?

Yes. PHPcs works fine when running individually

Hi, I've stumbled on this bug too, Windows 7, from commandline:

D:\Darek\CD3\server>arc lint cd3\server\core\storedviews.py --trace
libphutil loaded from 'D:\Darek\Arcanist\libphutil\src'.
arcanist loaded from 'D:\Darek\Arcanist\arcanist\src'.
Working Copy: Reading .arcconfig from "D:\Darek\CD3/.arcconfig".
Working Copy: Path "D:\Darek\CD3\server" is part of `svn` working copy "D:\Darek\CD3".
Working Copy: Project root is at "D:\Darek\CD3".
>>> [0] <exec> $ where "pep8"
<<< [0] <exec> 124,006 us
Examining paths for linter "mainSources".
Examining path 'server\cd3\server\core\storedviews.py'...
Testing "include" rules.
Path matches include rule: /.*\.py/
Path matches.
Found 1 matching paths for linter "mainSources".
>>> [1] <exec> $ where "pep8"
<&
lt;< [1] <exec> 118,005 us
>>> [2] <exec> $ where "pep8"
<<< [2] <exec> 115,007 us
>>> [3] <exec> $ where "pep8"
<<< [3] <exec> 118,006 us
>>> [4] <exec> $ where "pep8"
<<< [4] <exec> 114,005 us
>>> [5] <exec> $ where "pep8"
<<< [5] <exec> 119,006 us
>>> [6] <exec> $ "pep8" --version
<<< [6] <exec> 194,010 us
>>> [7] <lint> ArcanistPEP8Linter <paths = 1>
>>> [8] <exec> $ where "pep8"
<<< [8] <exec> 116,007 us
>>> [9] <exec> $ where "pep8"
<<< [9] <exec> 117,007 us
>>> [10] <exec> $ where "pep8"
<<< [10] <exec> 117,006 us
>>> [11] <exec> $ where "pep8"
<<< [11] <exec> 116,006 us
>>> [12] <exec> $ where "pep8"
<<< [12] <exec> 116,006 us
>>> [13] <exec> $ "pep8"  --max-line-length=120 --ignore=W293 "D:\Darek\CD3\server\cd3\server\core\storedviews.py"
<<< [7] <lint> 593,034 us
>>> [14] <lint> ArcanistPEP8Linter
<<< [13] <exec> 13,000 us
<<< [14] <lint> 9,001 us

[2014-04-08 06:34:17] EXCEPTION: (PhutilAggregateException) Some linters failed:
    - ArcanistPEP8Linter: CommandException: Command failed with error #1!
    COMMAND
    "pep8"  --max-line-length=120 --ignore=W293 "D:\Darek\CD3\server\cd3\server\core\storedviews.py"

    STDOUT
    (empty)

    STDERR
    Nazwa pliku, nazwa katalogu lub składnia etykiety woluminu jest niepoprawna.
    at [D:\Darek\Arcanist\arcanist\src\lint\engine\ArcanistLintEngine.php:370]
#0 ArcanistLintEngine::run() called at [D:\Darek\Arcanist\arcanist\src\workflow\ArcanistLintWorkflow.php:359]
#1 ArcanistLintWorkflow::run() called at [D:\Darek\Arcanist\arcanist\scripts\arcanist.php:322]

Message from STDERR "Nazwa pliku, nazwa katalogu lub składnia etykiety woluminu jest niepoprawna." means "The filename, directory name, or volume label syntax is incorrect."

When I run this command as is:

D:\Darek\CD3\server>"pep8" --max-line-length=120 --ignore=W293 "D:\Darek\CD3\server\cd3\server\core\storedviews.py"
D:\Darek\CD3\server\cd3\server\core\storedviews.py:15:15: E401 multiple imports on one line
D:\Darek\CD3\server\cd3\server\core\storedviews.py:33:1: E302 expected 2 blank lines, found 1
D:\Darek\CD3\server\cd3\server\core\storedviews.py:35:21: E128 continuation line under-indented for visual indent
D:\Darek\CD3\server\cd3\server\core\storedviews.py:50:51: E251 unexpected spaces around keyword / parameter equals
D:\Darek\CD3\server\cd3\server\core\storedviews.py:50:53: E251 unexpected spaces around keyword / parameter equals
D:\Darek\CD3\server\cd3\server\core\storedviews.py:50:68: E251 unexpected spaces around keyword / parameter equals
D:\Darek\CD3\server\cd3\server\core\storedviews.py:50:70: E251 unexpected spaces around keyword / parameter equals
D:\Darek\CD3\server\cd3\server\core\storedviews.py:50:88: E251 unexpected spaces around keyword / parameter equals
D:\Darek\CD3\server\cd3\server\core\storedviews.py:50:90: E251 unexpected spaces around keyword / parameter equals
... and so on ...

Could you help me and suggest what could be wrong? I may try to fix somehow this bug by myself, but for now I dont know where even start ;)

I believe I've discovered the issue to be related to PHP's proc_open, which is used in the ExecFuture to execute phpcs. I'm not sure yet what the fix is, but thought I'd provide my findings in case someone from the Phab team has an idea. At first glance, it seems like the only solution is to modify ExecFuture to wrap the command in double quotes before opening it with proc_open.

Here's what I did to discover the problem...

I created a file: test.php with the following in it:

<?
$descriptorspec = array(
0 => array("pipe", "r"),
1 => array("pipe", "w"),
2 => array("file", "error_output.txt", "a")
);
$process = proc_open('"phpcs" "--report=xml" "test.php"', $descriptorspec, $pipes);
if (is_resource($process)) {
    fclose($pipes[0]);

    echo stream_get_contents($pipes[1]);
    fclose($pipes[1]);
    $return_value = proc_close($process);

    echo "command returned $return_value\n";
}

?>

When I copy/paste the command into a command window, it executes fine. But when running through proc_open, it outputs an error inside error_output.txt that says:

'phpcs" "--report' is not recognized as an internal or external command, operable program or batch file.

Now, if you modify the proc_open to have double quotes around it like this:

$process = proc_open('""phpcs" "--report=xml" "test.php""', $descriptorspec, $pipes);

then everything works fine and you should get output similar to this:

<?xml version="1.0" encoding="UTF-8"?>
<phpcs version="1.5.3">
<file name="D:\eLYK Projects\ErrorSet\test.php" errors="3" warnings="0">
 <error line="1" column="1" source="Generic.PHP.DisallowShortOpenTag.Found" seve
rity="5">Short PHP opening tag used; expected &quot;&lt;?php&quot; but found &qu
ot;&lt;?&quot;</error>
 <error line="1" column="1" source="Generic.Files.LineEndings.InvalidEOLChar" se
verity="5">End of line character is invalid; expected &quot;\n&quot; but found &
quot;\r\n&quot;</error>
 <error line="1" column="3" source="PEAR.Commenting.FileComment.Missing" severit
y="5">Missing file doc comment</error>
</file>
</phpcs>
command returned 1

There is a bug over on php.net, but it looks like it's not being actively worked on: https://bugs.php.net/bug.php?id=49139

For those that don't want to read through the bug report at php.net, this is what's happening that's causing this linting process to fail...

proc_open is opening the process by calling "cmd.exe /c [cmd]" where [cmd] is the process to be opened.

From cmd /?, quotes are processed as follows:

If /C or /K is specified, then the remainder of the command line after
the switch is processed as a command line, where the following logic is
used to process quote (") characters:

 1.  If all of the following conditions are met, then quote characters
     on the command line are preserved:

     - no /S switch
     - exactly two quote characters
     - no special characters between the two quote characters,
       where special is one of: &<>()@^|
     - there are one or more whitespace characters between the
       two quote characters
     - the string between the two quote characters is the name
       of an executable file.

 2.  Otherwise, old behavior is to see if the first character is
     a quote character and if so, strip the leading character and
     remove the last quote character on the command line, preserving
     any text after the last quote character.

Method #1 above is not used to process quotes because the exactly two quote characters condition is not met. Method #2 is the logic being used.

According to method #2, if you specify "phpcs" "--report=xml" "test.php" as the process to open, proc_open will execute cmd.exe /c "phpcs" "--report=xml" "test.php" which, in turn, will strip the first quote and the last quote to give a final command string of phpcs" "--report=xml" "test.php which when run via copy/paste will return an error. Similarly, using the previous poster's commands, if you copy/paste "pep8" --max-line-length=120 --ignore=W293 "D:\Darek\CD3\server\cd3\server\core\storedviews.py" into a command prompt, it will run fine. But if you modify it the way proc_open ultimately will (although proc_open isn't really doing it, it's Windows' cmd.exe), you get pep8" --max-line-length=120 --ignore=W293 "D:\Darek\CD3\server\cd3\server\core\storedviews.py which throws the The filename, directory name, or volume label syntax is incorrect. error when copy/pasted. A similar thing happens with the OP's phpcs command.

This is why the recommended work-around in the php.net bug report is to enclose the command in double quotes; the quotes added will simply be stripped out when cmd.exe /c runs.

There's also the bypass_shell option you can pass to the other_options parameter, which is supposed to bypass the shell. However, when implemented in my test.php, I just get a warning as follows: PHP Warning: proc_open(): CreateProcess failed, error code - 2 in D:\eLYK Projects\ErrorSet\test.php on line 7

It appears that the fix for this is to detect Windows platforms and simply add double quotes around the whole command. There could be an issue with linting processes that don't require arguments and thus satisfy all the conditions specified in method #1 above for quote processing, but I think that since all the arguments are being escaped with quotes via csprintf, there will never be only 2 quotes in any linting command...

@hach-que, I think you ran into this kind of thing too -- does some approximation of this fix things?

diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php
index f9c1242..60d6590 100644
--- a/src/future/exec/ExecFuture.php
+++ b/src/future/exec/ExecFuture.php
@@ -631,6 +631,13 @@ final class ExecFuture extends Future {
         $trap = null;
       }
 
+      // Survive insane cmd.exe quoting behavior, see T4395.
+      if (phutil_is_windows()) {
+        if (strpos($unmasked_command, '"') !== false) {
+          $unmasked_command = '"'.$unmasked_command.'"';
+        }
+      }
+
       $proc = @proc_open(
         $unmasked_command,
         self::$descriptorSpec,

The solution for us was to put the command in a batch file and then execute the batch file. I'm not at work right now so I can't confirm whether that fix actually resolves the problem we had.

I can confirm that your fix makes the problem go away at my Windows pc. Nice :)

I can confirm that your fix makes the problem go away at my Windows pc. Nice :)

Which fix are you talking about?

As far as the diff referenced by @epriestley, I can confirm that it did fix the issue in my case.