From b4e43327fb52287ee3643caef8a6f7635eebbc43 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Fri, 13 Sep 2024 07:32:41 +0900 Subject: [PATCH] Allow `git push --dry-run` to store metadata --- README.md | 4 +- src/main.rs | 222 ++++++++++++++++++++++++++++----------------------- tests/push.t | 26 ++++++ 3 files changed, 153 insertions(+), 99 deletions(-) diff --git a/README.md b/README.md index d44d030b0..b2c991405 100644 --- a/README.md +++ b/README.md @@ -196,9 +196,11 @@ preference with one of the following values: - `always` - `never` - `phase` +- `force` `phase` is the default described above. `always` and `never` are -self-explanatory. +self-explanatory. `force` has the same meaning as `always`, but also +forces `git push --dry-run` to store metadata. Cinnabar clone: --------------- diff --git a/src/main.rs b/src/main.rs index 3f77f797e..64cd27141 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4478,6 +4478,39 @@ fn check_graft_refs() { } } +#[derive(PartialEq, Eq)] +enum RecordMetadata { + Never, + Phase, + Always, + // Like Always, but also applies to dry-run + Force, +} + +impl TryFrom<&OsStr> for RecordMetadata { + type Error = String; + + fn try_from(value: &OsStr) -> Result { + value + .to_str() + .and_then(|s| { + Some(match s { + "never" => RecordMetadata::Never, + "phase" => RecordMetadata::Phase, + "always" => RecordMetadata::Always, + "force" => RecordMetadata::Force, + _ => return None, + }) + }) + .ok_or_else(|| { + format!( + "`{}` is not one of `never`, `phase`, `always` or `force`", + value.as_bytes().as_bstr() + ) + }) + } +} + fn remote_helper_push( store: &mut Store, conn: &mut dyn HgRepo, @@ -4530,6 +4563,11 @@ fn remote_helper_push( } }); + let data = get_config_remote("data", remote) + .filter(|d| !d.is_empty()) + .as_deref() + .map_or(Ok(RecordMetadata::Phase), RecordMetadata::try_from)?; + let mut pushed = ChangesetHeads::new(); let result = (|| { let push_commits = push_refs.iter().filter_map(|(c, _, _)| *c).collect_vec(); @@ -4692,7 +4730,7 @@ fn remote_helper_push( } let mut result = None; - if !push_commits.is_empty() && !dry_run { + if !push_commits.is_empty() && (!dry_run || data == RecordMetadata::Force) { conn.require_capability(b"unbundle"); let b2caps = conn @@ -4718,6 +4756,7 @@ fn remote_helper_push( .unwrap_or(1); (BundleSpec::V2None, version) }; + // TODO: Ideally, for dry-run, we wouldn't even create a temporary file. let tempfile = tempfile::Builder::new() .prefix("hg-bundle-") .suffix(".hg") @@ -4734,40 +4773,43 @@ fn remote_helper_push( version == 2, )?; drop(file); - let file = File::open(path).unwrap(); - let empty_heads = [HgChangesetId::NULL]; - let heads = if force { - None - } else if no_topological_heads { - Some(&empty_heads[..]) - } else { - Some(&info.topological_heads[..]) - }; - let response = conn.unbundle(heads, file); - match response { - UnbundleResponse::Bundlev2(data) => { - let mut bundle = BundleReader::new(data).unwrap(); - while let Some(part) = bundle.next_part().unwrap() { - match part.part_type.as_bytes() { - b"reply:changegroup" => { - // TODO: should check in-reply-to param. - let response = part.get_param("return").unwrap(); - result = u32::from_str(response).ok(); - } - b"error:abort" => { - let mut message = part.get_param("message").unwrap().to_string(); - if let Some(hint) = part.get_param("hint") { - message.push_str("\n\n"); - message.push_str(hint); + if !dry_run { + let file = File::open(path).unwrap(); + let empty_heads = [HgChangesetId::NULL]; + let heads = if force { + None + } else if no_topological_heads { + Some(&empty_heads[..]) + } else { + Some(&info.topological_heads[..]) + }; + let response = conn.unbundle(heads, file); + match response { + UnbundleResponse::Bundlev2(data) => { + let mut bundle = BundleReader::new(data).unwrap(); + while let Some(part) = bundle.next_part().unwrap() { + match part.part_type.as_bytes() { + b"reply:changegroup" => { + // TODO: should check in-reply-to param. + let response = part.get_param("return").unwrap(); + result = u32::from_str(response).ok(); + } + b"error:abort" => { + let mut message = + part.get_param("message").unwrap().to_string(); + if let Some(hint) = part.get_param("hint") { + message.push_str("\n\n"); + message.push_str(hint); + } + error!(target: "root", "{}", message); } - error!(target: "root", "{}", message); + _ => {} } - _ => {} } } - } - UnbundleResponse::Raw(response) => { - result = u32::from_bytes(&response).ok(); + UnbundleResponse::Raw(response) => { + result = u32::from_bytes(&response).ok(); + } } } } @@ -4851,76 +4893,60 @@ fn remote_helper_push( stdout.flush().unwrap(); } - let data = get_config_remote("data", remote); - let data = data - .as_deref() - .and_then(|d| (!d.is_empty()).then_some(d)) - .unwrap_or_else(|| OsStr::new("phase")); - let valid = [ - OsStr::new("never"), - OsStr::new("phase"), - OsStr::new("always"), - ]; - if !valid.contains(&data) { - die!( - "`{}` is not one of `never`, `phase` or `always`", - data.as_bytes().as_bstr() - ); - } - let rollback = if status.is_empty() || pushed.is_empty() || dry_run { - true - } else { - match data.to_str().unwrap() { - "always" => false, - "never" => true, - "phase" => { - let phases = conn.phases(); - let phases = ByteSlice::lines(&*phases) - .filter_map(|l| { - l.splitn_exact(b'\t') - .map(|[k, v]| (k.as_bstr(), v.as_bstr())) - }) - .collect::>(); - let drafts = (!phases.contains_key(b"publishing".as_bstr())) - .then(|| { - phases - .into_iter() - .filter_map(|(phase, is_draft)| { - u32::from_bytes(is_draft).ok().and_then(|is_draft| { - (is_draft > 0).then(|| HgChangesetId::from_bytes(phase)) + let rollback = + if status.is_empty() || pushed.is_empty() || (dry_run && data != RecordMetadata::Force) { + true + } else { + match data { + RecordMetadata::Force | RecordMetadata::Always => false, + RecordMetadata::Never => true, + RecordMetadata::Phase => { + let phases = conn.phases(); + let phases = ByteSlice::lines(&*phases) + .filter_map(|l| { + l.splitn_exact(b'\t') + .map(|[k, v]| (k.as_bstr(), v.as_bstr())) + }) + .collect::>(); + let drafts = (!phases.contains_key(b"publishing".as_bstr())) + .then(|| { + phases + .into_iter() + .filter_map(|(phase, is_draft)| { + u32::from_bytes(is_draft).ok().and_then(|is_draft| { + (is_draft > 0).then(|| HgChangesetId::from_bytes(phase)) + }) }) - }) - .collect::, _>>() - }) - .transpose() - .unwrap() - .unwrap_or_default(); - if drafts.is_empty() { - false - } else { - // Theoretically, we could have commits with no - // metadata that the remote declares are public, while - // the rest of our push is in a draft state. That is - // however so unlikely that it's not worth the effort - // to support partial metadata storage. - !reachable_subset( - pushed - .heads() - .copied() - .filter_map(|h| h.to_git(store)) - .map(Into::into), - drafts - .iter() - .copied() - .filter_map(|h| h.to_git(store)) - .map(Into::into), - ) - .is_empty() + .collect::, _>>() + }) + .transpose() + .unwrap() + .unwrap_or_default(); + if drafts.is_empty() { + false + } else { + // Theoretically, we could have commits with no + // metadata that the remote declares are public, while + // the rest of our push is in a draft state. That is + // however so unlikely that it's not worth the effort + // to support partial metadata storage. + !reachable_subset( + pushed + .heads() + .copied() + .filter_map(|h| h.to_git(store)) + .map(Into::into), + drafts + .iter() + .copied() + .filter_map(|h| h.to_git(store)) + .map(Into::into), + ) + .is_empty() + } } } - _ => unreachable!(), - } - }; + }; if rollback { unsafe { do_cleanup(1); diff --git a/tests/push.t b/tests/push.t index 186733512..12c1cf0cf 100755 --- a/tests/push.t +++ b/tests/push.t @@ -364,6 +364,32 @@ Server is now non-publishing, so metadata is unchanged. WARNING Pushing a new root To hg::.*/push.t/xyz (re) + 687e015...7ca6a3c 7ca6a3c32ec0dbcbcd155b2be6e2f4505012c273 -> branches/default/tip (forced update) + +Whatever happens, --dry-run doesn't store metadata + + $ git -C xyz-git cinnabar rollback --candidates + a2341d430e5acddf9481eabcad901fda12d023d3 (current) + 8b8194eefb69ec89edc35dafb965311fe48c49d0 + 2836e453f32b1ecccd3acca412f75b07c88176bf + +There is an extra mode for cinnabar.data that forces --dry-run to store metadata + + $ git -c cinnabar.data=force -C xyz-git push -f --dry-run origin 7ca6a3c32ec0dbcbcd155b2be6e2f4505012c273:refs/heads/branches/default/tip + \r (no-eol) (esc) + WARNING Pushing a new root + To hg::.*/push.t/xyz (re) + + 687e015...7ca6a3c 7ca6a3c32ec0dbcbcd155b2be6e2f4505012c273 -> branches/default/tip (forced update) + $ git -C xyz-git cinnabar rollback --candidates + 4305cef3fa610b3370f64ce10d2b50693a904278 (current) + a2341d430e5acddf9481eabcad901fda12d023d3 + 8b8194eefb69ec89edc35dafb965311fe48c49d0 + 2836e453f32b1ecccd3acca412f75b07c88176bf + $ git -C xyz-git cinnabar rollback + $ git -C xyz-git cinnabar rollback --candidates + a2341d430e5acddf9481eabcad901fda12d023d3 (current) + 8b8194eefb69ec89edc35dafb965311fe48c49d0 + 2836e453f32b1ecccd3acca412f75b07c88176bf + $ git -c cinnabar.data=always -C xyz-git push -f origin 7ca6a3c32ec0dbcbcd155b2be6e2f4505012c273:refs/heads/branches/default/tip \r (no-eol) (esc) WARNING Pushing a new root