Skip to content

Commit

Permalink
fix!: handle symbolic ref updates far more gracefully and with more l…
Browse files Browse the repository at this point in the history 
…ogical consistency.

Previously, refspecs couldn't be used to update sybolic references locally, particularly because the logic
to do so correctly isn't trivial and `git` itself also seems to cover only the most common cases.

However, the logic now changed so that remote updates will only be rejected if

* fast-forward rules are violated
* the local ref is currently checked out

This makes it possible to update unborn remote refs to local refs if these are new or unborn themselves.
This allows to create mirrors more easily and allows us to handle `HEAD` without special casing it.
Bare repositories have an easier time to update local symbolic refs.
  • Loading branch information
@Byron
Byron committed Jul 26, 2023
1 parent c2f3420 commit 9a4bb18
Show file tree
Hide file tree
Showing 7 changed files with 283 additions and 87 deletions.
5 changes: 5 additions & 0 deletions gix/src/reference/log.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ impl<'repo> Reference<'repo> {
pub fn log_iter(&self) -> gix_ref::file::log::iter::Platform<'_, '_> {
self.inner.log_iter(&self.repo.refs)
}

/// Return true if a reflog is present for this reference.
pub fn log_exists(&self) -> bool {
self.inner.log_exists(&self.repo.refs)
}
}

/// Generate a message typical for git commit logs based on the given `operation`, commit `message` and `num_parents` of the commit.
Expand Down
180 changes: 116 additions & 64 deletions gix/src/remote/connection/fetch/update_refs/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{collections::BTreeMap, convert::TryInto, path::PathBuf};
use gix_odb::{Find, FindExt};
use gix_ref::{
transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog},
Target, TargetRef,
Target,
};

use crate::{
Expand All @@ -23,12 +23,12 @@ pub mod update;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Update {
/// The way the update was performed.
pub mode: update::Mode,
pub mode: Mode,
/// The index to the edit that was created from the corresponding mapping, or `None` if there was no local ref.
pub edit_index: Option<usize>,
}

impl From<update::Mode> for Update {
impl From<Mode> for Update {
fn from(mode: Mode) -> Self {
Update { mode, edit_index: None }
}
Expand All @@ -42,6 +42,13 @@ impl From<update::Mode> for Update {
/// `action` is the prefix used for reflog entries, and is typically "fetch".
///
/// It can be used to produce typical information that one is used to from `git fetch`.
///
/// We will reject updates only if…
///
/// * …fast-forward rules are violated
/// * …the local ref is currently checked out
///
/// With these safeguards in place, one can handle each naturally and implement mirrors or bare repos easily.
#[allow(clippy::too_many_arguments)]
pub(crate) fn update(
repo: &Repository,
Expand Down Expand Up @@ -76,46 +83,55 @@ pub(crate) fn update(
})
},
) {
let remote_id = match remote.as_id() {
Some(id) => id,
None => continue,
};
if dry_run == fetch::DryRun::No && !repo.objects.contains(remote_id) {
let update = if is_implicit_tag {
update::Mode::ImplicitTagNotSentByRemote.into()
} else {
update::Mode::RejectedSourceObjectNotFound { id: remote_id.into() }.into()
};
updates.push(update);
continue;
// `None` only if unborn.
let remote_id = remote.as_id();
if matches!(dry_run, fetch::DryRun::No) && !remote_id.map_or(true, |id| repo.objects.contains(id)) {
if let Some(remote_id) = remote_id.filter(|id| !repo.objects.contains(id)) {
let update = if is_implicit_tag {
Mode::ImplicitTagNotSentByRemote.into()
} else {
Mode::RejectedSourceObjectNotFound { id: remote_id.into() }.into()
};
updates.push(update);
continue;
}
}
let checked_out_branches = worktree_branches(repo)?;
let (mode, edit_index) = match local {
Some(name) => {
let (mode, reflog_message, name, previous_value) = match repo.try_find_reference(name)? {
Some(existing) => {
if let Some(wt_dir) = checked_out_branches.get(existing.name()) {
let mode = update::Mode::RejectedCurrentlyCheckedOut {
let mode = Mode::RejectedCurrentlyCheckedOut {
worktree_dir: wt_dir.to_owned(),
};
updates.push(mode.into());
continue;
}
match existing.target() {
TargetRef::Symbolic(_) => {
updates.push(update::Mode::RejectedSymbolic.into());
continue;
}
TargetRef::Peeled(local_id) => {
let previous_value =
PreviousValue::MustExistAndMatch(Target::Peeled(local_id.to_owned()));

match existing
.try_id()
.map_or_else(|| existing.clone().peel_to_id_in_place(), Ok)
.map(|id| id.detach())
{
Ok(local_id) => {
let remote_id = match remote_id {
Some(id) => id,
None => {
// we don't allow to go back to unborn state if there is a local reference already present.
// Note that we will be changing it to a symbolic reference just fine.
updates.push(Mode::RejectedToReplaceWithUnborn.into());
continue;
}
};
let previous_value = PreviousValue::MustExistAndMatch(existing.target().into_owned());
let (mode, reflog_message) = if local_id == remote_id {
(update::Mode::NoChangeNeeded, "no update will be performed")
(Mode::NoChangeNeeded, "no update will be performed")
} else if let Some(gix_ref::Category::Tag) = existing.name().category() {
if spec.allow_non_fast_forward() {
(update::Mode::Forced, "updating tag")
(Mode::Forced, "updating tag")
} else {
updates.push(update::Mode::RejectedTagUpdate.into());
updates.push(Mode::RejectedTagUpdate.into());
continue;
}
} else {
Expand All @@ -129,16 +145,16 @@ pub(crate) fn update(
.and_then(|c| {
c.committer().map(|a| a.time.seconds).map_err(|_| ())
}).and_then(|local_commit_time|
remote_id
.to_owned()
.ancestors(|id, buf| repo.objects.find_commit_iter(id, buf))
.sorting(
gix_traverse::commit::Sorting::ByCommitTimeNewestFirstCutoffOlderThan {
seconds: local_commit_time
},
)
.map_err(|_| ())
);
remote_id
.to_owned()
.ancestors(|id, buf| repo.objects.find_commit_iter(id, buf))
.sorting(
gix_traverse::commit::Sorting::ByCommitTimeNewestFirstCutoffOlderThan {
seconds: local_commit_time
},
)
.map_err(|_| ())
);
match ancestors {
Ok(mut ancestors) => {
ancestors.any(|cid| cid.map_or(false, |c| c.id == local_id))
Expand All @@ -153,20 +169,34 @@ pub(crate) fn update(
};
if is_fast_forward {
(
update::Mode::FastForward,
Mode::FastForward,
matches!(dry_run, fetch::DryRun::Yes)
.then(|| "fast-forward (guessed in dry-run)")
.unwrap_or("fast-forward"),
)
} else if force {
(update::Mode::Forced, "forced-update")
(Mode::Forced, "forced-update")
} else {
updates.push(update::Mode::RejectedNonFastForward.into());
updates.push(Mode::RejectedNonFastForward.into());
continue;
}
};
(mode, reflog_message, existing.name().to_owned(), previous_value)
}
Err(crate::reference::peel::Error::ToId(gix_ref::peel::to_id::Error::Follow(_))) => {
// An unborn reference, always allow it to be changed to whatever the remote wants.
(
if existing.target().try_name().map(|name| name.as_bstr()) == remote.as_target() {
Mode::NoChangeNeeded
} else {
Mode::Forced
},
"change unborn ref",
existing.name().to_owned(),
PreviousValue::MustExistAndMatch(existing.target().into_owned()),
)
}
Err(err) => return Err(err.into()),
}
}
None => {
Expand All @@ -177,10 +207,10 @@ pub(crate) fn update(
_ => "storing ref",
};
(
update::Mode::New,
Mode::New,
reflog_msg,
name,
PreviousValue::ExistingMustMatch(Target::Peeled(remote_id.to_owned())),
PreviousValue::ExistingMustMatch(new_value_by_remote(&remote, mappings)?),
)
}
};
Expand All @@ -192,36 +222,18 @@ pub(crate) fn update(
message: message.compose(reflog_message),
},
expected: previous_value,
new: if let Source::Ref(gix_protocol::handshake::Ref::Symbolic { target, .. }) = &remote {
match mappings.iter().find_map(|m| {
m.remote.as_name().and_then(|name| {
(name == target)
.then(|| m.local.as_ref().and_then(|local| local.try_into().ok()))
.flatten()
})
}) {
Some(local_branch) => {
// This is always safe because…
// - the reference may exist already
// - if it doesn't exist it will be created - we are here because it's in the list of mappings after all
// - if it exists and is updated, and the update is rejected due to non-fastforward for instance, the
// target reference still exists and we can point to it.
Target::Symbolic(local_branch)
}
None => Target::Peeled(remote_id.into()),
}
} else {
Target::Peeled(remote_id.into())
},
new: new_value_by_remote(remote, mappings)?,
},
name,
// We must not deref symrefs or we will overwrite their destination, which might be checked out
// and we don't check for that case.
deref: false,
};
let edit_index = edits.len();
edits.push(edit);
(mode, Some(edit_index))
}
None => (update::Mode::NoChangeNeeded, None),
None => (Mode::NoChangeNeeded, None),
};
updates.push(Update { mode, edit_index })
}
Expand Down Expand Up @@ -258,6 +270,46 @@ pub(crate) fn update(
Ok(update::Outcome { edits, updates })
}

fn new_value_by_remote(remote: &Source, mappings: &[fetch::Mapping]) -> Result<Target, update::Error> {
let remote_id = remote.as_id();
Ok(
if let Source::Ref(
gix_protocol::handshake::Ref::Symbolic { target, .. } | gix_protocol::handshake::Ref::Unborn { target, .. },
) = &remote
{
match mappings.iter().find_map(|m| {
m.remote.as_name().and_then(|name| {
(name == target)
.then(|| m.local.as_ref().and_then(|local| local.try_into().ok()))
.flatten()
})
}) {
// Map the target on the remote to the local branch name, which should be covered by refspecs.
Some(local_branch) => {
// This is always safe because…
// - the reference may exist already
// - if it doesn't exist it will be created - we are here because it's in the list of mappings after all
// - if it exists and is updated, and the update is rejected due to non-fastforward for instance, the
// target reference still exists and we can point to it.
Target::Symbolic(local_branch)
}
None => match remote_id {
None => {
// An unborn remote branch pointing to a target ref.
// There is no local mapping, but if we are here it's safe to make the edit anyway.
// This happens when dealing with `HEAD:HEAD` mappings, for instance, which is in bare repos
// or for unborn branches both remotely and here.
Target::Symbolic(target.try_into()?)
}
Some(id) => Target::Peeled(id.to_owned()),
},
}
} else {
Target::Peeled(remote_id.expect("unborn case handled earlier").to_owned())
},
)
}

fn worktree_branches(repo: &Repository) -> Result<BTreeMap<gix_ref::FullName, PathBuf>, update::Error> {
let mut map = BTreeMap::new();
if let Some((wt_dir, head_ref)) = repo.work_dir().zip(repo.head_ref().ok().flatten()) {
Expand Down
Loading

0 comments on commit 9a4bb18

Please sign in to comment.