Skip to content
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

Detect/install command-line git and invoke #3419

Merged
merged 4 commits into from
Jul 25, 2024
Merged

Detect/install command-line git and invoke #3419

merged 4 commits into from
Jul 25, 2024

Conversation

ssparach
Copy link
Contributor

Summary of the pull request

This PR adds a new class which allows using the git executable on machine to run commands such as status instead of using libgit2sharp.
This PR also includes basic unit test.

References and relevant issues

Detailed description of the pull request / Additional comments

The main functionality of the class is:

  • Detect git executable path on machine and save path to a config file
  • use the git executable path to invoke git with a set of commands

Validation steps performed

Local Build
Release build of msix in Developer Command Prompt
Unit tests

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated
  • Telemetry compliance tasks completed for added/updated events

@ssparach ssparach requested a review from DefaultRyan July 11, 2024 23:29
jaholme
jaholme reviewed Jul 17, 2024
View reviewed changes
extensions/GitExtension/FileExplorerGitIntegration/Models/GitCommandRunner.cs Outdated
{
System.Diagnostics.Process.Start("git.exe");
gitExeFound = true;
StoreGitExeInstallPath("git.exe");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not run "where git" in cmd and parse the output?
C:>where git
C:\Program Files\Git\cmd\git.exe
D:\work\depot_tools\git.bat

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly we're trying to be somewhat resilient to users not having git.exe on the path, and try some sensible fallback directories. But to be fair, the first option we try is the one the path.

Or am I misremembering that "where" depends on the PATH variable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're correct where depends on the path variable. my comment was why execute "git.exe" to figure out if its on the path when we have an existing utility that will tell us if it is and precisely in what location.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're sort of agreeing with each other. From the point of this code, if it's on the PATH, we don't care where it is - we'll just execute git.exe without qualifying the path. The other options are for "Hmm... it's not on PATH. Let's try a few other well-known locations to see if we can find something that works".

@ssparach ssparach requested a review from DefaultRyan July 19, 2024 18:54
@ssparach ssparach requested a review from DefaultRyan July 24, 2024 17:28
@ssparach ssparach merged commit 97aa4ad into feature/fileexplorer-sourcecontrol-integration Jul 25, 2024
4 checks passed
ssparach added a commit that referenced this pull request Jul 31, 2024
@ssparach
Detect/install command-line git and invoke (#3419)
49132f3 
* detect git and invoke functionality

* address PR comments

* address PR feedback

* address style comments
ssparach added a commit that referenced this pull request Jul 31, 2024
@ssparach
Detect/install command-line git and invoke (#3419)
052cd31 
* detect git and invoke functionality

* address PR comments

* address PR feedback

* address style comments
ssparach added a commit that referenced this pull request Jul 31, 2024
@ssparach
Detect/install command-line git and invoke (#3419)
1b1dc2b 
* detect git and invoke functionality

* address PR comments

* address PR feedback

* address style comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaholme jaholme jaholme left review comments

@DefaultRyan DefaultRyan DefaultRyan approved these changes

@dhoehna dhoehna dhoehna approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ssparach @dhoehna @DefaultRyan @jaholme