Suggestion on git-push --porcelain

Hi,

this is regarding the recent patch series from Larry. (I thought replying to any of the patch messages was appropriate, I couldn't find a cover-letter, so I'm starting a new thread.)

Around June last year, a patch from Larry was made to add the --porcelain option, so as to produce machine-readable output regarding ref status.

The latest patch series goes a step further, and tries to change output - for example, suppressing user-friendly advice and giving "To: <destination>".

I think this is an untenable path - adding/suppressing output of certain messages for porcelain writers, while trying to keep things fixed enough for porcelain writers to depend on. We will also have to keep and eye out for future patches from adding fprintfs to stdout and stderr that may break porcelain scripts.

I believe a better approach would be to prefix messages intended for porcelain writers. For example, a push session might look like this:

  $ git push --porcelain
  PORCELAIN To git://foo.com/git/myrepo.git
  PORCELAIN uptodate refs/heads/baz:refs/heads/baz 1234ab ba4321
  PORCELAIN nonff refs/heads/bar:refs/heads/bar 2345cd 3456de

This is an "positive" approach, in the sense that we don't remove anything from the current output; we just add more printf("PORCELAIN") lines to wherever is appropriate.

Tay Ray Chuan <rctay89@gmail.com> writes:

Sorry, but I don't see what that would solve. For example, we used not to give the destination to the standard output stream, but that line carries a necessary information and Larry's series corrects that.

In your "prefix with PORCELAIN" scheme, such a change will start adding a new line "PORCELAIN To ..." that older implementations may not be prepared to see.

Other than making the output more noisy, I do not see an improvement here.

On Tue, Feb 09, 2010 at 06:57:46PM -0800, Junio C Hamano wrote:

I think he is trying to future-proof any additional output that push (or remote helpers) produce. I don't think it is really worth it, though. All of that should be going to stderr, and thus would be, at worst, noise on the terminal. I don't think it is that hard or error-prone a rule to send such cruft to stderr.

-Peff

Hi,

On Wed, Feb 10, 2010 at 12:54 PM, Jeff King <peff@peff.net> wrote:

Yes. By using this (or some other output scheme), we're deliberately marking out output for porcelain scripts.

on examining the code paths, there's a fair bit of work needed to ensure this. Apart from transport.c (transport-helper.c and by extension, remote helpers, looks ok), we would have to look at the individual builtin-transport implementations for git and bundle.

Also, as you mentioned above, we would also have to take care future transport implementations send messages in the desired fashion so that we wouldn't break porcelain scripts. If we have an output scheme, then such risks are minimized.

Hi,

On Wed, Feb 10, 2010 at 10:57 AM, Junio C Hamano <gitster@pobox.com> wrote:

I sense a chicken-and-egg situation here.

Printing a "To: <destination>" to stdout is a correction as far as the current non-prefixed output scheme is concerned.

If a prefixed scheme (or some other output scheme) is adopted, then where, or whether, the "To: <destination>" is printed, is not relevant to porcelain script writers - they just get ignored.

The original --porcelain patch only "porcelained" the output for ref updates, such as

  = TAB refs/heads/master:refs/heads/master TAB [up to date]
  - TAB :refs/heads/foobar TAB [deleted]
 ...

In addition, the "To: <destination>" patch --porcelain is fairly recent - I don't think it has left 'pu', has it? Should we worry about breaking compatibility on something not out yet?

Tay Ray Chuan <rctay89@gmail.com> writes:

Sorry. I don't get this. Are you saying that you will write "To" without the prefix in order to make older scripts ignore it? What then would newer scripts that want to use the information do? Pay attention to lines that do not start with your prefix? Then what good does it do to introduce such a prefix to the output?

I think what you need is not a prefix or any syntax but a rule that says "ignore things that you do not understand" and possibly a way for the output stream to say "if you do not understand this, you are too old to correctly process this stream---please abort, because ignoring this will make you produce an incorrect result".

* Tay Ray Chuan (rctay89@gmail.com) [100209 21:34]:

While I agree with you in principle, I'm still advocating that we make these two changes. *Especially* suppressing the advice. That advice did not exist last June, so the output format has already been changed. Also, the advice never should have gone to the standard output in the first place. All the other instances of output like that go to standard error.

As for the "To: " lines, unfortunately the lack of them was a pretty serious design flaw in the original patch :(

Actually, What I'm proposing is something very similar to this: I think that the output for the porcelain writer and *only* the output for the porcelain writer should go to standard output, and everything else should go to standard error.

       --larry