{"thread":{"id":"22595","subject":"Suggestion on git-push --porcelain","startedAt":"2010-02-10T02:34:00Z","lastAt":"2010-02-10T19:14:41Z","messageCount":7,"participants":["Tay Ray Chuan","Junio C Hamano","Larry D'Anna","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"134107","messageId":"be6fef0d1002091834i1c4b202cp5afacc326bd1a4d6@mail.gmail.com","threadId":"22595","inReplyTo":null,"subject":"Suggestion on git-push --porcelain","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-10T02:34:00Z","receivedAt":"2010-02-10T02:34:00Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nthis is regarding the recent patch series from Larry. (I thought\nreplying to any of the patch messages was appropriate, I couldn't find\na cover-letter, so I'm starting a new thread.)\n\nAround June last year, a patch from Larry was made to add the\n--porcelain option, so as to produce machine-readable output regarding\nref status.\n\nThe latest patch series goes a step further, and tries to change\noutput - for example, suppressing user-friendly advice and giving \"To:\n\".\n\nI think this is an untenable path - adding/suppressing output of\ncertain messages for porcelain writers, while trying to keep things\nfixed enough for porcelain writers to depend on. We will also have to\nkeep and eye out for future patches from adding fprintfs to stdout and\nstderr that may break porcelain scripts.\n\nI believe a better approach would be to prefix messages intended for\nporcelain writers. For example, a push session might look like this:\n\n $ git push --porcelain\n PORCELAIN To git://foo.com/git/myrepo.git\n PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321\n PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de\n\nThis is an \"positive\" approach, in the sense that we don't remove\nanything from the current output; we just add more printf(\"PORCELAIN\")\nlines to wherever is appropriate.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134108","messageId":"7vmxzhn6fp.fsf@alter.siamese.dyndns.org","threadId":"22595","inReplyTo":"be6fef0d1002091834i1c4b202cp5afacc326bd1a4d6@mail.gmail.com","subject":"Re: Suggestion on git-push --porcelain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-10T02:57:46Z","receivedAt":"2010-02-10T02:57:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> $ git push --porcelain\n> PORCELAIN To git://foo.com/git/myrepo.git\n> PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321\n> PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de\n>\n> This is an \"positive\" approach, in the sense that we don't remove\n> anything from the current output; we just add more printf(\"PORCELAIN\")\n> lines to wherever is appropriate.\n\nSorry, but I don't see what that would solve. For example, we used not to\ngive the destination to the standard output stream, but that line carries\na necessary information and Larry's series corrects that.\n\nIn your \"prefix with PORCELAIN\" scheme, such a change will start adding a\nnew line \"PORCELAIN To ...\" that older implementations may not be prepared\nto see.\n\nOther than making the output more noisy, I do not see an improvement here.\n"},{"id":"134112","messageId":"20100210033557.GB5205@cthulhu","threadId":"22595","inReplyTo":"be6fef0d1002091834i1c4b202cp5afacc326bd1a4d6@mail.gmail.com","subject":"Re: Suggestion on git-push --porcelain","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-10T03:35:57Z","receivedAt":"2010-02-10T03:35:57Z","isPatch":false,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Tay Ray Chuan (rctay89@gmail.com) [100209 21:34]:\n> Hi,\n> \n> this is regarding the recent patch series from Larry. (I thought\n> replying to any of the patch messages was appropriate, I couldn't find\n> a cover-letter, so I'm starting a new thread.)\n> \n> Around June last year, a patch from Larry was made to add the\n> --porcelain option, so as to produce machine-readable output regarding\n> ref status.\n> \n> The latest patch series goes a step further, and tries to change\n> output - for example, suppressing user-friendly advice and giving \"To:\n> \".\n> \n> I think this is an untenable path - adding/suppressing output of\n> certain messages for porcelain writers, while trying to keep things\n> fixed enough for porcelain writers to depend on. We will also have to\n> keep and eye out for future patches from adding fprintfs to stdout and\n> stderr that may break porcelain scripts.\n\nWhile I agree with you in principle, I'm still advocating that we make these two\nchanges. *Especially* suppressing the advice. That advice did not exist last\nJune, so the output format has already been changed. Also, the advice never\nshould have gone to the standard output in the first place. All the other\ninstances of output like that go to standard error.\n\nAs for the \"To: \" lines, unfortunately the lack of them was a pretty serious\ndesign flaw in the original patch :(\n\n> I believe a better approach would be to prefix messages intended for\n> porcelain writers. For example, a push session might look like this:\n> \n> $ git push --porcelain\n> PORCELAIN To git://foo.com/git/myrepo.git\n> PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321\n> PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de\n> \n> This is an \"positive\" approach, in the sense that we don't remove\n> anything from the current output; we just add more printf(\"PORCELAIN\")\n> lines to wherever is appropriate.\n\nActually, What I'm proposing is something very similar to this: I think that the\noutput for the porcelain writer and *only* the output for the porcelain writer\nshould go to standard output, and everything else should go to standard error.\n\n\n --larry\n"},{"id":"134117","messageId":"20100210045446.GC28526@coredump.intra.peff.net","threadId":"22595","inReplyTo":"7vmxzhn6fp.fsf@alter.siamese.dyndns.org","subject":"Re: Suggestion on git-push --porcelain","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-10T04:54:46Z","receivedAt":"2010-02-10T04:54:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 09, 2010 at 06:57:46PM -0800, Junio C Hamano wrote:\n\n> Tay Ray Chuan writes:\n> \n> > $ git push --porcelain\n> > PORCELAIN To git://foo.com/git/myrepo.git\n> > PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321\n> > PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de\n> >\n> > This is an \"positive\" approach, in the sense that we don't remove\n> > anything from the current output; we just add more printf(\"PORCELAIN\")\n> > lines to wherever is appropriate.\n> \n> Sorry, but I don't see what that would solve. For example, we used not to\n> give the destination to the standard output stream, but that line carries\n> a necessary information and Larry's series corrects that.\n\nI think he is trying to future-proof any additional output that push (or\nremote helpers) produce. I don't think it is really worth it, though.\nAll of that should be going to stderr, and thus would be, at worst,\nnoise on the terminal. I don't think it is that hard or error-prone a\nrule to send such cruft to stderr.\n\n-Peff\n"},{"id":"134137","messageId":"be6fef0d1002100318v902689xc937cd332ac752b3@mail.gmail.com","threadId":"22595","inReplyTo":"7vmxzhn6fp.fsf@alter.siamese.dyndns.org","subject":"Re: Suggestion on git-push --porcelain","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-10T11:18:32Z","receivedAt":"2010-02-10T11:18:32Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Feb 10, 2010 at 10:57 AM, Junio C Hamano wrote:\n> Tay Ray Chuan writes:\n>\n>>   $ git push --porcelain\n>>   PORCELAIN To git://foo.com/git/myrepo.git\n>>   PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321\n>>   PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de\n>>\n>> This is an \"positive\" approach, in the sense that we don't remove\n>> anything from the current output; we just add more printf(\"PORCELAIN\")\n>> lines to wherever is appropriate.\n>\n> Sorry, but I don't see what that would solve.  For example, we used not to\n> give the destination to the standard output stream, but that line carries\n> a necessary information and Larry's series corrects that.\n\nI sense a chicken-and-egg situation here.\n\nPrinting a \"To: \" to stdout is a correction as far as the\ncurrent non-prefixed output scheme is concerned.\n\nIf a prefixed scheme (or some other output scheme) is adopted, then\nwhere, or whether, the \"To: \" is printed, is not relevant\nto porcelain script writers - they just get ignored.\n\n> In your \"prefix with PORCELAIN\" scheme, such a change will start adding a\n> new line \"PORCELAIN To ...\" that older implementations may not be prepared\n> to see.\n\nThe original --porcelain patch only \"porcelained\" the output for ref\nupdates, such as\n\n = TAB refs/heads/master:refs/heads/master TAB [up to date]\n - TAB :refs/heads/foobar TAB [deleted]\n ...\n\nIn addition, the \"To: \" patch --porcelain is fairly\nrecent - I don't think it has left 'pu', has it? Should we worry about\nbreaking compatibility on something not out yet?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134138","messageId":"be6fef0d1002100328g4dac78f6y88497aca791a17fc@mail.gmail.com","threadId":"22595","inReplyTo":"20100210045446.GC28526@coredump.intra.peff.net","subject":"Re: Suggestion on git-push --porcelain","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-10T11:28:11Z","receivedAt":"2010-02-10T11:28:11Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Feb 10, 2010 at 12:54 PM, Jeff King wrote:\n> On Tue, Feb 09, 2010 at 06:57:46PM -0800, Junio C Hamano wrote:\n>\n>> Tay Ray Chuan writes:\n>>\n>> >   $ git push --porcelain\n>> >   PORCELAIN To git://foo.com/git/myrepo.git\n>> >   PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321\n>> >   PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de\n>> >\n>> > This is an \"positive\" approach, in the sense that we don't remove\n>> > anything from the current output; we just add more printf(\"PORCELAIN\")\n>> > lines to wherever is appropriate.\n>>\n>> Sorry, but I don't see what that would solve.  For example, we used not to\n>> give the destination to the standard output stream, but that line carries\n>> a necessary information and Larry's series corrects that.\n>\n> I think he is trying to future-proof any additional output that push (or\n> remote helpers) produce.\n\nYes. By using this (or some other output scheme), we're deliberately\nmarking out output for porcelain scripts.\n\n> I don't think it is really worth it, though.\n> All of that should be going to stderr, and thus would be, at worst,\n> noise on the terminal. I don't think it is that hard or error-prone a\n> rule to send such cruft to stderr.\n\non examining the code paths, there's a fair bit of work needed to\nensure this. Apart from transport.c (transport-helper.c and by\nextension, remote helpers, looks ok), we would have to look at the\nindividual builtin-transport implementations for git and bundle.\n\nAlso, as you mentioned above, we would also have to take care future\ntransport implementations send messages in the desired fashion so that\nwe wouldn't break porcelain scripts. If we have an output scheme, then\nsuch risks are minimized.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134167","messageId":"7vaavgaony.fsf@alter.siamese.dyndns.org","threadId":"22595","inReplyTo":"be6fef0d1002100318v902689xc937cd332ac752b3@mail.gmail.com","subject":"Re: Suggestion on git-push --porcelain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-10T19:14:41Z","receivedAt":"2010-02-10T19:14:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan writes:\n\n> I sense a chicken-and-egg situation here.\n>\n> Printing a \"To: \" to stdout is a correction as far as the\n> current non-prefixed output scheme is concerned.\n>\n> If a prefixed scheme (or some other output scheme) is adopted, then\n> where, or whether, the \"To: \" is printed, is not relevant\n> to porcelain script writers - they just get ignored.\n\nSorry. I don't get this. Are you saying that you will write \"To\" without\nthe prefix in order to make older scripts ignore it? What then would newer\nscripts that want to use the information do? Pay attention to lines that\ndo not start with your prefix? Then what good does it do to introduce\nsuch a prefix to the output?\n\nI think what you need is not a prefix or any syntax but a rule that says\n\"ignore things that you do not understand\" and possibly a way for the\noutput stream to say \"if you do not understand this, you are too old to\ncorrectly process this stream---please abort, because ignoring this will\nmake you produce an incorrect result\".\n"}]}