On Wed, Jun 5, 2013 at 1:13 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Felipe Contreras <felipe.contre...@gmail.com> writes:
>> On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Felipe Contreras <felipe.contre...@gmail.com> writes:
>>>> You didn't answer, what happens when you run --skip-empty and 
>>>> --allow-empty?
>>> I'll answer to a slightly different question: What should happen?
>>> I think it should error out, because --allow-empty is about
>>> "allowing empty commits to be preserved", and --skip-empty is about
>>> "skipping empty commits (as it says on the package)".
>> Exactly, so they are related after all. However, --allow-empty says this:
>> "By default, cherry-picking an empty commit will fail,"
> OK, that is a very good point.  Especially because by the time
> reader reaches this new description, --allow-empty has already
> mentioned this, we can just be brief and it is sufficient to say
> "Instead of failing," upfront.
>> In fact, it might make sense to change the default in v2.0 to
>> --empty-commits=skip. If it makes sense in 'git rebase', why doesn't
>> it in 'git cherry-pick'?
> I think the primary reason behind the "Why are you picking a no-op?
> Let me stop to make sure you didn't make a mistake." is because
> cherry-pick and revert have long been operations for a single commit
> explicitly given by the user, not bunch of commits in a range.

Yeah, but this has changed already.

> These two operating modes are conceptually very different, even when
> we consider scripted use.  A script that feeds one commit at a time
> has a chance to do patch equivalence or user-interactive filtering
> on its own, and would be helped by the "are you sure you meant to
> record that no-op?" check if it filtered in a wrong way (e.g. the
> user specified an empty commit by mistake).  One that feeds a range,
> on the other hand, relies on or at least expects cherry-pick to have
> such smart, and a smarter cherry-pick could select what to pick from
> the given range.

How would a script know that a single pick ends up as a no-op? It
can't know what is the reason the cherry-pick failed. The only way to
know for sure is to check the last commit, and for that we don't need
the last cherry-pick to fail.

> In the longer term, especially if we envision to move large part of
> logic to prepare the sequencer insn list from rebase to cherry-pick,
> a ranged cherry-pick should learn a way to filter the range with
> patch equivalence, for example.  Once that happens, the behaviour
> would become inconsistent at the end-user level if we stopped at a
> commit only because it was not exactly patch equivalent to another
> one that is already on the branch we are cherry-picking to, but
> ended up to be a no-op, while we did not stop at another commit
> because patch equivalence filtered it out beforehand to skip it.
> Skipping a no-op by default would remove that inconsistency.
> So in that sense, one could argue that it may be a bugfix to skip
> commits that become no-op when replayed, when picking a range of
> commits (but not a single one).  And I do not think you would need
> to wait until 2.0 for such a change.

Right. But first we need to agree that failing an empty cherry-pick
serves no purpose.

Felipe Contreras
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to