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 26, 2025
1 parent e58713c commit 1857065
Show file tree
Hide file tree
Showing 2 changed files with 44 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
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

0 comments on commit 1857065

Please sign in to comment.