post-review: Fix handling of spaces in file paths when using Perforce

 






 

 


  Review Request #815
  —
  Created April 14, 2009 and submitted

 


















 

  

   Information

  





 
 

  buzzardsoft
 









 
 

  Review Board SVN (deprecated)
 









 
 

  
 









 
 

  840
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Fixes a problem where changes files with spaces in their paths would end up appearing as deleted content in Review Board diffs.  Changed the Perforce output format to use the -ztag switch, and then modified the parsing code to pull out the full path including the space(s).

This was a change that I originally attached to the following issue http://code.google.com/p/reviewboard/issues/detail?id=840

 








  


 
 

  
Tested on Mac OS X with a changed file that has a space in it's full path.

 







 









 





  








 









 






  

 








 














 





 

  

   





 EH





   

   





  

 



 

  


   

   

    
 
ehuss


    
   


  

  

  



    




  1. 
 
    
  
    
   
    My recent patch has added a _run_p4 method which uses the marshal API.  You could change this to:

where_output = self._run_p4(['where', depot_path])
return where_output['path']

This might be a little more reliable than parsing text output.

    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I don't see this in the current distribution of RBTools.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      It was checked in about a month ago.  Looks like the last release of RBTools was about 2 months ago.  You can see the code here: http://code.google.com/p/reviewboard/source/browse/trunk/rbtools/scripts/post-review
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      Yeah, you'll want to develop only against SVN. It's too easy to stay out of sync otherwise.

We'll get to another RBTools release before long. Been concentrating on work and on Review Board itself.
      
 
      

      4. 



    4. 
 
 
      
 
      
  
 
      
 
      
  
      To clarify, it looks to me like _run_p4 was in the version of post-review on the trunk, but not the spaces-in-pathnames fix.  I've now updated this review with a new and improved fix.
      
 
      

      5. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   





 BU





   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
I finally got around to implementing the change suggested by Eric Huss to use the _run_p4 method.   It seems to work in my light testing.

   

  








  

   

    Diff:
   

   

    
 Revision 2 (+4 -12)
Show changes
 
    /trunk/rbtools/scripts/post-review 
 

   

  












  

 
















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Looks good, thanks. Committed as r1990.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3.