Skip to content

preserve commit signatures #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 3, 2022
Merged

preserve commit signatures #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6350758
add test for prefix filter
RalfJung Oct 1, 2022
458d163
preserve commit signatures
RalfJung Oct 1, 2022
e4e3308
bump cache version
RalfJung Oct 3, 2022
2b7368b
Merge branch 'master' into gpgsig
christian-schilling Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::*;
use std::collections::HashMap;

const CACHE_VERSION: u64 = 12;
const CACHE_VERSION: u64 = 13;

lazy_static! {
static ref DB: std::sync::Mutex<Option<sled::Db>> = std::sync::Mutex::new(None);
Expand Down
12 changes: 12 additions & 0 deletions src/history.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,18 @@ pub fn rewrite_commit(
parents,
)?;

if let Ok((sig, _)) = repo.extract_signature(&base.id(), None) {
Copy link
Contributor Author

@RalfJung RalfJung Oct 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None here and in the commit_signed below refers to the name of the signature field. Looks like that can be customized. Roundtrips will still not work if there are commits that use a non-standard name for that field. I don't know how to get git to tell us tabout non-standard fields... really ideally we'd have an API to list all header fields, and preserve them all while only adjusting the tree, but I am not sure if that is possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's quite interesting: We could maybe at some point even use that for out own metadata. Good to know.

About getting all extra header fields: Worst case we have to parse the commit buffer ourselves. But I think this is good enough for now.

One little thing: The constant CACHE_VERSION in src/cache.rs should be incremented whenever we make a change that can affect the output sha's of any filter. This is to avoid strange effects that could happen when somebody uses a cache that was computed using the old version and things could get mixed up.

// Re-create the object with the original signature (which of course does not match any
// more, but this is needed to guarantee perfect round-trips).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW there is a comment a few lines up that needs updating now -- or we could entirely remove that equality check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that equality check can be removed. It's value as an optimization is super low as it is an quite rare case.

let b = b
.as_str()
.ok_or_else(|| josh_error("non-UTF-8 signed commit"))?;
let sig = sig
.as_str()
.ok_or_else(|| josh_error("non-UTF-8 signature"))?;
return Ok(repo.commit_signed(b, sig, None)?);
}

return Ok(repo.odb()?.write(git2::ObjectType::Commit, &b)?);
}

Expand Down
37 changes: 37 additions & 0 deletions tests/filter/gpgsig.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
$ git init -q 1> /dev/null

Initial commit
$ echo contents1 > file1
$ git add .
$ git commit -m "add file1" 1> /dev/null

Now create another commit for the same tree, but with a gpgsig
$ git rev-parse HEAD
0b4cf6c9efbbda1eada39fa9c1d21d2525b027bb
$ git cat-file commit HEAD
tree 3d77ff51363c9825cc2a221fc0ba5a883a1a2c72
author Josh <josh@example.com> 1112911993 +0000
committer Josh <josh@example.com> 1112911993 +0000

add file1
$ git hash-object -t commit -w --stdin <<EOF
> tree 3d77ff51363c9825cc2a221fc0ba5a883a1a2c72
> author Josh <josh@example.com> 1112911993 +0000
> committer Josh <josh@example.com> 1112911993 +0000
> gpgsig hello
>
> add file1
> EOF
cb22ebb8e47b109f7add68b1043e561e0db09802
$ git reset --hard cb22ebb8e47b109f7add68b1043e561e0db09802 1>/dev/null

Apply a josh round-trip to this.
$ josh-filter :prefix=extra refs/heads/master --update refs/heads/filtered
$ josh-filter :/extra refs/heads/filtered --update refs/heads/double-filtered

And compare. Should be the same commit for both.
If 0b4cf6c9efbbda1eada39fa9c1d21d2525b027bb shows up then the signature was lost.
$ git diff master double-filtered
$ git rev-parse master double-filtered
cb22ebb8e47b109f7add68b1043e561e0db09802
cb22ebb8e47b109f7add68b1043e561e0db09802
16 changes: 16 additions & 0 deletions tests/filter/prefix.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
$ git init -q 1> /dev/null

Initial commit
$ echo contents1 > file1
$ git add .
$ git commit -m "add file1" 1> /dev/null

Apply prefix filter
$ josh-filter -s :prefix=subtree refs/heads/master --update refs/heads/filtered
[1] :prefix=subtree

$ git log --graph --pretty=%s refs/heads/filtered
* add file1

$ git ls-tree --name-only -r refs/heads/filtered
subtree/file1
2 changes: 1 addition & 1 deletion tests/proxy/amend_patchset.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"real_repo.git" = [':/sub3']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/authentication.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
4 changes: 2 additions & 2 deletions tests/proxy/caching.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down Expand Up @@ -156,7 +156,7 @@
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_absent_head.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_all.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_blocked.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_invalid_url.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_locked_refs.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_prefix.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_sha.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subsubtree.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subtree.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subtree_no_master.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_subtree_tags.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/clone_with_meta.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/empty_commit.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ should still be included.
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/get_version.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/import_export.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ Flushed credential cache
"repo2.git" = [':prefix=repo2']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/join_with_merge.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"real_repo.git" = [':prefix=sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/markers.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/no_proxy.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
$ bash ${TESTDIR}/destroy_test_env.sh
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_new_branch.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Check the branch again
"real_repo.git" = [':/sub']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_prefix.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Make sure all temporary namespace got removed
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review_already_in.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ test for that.
"real_repo.git" = []
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review_nop_behind.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ This is a regression test for that problem.
"real_repo.git" = []
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review_old.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ Flushed credential cache
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_review_topic.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Make sure all temporary namespace got removed
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_stacked.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ Make sure all temporary namespace got removed
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_stacked_split.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Make sure all temporary namespace got removed
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_stacked_sub.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ Make sure all temporary namespace got removed
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_subdir_prefix.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
]
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_subtree.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Make sure all temporary namespace got removed
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
2 changes: 1 addition & 1 deletion tests/proxy/push_subtree_two_repos.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ Put a double slash in the URL to see that it also works
"real_repo.git" = [':/sub1']
.
|-- josh
| `-- 12
| `-- 13
| `-- sled
| |-- blobs
| |-- conf
Expand Down
Loading