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.

Diff Revision 2 (Latest)

orig
1
2

Commits

First Last 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. - Tested that startupinfo changes to Popen invocation did the correct thing for Windows hosts.
c5aa7241d5998e1c04201940c8194bcae3f6ebbd David Trowbridge
reviewboard/scmtools/clearcase.py
Loading...