patch for perforce. If changing the security level of perforce server, perhaps fail to login perforce when creating diffs.

Review Request #1537 — Created April 27, 2010 and discarded

Information

Review Board
master, 1.0.x

Reviewers

patch for perforce. If changing the security level of perforce server, perhaps fail to login perforce when creating diffs.

 
SE
chipx86
  1. I really don't understand the purpose of this change or why it's done the way it is. It concerns me, since we're calling out to an external program and commenting out code known to work in some environments. Please add a more detailed description discussing the rationale for the change and how this fixes the problem.
    1. This patch if to to solve this issue:
      
      Traceback (most recent call last):
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\views.py", line 153, in view_diff
          interdiffset, highlighting, True)
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\diffutils.py", line 623, in get_diff_files
          large_data=True)
        File "c:\python25\lib\site-packages\Djblets-0.6.1-py2.5.egg\djblets\util\misc.py", line 166, in cache_memoize
          data = lookup_callable()
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\diffutils.py", line 622, in <lambda>
          enable_syntax_highlighting),
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\diffutils.py", line 344, in get_chunks
          old = get_original_file(filediff)
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\diffutils.py", line 251, in get_original_file
          large_data=True)[0]
        File "c:\python25\lib\site-packages\Djblets-0.6.1-py2.5.egg\djblets\util\misc.py", line 166, in cache_memoize
          data = lookup_callable()
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\diffutils.py", line 250, in <lambda>
          data = cache_memoize(key, lambda: [fetch_file(file, revision)],
        File "c:\python25\lib\site-packages\ReviewBoard-1.0.8-py2.5.egg\reviewboard\diffviewer\diffutils.py", line 229, in fetch_file
          data = tool.get_file(file, revision)
        File "C:\Python25\lib\site-packages\reviewboard-1.0.8-py2.5.egg\reviewboard\scmtools\perforce.py", line 101, in get_file
          raise SCMError('\n'.join(line.lstrip("\t") for line in error))
      SCMError: Password not allowed at this server security level, use 'p4 login'.
      
      Now I change to use p4 API.
      
  2. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    We can't use echo like this. We might not be running on a system where it works. Please use the p4 api if at all possible, too.
  3. 
      
SE
SE
SE
SE
  1. 
      
  2. reviewboard/scmtools/perforce.py (Diff revision 3)
     
     
    Must restore password after executing p4.run_login(), or will fail to post requests. Don't know the reason. it seems to be a bug of p4svn.
  3. 
      
SE
Review request changed
Change Summary:
Hope to fix in 1.0.x branch ASAP.
Branch:
master
master, 1.0.x
AR
  1. Using "p4 login" is what my patch (http://reviews.reviewboard.org/r/1445/) also does. It should now include all of the changes Christian requested some time ago.
    What it does different is that it won't enforce "p4 login" but makes it optional depending on a configuration setting.
    
    What I don't understand is why you call the login command with each connect. Running the login command should replace both user and password attributes of the p4 class with the ticket credentials so those will be used for further p4 calls. I don't see the necessity to redo that step with each connect.
    1. Yes, your changes look good and including my changes. My patch is just used for my team. Thanks!
  2.