On 4/15/2023 3:07 PM, Elijah Newren wrote: > On Fri, Apr 14, 2023 at 7:23 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: >> Sending arbitrary command-line arguments to setup_revisions() >> creates an opportunity for behavior you are not expecting. >> [unstated: and what about other similar options?] > > I'd really rather not have an allowlist of which revision options > should be allowed for use by git-replay. A denylist might be okay, if > also implemented for fast-export, but I'm more of the opinion that we > just document that both commands only make sense with "contiguous" > history. > > Most importantly, though, at a minimum I do absolutely want to allow > several negative revisions to be specified, several positive revisions > to be specified, and special flags like --ancestry-path or > --first-parent to be specified. There may well be additional flags, > both current and future, that should be allowed too. > > In short, I don't want another limited rebase; I want a more generic tool. The primary value of git-replay (to me) is that we can do "simple" rebases without a worktree or user interaction. To me, simple rebases mean apply a linear sequence of commits from a single branch onto another branch (and specifying an "old base" is simple enough to include here). It also means that we abort completely if there is a conflict (and handle conflict resolution in later changes). The sooner we can deliver that, the better we can deliver on its potential (and expand its functionality to be more generic). And this is generally my preference, but we shouldn't get functionality "by accident" but instead do it with full intention and strong testing. I will always think that certain shortcuts should not be used. This includes passing arguments directly to setup_revisions() and using pathspec parsing when a path string will work just fine. If we have a need for a feature, then we should add it explicitly and carefully. The surface area of setup_revisions() is far too large to have confidence that we are not exposing bugs for users to trip on later. Thanks, -Stolee
