{"thread":{"id":"34733","subject":"Should \"git apply --check\" imply verbose?","startedAt":"2013-08-20T15:11:54Z","lastAt":"2013-08-20T22:37:58Z","messageCount":13,"participants":["Paul Gortmaker","Junio C Hamano","Jonathan Nieder","Steven Rostedt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"225522","messageId":"5213873A.6010003@windriver.com","threadId":"34733","inReplyTo":null,"subject":"Should \"git apply --check\" imply verbose?","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2013-08-20T15:11:54Z","receivedAt":"2013-08-20T15:11:54Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"TL;DR -- \"git apply --reject\" implies verbose, but the similar\n\"git apply --check\" does not, which seems inconsistent.\n\nBackground: A common (non-git) workflow can be to use \"patch --dry-run\"\nto inspect whether a patch is feasible, and then use patch again\na 2nd time (w/o --dry-run) to actually apply it (and then work\nthrough the rejects).\n\nYou can also do the above in a git repo, but you lose out because\n\"patch\" doesn't (yet) capture the patched function names[1] in the\nrejected hunks, making it hard to double check your work.\n\nMy initial thought was to replace the above two steps with\n\"git apply --check ...\" and then \"git apply --reject ...\" so\nthat I could just abandon using patch altogether.\n\nThat works great, with just one snag that had me go reading the\nsource. It seems that \"git apply --reject\" is verbose, and kind\nof looks like the identical output I'd get if I used patch. But\n\"git apply --check\" is quite reserved in its output and doesn't\nlook at all like \"patch --dry-run\". I initially believed that\n\"--check\" was stopping at the 1st failure, based on the output.\n\nOnly when I read the source did I realize it was checking all the\nhunks silently, and adding a \"-v\" would make it similar to the\noutput from \"patch --dry-run\".\n\nNot a critical issue by any means, but having the \"-v\" implied\nby \"--check\" (or perhaps having both default to non-verbose?)\nmight save other users from getting confused in the same way.\n\nThanks,\nPaul.\n--\n\n[1] https://savannah.gnu.org/bugs/index.php?39819\n"},{"id":"225534","messageId":"xmqqioz06y9m.fsf@gitster.dls.corp.google.com","threadId":"34733","inReplyTo":"5213873A.6010003@windriver.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-20T17:57:25Z","receivedAt":"2013-08-20T17:57:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Gortmaker writes:\n\n> TL;DR -- \"git apply --reject\" implies verbose, but the similar\n> \"git apply --check\" does not, which seems inconsistent.\n\nHmmm, I am of two minds. From purely idealistic point of view, I\ncan see why defaulting both to non-verbose may look a more\nattractive way to go, but I have my reservations that is more than\nthe usual change-aversion.\n\nHistorically, \"check\" was primarily meant to see if the patch is\napplicable cleanly in scripts, and we never thought it would make\nany sense to make it verbose by default. \n\nOn the other hand, the operation of \"reject\", which was a much later\ninvention, was primarily meant to be observed by humans to see how\nthe patch failed to cleanly apply and where, to help them decide\nwhere to look in the target to wiggle the rejected hunk into (even\nwhen it is driven from a script). It did not make much sense to\nsquelch its output.\n\nIn addition, because \"check\" is an idempotent operation that does\nnot touch anything in the index or the working tree, running with\n\"check\" and then \"check verbose\" is possible if somebody runs it\nwithout verbose and then decides later that s/he wants to see the\ndetails. But \"reject\" does touch the working tree files with\napplicable hunks, so after a quiet \"reject\", there is no way to see\nthe verbose output like you can with \"check\".\n"},{"id":"225540","messageId":"5213B95D.3040409@windriver.com","threadId":"34733","inReplyTo":"xmqqioz06y9m.fsf@gitster.dls.corp.google.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2013-08-20T18:45:49Z","receivedAt":"2013-08-20T18:45:49Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"On 13-08-20 01:57 PM, Junio C Hamano wrote:\n> Paul Gortmaker writes:\n> \n>> TL;DR -- \"git apply --reject\" implies verbose, but the similar\n>> \"git apply --check\" does not, which seems inconsistent.\n> \n> Hmmm, I am of two minds. From purely idealistic point of view, I\n> can see why defaulting both to non-verbose may look a more\n> attractive way to go, but I have my reservations that is more than\n> the usual change-aversion.\n\nOK, so given your feedback, how do you feel about a patch to the\ndocumentation that indicates to use \"-v\" in combination with the\n\"--check\" to get equivalent \"patch --dry-run\" behaviour? If that\nhad existed, I'd have not gone rummaging around in the source, so\nthat should be good enough to help others avoid the same...\n\nP.\n--\n\n> \n> Historically, \"check\" was primarily meant to see if the patch is\n> applicable cleanly in scripts, and we never thought it would make\n> any sense to make it verbose by default. \n> \n> On the other hand, the operation of \"reject\", which was a much later\n> invention, was primarily meant to be observed by humans to see how\n> the patch failed to cleanly apply and where, to help them decide\n> where to look in the target to wiggle the rejected hunk into (even\n> when it is driven from a script). It did not make much sense to\n> squelch its output.\n> \n> In addition, because \"check\" is an idempotent operation that does\n> not touch anything in the index or the working tree, running with\n> \"check\" and then \"check verbose\" is possible if somebody runs it\n> without verbose and then decides later that s/he wants to see the\n> details. But \"reject\" does touch the working tree files with\n> applicable hunks, so after a quiet \"reject\", there is no way to see\n> the verbose output like you can with \"check\".\n> \n"},{"id":"225541","messageId":"20130820185127.GG4110@google.com","threadId":"34733","inReplyTo":"5213B95D.3040409@windriver.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-08-20T18:51:27Z","receivedAt":"2013-08-20T18:51:27Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Paul,\n\nPaul Gortmaker wrote:\n\n> OK, so given your feedback, how do you feel about a patch to the\n> documentation that indicates to use \"-v\" in combination with the\n> \"--check\" to get equivalent \"patch --dry-run\" behaviour?\n\nSounds like a good idea to me.\n\nI assume you mean a note in the OPTIONS or EXAMPLES section of\nDocumentation/git-apply.txt?\n\nThanks,\nJonathan\n"},{"id":"225544","messageId":"5213BCA6.9040501@windriver.com","threadId":"34733","inReplyTo":"20130820185127.GG4110@google.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2013-08-20T18:59:50Z","receivedAt":"2013-08-20T18:59:50Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"On 13-08-20 02:51 PM, Jonathan Nieder wrote:\n> Hi Paul,\n> \n> Paul Gortmaker wrote:\n> \n>> OK, so given your feedback, how do you feel about a patch to the\n>> documentation that indicates to use \"-v\" in combination with the\n>> \"--check\" to get equivalent \"patch --dry-run\" behaviour?\n> \n> Sounds like a good idea to me.\n> \n> I assume you mean a note in the OPTIONS or EXAMPLES section of\n> Documentation/git-apply.txt?\n\nI hadn't looked exactly where yet, but wherever makes sense and\nwherever appears in TFM.\n\nP.\n--\n\n> \n> Thanks,\n> Jonathan\n> \n"},{"id":"225545","messageId":"xmqqzjsc5ggp.fsf@gitster.dls.corp.google.com","threadId":"34733","inReplyTo":"5213B95D.3040409@windriver.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-20T19:07:18Z","receivedAt":"2013-08-20T19:07:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Gortmaker writes:\n\n> OK, so given your feedback, how do you feel about a patch to the\n> documentation that indicates to use \"-v\" in combination with the\n> \"--check\" to get equivalent \"patch --dry-run\" behaviour? If that\n> had existed, I'd have not gone rummaging around in the source, so\n> that should be good enough to help others avoid the same...\n\nI do not think it is necessarily a good idea to assume that people\nwho are learning \"git apply\" know how GNU patch works.\n\nBut I do agree that the description of -v, --verbose has a lot of\nroom for improvement.\n\n\tReport progress to stderr. By default, only a message about the\n\tcurrent patch being applied will be printed. This option will cause\n\tadditional information to be reported.\n\nIt is totally unclear what \"additional information\" is reported at\nall.\n\nThanks.\n"},{"id":"225546","messageId":"20130820151554.6afbcb7f@gandalf.local.home","threadId":"34733","inReplyTo":"xmqqzjsc5ggp.fsf@gitster.dls.corp.google.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Steven Rostedt","fromEmail":"rostedt@goodmis.org","sentAt":"2013-08-20T19:15:54Z","receivedAt":"2013-08-20T19:15:54Z","isPatch":false,"sender":{"key":"rostedt@goodmis.org","avatar":"https://gravatar.com/avatar/cc188bf330d625ec6a7a2d0b6f4829dc777963e8dab83d943691dc31c5095227?d=mp&s=160"},"body":"On Tue, 20 Aug 2013 12:07:18 -0700\nJunio C Hamano wrote:\n\n> Paul Gortmaker writes:\n> \n> > OK, so given your feedback, how do you feel about a patch to the\n> > documentation that indicates to use \"-v\" in combination with the\n> > \"--check\" to get equivalent \"patch --dry-run\" behaviour? If that\n> > had existed, I'd have not gone rummaging around in the source, so\n> > that should be good enough to help others avoid the same...\n> \n> I do not think it is necessarily a good idea to assume that people\n> who are learning \"git apply\" know how GNU patch works.\n\nLinus told me that \"git apply\" was basically a replacement for patch.\nWhy would you think it would not be a good idea to assume that people\nwould not be familiar with how GNU patch works?\n\nIs it because you expect \"git apply\" to eventually replace patch all\nout, and want no dependencies on its knowledge?\n\n-- Steve\n\n\n> \n> But I do agree that the description of -v, --verbose has a lot of\n> room for improvement.\n> \n> \tReport progress to stderr. By default, only a message about the\n> \tcurrent patch being applied will be printed. This option will cause\n> \tadditional information to be reported.\n> \n> It is totally unclear what \"additional information\" is reported at\n> all.\n> \n> Thanks.\n"},{"id":"225552","messageId":"7v7gfgkuyo.fsf@alter.siamese.dyndns.org","threadId":"34733","inReplyTo":"20130820151554.6afbcb7f@gandalf.local.home","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-20T19:45:03Z","receivedAt":"2013-08-20T19:45:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Rostedt writes:\n\n>> I do not think it is necessarily a good idea to assume that people\n>> who are learning \"git apply\" know how GNU patch works.\n>\n> Linus told me that \"git apply\" was basically a replacement for patch.\n> Why would you think it would not be a good idea to assume that people\n> would not be familiar with how GNU patch works?\n\nThe audience of Git these days are far more widely spread than the\nkernel circle. I am not opposed to _helping_ those who happen to\nknow \"patch\", but I was against a description that assumes readers\nknow it, i.e. making it a requirement to know \"patch\" to understand\n\"apply\".\n\n>> But I do agree that the description of -v, --verbose has a lot of\n>> room for improvement.\n>> \n>> \tReport progress to stderr. By default, only a message about the\n>> \tcurrent patch being applied will be printed. This option will cause\n>> \tadditional information to be reported.\n>> \n>> It is totally unclear what \"additional information\" is reported at\n>> all.\n\nIn other words, your enhancement to the documentation could go like:\n\n\t... By default, ... With this option, you will additionally\n\tsee such and such and such in the output (this is similar to\n\twhat \"patch --dry-run\" would give you). See the EXAMPLES\n\tsection to get a feel of how it looks like.\n\nand I would not be opposed, as long as \"such and such and such\" are\nwritten in such a way that the reader does not have to have a prior\nexperience with GNU patch in order to understand it.\n\nClear?\n"},{"id":"225555","messageId":"20130820155433.217abb3e@gandalf.local.home","threadId":"34733","inReplyTo":"7v7gfgkuyo.fsf@alter.siamese.dyndns.org","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Steven Rostedt","fromEmail":"rostedt@goodmis.org","sentAt":"2013-08-20T19:54:33Z","receivedAt":"2013-08-20T19:54:33Z","isPatch":false,"sender":{"key":"rostedt@goodmis.org","avatar":"https://gravatar.com/avatar/cc188bf330d625ec6a7a2d0b6f4829dc777963e8dab83d943691dc31c5095227?d=mp&s=160"},"body":"On Tue, 20 Aug 2013 12:45:03 -0700\nJunio C Hamano wrote:\n\n> Steven Rostedt writes:\n> \n> >> I do not think it is necessarily a good idea to assume that people\n> >> who are learning \"git apply\" know how GNU patch works.\n> >\n> > Linus told me that \"git apply\" was basically a replacement for patch.\n> > Why would you think it would not be a good idea to assume that people\n> > would not be familiar with how GNU patch works?\n> \n> The audience of Git these days are far more widely spread than the\n> kernel circle. I am not opposed to _helping_ those who happen to\n> know \"patch\", but I was against a description that assumes readers\n> know it, i.e. making it a requirement to know \"patch\" to understand\n> \"apply\".\n\nPatch is used by much more than just the kernel folks ;-) I've been\nusing patch much longer than I've been doing kernel development.\n\n\n> \n> >> But I do agree that the description of -v, --verbose has a lot of\n> >> room for improvement.\n> >> \n> >> \tReport progress to stderr. By default, only a message about the\n> >> \tcurrent patch being applied will be printed. This option will cause\n> >> \tadditional information to be reported.\n> >> \n> >> It is totally unclear what \"additional information\" is reported at\n> >> all.\n> \n> In other words, your enhancement to the documentation could go like:\n> \n> \t... By default, ... With this option, you will additionally\n> \tsee such and such and such in the output (this is similar to\n> \twhat \"patch --dry-run\" would give you). See the EXAMPLES\n> \tsection to get a feel of how it looks like.\n> \n> and I would not be opposed, as long as \"such and such and such\" are\n> written in such a way that the reader does not have to have a prior\n> experience with GNU patch in order to understand it.\n> \n> Clear?\n\nLooks good to me. Paul, what do you think?\n\nThanks,\n\n-- Steve\n"},{"id":"225585","messageId":"5213CF53.5010306@windriver.com","threadId":"34733","inReplyTo":"20130820155433.217abb3e@gandalf.local.home","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2013-08-20T20:19:31Z","receivedAt":"2013-08-20T20:19:31Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"On 13-08-20 03:54 PM, Steven Rostedt wrote:\n> On Tue, 20 Aug 2013 12:45:03 -0700\n> Junio C Hamano wrote:\n> \n>> Steven Rostedt writes:\n>>\n>>>> I do not think it is necessarily a good idea to assume that people\n>>>> who are learning \"git apply\" know how GNU patch works.\n>>>\n>>> Linus told me that \"git apply\" was basically a replacement for patch.\n>>> Why would you think it would not be a good idea to assume that people\n>>> would not be familiar with how GNU patch works?\n>>\n>> The audience of Git these days are far more widely spread than the\n>> kernel circle. I am not opposed to _helping_ those who happen to\n>> know \"patch\", but I was against a description that assumes readers\n>> know it, i.e. making it a requirement to know \"patch\" to understand\n>> \"apply\".\n> \n> Patch is used by much more than just the kernel folks ;-) I've been\n> using patch much longer than I've been doing kernel development.\n> \n> \n>>\n>>>> But I do agree that the description of -v, --verbose has a lot of\n>>>> room for improvement.\n>>>>\n>>>> \tReport progress to stderr. By default, only a message about the\n>>>> \tcurrent patch being applied will be printed. This option will cause\n>>>> \tadditional information to be reported.\n>>>>\n>>>> It is totally unclear what \"additional information\" is reported at\n>>>> all.\n>>\n>> In other words, your enhancement to the documentation could go like:\n>>\n>> \t... By default, ... With this option, you will additionally\n>> \tsee such and such and such in the output (this is similar to\n>> \twhat \"patch --dry-run\" would give you). See the EXAMPLES\n>> \tsection to get a feel of how it looks like.\n>>\n>> and I would not be opposed, as long as \"such and such and such\" are\n>> written in such a way that the reader does not have to have a prior\n>> experience with GNU patch in order to understand it.\n>>\n>> Clear?\n> \n> Looks good to me. Paul, what do you think?\n\nYep, I'll write something up tomorrow which loosely matches the above.\n\nThanks,\nPaul.\n--\n\n> \n> Thanks,\n> \n> -- Steve\n> \n"},{"id":"225562","messageId":"xmqqtxikujfn.fsf@gitster.dls.corp.google.com","threadId":"34733","inReplyTo":"20130820155433.217abb3e@gandalf.local.home","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-20T21:43:56Z","receivedAt":"2013-08-20T21:43:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Rostedt writes:\n\n>> > Linus told me that \"git apply\" was basically a replacement for patch.\n>> > Why would you think it would not be a good idea to assume that people\n>> > would not be familiar with how GNU patch works?\n>> \n>> The audience of Git these days are far more widely spread than the\n>> kernel circle. I am not opposed to _helping_ those who happen to\n>> know \"patch\", but I was against a description that assumes readers\n>> know it, i.e. making it a requirement to know \"patch\" to understand\n>> \"apply\".\n>\n> Patch is used by much more than just the kernel folks ;-) I've been\n> using patch much longer than I've been doing kernel development.\n\nYeah, I was familiar with \"patch\" when I started Git, too ;-).\n\nBut only folks in the kernel circle will be told by Linus the\nsimilarity between apply and patch, no?\n\nIn any case...\n\n>> In other words, your enhancement to the documentation could go like:\n>> \n>> \t... By default, ... With this option, you will additionally\n>> \tsee such and such and such in the output (this is similar to\n>> \twhat \"patch --dry-run\" would give you). See the EXAMPLES\n>> \tsection to get a feel of how it looks like.\n>> \n>> and I would not be opposed, as long as \"such and such and such\" are\n>> written in such a way that the reader does not have to have a prior\n>> experience with GNU patch in order to understand it.\n\n... I forgot to also add: And by mentioning \"similar to\", people who\nare familiar with \"patch\" are also helped by their pre-existing\nknowledge, so both kinds of people win.\n\nThanks.\n"},{"id":"225563","messageId":"xmqqppt8ujek.fsf@gitster.dls.corp.google.com","threadId":"34733","inReplyTo":"5213CF53.5010306@windriver.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-20T21:44:35Z","receivedAt":"2013-08-20T21:44:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Gortmaker writes:\n\n>> Looks good to me. Paul, what do you think?\n>\n> Yep, I'll write something up tomorrow which loosely matches the above.\n\nThanks.\n"},{"id":"225568","messageId":"20130820183758.436dee18@gandalf.local.home","threadId":"34733","inReplyTo":"xmqqtxikujfn.fsf@gitster.dls.corp.google.com","subject":"Re: Should \"git apply --check\" imply verbose?","fromName":"Steven Rostedt","fromEmail":"rostedt@goodmis.org","sentAt":"2013-08-20T22:37:58Z","receivedAt":"2013-08-20T22:37:58Z","isPatch":false,"sender":{"key":"rostedt@goodmis.org","avatar":"https://gravatar.com/avatar/cc188bf330d625ec6a7a2d0b6f4829dc777963e8dab83d943691dc31c5095227?d=mp&s=160"},"body":"On Tue, 20 Aug 2013 14:43:56 -0700\nJunio C Hamano wrote:\n\n \n> But only folks in the kernel circle will be told by Linus the\n> similarity between apply and patch, no?\n\nWell, there was a time when Linus was making his rounds showcasing git\nmore than Linux, to people that were not kernel developers.\n\n-- Steve\n"}]}