{"thread":{"id":"60683","subject":"Does extending `--empty` to git-cherry-pick make sense?","startedAt":"2024-01-04T06:57:56Z","lastAt":"2024-01-05T03:20:49Z","messageCount":4,"participants":["Brian Lyles","Taylor Blau","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"486281","messageId":"CAHPHrSevBdQF0BisR8VK=jM=wj1dTUYEVrv31gLerAzL9=Cd8Q@mail.gmail.com","threadId":"60683","inReplyTo":null,"subject":"Does extending `--empty` to git-cherry-pick make sense?","fromName":"Brian Lyles","fromEmail":"brianmlyles@gmail.com","sentAt":"2024-01-04T06:57:18Z","receivedAt":"2024-01-04T06:57:56Z","isPatch":false,"sender":{"key":"brianmlyles@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1123282?v=4"},"body":"The `--empty=(keep|drop|ask)` option added to git-rebase in\ne98c4269c86019bfe057a91b4305f784365b6f0b seems like it would be\napplicable to git-cherry-pick (and maybe git-revert for completeness?).\nWhile the shared sequencer code likely would already handle this fairly\nwell (at a cursory glance from someone with very little knowledge of C\nor git's codebase, admittedly), it's only exposed via git-rebase.\n\nThe use case in which this came up involves a semi-automated workflow\nfor moving commits between branches. At a high level, a tool is:\n\n- Identifying commits to be picked based on a specific trailer value\n- Using git-cherry-pick with `-x` to pick those commits\n- Relying on the presence of the `(cherry picked from commit ...)`\n trailer to avoid re-picking commits that have already been picked\n\nIf the picked commits are then rewritten in the upstream such that there\nare squashes or fixups, that trailer can end up missing in the upstream.\nThe next time the tool runs, it will end up trying to re-pick commits\nthat are already represented there. As a result, those commits become\nempty upon being picked a second time and the cherry-pick ends up\nbreaking for the user to resolve:\n\n $ git cherry-pick main\n On branch feature\n You are currently cherry-picking commit cfd7cd9.\n (all conflicts fixed: run \"git cherry-pick --continue\")\n (use \"git cherry-pick --skip\" to skip this patch)\n (use \"git cherry-pick --abort\" to cancel the cherry-pick operation)\n\n nothing to commit, working tree clean\n The previous cherry-pick is now empty, possibly due to conflict resolution.\n If you wish to commit it anyway, use:\n\n git commit --allow-empty\n\n Otherwise, please use 'git cherry-pick --skip'\n\nI'll spare the details, but we do expect this to happen with enough\nfrequency that we'd really like to be able to prevent it. The\n`--empty=drop` option sounds like exactly what we want here:\n\n --empty=(drop|keep|ask)\n How to handle commits that are not empty to start and are not\n clean cherry-picks of any upstream commit, but which become\n empty after rebasing (because they contain a subset of already\n upstream changes). With drop (the default), commits that become\n empty are dropped.\n\nIs there any real barrier to exposing that option to git-cherry-pick as\nwell? Was this an oversight, or intentionally left out? The\ncorresponding commit message doesn't seem to indicate any specific\nreason for limiting it to git-rebase.\n\nThanks,\n-Brian Lyles\n"},{"id":"486310","messageId":"ZZcIG+mNXhZ0rHw3@nand.local","threadId":"60683","inReplyTo":"CAHPHrSevBdQF0BisR8VK=jM=wj1dTUYEVrv31gLerAzL9=Cd8Q@mail.gmail.com","subject":"Re: Does extending `--empty` to git-cherry-pick make sense?","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-01-04T19:33:47Z","receivedAt":"2024-01-04T19:33:49Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"[+cc Elijah]\n\nOn Thu, Jan 04, 2024 at 12:57:18AM -0600, Brian Lyles wrote:\n> Is there any real barrier to exposing that option to git-cherry-pick as\n> well? Was this an oversight, or intentionally left out? The\n> corresponding commit message doesn't seem to indicate any specific\n> reason for limiting it to git-rebase.\n\nI am not nearly as familiar with this code as Elijah is, but this\ncertainly appears possible by setting the `drop_redundant_commits` and\n`keep_redundant_commits` flags in the replay_opts struct.\n\nI don't see any fundamental reason why cherry-pick shouldn't have the\nsame functionality.\n\nThanks,\nTaylor\n"},{"id":"486334","messageId":"CABPp-BGJfvBhO_zEX8nLoa8WNsjmwvtZ2qOjmYm9iPoZg4SwPw@mail.gmail.com","threadId":"60683","inReplyTo":"ZZcIG+mNXhZ0rHw3@nand.local","subject":"Re: Does extending `--empty` to git-cherry-pick make sense?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2024-01-05T02:28:00Z","receivedAt":"2024-01-05T02:28:31Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Jan 4, 2024 at 11:33 AM Taylor Blau wrote:\n>\n> [+cc Elijah]\n>\n> On Thu, Jan 04, 2024 at 12:57:18AM -0600, Brian Lyles wrote:\n> > Is there any real barrier to exposing that option to git-cherry-pick as\n> > well? Was this an oversight, or intentionally left out? The\n> > corresponding commit message doesn't seem to indicate any specific\n> > reason for limiting it to git-rebase.\n>\n> I am not nearly as familiar with this code as Elijah is, but this\n> certainly appears possible by setting the `drop_redundant_commits` and\n> `keep_redundant_commits` flags in the replay_opts struct.\n>\n> I don't see any fundamental reason why cherry-pick shouldn't have the\n> same functionality.\n\nI was indeed focused on simply getting the multiple rebase backends to\nhave consistent behavior (we had like 4-5 backends at the time,\nright?) and just didn't consider cherry-pick at the time. Now that\nthose are more consistent (though there's still work to be done on\nthat end too), cherry-pick and rebase really ought to share a lot more\nbetween each other, from command line flags, to temporary control\nfiles written, to more shared code paths. Adding an --empty flag to\ncherry-pick as a step along the way makes sense.\n\nNote that git-am also gained a similar flag in the meantime, but\nchanged the names slightly: --empty=(stop,drop,keep). I think 'stop'\nis a better name than 'ask', and we really should make rebase suggest\n'stop' instead (but keep 'ask' as a synonym for 'stop', for backwards\ncompatibility). Also, I kind of want to replace 'keep' with 'roll' in\nthe --empty option for both git-am and git-rebase, while keeping\n'keep' as a synonym for 'roll'. But I'm not sure if others on the\nlist would go along with it...\n"},{"id":"486336","messageId":"CAHPHrSe51_y445m0Hn_SqdpwJNKLrKHKm3LzY=APG+QzBygt7w@mail.gmail.com","threadId":"60683","inReplyTo":"CABPp-BGJfvBhO_zEX8nLoa8WNsjmwvtZ2qOjmYm9iPoZg4SwPw@mail.gmail.com","subject":"Re: Does extending `--empty` to git-cherry-pick make sense?","fromName":"Brian Lyles","fromEmail":"brianmlyles@gmail.com","sentAt":"2024-01-05T03:20:10Z","receivedAt":"2024-01-05T03:20:49Z","isPatch":false,"sender":{"key":"brianmlyles@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1123282?v=4"},"body":"On Thu, Jan 4, 2024 at 8:28 PM Elijah Newren wrote:\n>\n> I was indeed focused on simply getting the multiple rebase backends to\n> have consistent behavior (we had like 4-5 backends at the time,\n> right?) and just didn't consider cherry-pick at the time. Now that\n> those are more consistent (though there's still work to be done on\n> that end too), cherry-pick and rebase really ought to share a lot more\n> between each other, from command line flags, to temporary control\n> files written, to more shared code paths. Adding an --empty flag to\n> cherry-pick as a step along the way makes sense.\n\nI appreciate the insight from both of you.\n\nIt sounds like I'll be diving into some C code for the first time in\nover a decade, then! I'll plan to take a crack at this soon.\n\n-Brian Lyles\n"}]}