Re: [PATCH] format-patch: avoid generation of empty patches
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 10, 2009, 20:41 UTC
- Message-ID
- <7vy6xivqpu.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <d77df1110901100801s463bb43bt701a95df14f167d8@mail.gmail.com>
"Nathan W. Panike" <nathan.panike@gmail.com> writes:
Show 9 quoted lines
> On Sat, Jan 10, 2009 at 5:39 AM, Alexander Potashev > <aspotashev@gmail.com> wrote: > ... >> >> + if (!commit->parents && !rev.show_root_diff) >> + break; > > Do you really want to stop getting commits? It seems like the break > statement here should be a continue.
You can give a commit range that has two independent roots. The above "break" is wrong.
The variable is called show_root_DIFF, not show_root_COMMIT; even if you have "log.showroot = false", "git log -p" output would still give you the initial commit, but without the patch text, no?
But that is not Alexander's fault; it is mine.
I think "log -p" and "format-patch" can and should behave differently in this case. "log -p" is for people who already _have_ the history and would want to inspect how it evolved, and it is reasonable if some people want to say "the very initial huge import is not interesting to me while reviewing the history", and turning it off makes sense for them (in fact, the default was initially that way).
On the other hand, "format-patch" is about exporting a part of your history so that you can mechanincally replay it elsewhere, and I do not think of a reasonable justification not to export a root commit fully if the range user asked for happens to contain one.
I agree with Alexander that we should not output just the message without the patch text, but I think the right solution is to show both. not to skip root.
-- >8 -- format-patch: show patch text for the root commit
Even without --root specified, if the range given on the command line happens to include a root commit, we should include its patch text in the output.
This fix deliberately ignores log.showroot configuration variable because "format-patch" and "log -p" can and should behave differently in this case, as the former is about exporting a part of your history in a form that is replayable elsewhere and just giving the commit log message without the patch text does not make any sense for that purpose.
Noticed and fix originally attempted by Nathan W. Panike; credit goes to Alexander Potashev for injecting sanity to my initial (broken) fix that used the value from log.showroot configuration, which was misguided.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin-log.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git c/builtin-log.c w/builtin-log.c index 4a02ee9..91e5412 100644 --- c/builtin-log.c +++ w/builtin-log.c @@ -935,6 +935,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) * get_revision() to do the usual traversal. */ } + + /* + * We cannot move this anywhere earlier because we do want to + * know if --root was given explicitly from the comand line. + */ + rev.show_root_diff = 1; + if (cover_letter) { /* remember the range */ int i;