Centralize cleartool invocation and fix up windows shell issues.

Review Request #11298 — Created Nov. 23, 2020 and submitted

david
Review Board
release-3.0.x
reviewboard

The ClearCase SCM backend invokes cleartool a lot, and every time was
assembling a big Popen call and duplicating error handling. This change
centralizes all of that into a single method, while also improving
unicode/bytes compatibility, logging, and console shell handling for
windows.

The windows issue is the most interesting one here. Some windows
applications are marked as requiring a console (as opposed to using
windows), and invoking them would always pop up a console window,
regardless of whether input and output was actually possible.
cleartool is one such executable. An existing workaround had been
added to use shell=True, but this was only enabled when using Python
2.7 and Windows 7 (the initial patch was not written with
future-proofing in mind), and relying on the shell opens up a variety of
other potential issues, including security concerns. The startupinfo
flag to Popen allows actually controlling this at a lower level.

  • Ran unit tests.
  • Tested that startupinfo changes to Popen invocation did the correct
    thing for Windows hosts.
Summary
Centralize cleartool invocation and fix up windows shell issues.
Description From Last Updated

"Returns" goes before "Raises".

chipx86chipx86

F821 undefined name 'six'

reviewbotreviewbot

Review Bot has its complaint, but you can also just have this be: if sys.version_info[:2] >= (3, 6): ... which ...

chipx86chipx86

"Returns" before "Raises".

chipx86chipx86

"Returns" before "Raises".

chipx86chipx86

"Returns" before "Raises". Same with others below.

chipx86chipx86
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
  1. 
      
  2. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
     
     
     
     
     
     

    "Returns" goes before "Raises".

  3. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    Review Bot has its complaint, but you can also just have this be:

    if sys.version_info[:2] >= (3, 6):
      ...
    

    which is probably more clear.

  4. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
     
     
     
     
     
     

    "Returns" before "Raises".

  5. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
     
     
     
     
     
     

    "Returns" before "Raises".

  6. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
     
     
     
     
     
     

    "Returns" before "Raises".

    Same with others below.

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

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (6ee4835)
Loading...