Skip to content

Change how the git CLI subprocess is executed #595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Change how the git CLI subprocess is executed #595

wants to merge 1 commit into from

Conversation

jcouball
Copy link
Member

@jcouball jcouball commented Aug 23, 2022 •
edited
Loading

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Ensure all commits include DCO sign-off.
  • Ensure that your contributions pass unit testing.
  • Ensure that your contributions contain documentation if applicable.

Description

In the current implementation, all calls to git are done using backticks which has several disadvantages:

  • STDERR is not captured so can not be processed by this gem
  • STDERR is always passed through to the terminal
  • Changes to the environment's subprocess have to be global which could impact users of this gem
  • Arguments passed on the command line go through the system shell making it difficult to escape args correctly for all platforms AND leaving this gem open to security vulnerabilities

Desired Result

Implement a cross platform (Windows, Linux, and Mac) solution that can:

  • Independently capture STDOUT and STDERR
  • Stream STDOUT and STDERR to the terminal if needed for debugging
  • Provide a customized environment for the git command that does not impact users of this gem
  • Pass command line args to the git command without being interpreted by the system shell

@jcouball jcouball force-pushed the stderr_in_diff branch 14 times, most recently from 09bb364 to 19077ed Compare August 25, 2022 23:58
@jcouball jcouball force-pushed the stderr_in_diff branch 11 times, most recently from a5d8789 to a3c27d2 Compare December 16, 2022 16:42
@jcouball jcouball force-pushed the stderr_in_diff branch 3 times, most recently from 423ba23 to 1d827af Compare February 12, 2023 17:45
@jcouball jcouball changed the title Use Open3.capture2e instead of backticks for subprocesses Separately capture STDOUT and STDERR when calling subprocesses Feb 12, 2023
@jcouball jcouball changed the title Separately capture STDOUT and STDERR when calling subprocesses Change how the git CLI is run Feb 12, 2023
@jcouball jcouball changed the title Change how the git CLI is run Change how the git CLI subprocess is executed Feb 12, 2023
@jcouball
Integrate process_executor gem for controlling the git subprocess
597e697 
Signed-off-by: James Couball <jcouball@yahoo.com>
@jcouball
Copy link
Member Author

Closing in favor of #617

@jcouball jcouball closed this Feb 12, 2023
@jcouball jcouball deleted the stderr_in_diff branch February 16, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jcouball