From 0008f1b74efeb8c040ab8774389b27fb122133be Mon Sep 17 00:00:00 2001 From: hmirin Date: Thu, 29 Oct 2026 00:37:49 +0900 Subject: [PATCH] Fix the local benchmark, clean up keys from a failed init, and stale v6 paths - scripts/benchmark-local.sh counted files in storage/policies, which format 5 and later never create, so it stopped under set -e. The count and its summary field are removed. - git-e2ee init deletes a key it generated in the same run when a later step fails, instead of leaving an orphaned key file. A key the user passed is never touched. - scripts/benchmark-comparison.sh snapshotted storage/objects, which format 6 renamed to packs/. - docs/benchmarks.md notes that its recorded run used the old carrier branch and attributes, and what format 6 uses instead. --- docs/benchmarks.md | 4 +++- scripts/benchmark-comparison.sh | 8 ++++---- scripts/benchmark-local.sh | 4 +--- src/main.rs | 30 ++++++++++++++++++++++-------- tests/cli_targets.rs | 14 ++++++++++++++ 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/docs/benchmarks.md b/docs/benchmarks.md index d83f966..7ea44e2 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -81,7 +81,9 @@ fresh bare remotes, clients, cache directories, and a temporary GPG home. Git auto-GC, maintenance, and receive auto-GC were disabled. The bare receivers used `receive.unpackLimit=0`; the carrier receiver also used `attr.tree=refs/heads/git-remote-e2ee`, so it could read the committed -`e2ee/** -delta` attribute. The E2EE push and returning-client caches were +`e2ee/** -delta` attribute. (That run used an earlier layout; the current +layout, format 6, uses branch `main` and `* -delta` at the branch root, so +reproduce with `attr.tree=refs/heads/main`.) The E2EE push and returning-client caches were isolated per round under the temporary benchmark directory. Each tiny commit replaced one line in Godot's tracked README and was pushed and fetched before the next. See `scripts/benchmark-git-backend.sh` for the complete harness. diff --git a/scripts/benchmark-comparison.sh b/scripts/benchmark-comparison.sh index 40bdbe0..80496d5 100755 --- a/scripts/benchmark-comparison.sh +++ b/scripts/benchmark-comparison.sh @@ -108,7 +108,7 @@ tree_logical_bytes() { object_snapshot() { local root=$1 local object digest - find "$root/objects" -type f -print | LC_ALL=C sort | while IFS= read -r object; do + find "$root/packs" -type f -print | LC_ALL=C sort | while IFS= read -r object; do if [[ $time_style == bsd ]]; then digest=$(shasum -a 256 "$object" | awk '{print $1}') else @@ -291,7 +291,7 @@ repository_root=$(sed -n 's/^[[:space:]]*"repository_root": "\([^"]*\)",$/\1/p' --output "$work/new-user.key.json" >/dev/null "$git_e2ee" device-export --key "$work/new-user.key.json" \ --output "$work/new-user.public.json" >/dev/null -object_snapshot "$work/e2ee-storage" >"$work/objects-before-device-add.sha256" +object_snapshot "$work/e2ee-storage" >"$work/packs-before-device-add.sha256" run_phase add_user e2ee device-add "$work/e2ee-storage" "$work/e2ee-new-client" \ "$git_e2ee" device-add "e2ee::dir:$work/e2ee-storage" "$work/new-user.public.json" \ --label new-user --key "$work/repository.key.json" @@ -312,8 +312,8 @@ if (( total_readers > 2 )); then active_reader_key=$reader_key done fi -object_snapshot "$work/e2ee-storage" >"$work/objects-after-device-add.sha256" -cmp "$work/objects-before-device-add.sha256" "$work/objects-after-device-add.sha256" +object_snapshot "$work/e2ee-storage" >"$work/packs-after-device-add.sha256" +cmp "$work/packs-before-device-add.sha256" "$work/packs-after-device-add.sha256" run_phase add_user e2ee fetch-new-user "$work/e2ee-storage" \ "$work/e2ee-new-client" \ "$git_e2ee" fetch --storage "$work/e2ee-storage" \ diff --git a/scripts/benchmark-local.sh b/scripts/benchmark-local.sh index aa9d30a..fb7a6db 100755 --- a/scripts/benchmark-local.sh +++ b/scripts/benchmark-local.sh @@ -185,7 +185,6 @@ final_storage_logical=$(tree_logical_bytes "$work/storage") final_pack_logical=$(tree_logical_bytes "$work/storage/packs") final_pack_files=$(find "$work/storage/packs" -type f | wc -l | tr -d ' ') manifest_files=$(find "$work/storage/manifests" -type f | wc -l | tr -d ' ') -policy_files=$(find "$work/storage/policies" -type f | wc -l | tr -d ' ') crate_sha=$(git -C "$project_root" rev-parse HEAD) if [[ -n $(git -C "$project_root" status --porcelain=v1) ]]; then crate_dirty=true @@ -214,8 +213,7 @@ cat >"$results_dir/summary.json" < Result<()> { if in_repository && git_config(&format!("remote.{name}.url"), false)?.is_some() { bail!("Git remote {name} already exists; choose another name with --name") } + let mut created_key = None; let key = match key { Some(path) if path.exists() => path, requested => { @@ -255,18 +256,31 @@ fn main() -> Result<()> { }; generated.write_new(&path)?; println!("created key {} (administrator)", path.display()); + created_key = Some(path.clone()); path } }; - let target = Target { - storage, - key: std::path::absolute(key)?, + let initialized = (|| -> Result { + let target = Target { + storage, + key: std::path::absolute(key)?, + }; + let pin = admin_pin_path(&target.key)?; + with_repository!(&target, |repository| { + repository.initialize(&label)?; + repository.pin_admin_state(&pin)?; + }); + Ok(target) + })(); + let target = match initialized { + Ok(target) => target, + Err(error) => { + if let Some(path) = created_key { + let _ = std::fs::remove_file(path); + } + return Err(error); + } }; - let pin = admin_pin_path(&target.key)?; - with_repository!(&target, |repository| { - repository.initialize(&label)?; - repository.pin_admin_state(&pin)?; - }); println!("initialized {}", target.storage); if in_repository { run_git(&["remote", "add", &name, &target.storage.to_string()])?; diff --git a/tests/cli_targets.rs b/tests/cli_targets.rs index bb0fd95..9be94ca 100644 --- a/tests/cli_targets.rs +++ b/tests/cli_targets.rs @@ -141,6 +141,20 @@ fn init_refuses_an_existing_remote_before_creating_anything() { sandbox.git(&repo, &["push", "-q", "private", "main"]); } +#[test] +fn failed_init_removes_the_key_it_created() { + let sandbox = Sandbox::new(); + let url = format!("e2ee::dir:{}", sandbox.path().join("storage").display()); + + success(&sandbox.e2ee(sandbox.path(), &["init", &url])); + assert_eq!(sandbox.keys().len(), 1); + + let second = sandbox.e2ee(sandbox.path(), &["init", &url]); + assert!(!second.status.success()); + let keys = sandbox.keys(); + assert_eq!(keys.len(), 1, "{keys:?}"); +} + #[test] fn verify_takes_a_carrier_url_outside_a_repository() { let sandbox = Sandbox::new();