threads / discuss / 34733

Should "git apply --check" imply verbose?

Subject: Should "git apply --check" imply verbose?

## tl;dr

13 messages between Aug 20, 2013 and Aug 20, 2013.

replies: 12people: 4as markdown or json

Paul Gortmaker· Aug 20, 2013, 15:11 UTC · lore

TL;DR -- "git apply --reject" implies verbose, but the similar "git apply --check" does not, which seems inconsistent.

Background:  A common (non-git) workflow can be to use "patch --dry-run"
to inspect whether a patch is feasible, and then use patch again
a 2nd time (w/o --dry-run) to actually apply it (and then work
through the rejects).

You can also do the above in a git repo, but you lose out because "patch" doesn't (yet) capture the patched function names[1] in the rejected hunks, making it hard to double check your work.

My initial thought was to replace the above two steps with "git apply --check ..." and then "git apply --reject ..." so that I could just abandon using patch altogether.

That works great, with just one snag that had me go reading the source. It seems that "git apply --reject" is verbose, and kind of looks like the identical output I'd get if I used patch. But "git apply --check" is quite reserved in its output and doesn't look at all like "patch --dry-run". I initially believed that "--check" was stopping at the 1st failure, based on the output.

Only when I read the source did I realize it was checking all the hunks silently, and adding a "-v" would make it similar to the output from "patch --dry-run".

Not a critical issue by any means, but having the "-v" implied by "--check" (or perhaps having both default to non-verbose?) might save other users from getting confused in the same way.

Thanks, Paul. --

[1] https://savannah.gnu.org/bugs/index.php?39819
Junio C Hamano· Aug 20, 2013, 17:57 UTC · re: Paul Gortmaker · lore

Re: Should "git apply --check" imply verbose?

Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> TL;DR -- "git apply --reject" implies verbose, but the similar
> "git apply --check" does not, which seems inconsistent.

Hmmm, I am of two minds. From purely idealistic point of view, I can see why defaulting both to non-verbose may look a more attractive way to go, but I have my reservations that is more than the usual change-aversion.

Historically, "check" was primarily meant to see if the patch is applicable cleanly in scripts, and we never thought it would make any sense to make it verbose by default.

On the other hand, the operation of "reject", which was a much later invention, was primarily meant to be observed by humans to see how the patch failed to cleanly apply and where, to help them decide where to look in the target to wiggle the rejected hunk into (even when it is driven from a script). It did not make much sense to squelch its output.

In addition, because "check" is an idempotent operation that does not touch anything in the index or the working tree, running with "check" and then "check verbose" is possible if somebody runs it without verbose and then decides later that s/he wants to see the details. But "reject" does touch the working tree files with applicable hunks, so after a quiet "reject", there is no way to see the verbose output like you can with "check".

Paul Gortmaker· Aug 20, 2013, 18:45 UTC · re: Junio C Hamano · lore

Re: Should "git apply --check" imply verbose?

On 13-08-20 01:57 PM, Junio C Hamano wrote:
Show 9 quoted lines
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
>> TL;DR -- "git apply --reject" implies verbose, but the similar
>> "git apply --check" does not, which seems inconsistent.
> 
> Hmmm, I am of two minds.  From purely idealistic point of view, I
> can see why defaulting both to non-verbose may look a more
> attractive way to go, but I have my reservations that is more than
> the usual change-aversion.

OK, so given your feedback, how do you feel about a patch to the documentation that indicates to use "-v" in combination with the "--check" to get equivalent "patch --dry-run" behaviour? If that had existed, I'd have not gone rummaging around in the source, so that should be good enough to help others avoid the same...

P. --

Show 20 quoted lines
> 
> Historically, "check" was primarily meant to see if the patch is
> applicable cleanly in scripts, and we never thought it would make
> any sense to make it verbose by default.  
> 
> On the other hand, the operation of "reject", which was a much later
> invention, was primarily meant to be observed by humans to see how
> the patch failed to cleanly apply and where, to help them decide
> where to look in the target to wiggle the rejected hunk into (even
> when it is driven from a script).  It did not make much sense to
> squelch its output.
> 
> In addition, because "check" is an idempotent operation that does
> not touch anything in the index or the working tree, running with
> "check" and then "check verbose" is possible if somebody runs it
> without verbose and then decides later that s/he wants to see the
> details.  But "reject" does touch the working tree files with
> applicable hunks, so after a quiet "reject", there is no way to see
> the verbose output like you can with "check".
> 
Jonathan Nieder· Aug 20, 2013, 18:51 UTC · re: Paul Gortmaker · lore

Re: Should "git apply --check" imply verbose?

Hi Paul,
Paul Gortmaker wrote:
> OK, so given your feedback, how do you feel about a patch to the
> documentation that indicates to use "-v" in combination with the
> "--check" to get equivalent "patch --dry-run" behaviour?
Sounds like a good idea to me.

I assume you mean a note in the OPTIONS or EXAMPLES section of Documentation/git-apply.txt?

Thanks, Jonathan

Paul Gortmaker· Aug 20, 2013, 18:59 UTC · re: Jonathan Nieder · lore

Re: Should "git apply --check" imply verbose?

On 13-08-20 02:51 PM, Jonathan Nieder wrote:
Show 12 quoted lines
> Hi Paul,
> 
> Paul Gortmaker wrote:
> 
>> OK, so given your feedback, how do you feel about a patch to the
>> documentation that indicates to use "-v" in combination with the
>> "--check" to get equivalent "patch --dry-run" behaviour?
> 
> Sounds like a good idea to me.
> 
> I assume you mean a note in the OPTIONS or EXAMPLES section of
> Documentation/git-apply.txt?

I hadn't looked exactly where yet, but wherever makes sense and wherever appears in TFM.

P. --

> 
> Thanks,
> Jonathan
> 
Junio C Hamano· Aug 20, 2013, 19:07 UTC · re: Paul Gortmaker · lore

Re: Should "git apply --check" imply verbose?

Paul Gortmaker <paul.gortmaker@windriver.com> writes:
Show 5 quoted lines
> OK, so given your feedback, how do you feel about a patch to the
> documentation that indicates to use "-v" in combination with the
> "--check" to get equivalent "patch --dry-run" behaviour?   If that
> had existed, I'd have not gone rummaging around in the source, so
> that should be good enough to help others avoid the same...

I do not think it is necessarily a good idea to assume that people who are learning "git apply" know how GNU patch works.

But I do agree that the description of -v, --verbose has a lot of room for improvement.

	Report progress to stderr. By default, only a message about the
	current patch being applied will be printed. This option will cause
	additional information to be reported.

It is totally unclear what "additional information" is reported at all.

Thanks.
Steven Rostedt· Aug 20, 2013, 19:15 UTC · re: Junio C Hamano · lore

Re: Should "git apply --check" imply verbose?

On Tue, 20 Aug 2013 12:07:18 -0700 Junio C Hamano <gitster@pobox.com> wrote:

Show 10 quoted lines
> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> 
> > OK, so given your feedback, how do you feel about a patch to the
> > documentation that indicates to use "-v" in combination with the
> > "--check" to get equivalent "patch --dry-run" behaviour?   If that
> > had existed, I'd have not gone rummaging around in the source, so
> > that should be good enough to help others avoid the same...
> 
> I do not think it is necessarily a good idea to assume that people
> who are learning "git apply" know how GNU patch works.

Linus told me that "git apply" was basically a replacement for patch. Why would you think it would not be a good idea to assume that people would not be familiar with how GNU patch works?

Is it because you expect "git apply" to eventually replace patch all out, and want no dependencies on its knowledge?

-- Steve
Show 12 quoted lines
> 
> But I do agree that the description of -v, --verbose has a lot of
> room for improvement.
> 
> 	Report progress to stderr. By default, only a message about the
> 	current patch being applied will be printed. This option will cause
> 	additional information to be reported.
> 
> It is totally unclear what "additional information" is reported at
> all.
> 
> Thanks.
Junio C Hamano· Aug 20, 2013, 19:45 UTC · re: Steven Rostedt · lore

Re: Should "git apply --check" imply verbose?

Steven Rostedt <rostedt@goodmis.org> writes:
Show 6 quoted lines
>> I do not think it is necessarily a good idea to assume that people
>> who are learning "git apply" know how GNU patch works.
>
> Linus told me that "git apply" was basically a replacement for patch.
> Why would you think it would not be a good idea to assume that people
> would not be familiar with how GNU patch works?

The audience of Git these days are far more widely spread than the kernel circle. I am not opposed to _helping_ those who happen to know "patch", but I was against a description that assumes readers know it, i.e. making it a requirement to know "patch" to understand "apply".

Show 9 quoted lines
>> But I do agree that the description of -v, --verbose has a lot of
>> room for improvement.
>> 
>> 	Report progress to stderr. By default, only a message about the
>> 	current patch being applied will be printed. This option will cause
>> 	additional information to be reported.
>> 
>> It is totally unclear what "additional information" is reported at
>> all.
In other words, your enhancement to the documentation could go like:
	... By default, ... With this option, you will additionally
	see such and such and such in the output (this is similar to
	what "patch --dry-run" would give you).  See the EXAMPLES
	section to get a feel of how it looks like.

and I would not be opposed, as long as "such and such and such" are written in such a way that the reader does not have to have a prior experience with GNU patch in order to understand it.

Clear?
Steven Rostedt· Aug 20, 2013, 19:54 UTC · re: Junio C Hamano · lore

Re: Should "git apply --check" imply verbose?

On Tue, 20 Aug 2013 12:45:03 -0700 Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines
> Steven Rostedt <rostedt@goodmis.org> writes:
> 
> >> I do not think it is necessarily a good idea to assume that people
> >> who are learning "git apply" know how GNU patch works.
> >
> > Linus told me that "git apply" was basically a replacement for patch.
> > Why would you think it would not be a good idea to assume that people
> > would not be familiar with how GNU patch works?
> 
> The audience of Git these days are far more widely spread than the
> kernel circle.  I am not opposed to _helping_ those who happen to
> know "patch", but I was against a description that assumes readers
> know it, i.e. making it a requirement to know "patch" to understand
> "apply".

Patch is used by much more than just the kernel folks ;-) I've been using patch much longer than I've been doing kernel development.

Show 23 quoted lines
> 
> >> But I do agree that the description of -v, --verbose has a lot of
> >> room for improvement.
> >> 
> >> 	Report progress to stderr. By default, only a message about the
> >> 	current patch being applied will be printed. This option will cause
> >> 	additional information to be reported.
> >> 
> >> It is totally unclear what "additional information" is reported at
> >> all.
> 
> In other words, your enhancement to the documentation could go like:
> 
> 	... By default, ... With this option, you will additionally
> 	see such and such and such in the output (this is similar to
> 	what "patch --dry-run" would give you).  See the EXAMPLES
> 	section to get a feel of how it looks like.
> 
> and I would not be opposed, as long as "such and such and such" are
> written in such a way that the reader does not have to have a prior
> experience with GNU patch in order to understand it.
> 
> Clear?
Looks good to me. Paul, what do you think?
Thanks,
-- Steve
Paul Gortmaker· Aug 20, 2013, 20:19 UTC · re: Steven Rostedt · lore

Re: Should "git apply --check" imply verbose?

On 13-08-20 03:54 PM, Steven Rostedt wrote:
Show 47 quoted lines
> On Tue, 20 Aug 2013 12:45:03 -0700
> Junio C Hamano <gitster@pobox.com> wrote:
> 
>> Steven Rostedt <rostedt@goodmis.org> writes:
>>
>>>> I do not think it is necessarily a good idea to assume that people
>>>> who are learning "git apply" know how GNU patch works.
>>>
>>> Linus told me that "git apply" was basically a replacement for patch.
>>> Why would you think it would not be a good idea to assume that people
>>> would not be familiar with how GNU patch works?
>>
>> The audience of Git these days are far more widely spread than the
>> kernel circle.  I am not opposed to _helping_ those who happen to
>> know "patch", but I was against a description that assumes readers
>> know it, i.e. making it a requirement to know "patch" to understand
>> "apply".
> 
> Patch is used by much more than just the kernel folks ;-)  I've been
> using patch much longer than I've been doing kernel development.
> 
> 
>>
>>>> But I do agree that the description of -v, --verbose has a lot of
>>>> room for improvement.
>>>>
>>>> 	Report progress to stderr. By default, only a message about the
>>>> 	current patch being applied will be printed. This option will cause
>>>> 	additional information to be reported.
>>>>
>>>> It is totally unclear what "additional information" is reported at
>>>> all.
>>
>> In other words, your enhancement to the documentation could go like:
>>
>> 	... By default, ... With this option, you will additionally
>> 	see such and such and such in the output (this is similar to
>> 	what "patch --dry-run" would give you).  See the EXAMPLES
>> 	section to get a feel of how it looks like.
>>
>> and I would not be opposed, as long as "such and such and such" are
>> written in such a way that the reader does not have to have a prior
>> experience with GNU patch in order to understand it.
>>
>> Clear?
> 
> Looks good to me. Paul, what do you think?
Yep, I'll write something up tomorrow which loosely matches the above.

Thanks, Paul. --

Show 5 quoted lines
> 
> Thanks,
> 
> -- Steve
> 
Junio C Hamano· Aug 20, 2013, 21:44 UTC · re: Paul Gortmaker · lore

Re: Should "git apply --check" imply verbose?

Paul Gortmaker <paul.gortmaker@windriver.com> writes:
>> Looks good to me. Paul, what do you think?
>
> Yep, I'll write something up tomorrow which loosely matches the above.
Thanks.
Junio C Hamano· Aug 20, 2013, 21:43 UTC · re: Steven Rostedt · lore

Re: Should "git apply --check" imply verbose?

Steven Rostedt <rostedt@goodmis.org> writes:
Show 12 quoted lines
>> > Linus told me that "git apply" was basically a replacement for patch.
>> > Why would you think it would not be a good idea to assume that people
>> > would not be familiar with how GNU patch works?
>> 
>> The audience of Git these days are far more widely spread than the
>> kernel circle.  I am not opposed to _helping_ those who happen to
>> know "patch", but I was against a description that assumes readers
>> know it, i.e. making it a requirement to know "patch" to understand
>> "apply".
>
> Patch is used by much more than just the kernel folks ;-)  I've been
> using patch much longer than I've been doing kernel development.
Yeah, I was familiar with "patch" when I started Git, too ;-).

But only folks in the kernel circle will be told by Linus the similarity between apply and patch, no?

In any case...
Show 10 quoted lines
>> In other words, your enhancement to the documentation could go like:
>> 
>> 	... By default, ... With this option, you will additionally
>> 	see such and such and such in the output (this is similar to
>> 	what "patch --dry-run" would give you).  See the EXAMPLES
>> 	section to get a feel of how it looks like.
>> 
>> and I would not be opposed, as long as "such and such and such" are
>> written in such a way that the reader does not have to have a prior
>> experience with GNU patch in order to understand it.

... I forgot to also add: And by mentioning "similar to", people who are familiar with "patch" are also helped by their pre-existing knowledge, so both kinds of people win.

Thanks.
Steven Rostedt· Aug 20, 2013, 22:37 UTC · re: Junio C Hamano · lore

Re: Should "git apply --check" imply verbose?

On Tue, 20 Aug 2013 14:43:56 -0700 Junio C Hamano <gitster@pobox.com> wrote:

 
> But only folks in the kernel circle will be told by Linus the
> similarity between apply and patch, no?

Well, there was a time when Linus was making his rounds showcasing git more than Linux, to people that were not kernel developers.

-- Steve

← back to recent threads