preserve commit signatures #958
Conversation
mostly for me to learn how to write these tests
|
For instance, the docs for importing say but currently this is only true if the original project did not contain any signed commits. |
| @@ -196,6 +196,18 @@ pub fn rewrite_commit( | |||
| parents, | |||
| )?; | |||
|
|
|||
| if let Ok((sig, _)) = repo.extract_signature(&base.id(), None) { | |||
| // 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). | |||
There was a problem hiding this comment.
FWIW there is a comment a few lines up that needs updating now -- or we could entirely remove that equality check.
There was a problem hiding this comment.
I think that equality check can be removed. It's value as an optimization is super low as it is an quite rare case.
| @@ -196,6 +196,18 @@ pub fn rewrite_commit( | |||
| parents, | |||
| )?; | |||
|
|
|||
| if let Ok((sig, _)) = repo.extract_signature(&base.id(), None) { | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Oh, that makes a ton of tests fail that have the version number somewhere in their output... |
Head branch was pushed to by a user without write access
Note that I don't really know what I am doing here. ;) This seems like a massive breaking change to me -- if someone's git history contains these signatures, now the commits exposed via josh will be all different. Is that a problem? What is the usual way to deal with such a problem? Should there be a flag? Were signatures actually dropped deliberately? So many questions.^^
The first commit is just me learning to write these tests, but I couldn't find an existing test specifically for the prefix filter so I figured I might submit it as well. If you prefer I can remove it.
Fixes #957