Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 8, 2009, 04:14 UTC
Message-ID: <7v63ikmz11.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1236418251-16947-1-git-send-email-chris_johnsen@pobox.com>

Chris Johnsen <chris_johnsen@pobox.com> writes:

Show 6 quoted lines

> When a cherry-pick of an empty commit is done, release the lock
> held on the index.
>
> The fix is the same as was applied to similar code in 4271666046.
>
> Signed-off-by: Chris Johnsen <chris_johnsen@pobox.com>

Thanks. Will apply. We should handle possible refactoring as a separate topic.

Show 7 quoted lines

> UNEVEN TREATMENT OF EMPTY CHANGES
>
> It seems that empty commits suffer uneven treatment under various
> patch-transport/history-rewriting mechanisms. They seem to be
> handled okay in the most of the core (fetch, push, bundle all
> seem to preserve empty commits, though I have not done rigorous
> testing).

They just transfer an existing history from one place to another without modifying, so it is unfortunately true that they preserve such a broken history with empty commits.

> 'format-patch', 'send-email', 'apply', 'am', 'rebase' (automatic,
> non-fast-forward; and --interactive).

These are all about creating history afresh, and they actively discourage empty commits to be (re)created.

There is no "uneven treatment".

> 36863af16e (git-commit --allow-empty) says "This is primarily for
> use by foreign scm interface scripts.". Is this the only case
> where empty commits _should_ be used?

If foreign scm recorded an empty commit, it would be nice to be able to recreate such a broken history _if the user wanted to_, and that is where the --allow-empty option can be used.