git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Pádraig Brady" <P@draigBrady.com>
Cc: git@vger.kernel.org
Subject: Re: git format-patch doesn't exclude merged hunks
Date: Wed, 16 May 2012 11:49:43 -0700	[thread overview]
Message-ID: <7vhavgc660.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <4FB3CAE3.6040608@draigBrady.com> ("Pádraig Brady"'s message of "Wed, 16 May 2012 16:42:27 +0100")

Pádraig Brady <P@draigBrady.com> writes:

> For reference the two commits in question are:
> https://github.com/openstack/nova/commit/7028d66
> https://github.com/openstack/nova/commit/26dc6b7
> Notice how both make the same change to Authors.

If you compare the changes these two commits introduce, you will also
notice that the "Authors" file is the _only_ common part of them.

"format-patch" (more precicely, the "git cherry" machinery that identifies
the same patch) does not _selectively_ drop only a part of a patch while
keeping the other parts.  It is not per "hunk", it is not even per "file".

This is very much on purpose, and I think it is a good design decision.

In this particular case, the behaviour does look suboptimal, but if you
think about it harder, you will realize that the perception comes largely
because in this particular commit, the change to the "Authors" file is the
least interesting part of the change.

Imagine a case where you were replaying a commit that changes a file
significantly and also changes another file in a trivial way, and where it
were the significant change that has already been applied to the receiving
codebase, not the insignificant change to "Authors" file.

Now imagine that format-patch dropped the part that brings in the
significant change as duplicate, and replayed only the insignificant part.
Most likely, the log message of the original commit explains what issue
that significant change tried to solve, and how the implementation in the
patch was determined to be an acceptable approach to solve it, and that is
what you will be recording for the replayed commit that only introduces
the remaining insignificant change.

I am not fundamentally opposed to the idea of (optionally) detecting and
selectively dropping parts of a patch to an entire file or even hunks that
have already applied, but it needs to have a way remind the user somewhere
in the workflow that it did so and the log message may no longer describe
what the change does.  Most likely it would have to be done when producing
format-patch output, but an approach to make it a responsibility to notice
and fix the resulting log message to the person who applies the output, I
would imagine.

  reply	other threads:[~2012-05-16 18:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-16 15:42 git format-patch doesn't exclude merged hunks Pádraig Brady
2012-05-16 18:49 ` Junio C Hamano [this message]
2012-05-16 19:04   ` Pádraig Brady
2012-05-16 19:12     ` Junio C Hamano
2012-05-16 20:13       ` Pádraig Brady
2012-05-16 22:42         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vhavgc660.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=P@draigBrady.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).