Sending informational messages from upload-pack

Sending informational messages from upload-pack

[Lukas Fleischer]

Hi,

It would be handy to be able to show a message to the user when
cloning/fetching from a repository (e.g. to show a warning if a
repository is deprecated). This should technically already be possible
using the current pack protocol and sidebands. However, to my knowledge,
there is no easy way to configure this on the server side; writing a
wrapper around git-upload-pack(1) or replacing git-upload-pack(1) seem
to be the only options.

What I have in mind is something like a post-upload hook whose stdout
and stderr are redirected to sideband 2 and 3, respectively. The commit
message of 20b20a22f (upload-pack: provide a hook for running
pack-objects, 2016-05-18) suggests that such a hook should be
implemented as a "config variable hook" rather than a regular hook.

One could think of additional parameters passed to such a hook. For the
purposes I intend to use this, no parameters are needed. However, a
fixed per-repository MOTD would be too inflexible since we are using
namespaces and database accesses to determine whether a repository is
"deprecated".

Am I missing any "easy" already supported way to add such messages
without patching Git or writing a git-upload-pack(1) wrapper? If not,
does this sound general and useful enough to become an official feature?
Are there any alternative suggestions on how to display such messages?

Regards,
Lukas

Re: Sending informational messages from upload-pack

[Jeff King]

On Mon, Feb 20, 2017 at 07:38:02PM +0100, Lukas Fleischer wrote:

> It would be handy to be able to show a message to the user when
> cloning/fetching from a repository (e.g. to show a warning if a
> repository is deprecated). This should technically already be possible
> using the current pack protocol and sidebands. However, to my knowledge,
> there is no easy way to configure this on the server side; writing a
> wrapper around git-upload-pack(1) or replacing git-upload-pack(1) seem
> to be the only options.

I wouldn't recommend wrapping upload-pack. You don't know you have a
sideband until partway through the upload-pack conversation. And clients
do not expect sideband at all until we get to the pack-sending part of
the protocol (I think; I just quickly verified the location of the
demuxer async code in fetch-pack.c, but I didn't dig into it in depth).

So I don't think you can do a MOTD or similar in a backwards-compatible
way. You're only allowed to talk if the conversation results in an
actual pack being sent.

> What I have in mind is something like a post-upload hook whose stdout
> and stderr are redirected to sideband 2 and 3, respectively. The commit
> message of 20b20a22f (upload-pack: provide a hook for running
> pack-objects, 2016-05-18) suggests that such a hook should be
> implemented as a "config variable hook" rather than a regular hook.

Yeah, because of the "upload-pack is special and untrusted" rule, this
can't be a regular hook. I think the config mechanism used by 20b20a22f
would be the right approach.

If my fetch-pack assertion above is right, technically the hook added by
20b20a22f is sufficient for your purposes, if your hook looks like:

  echo >&2 "pre-pack message"
  git pack-objects "$@"
  echo >72 "post-pack message"

but I would not be opposed to having pre-/post- hooks that run
separately, if only for the convenience of the admin.

> One could think of additional parameters passed to such a hook. For the
> purposes I intend to use this, no parameters are needed. However, a
> fixed per-repository MOTD would be too inflexible since we are using
> namespaces and database accesses to determine whether a repository is
> "deprecated".

There was a proposed post-upload-pack hook a long time ago that
collected clone/fetch stats, and we used it at GitHub for many years.
These days we use something much more invasive that dumps stats from
every git invocation over a Unix socket.

> Am I missing any "easy" already supported way to add such messages
> without patching Git or writing a git-upload-pack(1) wrapper? If not,
> does this sound general and useful enough to become an official feature?
> Are there any alternative suggestions on how to display such messages?

I don't think there's any other mechanism to do what you're asking,
aside from the hook in 20b20a22f.

-Peff

Re: Sending informational messages from upload-pack

[Lukas Fleischer]

On Mon, 20 Feb 2017 at 20:21:03, Jeff King wrote:
> On Mon, Feb 20, 2017 at 07:38:02PM +0100, Lukas Fleischer wrote:
> 
> > It would be handy to be able to show a message to the user when
> > cloning/fetching from a repository (e.g. to show a warning if a
> > repository is deprecated). This should technically already be possible
> > using the current pack protocol and sidebands. However, to my knowledge,
> > there is no easy way to configure this on the server side; writing a
> > wrapper around git-upload-pack(1) or replacing git-upload-pack(1) seem
> > to be the only options.
> 
> I wouldn't recommend wrapping upload-pack. You don't know you have a
> sideband until partway through the upload-pack conversation. And clients
> do not expect sideband at all until we get to the pack-sending part of
> the protocol (I think; I just quickly verified the location of the
> demuxer async code in fetch-pack.c, but I didn't dig into it in depth).

By wrapper I meant something that understands the pack protocol itself,
intercepts the traffic, forwards most of it to git-upload-pack(1) and
injects the message at the right time. I agree that it is a fairly ugly
workaround, though.

> [...]
> If my fetch-pack assertion above is right, technically the hook added by
> 20b20a22f is sufficient for your purposes, if your hook looks like:
> 
>   echo >&2 "pre-pack message"
>   git pack-objects "$@"
>   echo >72 "post-pack message"
> 
> but I would not be opposed to having pre-/post- hooks that run
> separately, if only for the convenience of the admin.
> [...]

I will give it a try. And I agree that it would still be convenient to
have pre-upload-pack and post-upload-pack hooks.

Regards,
Lukas