Fix the types and values of connection arguments for Perforce.

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

chipx86
Review Board
release-2.5.x
2119586...
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.

  • 0
  • 0
  • 0
  • 1
  • 1
Description From Last Updated
david
  1. 
      
  2. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     

    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.)

  3. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (e2c8808)
Loading...