Thread (6 messages) 6 messages, 3 authors, 2024-01-05

Re: rebase invoking pre-commit

From: Elijah Newren <hidden>
Date: 2023-12-23 21:37:52

On Thu, Dec 21, 2023 at 12:59 PM Sean Allred [off-list ref] wrote:
Is there a current reason why pre-commit shouldn't be invoked during
rebase, or is this just waiting for a reviewable patch?

This was brought up before at [1] in 2015, but that thread so old at
this point that it seemed prudent to double-check before investing time
in a developing and testing a patch.

[1]: https://lore.kernel.org/git/1m55i3m.1fum4zo1fpnhncM%25lists@haller-berlin.de/ (local)
I'm very opinionated here.  I'm just one person, so definitely take
this with a grain of salt, but in my view...

Personally, I think implementing any per-commit hook in rebase by
default is a mistake.  It enforces a
must-be-in-a-worktree-and-the-worktree-must-be-updated-with-every-replayed-commit
mindset, which I find highly problematic[2], even if that's "what we
always used to do".  Because of that, I would prefer to see this at
most be a command line flag.  However, we've already got a command
line flag that while not identical, is essentially equivalent: "--exec
$MY_SCRIPT" (it's not the same because it's a post-commit check, but
you get notification of any problematic commits, and an immediate stop
of the rebase for you to fix up the problematic commit; fixing up the
commit shouldn't be problematic since you are, after all, already
rebasing).

I see Phillip already responded and suggested not running the
pre-commit hook with every commit, but only upon the first commit
after a "git rebase --continue".  That seems far more reasonable to me
than running on every commit...though even that worries me in regards
to what assumptions that entails about what is present in the working
tree.  (For example, what about folks with large repositories, so
large that a branch switch or full checkout is extremely costly, and
which could benefit from resolving conflicts in a separate
sparse-checkout worktree, potentially much more sparse than their main
checkout?  And what if people like that really fast rebase resolution
(namely, done in a separate very sparse checkout which also has the
advantage of not polluting your current working tree) so much that
they use it on smaller repositories as well?  Can I not even
experiment with this idea because of the historical
per-commit-at-least-as-full-as-main-worktree-checkout assumptions
we've baked into rebase?)

While at it, I should also mention that I'm not a fan of the broken
pre-rebase hook; its design ties us to doing a single branch at a
time.  Maybe that hook is not quite as bad, though, since we already
broke that hook and no one seemed to care (in particular, when
--update-refs was implemented).  But if no one seems to care about
broken hooks, I think the proper answer is to either get rid of the
hook or fix it.

Anyway, as I mentioned, I'm quite opinionated here.  To the point that
I deemed git-rebase in its current form to possibly be unfixable
(after first putting a lot of work into improving it over the past
years) and eventually introduced a new "git-replay" command which I
hope to make into a competent replacement for it.  Given that I do
have another command to do my experiments, others on the list may
think it's fine to send rebase further down this route, but I was
hoping to avoid further drift so that there might be some way of
re-implementing rebase on top of my "git-replay" ideas/design.

Just my $0.02,
Elijah

[2] https://lore.kernel.org/git/20231124111044.3426007-1-christian.couder@gmail.com/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help