840: post-review can't diff correctly Perforce paths with spaces?

btse****@gmai***** (Google Code) (Is this you? Claim this profile.)
chipx86
 chipx86
May 28, 2009 
What's the URL of the page containing the problem? N/A


What steps will reproduce the problem?
1. Have Perforce
2. In your perforce system, have a path like //depot/dev/proj1/path with
spaces/file.cpp
3. Edit the file and setup to post-review the change list
4. Review board interprets it as a change of "deleted content"

What is the expected output? What do you see instead?
- I would have expected it to properly do the diffs and handle the path
with spaces.
- 
When doing post-review -d, the problem looks to be that the path agurment
to the diff command is not-escaped or complete

so i've seen something like

diff -urNp /cygdrive/c/TEMP/tmpatI2gl //btse-ws2//dev/proj1/path


- just 'path' instead of 'path with space/file.cpp'

What operating system are you using? What browser?

Windows XP, Firefox 3.0.1


Please provide any additional information below.

Is there some patch in the python script to properly escape these depot paths?
#1 btse****@gmai***** (Google Code) (Is this you? Claim this profile.) 
Ahh. After looking at the post-review script I see the comments in function:
_depot_to_local()

        # XXX: This breaks on filenames with spaces.
        return last_line.split(' ')[2]
Rats.
david
#2 david
  • +Confirmed
  • +Component-Scripts
#3 devl****@buzzard******** (Google Code) (Is this you? Claim this profile.) 
I'm attaching a patch that I believe fixes the "spaces in paths" problem with Perforce.  At a high level, it passes 
the "-ztag" flag to the "p4 where" command to split the Perforce and local paths onto separate lines.

This has only been tested briefly on Mac OS X (and I'm not a Python expert) so YMMV, especially on other 
platforms.
  • +
    post-review.patch 
    --- post-review	(revision 1894)
    +++ post-review	(working copy)
    @@ -1526,19 +1526,20 @@
             Given a path in the depot return the path on the local filesystem to
             the same file.
             """
    -        # $ p4 where //user/bvanzant/main/testing
    -        # //user/bvanzant/main/testing //bvanzant:test05:home/testing /home/bvanzant/home-versioned/testing
    -        where_output = execute(["p4", "where", depot_path], split_lines=True)
    +        # $ p4 -ztag where //user/bvanzant/main/testing
    +        # ... depotFile //user/bvanzant/main/testing
    +        # ... clientFile //bvanzant:test05:home/testing
    +        # ... path /home/bvanzant/home-versioned/testing
    +        where_output = execute(["p4", "-ztag", "where", depot_path], split_lines=True)
             # Take only the last line from the where command.  If you have a
             # multi-line view mapping with exclusions, Perforce will display
             # the exclusions in order, with the last line showing the actual
    -        # location.
    -
#4 shar*****@gmai***** (Google Code) (Is this you? Claim this profile.) 
I was facing the same issue with Perforce on windows a couple of days back. I applied
the patch and it worked nicely.
chipx86
#5 chipx86 
Committed as r1990.
  • -Confirmed
    +Fixed
  • -Priority-Medium
    -Component-Scripts
    +Priority-High
    +Component-RBTools
    +Milestone-Release1.0
  • +chipx86