Re: [PATCH] sequencer: refuse to revert shallow boundary commits by default

[Nakul V S <vsnakul@gmail.com>]

Hi Phillip,

> From the user's point of view, I think do_pick_commit() is a bit late
> to error out. It would be nicer to refuse to add a shallow commit to
> the todo list in the first place which means we'd need a helper
> function that's called from sequencer_make_script(),
> make_script_with_merges(), single_pick() and
> sequencer_pick_revisions().

You're right, and I hadn't thought about it from the user's side. I
tried "git revert <tip> <boundary>" with v1. It reverted the tip, made
a commit, then failed on the boundary and left .git/sequencer behind.
That's a messy state to be left in.

So v2 does what you describe. The check is made where each command
builds its list of commits, and they all call the one helper. If any
of the commits is a boundary, the command refuses up front: no commit
is created, HEAD doesn't move, and there is nothing to abort.

"rebase --apply" doesn't go through the sequencer, so it gets a small
check of its own before it rewinds HEAD. That check does nothing in a
repository that isn't shallow.

One thing to be aware of: if someone types a "pick" or "revert" line
for a boundary commit into the rebase todo list by hand, it won't be
caught, because the check runs when the list is generated and not
after the editor. Git doesn't catch that today either, and the person
would have to type the hash on purpose, so I think it's an acceptable
price for failing early. If you'd prefer, I can add a second check in
do_pick_commit() as well.


Thanks,
Nakul