Re: Very simple popen() code request, ground-shaking functionality openned by it

From: Jeff King <peff@peff.net>
Date: Sep 21, 2018, 23:39 UTC
Message-ID: <20180921233921.GA3323@sigill.intra.peff.net>
In-Reply-To: <87musajun7.fsf@evledraar.gmail.com>

On Sat, Sep 22, 2018 at 01:30:36AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

> >> This will allow users to free their creativity and provide probably
> >> dozens of custom Git progress bars.
> >
> > I don't personally feel that the existing progress bar is that bad, but
> > if anybody wants to pursue this, I think the most sensible path is:
> 
> I don't think it's bad either, but one thing that's really neat about
> Sebastian's suggestion is that it's using some UTF-8 terminal ASCII art
> to render an actual progress bar.

Yeah. I don't care myself, but I'm not opposed to somebody trying to spruce up the in-code bar, as long we can still handle the lowest common denominator cleanly (and remember that includes passing progress bars back over the remote sideband).

Show 18 quoted lines

> >   1. Add a trace_key for sending machine-readable progress output to a
> >      descriptor or file. E.g., via setting GIT_TRACE_PROGRESS=2 in the
> >      environment.
> >
> >   2. Teach the trace code to open a command for piping, so that you
> >      could do something like GIT_TRACE_PROGRESS='|mygauge'.
> >
> > That would make your use case work, and I think many other use cases
> > would benefit from both of those features independently.
> 
> Yup, that's all sensible, and would be great both for this and other
> stuff if we wanted true extensibility for this sort of thing.
> 
> I'll just add that a 3rd thing that would also make sense would be to
> add a feature to configure the value of these GIT_TRACE_*=* variables
> via the .gitconfig, that's been suggested before (too lazy to dig up a
> ML archive reference), and would make this as easy to configure as
> Sebastian's suggestion.

Heh, I almost included that in my original mail, but didn't want to get into the tangle of secondary concerns. But since you mention it... :)

One thing to watch out is that (2) and (3) combined may mean executing arbitrary code specified in the .git/config of an untrusted repository. This is already the case for many commands, but we've specifically tried to avoid it with git-upload-pack, making it safe to "git fetch" out of an untrusted repository. It's almost certain that you'd be able to trigger trace code from it.

There are a number of solutions floating around. We already have some upload-pack config which is smart enough to realize when its source is in-repo and handle it appropriately, and we've talked on the list about having some "I don't trust this repo" environment variable that would make Git operate in a more restricted way. I don't think we need to hash out the solution here, but I just want to mention that it's a thing that would have to dealt with before adding those two features.

(Actually, I guess you could argue that even reading existing trace specs out of config is potentially dangerous, since you can append to arbitrary files).

-Peff