feat(GB-1121): add sign-on-push functionality#14037
Conversation
github-actions
Bot
added
rust
slarse
force-pushed
the
GB-1121/add-sign-on-push-prototype
branch
from
7a10b9a to
d89fa32
Compare
slarse
force-pushed
the
GB-1121/add-sign-on-push-prototype
branch
from
d89fa32 to
1ef78ba
Compare
| /// Sign commits before pushing | ||
| #[clap(long, default_value_t = false, hide = true)] | ||
| pub sign_commits: bool, |
There was a problem hiding this comment.
I don't think this is what we wanna do, rather it should be setting in Git-config. But potentially we could start here for experimentation purposes while I dogfood the feature.
| let mut guard = ctx.exclusive_worktree_access(); | ||
| let perm = guard.write_permission(); | ||
| let id_map = IdMap::new_from_context(ctx, None, perm.read_permission())?; |
There was a problem hiding this comment.
We need exclusive access to be able to create a mutable workspace to sign commits.
| pub fn sign_commits<'ws, 'meta, M: RefMetadata>( | ||
| ref_name: impl Borrow<gix::refs::FullNameRef>, | ||
| repo: &gix::Repository, | ||
| workspace: &'ws mut but_graph::Workspace, | ||
| meta: &'meta mut M, | ||
| ) -> Result<SignCommitsOutcome<'ws, 'meta, M>> { | ||
| let graph = but_graph::Graph::from_head(repo, meta, but_graph::init::Options::limited())?; | ||
| let ref_name = ref_name.borrow(); | ||
|
|
||
| let Some(segment) = graph.segment_by_ref_name(ref_name) else { | ||
| anyhow::bail!("ref not found in graph: {ref_name}"); | ||
| }; | ||
|
|
||
| let mut editor = Editor::create_with_opts( | ||
| workspace, | ||
| meta, | ||
| &repo, | ||
| &GraphEditorOptions { | ||
| // SignCommit::Yes makes the signing cascade as necessary, s.t. already signed commits | ||
| // are resigned if any of their parents are forcibly signed. Without this, already | ||
| // signed commits that are parents of unsigned commits may themselves become unsigned | ||
| // as a result of the rebase. | ||
| default_sign_commit: SignCommit::Yes, | ||
| ..GraphEditorOptions::default() | ||
| }, | ||
| )?; | ||
|
|
||
| for graph_commit in &segment.commits { | ||
| if graph_commit.flags.contains(CommitFlags::Integrated) { | ||
| continue | ||
| } | ||
|
|
||
| let commit = but_core::Commit::from_id(graph_commit.id.attach(&repo))?; | ||
|
|
||
| if !commit.extra_headers().pgp_signature().is_some() { | ||
| let selector = editor.select_commit(graph_commit.id)?; | ||
| let mut pick = Pick::new_pick(graph_commit.id); | ||
| pick.sign_commit = SignCommit::Yes; | ||
| pick.pick_mode = PickMode::Force; | ||
| editor.replace(selector, Step::Pick(pick))?; | ||
| } | ||
| } | ||
|
|
||
| let rebase = editor.rebase()?; | ||
| Ok(SignCommitsOutcome { rebase }) | ||
| } |
There was a problem hiding this comment.
This represents what I want to do, but I'm not sure where to put this or how to split this. I can't pass in an editor both because I also need the graph to resolve the segment, but also because the editor must be instantiated in a particular way (SignCommit::Yes) for this to work as expected. So passing in an editor seems like the wrong thing to do; if it's not instantiated correctly the result won't be the expected one.
So while on the one hand I want to split this into "find" and "execute", I don't think it's the right way to go about it and this should probably be just a single, isolated operation.
| if args.sign_commits { | ||
| let (repo, mut ws, _db) = ctx.workspace_mut_and_db_with_perm(perm)?; | ||
| let mut meta = ctx.meta()?; | ||
| let ref_name: gix::refs::FullName = format!("refs/heads/{branch_name}").try_into()?; | ||
| let outcome = | ||
| but_workspace::commit::sign_commits::sign_commits(ref_name, &repo, &mut ws, &mut meta)?; | ||
|
|
||
| outcome.rebase.materialize()?; | ||
| } |
There was a problem hiding this comment.
Probably need to fetch here to minimize the risk of signing already integrated commits.
1 participant
Picking up sign-on-push again!
This is just a starting point and by no means done, I just want a basis for conversation.
Will add some comments on things to discuss tomorrow.