"arc patch" fails on filenames containing spaces


#1

Observed Behavior:
arc patch doesn’t correctly quote spaces in filenames.

katalon accept file names in following format

tests/katalon/Test Cases/$site/Validation/
tests/katalon/Test Cases/$site/Validation/Myaccount/
tests/katalon/Test Cases/$site/Validation/Myaccount/Invalid-inputs.tc
tests/katalon/Test Cases/$site/Validation/Myaccount/Valid-Inputs.tc

When try to patch phabricator revision with above files, getting following warning

patching file tests/katalon/Test
The next patch would create the file tests/katalon/Test,
which already exists! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
The next patch would create the file tests/katalon/Test,
which already exists! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
The next patch would create the file tests/katalon/Test,
which already exists! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
The next patch would create the file tests/katalon/Test,
which already exists! Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored

WARNING Some hunks could not be applied cleanly by the unix ‘patch’ utility. Your working copy may be different from the revision’s base, or you may be in the wrong subdirectory. You can export the raw patch file using ‘arc export --unified’, and then try to apply it by fiddling with options to ‘patch’ (particularly, -p), or manually. The output above, from ‘patch’, may be helpful in figuring out what went wrong.

it is trying to write the file tests/katalon/Test, which is truncated at the first space character.

Technical observation
If a file path contains spaces, it must be terminated with a tab.

Expected Behavior:
arc path should work properly for filenames with space

Phabricator Version:
arcanist 622862e0479956c98cf0327b5bcb17abada6a0ec (7 Nov 2017)
libphutil 3510bfbc8879ff9739495b32b3386c4bf644937c (7 Nov 2017)

Reproduction Steps:
Try to patch phabricator revision which has files with space in filename.


#2

Here is code which solved issue locally by appending ‘\t’ at the end of filenames, but I am not sure how to submit code change request to archanist project.

private function encodeGitTargetPath($path) {
// +++ b/X Y.txt\t
if (strpos($path, ’ ') !== false) {
$path = $path."\t";
}
return $path;
}

updated filepath variables in ‘toUnifiedDiff()’ in ‘arcanist/src/parser/ArcanistBundle.php’

if ($cur_path !== ‘/dev/null’ && $old_path !== ‘/dev/null’) {
$old_path = $cur_path;
}

$old_path = $this->encodeGitTargetPath($old_path);
$cur_path = $this->encodeGitTargetPath($cur_path);

$result[] = '— '.$old_path.$eol;
$result[] = '+++ '.$cur_path.$eol;


#3

This bug was probably fixed by https://secure.phabricator.com/D18869, on January 16, about 3 months before you reported it here. I suspect running arc upgrade will pick up the fix, since your version of arc is about 5 months out of date.

It’s important that you make sure bugs reproduce against up-to-date versions of the software before reporting them here. Many reports against out-of-date versions of the software are known bugs which have been fixed in more recent versions of the software.


In fact, your patch appears to be byte-for-byte identical to D18869, except that you’ve removed most of my comment in encodeGitTargetPath(). I have no idea what grand adventure led to you posting a copy of a patch already in the upstream for 3 months to this forum, asking for instructions on how to submit it to the upstream.


#4

Thanks for your reply.

Yes, I referred your ‘ArcanistBundle.php’ copy from here

My issue is, I am using Subversion with archanist, the code which has used to fix the issue with Git here https://secure.phabricator.com/D18869 the same code needs to used with subversion.

So the same function needs to be used in ‘toUnifiedDiff()’

  if ($cur_path !== '/dev/null' && $old_path !== '/dev/null') {
        $old_path = $cur_path;
  }
            
      $old_path = $this->encodeGitTargetPath($old_path);
      $cur_path = $this->encodeGitTargetPath($cur_path);
      
      $result[] = '--- '.$old_path.$eol;
      $result[] = '+++ '.$cur_path.$eol;

      $result[] = $hunk_changes;

Thanks,
Pratibha


#5

If you’d like this fixed, please file a new bug report and follow all the instructions. In particular, make sure your version is up-to-date and that your reproduction instructions include ALL the information that someone else needs in order to reproduce this issue. This report omitted that you’re using Subversion, which is critical to reproducing the problem you’re encountering.


#6

filed new bug here “arc patch” fails on filenames containing spaces with Subversion

Thanks