Centralize cleartool invocation and fix up windows shell issues.

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

Information

Review Board
release-3.0.x

Reviewers

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.

Changes between revision 1 and 2

orig
1
2

Commits

Summary ID Author
Centralize cleartool invocation and fix up windows shell issues.
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. Testing Done: - Ran unit tests. - startupinfo changes haven't yet been tested because I don't have a windows environment set up at the moment, but I'll make sure to verify its correctness before landing this.
c62c899160d09387a1da2487d61a49510f31cb1a David Trowbridge
Centralize cleartool invocation and fix up windows shell issues.
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. Testing Done: - Ran unit tests. - Tested that startupinfo changes to Popen invocation did the correct thing for Windows hosts.
c5aa7241d5998e1c04201940c8194bcae3f6ebbd David Trowbridge
reviewboard/scmtools/clearcase.py
Loading...