• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    Fix the types and values of connection arguments for Perforce.
    
 
    

    



 
    
 
    

  Review Request #10135
  —
  Created Aug. 31, 2018 and submitted 

 
    


    




    




    




 
    
  
    
   Information

  
    



    
 
 
    
  chipx86
 
    

    





    
 
 
    
  Review Board
 
    

    





    
 
 
    
  release-2.5.x
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  2119586...
 
    

    



 
    



 
    
  
    
   Reviewers

  
    



    
 
 
    
  reviewboard
 
    

    





    
 
 
    
  
 
    

    



 
    



    


    
 
    




  

    
 
 
    
  
    Over the years, the Perforce connection code went through many changes
    
designed to normalize strings so that we would ensure the Perforce
    
client would receive byte strings, no matter what was being passed in.
    
Some of these were no longer necessary, and in fact causing problems, as
    
None values were getting converted to 'None' text strings, which
    
could cause problems in very specific edge cases on Perforce deployments
    
involving clients named 'None' that had restricted client views.
    

    Along with this, these changes were only compatible with Python 2, as
    
modern versions of Perforce on Python 3 use Unicode strings.
    

    This change removes the unnecessary string conversion when populating
    
our PerforceClient based on the Repository settings, and updates the
    
P4 string setting to use the right string type for the right version
    
of Python.
    

    Unit tests were added to check that both PerforceClient and P4
    
received the proper values and string types.
    
 
    

    





  

    
 
 
    
  
    Unit tests passed.
    

    Verified connection against real-world servers.
    

    Tested on Review Board 2.5 and 3.0.
    
 
    

    




 
    

    





    
 
    









 





    
 
 
 
    
  
    
   
    
    
    
     
    
    
    
      
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
    
    
   
    
   
    
    
     
    
      Description
      From
      Last Updated
     
    
        
    



     
    
      
       
       
    Do we want to use str here? How does this behave on Python 3? Do we actually want force_bytes?
    
          		
      
       daviddavid
      
      
       
      
     
    

        
   
    
  
    
 
    

    




 
    

    



  
    
 
    

    


    












    
 




 

    


 













    
 





 
    
  
    
   
    



david




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 

















    2. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       reviewboard/scmtools/perforce.py
       


        (Diff revision 1)


       
      
     
      
          
    
     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   Show all issues
  
 
      

      



    
      Do we want to use str here? How does this behave on Python 3? Do we actually want force_bytes?
      
  
      
 
      

 
      
  


      
 
      
 
        

  


      1. 
 
 
        
 
        
  
 
        
 
        
  
        I checked the p4python source code. It has logic that takes these arguments passed in and checks that they're native Python strings (through an ifdef -- Unicode strings on Python 3, byte strings on Python 2). If you try to set to a byte string on Python 3, you get:
        

        

        >>> p4.password = b'foo'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: Cannot set attribute : password with value b'foo'
>>> p4.password = 'foo'
>>>

        

        

        I think we want force_str() though, since that handles encoding in the case of Python 2, like we've had before. If it's Python 3, we're taking Unicode and keeping it as Unicode, so no changes. If it's Python 2, we're taking Unicode and encoding to UTF-8, which force_str() does when on Python 2.
        

        (force_str() also does some other things, like accounting for native to native, or promises -- like ugettext_lazy -- to native, but they won't be invoked in our usage.)
        
 
        

        2. 


 
      


      

 
      

      3. 




    3. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      4. 


    

  
    
 
    

    












    
 





 
    
  
    
   
    



david




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Ship It!
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 















    2. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



chipx86




   
    
   
    
  
    
 
    


 
    
  
    

   
    
   
    
    
     Review request changed
    
    
   
    

  
    
  
    
  
    




    

  
    
   
    
    Status:
   
    
   
    

    Completed

   
    
  
    



  
    
   
    
    Change Summary:
   
    
   
    
    
    Pushed to release-3.0.x (e2c8808)