Should "git apply --check" imply verbose?

Should "git apply --check" imply verbose?

[Paul Gortmaker]

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

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

[Junio C Hamano]

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".

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

[Paul Gortmaker]

On 13-08-20 01:57 PM, Junio C Hamano wrote:
> 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.
--

> 
> 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".
> 

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

[Jonathan Nieder]

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

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

[Paul Gortmaker]

On 13-08-20 02:51 PM, Jonathan Nieder wrote:
> 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
> 

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

[Junio C Hamano]

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.

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.

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

[Steven Rostedt]

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

> 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


> 
> 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.

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

[Junio C Hamano]

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".

>> 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?

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

[Steven Rostedt]

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?

Thanks,

-- Steve

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

[Paul Gortmaker]

On 13-08-20 03:54 PM, Steven Rostedt wrote:
> 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.
--

> 
> Thanks,
> 
> -- Steve
> 

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

[Junio C Hamano]

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.

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

[Junio C Hamano]

Steven Rostedt <rostedt@goodmis.org> writes:

>> > 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...

>> 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.

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

[Steven Rostedt]

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