~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless
Switch repository
Loading…

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

Merging keeps a protected head branch, and a failed compaction spares the push

Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:client/small-fixes
.changeset/merge-keeps-protected-head.mdnew+7−0
View file
@@ -0,0 +1,7 @@
---
default: patch
---
# A merge no longer leaves a pull open when its head branch cannot be deleted
Merging a pull whose head branch is protected keeps that branch, and says why, instead of trying to delete it. A delete that is refused for another reason no longer fails the merge after the target has moved. A `git push` also no longer fails when the compaction after it does, and `KNOTLESS_INLINE_MAX` is held to 262144 bytes, the most a pack record takes.
crates/knotless/src/env.rs+6−1
View file
@@ -27,6 +27,9 @@ pub enum HandleResolver {
/// `KNOTLESS_APPVIEW` is unset.
pub const DEFAULT_APPVIEW: &str = "https://knotless.dev";
/// The most bytes a pack record holds inline: the lexicon's `maxLength`.
pub const MAX_INLINE: usize = 262_144;
#[derive(Clone, Debug)]
pub struct Env {
pub config_dir: PathBuf,
@@ -143,7 +146,7 @@ impl Env {
identifier: get("KNOTLESS_IDENTIFIER"),
password: get("KNOTLESS_PASSWORD"),
browser: get("KNOTLESS_BROWSER").filter(|v| !v.is_empty()),
inline_max: num("KNOTLESS_INLINE_MAX", 192 * 1024) as usize,
inline_max: (num("KNOTLESS_INLINE_MAX", 192 * 1024) as usize).min(MAX_INLINE),
chunk_size: num("KNOTLESS_CHUNK_SIZE", 4_000_000) as usize,
compact_threshold: num("KNOTLESS_COMPACT_THRESHOLD", 32) as usize,
compact_mode: match get("KNOTLESS_COMPACT").as_deref() {
@@ -283,6 +286,8 @@ mod tests {
);
assert_eq!(e.compact_mode, CompactMode::Off);
assert_eq!(e.inline_max, 10);
let big = env(&[("KNOTLESS_INLINE_MAX", "999999")]);
assert_eq!(big.inline_max, MAX_INLINE);
assert_eq!(e.chunk_size, 4_000_000);
assert_eq!(
env(&[("KNOTLESS_COMPACT", "inline")]).compact_mode,
crates/knotless/src/forge.rs+28−13
View file
@@ -2088,22 +2088,31 @@ pub async fn merge(
} else {
merge_commit(git, &tree, base, head, &item, &me).await?
};
let mut updates = vec![Update {
let updates = [Update {
src: Some(merged.clone()),
dst: sides.refname.clone(),
force: false,
}];
let others = assemble(&t.record, t.remote.did(), &entries);
let deleted = (!keep).then(|| spent(t.facts(), &item, &others)).flatten();
let kept = kept(&t, &item, &others, keep).filter(|_| deleted.is_none());
if let Some(gone) = &deleted {
updates.push(Update {
src: None,
dst: format!("refs/heads/{gone}"),
force: false,
});
}
let spent = (!keep).then(|| spent(t.facts(), &item, &others)).flatten();
let kept = kept(&t, &item, &others, keep).filter(|_| spent.is_none());
push_as(ctx, &t, git, &me, &updates).await?;
// The target has moved: a delete that is refused leaves the branch,
// not the pull open.
let deleted = match spent {
Some(gone) => {
let drop = [Update {
src: None,
dst: format!("refs/heads/{gone}"),
force: false,
}];
push_as(ctx, &t, git, &me, &drop)
.await
.is_ok()
.then_some(gone)
}
None => None,
};
let record = StateRecord {
repo: t.reference(),
subject: pull.to_string(),
@@ -2237,7 +2246,7 @@ pub(crate) fn refusal(
/// The head branch of `item`, once merged, when nothing needs it any more:
/// it is in the target itself, is neither the target branch nor the
/// default one, and no other open pull brings it in or targets it.
/// default one, is not protected (a gateway never deletes one), and no other open pull brings it in or targets it.
pub(crate) fn spent(t: Facts, item: &Item, items: &[Item]) -> Option<String> {
let head = item.head.as_ref()?;
let here = t.key;
@@ -2252,7 +2261,8 @@ pub(crate) fn spent(t: Facts, item: &Item, items: &[Item]) -> Option<String> {
other.uri != item.uri && other.state == "open" && (into || from)
};
let base = item.base.as_deref() == Some(&head.branch) || head.branch == default;
(mine && !base && !items.iter().any(named)).then(|| head.branch.clone())
let protected = t.record.rule(&head.branch).is_some();
(mine && !base && !protected && !items.iter().any(named)).then(|| head.branch.clone())
}
/// Why a merge of `item` on `t` keeps its head branch rather than
@@ -2274,6 +2284,7 @@ pub(crate) fn kept(t: &Target, item: &Item, items: &[Item], keep: bool) -> Optio
head_here(t.facts(), item).is_some(),
into,
named,
t.record.rule(branch).is_some(),
)
}
@@ -2287,6 +2298,7 @@ fn why_kept(
here: bool,
into: bool,
named: Option<&str>,
protected: bool,
) -> Option<String> {
let branch = &head.branch;
let why = match (keep, here, into, named) {
@@ -2294,6 +2306,7 @@ fn why_kept(
(_, false, ..) => format!("{branch} is in another repository, {}", head.repo.uri),
(_, _, true, _) => format!("{branch} is the branch it merges into, or the default branch"),
(.., Some(other)) => format!("{branch} is still named by another open pull, {other}"),
_ if protected => format!("{branch} is protected, so it is not deleted"),
_ => return None,
};
Some(why)
@@ -2989,7 +3002,7 @@ mod tests {
},
branch: "x".into(),
};
let why = |keep, here, into, named| why_kept(&head, keep, here, into, named);
let why = |keep, here, into, named| why_kept(&head, keep, here, into, named, false);
assert_eq!(
why(true, true, false, None).unwrap(),
"x was kept, as asked"
@@ -3008,6 +3021,8 @@ mod tests {
"x is still named by another open pull, at://p"
);
assert_eq!(why(false, true, false, None), None);
let protected = why_kept(&head, false, true, false, None, true);
assert_eq!(protected.unwrap(), "x is protected, so it is not deleted");
}
#[test]
crates/knotless/src/helper.rs+16−1
View file
@@ -185,7 +185,8 @@ impl Knotless {
match self.ctx.env.compact_mode {
CompactMode::Off => {}
CompactMode::Inline => {
compact::compact(&self.ctx, &self.remote, auth).await?;
let done = compact::compact(&self.ctx, &self.remote, auth).await;
said(done.map(drop));
}
CompactMode::Background => {
let exe = std::env::current_exe()?.with_file_name("knotless");
@@ -203,6 +204,14 @@ impl Knotless {
}
}
/// Says that a compaction after a push failed. The push has landed, so
/// git is not told.
fn said(done: Result<()>) {
if let Err(e) = done {
eprintln!("knotless: compaction after the push failed: {e:#}");
}
}
impl Knotless {
/// After a push that landed, claims the address git authors commits as here, the
/// first time `me` pushes from this machine, and says so. The push has
@@ -287,6 +296,12 @@ impl Backend for Knotless {
#[cfg(test)]
mod tests {
#[test]
fn a_failed_compaction_is_only_said() {
super::said(Err(anyhow::anyhow!("no room")));
super::said(Ok(()));
}
use super::*;
#[derive(Default)]
crates/knotless-serve/tests/forge.rs+9−0
View file
@@ -342,6 +342,15 @@ fn forks_pulls_reviews_and_merges() {
"{refs}"
);
assert!(!refs.contains("refs/heads/atop"), "{refs}");
// A protected head branch is not deleted, as a gateway would refuse it.
owner.git(&work, &["push", "-q", &url, "atop:guarded"]);
ok(&owner, &["protect", &url, "guarded"]);
let guarded = ok(&owner, &["pull", &url, "main", &url, "guarded", "Guarded"]);
listed(&owner, &url, &guarded);
let merged = ok(&owner, &["merge", &url, &guarded]);
assert!(!merged.contains("deleted"), "{merged}");
let (_, refs) = owner.cli(&["refs", &url], b"");
assert!(refs.contains("refs/heads/guarded"), "{refs}");
// Someone else's commit, elsewhere on the firehose, changes nothing.
av.app.observe(knotless_serve::firehose::Event::default());
docs/design/collaboration.md+5−4
View file
@@ -519,8 +519,9 @@ The steps:
reviews](#required-reviews), if its rule asks for any; then, if the
repository names runners, the merge gate. Only the owner may force.
6. Build the commit.
7. Push the branch update and, when spent (see below), the head branch's
deletion, in one push.
7. Push the branch update, then, when the head branch is spent (see
below), its deletion in a push of its own, so a delete that is refused
leaves the branch and not the pull open.
8. In one `applyWrites` in the merger's account, write the `merged` state
and any closing states.
@@ -547,8 +548,8 @@ Details:
committer, and a merge commit's author, is the merger, with email
`<did>@knotless.invalid`.
- A head branch is spent when it is in the target repository, is neither
the pull's base nor the default branch, and no other open pull targets or
brings it in. A fork's branch is never spent. `--keep-branch` keeps it.
the pull's base nor the default branch, is not protected, and no other
open pull targets or brings it in. A fork's branch is never spent. `--keep-branch` keeps it.
## Reverting a pull
docs/reference/cli.md+1−1
View file
@@ -681,7 +681,7 @@ account whose pull asks it, has it read the repositories again after
| Variable | Default | |
| --- | --- | --- |
| `KNOTLESS_INLINE_MAX` | 192 KiB | the largest pack kept inline in its record |
| `KNOTLESS_INLINE_MAX` | 192 KiB | the largest pack kept inline in its record, at most 256 KiB (262144 bytes), the lexicon's limit |
| `KNOTLESS_CHUNK_SIZE` | 4 MB, or the PDS's blob limit if smaller | the size of a large pack's, or a release file's, blob parts |
| `KNOTLESS_COMPACT` | `background` | when to compact: `background` (a detached `knotless compact`), `inline` (before the push returns) or `off` |
| `KNOTLESS_COMPACT_THRESHOLD` | 32 | the packs, counting the push's own, at which the owner's `git push` starts a compaction |