Fix up a variety of unicode/bytes issues with SVN.

 






 

 


  Review Request #9627
  —
  Created Feb. 12, 2018 and submitted 

 


















 

  

   Information

  





 
 

  david
 









 
 

  RBTools
 









 
 

  master
 









 
 

  4547
 









 
 

  
 









 
 

  4f91bf4...
 






 




 

  

   Reviewers

  





 
 

  rbtools
 









 
 

  
 






 








 





  


 
 

  
The SVN client was handling a variety of things using the wrong string

types, most notably filenames. In Python 3, filenames are expected to be

treated as str internally, and then serialized/deserialized using the

filesystem encoding (theoretically this is also the case in Python 2,

but the implicit casting makes it easy to be sloppy). This change fixes

those up so that we're at least sucessfully running the tests.


This also incidentally fixes bug 4547, which involves using

Popen.communicate() instead of Popen.wait().

 








  


 
 

  
Ran unit tests on Python 2.7 and 3.6.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Maybe worth doing this at the module level, sicne we need it a few times?

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Feels like this might be more readable as: return (diff_line.split(b'\n')[0].decode(fs_encoding), b'\n')

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
For Python, object is probably fine, but if we don't have an explicit type in a Returns, we can leave …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Can be six.PY3.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
E501 line too long (80 > 79 characters)

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/clients/svn.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    I'm confused about this, and feel like it's something I'm likely going to hit. What's the reason for the explicit ending index?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Pulling a single index out of bytes returns an int, not a bytes.
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/clients/svn.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Maybe worth doing this at the module level, sicne we need it a few times?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/clients/svn.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Feels like this might be more readable as:
    

    

    return (diff_line.split(b'\n')[0].decode(fs_encoding),
        b'\n')

    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/utils/process.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    Oh man thank you.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 






  6. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/utils/process.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    For Python, object is probably fine, but if we don't have an explicit type in a Returns, we can leave off the type. That will leave off the type portion of the docs in the field, showing only the description.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 






  7. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/utils/process.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Can be six.PY3.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    8. 




  8. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    9. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  











 



  

   
Commit:

   
    
-415850f7b386c1b3ca1b8b47a42c0351d20be2c2
+dbddf3a8856e08cc718a1e233b201559d47da9a0

   		
  




  

   
Diff:

   
    
 Revision 2 (+229 -206)
Show changes
 
    rbtools/clients/svn.py 
    rbtools/clients/tests/test_svn.py 
    rbtools/utils/diffs.py 
    rbtools/utils/process.py 
 

   		
  



 









 

  
Checks run (1 failed, 1 succeeded)



  

 flake8
 failed.
 





  

 JSHint
 passed.
 





 







 
flake8

 


 















  

 
















 



 

  

   




david




   

   

  

 



 















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to master (f158f41)





 


 










  

 









     

    

   

  

  

   
   Loading...