FWIW - I posted a review request yesterday that contains a different approach to fixing this:
http://reviews.review-board.org/r/815/ (The same changes are also posted to issue 840 as a patch file: http://code.google.com/p/reviewboard/issues/detail?id=840) -- Steven On Apr 14, 10:54 pm, roshan pius <roshanpiustho...@gmail.com> wrote: > Hi Christian, > > I am not too good at python programming, but i have got it working now.The > issue was definitely beacause of the spaces in the path in the local file. > > This was the culprit - "last_line.split(' ')[2]" in: > > def _depot_to_local(self, depot_path): > """ > 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) > # 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. > last_line = where_output[-1] > > # XXX: This breaks on filenames with spaces. > return last_line.split(' ')[2] > > In my case the last_line variable was : > > //<depot>/<branch>/<src-dir-path>/Assisted File Transfer/AftMain.cpp > //<worksapce>/<branch>/<src-dir-path>/Assisted File Transfer/AftMain.cpp > <worksapce-src-dir-path>\Assisted File Transfer\AftMain.cpp > > So last_line.split(' ')[2] returned : > Transfer/AftMain.cpp. > It should have been: > <worksapce-src-dir-path>\Assisted File Transfer\AftMain.cpp > > So I've made some changes(very crude ones, but it's working :-) ) > > The New Function Definition: > > def _depot_to_local(self, depot_path): > """ > 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) > # 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. > > debug("Local Path Info From Perforce: %s" % (where_output)) > > last_line = where_output[-1].strip() > debug("Last Line Of the Info: %s" % (last_line)) > > local_file_name=last_line.split('//')[1].rstrip().split('/')[-1].rstrip() > debug("File Name: %s" % (local_file_name)) > > file_path_dir=last_line.split(local_file_name)[-2].rstrip().lstrip() > debug("Directory Name: %s" % (file_path_dir)) > > file_path_name=string.join([file_path_dir,local_file_name],'') > debug("The File Path: %s" %(file_path_name)) > > return file_path_name > > Maybe you can make it better and submit a fix for the issue. > Roshan Pius > > On Wed, Apr 15, 2009 at 2:15 AM, Christian Hammond <chip...@chipx86.com> > wrote: > > > Yeah, it's definitely due to spaces in the filenames. Eris Huss did some > > work that should lead to a fix for this, I believe, but someone > encountering > > this problem needs to find where it's breaking and update the code to make > > this work. > > > Christian > > > -- > > Christian Hammond - chip...@chipx86.com > > Review Board -http://www.review-board.org > > VMware, Inc. -http://www.vmware.com > > > On Tue, Apr 14, 2009 at 6:37 AM, roshan pius <roshanpiustho...@gmail.com> > > wrote: > > >> The complete output of post-review -d : > > >> >>> p4 info > >> >>> repository info: Path: xxxxxxxx:1666, Base path: None, Supports > >> >>> changesets: True > >> >>> Generating diff for changenum 10020 > >> >>> p4 describe -s 10020 > >> >>> Processing edit of //<branch>/host/src/Assisted File > >> >>> Transfer/AftMain.cpp > >> >>> Writing "//<branch>/host/src/Assisted File Transfer/AftMain.cpp#1" to > >> >>> "c:\docume~1\rpius\locals~1\temp\tmptkche3" > >> >>> p4 print -q "//<branch>/host/src/Assisted File > > Transfer/AftMain.cpp#1">> >>> p4 where "//<branch>/host/src/Assisted File > Transfer/AftMain.cpp" > >> >>> diff -urNp c:\docume~1\rpius\locals~1\temp\tmptkche3 > >> >>> Transfer/AftMain.cpp > >> >>> Processing edit of //<branch>/host/src/Assisted File > >> >>> Transfer/ConfigReader.cpp > >> >>> Writing "//<branch>/host/src/Assisted File > >> >>> Transfer/ConfigReader.cpp#1" to > > "c:\docume~1\rpius\locals~1\temp\tmptkche3">> >>> p4 print -q > "//<branch>/host/src/Assisted File > >> >>> Transfer/ConfigReader.cpp#1" > >> >>> p4 where "//<branch>/host/src/Assisted File > > Transfer/ConfigReader.cpp">> >>> diff -urNp > c:\docume~1\rpius\locals~1\temp\tmptkche3 > >> >>> Transfer/ConfigReader.cpp > >> >>> Processing edit of //<branch>/host/src/Assisted File > Transfer/StdAfx.h > >> >>> Writing "//<branch>/host/src/Assisted File Transfer/StdAfx.h#1" to > >> >>> "c:\docume~1\rpius\locals~1\temp\tmptkche3" > >> >>> p4 print -q "//<branch>/host/src/Assisted File Transfer/StdAfx.h#1" > >> >>> p4 where "//<branch>/host/src/Assisted File Transfer/StdAfx.h" > >> >>> diff -urNp c:\docume~1\rpius\locals~1\temp\tmptkche3 > Transfer/StdAfx.h > >> >>> Looking for 'xxx.xxx.xxx.xxx /' cookie in C:\Documents and > >> >>> Settings\<username>\Local Settings\Application > > Data\.post-review-cookies.txt>> >>> Loaded valid cookie -- no login required > >> >>> Attempting to create review request for 10020 > >> >>> HTTP POSTing to > >> >>>http://xxx.xxx.xxx.xxx:80/api/json/reviewrequests/new/: > > {'repository_path':>> >>> 'xxxxxxxx:1666', 'changenum': '10020'} > >> >>> Review request already exists. Updating it... > >> >>> HTTP POSTing to > > http://xxx.xxx.xxx.xxx:80/api/json/reviewrequests/186/update_from_cha... > : > > >> >>> {} > >> >>> Review request created > >> >>> Uploading diff, size: 37349 > >> >>> HTTP POSTing to > >> >>>http://xxx.xxx.xxx.xxx:80/api/json/reviewrequests/186/diff/new/:{} > >> Review request #186 posted. > > >>http://xxx.xxx.xxx.xxx:80/r/186 > > >> Thanks, > >> Roshan Pius > > >> On Tue, Apr 14, 2009 at 6:15 PM, Raghu <raghu...@gmail.com> wrote: > > >> > Can you provide the complete output of post-review -d? My first guess > >> > would be that it is due to a space in the file path. > > >> > -Raghu > > >> > On Apr 13, 10:25 am, roshanpius <roshanpiustho...@gmail.com> wrote: > >> >> Hi , > > >> >> I've been using post-review tool for posting reviews for Review Board > >> >> for a Perforce Repository. I've integrated the post-review tool into > >> >> P4V which is the GUI P4 Client For Windows. This worked fine till now. > > >> >> But now i've created a new workspace and tried using post-review, but > >> >> the diff didn't fetch the file from my hard disk. It was fetching the > >> >> correct file from Repository, but not finding the correct edited file > >> >> on my Workspace. > > >> >> I ran post-review with -d flag to get the debug logs and i noticed > >> >> that the path on the Workspace that the diff was trying to fetch was > >> >> completely wrong: > > >> >> diff -urNp c:\docume~1\rpius\locals~1\temp\tmpr_q3cf Transfer/ > >> >> ConfigReader.cpp > > >> >> It should have been: > > >> >> diff -urNp c:\docume~1\rpius\locals~1\temp\tmpr_q3cf > C:\Unicode\jordan- > >> >> unicode\host\src\Assisted File Transfer/ConfigReader.cpp > > >> >> I'm using post-review with the following arguments in the P4V Client: > > >> >> python C:\Python26\post-review.py %c --p4-client $c --p4-port $p > > >> >> Do i need to change something in the post-review Configuration for the > >> >> new workspace ? --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "reviewboard" group. To post to this group, send email to reviewboard@googlegroups.com To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en -~----------~----~----~----~------~----~------~--~---
