Skip to content

Commit

Permalink
git: remove git2 from git subprocessing code paths in lib
Browse files Browse the repository at this point in the history
Currently, the subprocessing code paths in the library were relying on
git2 to figure out if a remote exists. This is because when git errors
out for a remote not existing it outputs the same error as when the
repository is not found.

As the cli tool makes a distinction between the two (e.g., after
cloning, the remote must exist and as such we cannot send out an error
that is shared by `no remote found` and `no repository found`), this was a
hack put in place to sort out the difference.

It calls out to `git remote` to list out the remotes.
  • Loading branch information
bsdinis committed Jan 24, 2025
1 parent f3bbc50 commit ad6f3df
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 21 deletions.
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
27 changes: 27 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,20 @@ impl<'a> GitSubprocessContext<'a> {
Ok(())
}

/// 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 +336,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

0 comments on commit ad6f3df

Please sign in to comment.