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.

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

git: remove git2 from git subprocessing code paths in lib #5443

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 13 additions & 21 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1601,15 +1601,14 @@ impl<'a> GitFetch<'a> {
branch_names: &[StringPattern],
remote_name: &str,
) -> Result<Option<String>, GitFetchError> {
// check the remote exists
// TODO: we should ideally find a way to do this without git2
let _remote = self.git_repo.find_remote(remote_name).map_err(|err| {
if is_remote_not_found_err(&err) {
GitFetchError::NoSuchRemote(remote_name.to_string())
} else {
GitFetchError::InternalGitError(err)
}
})?;
let git_ctx =
GitSubprocessContext::from_git2(self.git_repo, &self.git_settings.executable_path);

let remotes = git_ctx.spawn_remote()?;
if !remotes.contains(remote_name) {
return Err(GitFetchError::NoSuchRemote(remote_name.to_string()));
}

// At this point, we are only updating Git's remote tracking branches, not the
// local branches.
let mut remaining_refspecs: Vec<_> = Self::expand_refspecs(remote_name, branch_names)?;
Expand All @@ -1618,9 +1617,6 @@ impl<'a> GitFetch<'a> {
return Ok(None);
}

let git_ctx =
GitSubprocessContext::from_git2(self.git_repo, &self.git_settings.executable_path);

let mut branches_to_prune = Vec::new();
// git unfortunately errors out if one of the many refspecs is not found
//
Expand Down Expand Up @@ -2019,16 +2015,12 @@ fn subprocess_push_refs(
qualified_remote_refs_expected_locations: &HashMap<&str, Option<&CommitId>>,
refspecs: &[RefSpec],
) -> Result<(), GitPushError> {
// check the remote exists
// TODO: we should ideally find a way to do this without git2
let _remote = git_repo.find_remote(remote_name).map_err(|err| {
if is_remote_not_found_err(&err) {
GitPushError::NoSuchRemote(remote_name.to_string())
} else {
GitPushError::InternalGitError(err)
}
})?;
let git_ctx = GitSubprocessContext::from_git2(git_repo, git_executable_path);
// check the remote exists
let remotes = git_ctx.spawn_remote()?;
if !remotes.contains(remote_name) {
return Err(GitPushError::NoSuchRemote(remote_name.to_string()));
}

let mut remaining_remote_refs: HashSet<_> = qualified_remote_refs_expected_locations
.keys()
Expand Down
31 changes: 31 additions & 0 deletions lib/src/git_subprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//
use std::collections::HashSet;
use std::num::NonZeroU32;
use std::path::Path;
use std::path::PathBuf;
Expand Down Expand Up @@ -156,6 +157,24 @@ impl<'a> GitSubprocessContext<'a> {
Ok(())
}

// TODO: we should remove this in lieu of using the gix API.
// It is not trivial at this point because gix caches the config in memory
// (which is where the configured remotes live). This means that the
// externally modified remote is not found on clone.
/// List the configured remotes
///
/// `git remote`
///
/// Prints a remote per line
pub(crate) fn spawn_remote(&self) -> Result<HashSet<String>, GitSubprocessError> {
let mut command = self.create_command();
command.stdout(Stdio::piped());
command.arg("remote");
let output = self.spawn_cmd(command)?;

parse_git_remote_output(output)
}

/// How we retrieve the remote's default branch:
///
/// `git remote show <remote_name>`
Expand Down Expand Up @@ -321,6 +340,18 @@ fn parse_git_branch_prune_output(output: Output) -> Result<(), GitSubprocessErro
Err(external_git_error(&output.stderr))
}

fn parse_git_remote_output(output: Output) -> Result<HashSet<String>, GitSubprocessError> {
if !output.status.success() {
return Err(external_git_error(&output.stderr));
}

Ok(output
.stdout
.lines()
.map(|line| line.to_str_lossy().into_owned())
.collect())
}

fn parse_git_remote_show_output(output: Output) -> Result<Output, GitSubprocessError> {
if output.status.success() {
return Ok(output);
Expand Down
Loading