git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Enhancements to git-protocoll
@ 2012-07-28 21:41 Fredrik Gustafsson
  2012-07-29  6:58 ` Junio C Hamano
  2012-07-29 10:37 ` Sitaram Chamarty
  0 siblings, 2 replies; 25+ messages in thread
From: Fredrik Gustafsson @ 2012-07-28 21:41 UTC (permalink / raw)
  To: gitster; +Cc: git

Hi,
sometimes git communicates with something that's not git on the other
side (gitolite and github for example).

Sometimes the server wants to communicate directly to the git user.

git isn't really designed for this. gitolite solves this by do user
interaction on STDERR instead. The bad thing about this is that it can
only be one-direction communication, for example error messages.

If git would allow for the user to interact direct with the server, a
lot of cool and and userfriendly features could be developed.

For example:
gitolite has something called wild repos[1]. The management is
cumbersome and if you misspell when you clone a repo you might instead
create a new repo.

This could have been avoided with a simply:
"Do you want to create a new repo[Yn]"

To fix this, git protocol should have a command for printing input to
STDOUT and accepting input on STDIN which should be sent to the server.
And a command to switch back to orginal of course.

The server could then switch to user interaction, do that and then
switch back to normal operation.

Before eventually starting to implement this, I would know your
opinions. This feature would be wortless if it's not in the official git.

[1] http://sitaramc.github.com/gitolite/wild.html
-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-28 21:41 Enhancements to git-protocoll Fredrik Gustafsson
@ 2012-07-29  6:58 ` Junio C Hamano
  2012-07-29 14:24   ` Fredrik Gustafsson
  2012-07-29 10:37 ` Sitaram Chamarty
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2012-07-29  6:58 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: git

Fredrik Gustafsson <iveqy@iveqy.com> writes:

> Sometimes the server wants to communicate directly to the git user.
> ...
> For example:
> gitolite has something called wild repos[1]. The management is
> cumbersome and if you misspell when you clone a repo you might instead
> create a new repo.
>
> This could have been avoided with a simply:
> "Do you want to create a new repo[Yn]"

I do not think the automatic repository creation done by gitolite is
a good use case or example for whatever you seem to be advocating.

IIUC, the auto-creation in gitolite-shell::main() is done way before
gitolite-shell (which is used as a login shell for incoming ssh
sessions) creates a new git repository, goes into it and spawns the
git-receive-pack command.  It all happens outside Git.

    # auto-create?
    if ( repo_missing($repo) and access( $repo, $user, '^C', 'any' ) !~ /DENIED/ ) {
        require Gitolite::Conf::Store;
        Gitolite::Conf::Store->import;
        new_wild_repo( $repo, $user, $aa );
        gl_log( 'create', $repo, $user, $aa );
    }

The "access()" we see here is not the Perl builtin access(), but is
a function defined in src/lib/Gitolite/Conf/Load.pm; that would be
the place to allow the incoming ssh session to talk back to the end
user, but at that point there is no Git processing on the server
end.

While I am not fundamentally opposed to adding yet another sideband
channel to the git protocol, I do not think adding user interaction
at random places in the protocol exchange is a viable or useful
approach to implement an enhanced server that works with both
enhanced and vanilla clients (and the same is true for enhanced
client that works with both enhanced and vanilla server).

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-28 21:41 Enhancements to git-protocoll Fredrik Gustafsson
  2012-07-29  6:58 ` Junio C Hamano
@ 2012-07-29 10:37 ` Sitaram Chamarty
  2012-07-29 14:13   ` Fredrik Gustafsson
  1 sibling, 1 reply; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-29 10:37 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git

On Sun, Jul 29, 2012 at 3:11 AM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> Hi,
> sometimes git communicates with something that's not git on the other
> side (gitolite and github for example).
>
> Sometimes the server wants to communicate directly to the git user.
>
> git isn't really designed for this. gitolite solves this by do user
> interaction on STDERR instead. The bad thing about this is that it can
> only be one-direction communication, for example error messages.
>
> If git would allow for the user to interact direct with the server, a
> lot of cool and and userfriendly features could be developed.
>
> For example:
> gitolite has something called wild repos[1]. The management is
> cumbersome and if you misspell when you clone a repo you might instead
> create a new repo.

For the record, although it cannot do the yes/no part, if you want to
disable auto-creation on a fetch/clone (read operation) it's trivial
to add a "PRE_CREATE" trigger to do that.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 10:37 ` Sitaram Chamarty
@ 2012-07-29 14:13   ` Fredrik Gustafsson
  2012-07-29 14:25     ` Sitaram Chamarty
  0 siblings, 1 reply; 25+ messages in thread
From: Fredrik Gustafsson @ 2012-07-29 14:13 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Fredrik Gustafsson, gitster, git

On Sun, Jul 29, 2012 at 04:07:13PM +0530, Sitaram Chamarty wrote:
> On Sun, Jul 29, 2012 at 3:11 AM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> > Hi,
> > sometimes git communicates with something that's not git on the other
> > side (gitolite and github for example).
> >
> > Sometimes the server wants to communicate directly to the git user.
> >
> > git isn't really designed for this. gitolite solves this by do user
> > interaction on STDERR instead. The bad thing about this is that it can
> > only be one-direction communication, for example error messages.
> >
> > If git would allow for the user to interact direct with the server, a
> > lot of cool and and userfriendly features could be developed.
> >
> > For example:
> > gitolite has something called wild repos[1]. The management is
> > cumbersome and if you misspell when you clone a repo you might instead
> > create a new repo.
> 
> For the record, although it cannot do the yes/no part, if you want to
> disable auto-creation on a fetch/clone (read operation) it's trivial
> to add a "PRE_CREATE" trigger to do that.

Thanks, however I think auto-creation is a great feature for some cases
and I think there can be even more useable functions if we could get
user interaction.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29  6:58 ` Junio C Hamano
@ 2012-07-29 14:24   ` Fredrik Gustafsson
  2012-07-29 14:39     ` Sitaram Chamarty
  0 siblings, 1 reply; 25+ messages in thread
From: Fredrik Gustafsson @ 2012-07-29 14:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Gustafsson, git

On Sat, Jul 28, 2012 at 11:58:09PM -0700, Junio C Hamano wrote:
> Fredrik Gustafsson <iveqy@iveqy.com> writes:
> 
> > Sometimes the server wants to communicate directly to the git user.
> > ...
> > For example:
> > gitolite has something called wild repos[1]. The management is
> > cumbersome and if you misspell when you clone a repo you might instead
> > create a new repo.
> >
> > This could have been avoided with a simply:
> > "Do you want to create a new repo[Yn]"
> 
> I do not think the automatic repository creation done by gitolite is
> a good use case or example for whatever you seem to be advocating.
> 
> IIUC, the auto-creation in gitolite-shell::main() is done way before
> gitolite-shell (which is used as a login shell for incoming ssh
> sessions) creates a new git repository, goes into it and spawns the
> git-receive-pack command.  It all happens outside Git.
> 
>     # auto-create?
>     if ( repo_missing($repo) and access( $repo, $user, '^C', 'any' ) !~ /DENIED/ ) {
>         require Gitolite::Conf::Store;
>         Gitolite::Conf::Store->import;
>         new_wild_repo( $repo, $user, $aa );
>         gl_log( 'create', $repo, $user, $aa );
>     }
> 
> The "access()" we see here is not the Perl builtin access(), but is
> a function defined in src/lib/Gitolite/Conf/Load.pm; that would be
> the place to allow the incoming ssh session to talk back to the end
> user, but at that point there is no Git processing on the server
> end.

That's a feature. It means that the impact on git would be rather small,
we don't have to involve server-side git at all. The problem so solve is
how to get client-side git to pass through STDIN and STDOUT (just as is
done with STDERR right now). I see this as a gitolite <-> client-git
interaction case. No server-git should be involved.

All the use casese I can imagine will be done before (or after)
serverside git is executed.

> While I am not fundamentally opposed to adding yet another sideband
> channel to the git protocol, I do not think adding user interaction
> at random places in the protocol exchange is a viable or useful
> approach to implement an enhanced server that works with both
> enhanced and vanilla clients (and the same is true for enhanced
> client that works with both enhanced and vanilla server).

Do we mean the same thing with "git protocol"? I specify the protocol as
everything that happens between the server and the client. Are the
connection divided into multiple protocoll after eachother? (would it be
possible to execute git-user-interaction-protocoll first and the
git-protocoll and then git-user-interaction-protocoll again?).

The vanilla case would be easy to solve if the protocol has git version
in its handshake. The STDERR approach is already used and working. A
vanilla client would have the same functionality as today and en
enhanced client will have enhanced functionality.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 14:13   ` Fredrik Gustafsson
@ 2012-07-29 14:25     ` Sitaram Chamarty
  2012-07-29 15:05       ` Fredrik Gustafsson
  0 siblings, 1 reply; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-29 14:25 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git

On Sun, Jul 29, 2012 at 7:43 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Sun, Jul 29, 2012 at 04:07:13PM +0530, Sitaram Chamarty wrote:
>> On Sun, Jul 29, 2012 at 3:11 AM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
>> > Hi,
>> > sometimes git communicates with something that's not git on the other
>> > side (gitolite and github for example).
>> >
>> > Sometimes the server wants to communicate directly to the git user.
>> >
>> > git isn't really designed for this. gitolite solves this by do user
>> > interaction on STDERR instead. The bad thing about this is that it can
>> > only be one-direction communication, for example error messages.
>> >
>> > If git would allow for the user to interact direct with the server, a
>> > lot of cool and and userfriendly features could be developed.
>> >
>> > For example:
>> > gitolite has something called wild repos[1]. The management is
>> > cumbersome and if you misspell when you clone a repo you might instead
>> > create a new repo.
>>
>> For the record, although it cannot do the yes/no part, if you want to
>> disable auto-creation on a fetch/clone (read operation) it's trivial
>> to add a "PRE_CREATE" trigger to do that.
>
> Thanks, however I think auto-creation is a great feature for some cases
> and I think there can be even more useable functions if we could get
> user interaction.

For the record, I don't think I agree.  There's a place to create a
human-conversation, and there's a place not to.

If you want a dialog with the server, there should be *other* commands
that do that, instead of overloading git's own protocol.

Since you mentioned gitolite, consider copying the fork command
(src/commands/fork) and munging the code into an explicit wild repo
create.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 14:24   ` Fredrik Gustafsson
@ 2012-07-29 14:39     ` Sitaram Chamarty
  2012-07-29 20:51       ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-29 14:39 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: Junio C Hamano, git

On Sun, Jul 29, 2012 at 7:54 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Sat, Jul 28, 2012 at 11:58:09PM -0700, Junio C Hamano wrote:
>> Fredrik Gustafsson <iveqy@iveqy.com> writes:
>>
>> > Sometimes the server wants to communicate directly to the git user.
>> > ...
>> > For example:
>> > gitolite has something called wild repos[1]. The management is
>> > cumbersome and if you misspell when you clone a repo you might instead
>> > create a new repo.
>> >
>> > This could have been avoided with a simply:
>> > "Do you want to create a new repo[Yn]"
>>
>> I do not think the automatic repository creation done by gitolite is
>> a good use case or example for whatever you seem to be advocating.
>>
>> IIUC, the auto-creation in gitolite-shell::main() is done way before
>> gitolite-shell (which is used as a login shell for incoming ssh
>> sessions) creates a new git repository, goes into it and spawns the
>> git-receive-pack command.  It all happens outside Git.
>>
>>     # auto-create?
>>     if ( repo_missing($repo) and access( $repo, $user, '^C', 'any' ) !~ /DENIED/ ) {
>>         require Gitolite::Conf::Store;
>>         Gitolite::Conf::Store->import;
>>         new_wild_repo( $repo, $user, $aa );
>>         gl_log( 'create', $repo, $user, $aa );
>>     }
>>
>> The "access()" we see here is not the Perl builtin access(), but is
>> a function defined in src/lib/Gitolite/Conf/Load.pm; that would be
>> the place to allow the incoming ssh session to talk back to the end
>> user, but at that point there is no Git processing on the server
>> end.
>
> That's a feature. It means that the impact on git would be rather small,
> we don't have to involve server-side git at all. The problem so solve is
> how to get client-side git to pass through STDIN and STDOUT (just as is
> done with STDERR right now). I see this as a gitolite <-> client-git
> interaction case. No server-git should be involved.

Uggh, no.  Client-git should only talk to server-git.  It shouldn't be
talking first to some *other* program (in this case gitolite), and
then to to server-git.  That doesn't sound sane to me.

You should wrap this whole thing around something else that does it in
3 steps.  Check, create if needed, then the actual git command you
intend to run.  All this should be local to your environment, not
rolled into git; it's far too specific to be rolled into git itself,
if you ask me.

> All the use casese I can imagine will be done before (or after)
> serverside git is executed.
>
>> While I am not fundamentally opposed to adding yet another sideband
>> channel to the git protocol, I do not think adding user interaction
>> at random places in the protocol exchange is a viable or useful
>> approach to implement an enhanced server that works with both
>> enhanced and vanilla clients (and the same is true for enhanced
>> client that works with both enhanced and vanilla server).
>
> Do we mean the same thing with "git protocol"? I specify the protocol as
> everything that happens between the server and the client. Are the
> connection divided into multiple protocoll after eachother? (would it be
> possible to execute git-user-interaction-protocoll first and the
> git-protocoll and then git-user-interaction-protocoll again?).
>
> The vanilla case would be easy to solve if the protocol has git version
> in its handshake. The STDERR approach is already used and working. A
> vanilla client would have the same functionality as today and en
> enhanced client will have enhanced functionality.
>
> --
> Med vänliga hälsningar
> Fredrik Gustafsson
>
> tel: 0733-608274
> e-post: iveqy@iveqy.com
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Sitaram

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 14:25     ` Sitaram Chamarty
@ 2012-07-29 15:05       ` Fredrik Gustafsson
  2012-07-29 15:15         ` Sitaram Chamarty
  0 siblings, 1 reply; 25+ messages in thread
From: Fredrik Gustafsson @ 2012-07-29 15:05 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Fredrik Gustafsson, gitster, git

On Sun, Jul 29, 2012 at 07:55:36PM +0530, Sitaram Chamarty wrote:
> > Thanks, however I think auto-creation is a great feature for some cases
> > and I think there can be even more useable functions if we could get
> > user interaction.
> 
> For the record, I don't think I agree.  There's a place to create a
> human-conversation, and there's a place not to.
> 
> If you want a dialog with the server, there should be *other* commands
> that do that, instead of overloading git's own protocol.
> 
> Since you mentioned gitolite, consider copying the fork command
> (src/commands/fork) and munging the code into an explicit wild repo
> create.

I appriciate that you clearified you oppinion. Please excuse me if it
sounded as I in any way speaked for gitolite. I use gitolite as an
example becuase the target application in this case is unknown to most
people (think gitolite with db-backend for user permissions).

It's a valid design oppinion to not mix git protocoll with anything
else. But gitolite already does that. Gitolite already have user
interaction mixed with git interaction. Do you say to me that gitolite
is broken and should not do user interaction over git-commands? Then why
does wild repos exists and why does gitolite error messages exists?

We're already down that road, why not do it better?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 15:05       ` Fredrik Gustafsson
@ 2012-07-29 15:15         ` Sitaram Chamarty
  2012-07-29 15:41           ` Fredrik Gustafsson
  0 siblings, 1 reply; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-29 15:15 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git

On Sun, Jul 29, 2012 at 8:35 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Sun, Jul 29, 2012 at 07:55:36PM +0530, Sitaram Chamarty wrote:
>> > Thanks, however I think auto-creation is a great feature for some cases
>> > and I think there can be even more useable functions if we could get
>> > user interaction.
>>
>> For the record, I don't think I agree.  There's a place to create a
>> human-conversation, and there's a place not to.
>>
>> If you want a dialog with the server, there should be *other* commands
>> that do that, instead of overloading git's own protocol.
>>
>> Since you mentioned gitolite, consider copying the fork command
>> (src/commands/fork) and munging the code into an explicit wild repo
>> create.
>
> I appriciate that you clearified you oppinion. Please excuse me if it
> sounded as I in any way speaked for gitolite. I use gitolite as an
> example becuase the target application in this case is unknown to most
> people (think gitolite with db-backend for user permissions).
>
> It's a valid design oppinion to not mix git protocoll with anything
> else. But gitolite already does that. Gitolite already have user
> interaction mixed with git interaction. Do you say to me that gitolite
> is broken and should not do user interaction over git-commands? Then why
> does wild repos exists and why does gitolite error messages exists?
>
> We're already down that road, why not do it better?

I think you misunderstood how gitolite works.  Gitolite does not have
*any* user interaction other than sending some extra messages back via
STDERR if you're using a normal git client to do normal git operations
(clone/fetch/ls-remote).

Such messages are *no different* from something that an update or
pre-receive hook might send back even on a normal (no gitolite) git
server.

The only time that gitolite might have any user *interaction* is when
using "gitolite commands".  These do not run git at all (neither on
the client nor on the server), and in fact merely provide a convenient
way to allow users to run a controlled set of specific *shell*
commands.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 15:15         ` Sitaram Chamarty
@ 2012-07-29 15:41           ` Fredrik Gustafsson
  2012-07-29 18:22             ` Sitaram Chamarty
  0 siblings, 1 reply; 25+ messages in thread
From: Fredrik Gustafsson @ 2012-07-29 15:41 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Fredrik Gustafsson, gitster, git

On Sun, Jul 29, 2012 at 08:45:39PM +0530, Sitaram Chamarty wrote:
> I think you misunderstood how gitolite works.  Gitolite does not have
> *any* user interaction other than sending some extra messages back via
> STDERR if you're using a normal git client to do normal git operations
> (clone/fetch/ls-remote).

As you say, gitolite has userinteraction, and its not even standard to
git error messages. Try cloning a repo that doesn't exists via gitolite
and a regular ssh connection:

[iveqy@paksenarrion git]$ git clone ssh://gitolite@localhost/testing2
Cloning into testing2...
FATAL: R any testing2 id_rsa DENIED by fallthru
(or you mis-spelled the reponame)
fatal: The remote end hung up unexpectedly
[iveqy@paksenarrion git]$ git clone ssh://iveqy@localhost/testing2
Cloning into testing...
fatal: '/testing2' does not appear to be a git repository
fatal: The remote end hung up unexpectedly

> Such messages are *no different* from something that an update or
> pre-receive hook might send back even on a normal (no gitolite) git
> server.

As I showed above the "non-existing" repo is a case when it's different.
But you have a good point in hooks, of course a hook also should be able
to have two way user interaction.

> The only time that gitolite might have any user *interaction* is when
> using "gitolite commands".  These do not run git at all (neither on
> the client nor on the server), and in fact merely provide a convenient
> way to allow users to run a controlled set of specific *shell*
> commands.

I do understand how gitolite works. However this is off-topic to my
original question. I also do not have an oppinion on how gitolite
should work, I simply don't care. Gitolite is an widely acceptet git
tool, I see improvement opportunities in git to allow an other
program to utilize two-way user interaction all the time, this will not
effect gitolite at all.

So in my point of view, it's up to Junio if I shall continue explore this
path and maybe find a way of doing this in git, "the right way". Or if
this is something unwanted and gitolite and alike programs should
continue with STDERR "hacks".

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 15:41           ` Fredrik Gustafsson
@ 2012-07-29 18:22             ` Sitaram Chamarty
  0 siblings, 0 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-29 18:22 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: gitster, git

On Sun, Jul 29, 2012 at 9:11 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Sun, Jul 29, 2012 at 08:45:39PM +0530, Sitaram Chamarty wrote:
>> I think you misunderstood how gitolite works.  Gitolite does not have
>> *any* user interaction other than sending some extra messages back via
>> STDERR if you're using a normal git client to do normal git operations
>> (clone/fetch/ls-remote).
>
> As you say, gitolite has userinteraction, and its not even standard to

I think we differ on the meaning of the word "interaction".

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 14:39     ` Sitaram Chamarty
@ 2012-07-29 20:51       ` Junio C Hamano
  2012-07-29 21:26         ` Fredrik Gustafsson
  2012-07-29 21:38         ` Junio C Hamano
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2012-07-29 20:51 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Fredrik Gustafsson, git

Sitaram Chamarty <sitaramc@gmail.com> writes:

> Uggh, no.  Client-git should only talk to server-git.  It shouldn't be
> talking first to some *other* program (in this case gitolite), and
> then to to server-git.  That doesn't sound sane to me.
>
> You should wrap this whole thing around something else that does it in
> 3 steps.  Check, create if needed, then the actual git command you
> intend to run.  All this should be local to your environment, not
> rolled into git; it's far too specific to be rolled into git itself,
> if you ask me.

Thanks for saving me from having to state an obvious sanity ;-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 20:51       ` Junio C Hamano
@ 2012-07-29 21:26         ` Fredrik Gustafsson
  2012-07-30  0:37           ` Sitaram Chamarty
  2012-07-29 21:38         ` Junio C Hamano
  1 sibling, 1 reply; 25+ messages in thread
From: Fredrik Gustafsson @ 2012-07-29 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sitaram Chamarty, Fredrik Gustafsson, git

Sorry I missed this thread earlier. I'll drop this if it's not something
that's wanted.

On Sun, Jul 29, 2012 at 01:51:34PM -0700, Junio C Hamano wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
> 
> > Uggh, no.  Client-git should only talk to server-git.  It shouldn't be
> > talking first to some *other* program (in this case gitolite), and
> > then to to server-git.  That doesn't sound sane to me.

This is exactly the way gitolite works. It's placed between git-server
and git-client. Does some checks and approves a connection if some
criterias isn't met. See the example when trying to clone an
non-existing repo from gitolite. You won't get an git error but a
gitolite error.

I can understand why my idea is beeing rejected but I can't see why the
gitolite way should be considered sane. It seems more like an hack to
me (according to git design principles).

So from a git point of view, why is it sane for passing through STDERR
but not STDIN and STDOUT?

(I realize that this is a closed matter but would appriciate an
explanation solely for my own educational purpose).

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 20:51       ` Junio C Hamano
  2012-07-29 21:26         ` Fredrik Gustafsson
@ 2012-07-29 21:38         ` Junio C Hamano
  2012-07-30  1:04           ` Sitaram Chamarty
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2012-07-29 21:38 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Fredrik Gustafsson, git

Junio C Hamano <gitster@pobox.com> writes:

> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>> Uggh, no.  Client-git should only talk to server-git.  It shouldn't be
>> talking first to some *other* program (in this case gitolite), and
>> then to to server-git.  That doesn't sound sane to me.
>>
>> You should wrap this whole thing around something else that does it in
>> 3 steps.  Check, create if needed, then the actual git command you
>> intend to run.  All this should be local to your environment, not
>> rolled into git; it's far too specific to be rolled into git itself,
>> if you ask me.
>
> Thanks for saving me from having to state an obvious sanity ;-)

Having said all that, I am not fundamentally opposed to new features
added to the git protocol.  The responses so far from me in this
thread were primarily me reacting to (1) auto creation by gitolite
is a bad example and (2) the proposal sounded, at least to me, that
it wants to add random and uncontrolled interactions between the
sides outside the defined protocol exchange (e.g. "pusher connects,
pushee says what it has and what capabilities it supports, pusher
says what it wants to update how and chooses what capabilities it
wants to use in the exchange, sends the pack data, ..."), which is
an unworkable idea.

For people who do not understand how the git protocol works (the
proposal that started this thread included), a bit of clarification
on (2) may help.

Long time ago, we did not have "report-status" capability (think of
a "capability" as a "protocol extension").  The "git push" protocol
exchange ended with the pusher sending the pack data and that was
the end of conversation.

But we realized that it would be beneficial for the pushee to be
able to tell the pusher that the push failed at the protocol level,
so that the "git push" program can exit with an error code.  The
version of the "git push" program that was current at the time
exited with success unconditionally, as long as all protocol
exchange went well and all the pack data was sent successfully.  The
program's flow was not expecting any response once the pack data
started flowing from it to the pushee, so that is the only
reasonable behaviour.

So what did we do?  We added "report-status" capability to the
capability suite, and made new versions of "receive-pack" program
advertise it.  Existing versions of "git push" did not know about
this capability, so they did not ask for it and pusher and pushee
worked exactly as before.

But that way, the new versions of "git push" can learn to ask for
"report-status" capability to updated "receive-pack", and the new
versions of "receive-pack", after receiving and processing the pack
data stream, can send new messages the original protocol did not
allow it to send to "git push".

An important point to understand is that there is one more thing
that is needed.  The updated "git push" that asks for "report-status"
needed to learn how to interpret the new message and act on it.

Another example in the same protocol was the addition of "sideband"
capability.  Before that happened, there was no way to send the
error stream from the pushee to the pusher.  Unlike "report-status"
that happens at the very end, this actually changes the way how the
remainder of the protocol exchange proceeds once it is activated.

You could add a new capability that says "when in effect, both ends
can write to and read from new sidebands 4 and 5 to communicate out
of line", but that is not all that useful.  You need to make the
updated programs to agree on what should happen to the main
interaction when a new kind of communication is made out of band.
For example, you may ask "do you really mean it [Y/n]?" to the
client, pause the entire transaction until you hear Yes or No from
the client, and you may even choose to do something different from
the usual when the client says No, but you also need to update the
client to behave differently after that, perhaps by defining a new
conversation path in the protocol.  At that point, you would need to
handle real and concrete definition of the extended protocol and the
code on the both sides to support it _anyway_.

For example, imagine that you want to let a new user interaction to
verify the repository it is pushing to.  How would we do that with
your "adding random interaction between the server and client"?

The server end notices that the user is trying to push into one
repository.  throws "Do you really mean to push into repository
foo?"  Updated client that understands your "random interaction"
extention is capable of show this message, pauses until the user
says "Yes" or "No", and sends it back to the server end.  If the
answer is "No", the client may want to say "No, the repository I
really wanted to push into was not foo, but bar".

How did the client _know_ the "Do you really mean" message from the
server requires a Yes or No response in the above scenario?

How did the client know, when saying "No", to ask the real URL of
the repository to the end user, disconnect the current session with
the server, and restart itself using the real URL of the repository?

All of that need clearly defined new protocol exchange steps, state
transitions, and new conditional and new codepaths to implement
them.  It cannot be just "let's allow the server and the client to
randomly exchange packets out of band".

In short, I am not fundamentally opposed to protocol extension, as I
said in my very first response.

I however do not think allowing the server 'to communicate directly
to the git user' (which was the phrase used in the original message
that expressed the idea) is a good idea, at least for two reasons:

 - because we already have a proven-to-work-well way to extend the
   protocol via "capability"; and

 - because doing so is not useful for the purpose of extending the
   feature of the protocol,

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 21:26         ` Fredrik Gustafsson
@ 2012-07-30  0:37           ` Sitaram Chamarty
  0 siblings, 0 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-30  0:37 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: Junio C Hamano, git

On Mon, Jul 30, 2012 at 2:56 AM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> Sorry I missed this thread earlier. I'll drop this if it's not something
> that's wanted.
>
> On Sun, Jul 29, 2012 at 01:51:34PM -0700, Junio C Hamano wrote:
>> Sitaram Chamarty <sitaramc@gmail.com> writes:
>>
>> > Uggh, no.  Client-git should only talk to server-git.  It shouldn't be
>> > talking first to some *other* program (in this case gitolite), and
>> > then to to server-git.  That doesn't sound sane to me.
>
> This is exactly the way gitolite works. It's placed between git-server
> and git-client. Does some checks and approves a connection if some
> criterias isn't met. See the example when trying to clone an
> non-existing repo from gitolite. You won't get an git error but a
> gitolite error.
>
> I can understand why my idea is beeing rejected but I can't see why the
> gitolite way should be considered sane. It seems more like an hack to
> me (according to git design principles).
>
> So from a git point of view, why is it sane for passing through STDERR
> but not STDIN and STDOUT?

That is precisely the point.  The pack protocol (see
Documentation/pack-protocol.txt in the git sources) works with
STDIN/STDOUT.  Run with GIT_TRACE_PACKET=1 to see some of that info
fly past on various git commands.

It explicitly leaves STDERR for the purpose of providing user's extra
information, including errors.

Gitolite is merely using that same mechanism.

And I repeat, if you insist on calling what gitolite does an
"interaction" (which to me means two-way communication, not a one-way
error/warning/diagnostic stream), we do not have enough in common to
discuss this any more.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-29 21:38         ` Junio C Hamano
@ 2012-07-30  1:04           ` Sitaram Chamarty
  2012-07-30  1:21             ` Shawn Pearce
  2012-07-30  1:28             ` Junio C Hamano
  0 siblings, 2 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-30  1:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Gustafsson, git

On Mon, Jul 30, 2012 at 3:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sitaram Chamarty <sitaramc@gmail.com> writes:
>>
>>> Uggh, no.  Client-git should only talk to server-git.  It shouldn't be
>>> talking first to some *other* program (in this case gitolite), and
>>> then to to server-git.  That doesn't sound sane to me.
>>>
>>> You should wrap this whole thing around something else that does it in
>>> 3 steps.  Check, create if needed, then the actual git command you
>>> intend to run.  All this should be local to your environment, not
>>> rolled into git; it's far too specific to be rolled into git itself,
>>> if you ask me.
>>
>> Thanks for saving me from having to state an obvious sanity ;-)
>
> Having said all that, I am not fundamentally opposed to new features
> added to the git protocol.  The responses so far from me in this
> thread were primarily me reacting to (1) auto creation by gitolite
> is a bad example and (2) the proposal sounded, at least to me, that
> it wants to add random and uncontrolled interactions between the
> sides outside the defined protocol exchange (e.g. "pusher connects,
> pushee says what it has and what capabilities it supports, pusher
> says what it wants to update how and chooses what capabilities it
> wants to use in the exchange, sends the pack data, ..."), which is
> an unworkable idea.
>
> For people who do not understand how the git protocol works (the
> proposal that started this thread included), a bit of clarification
> on (2) may help.
>
> Long time ago, we did not have "report-status" capability (think of
> a "capability" as a "protocol extension").  The "git push" protocol
> exchange ended with the pusher sending the pack data and that was
> the end of conversation.
>
> But we realized that it would be beneficial for the pushee to be
> able to tell the pusher that the push failed at the protocol level,
> so that the "git push" program can exit with an error code.  The
> version of the "git push" program that was current at the time
> exited with success unconditionally, as long as all protocol
> exchange went well and all the pack data was sent successfully.  The
> program's flow was not expecting any response once the pack data
> started flowing from it to the pushee, so that is the only
> reasonable behaviour.
>
> So what did we do?  We added "report-status" capability to the
> capability suite, and made new versions of "receive-pack" program
> advertise it.  Existing versions of "git push" did not know about
> this capability, so they did not ask for it and pusher and pushee
> worked exactly as before.
>
> But that way, the new versions of "git push" can learn to ask for
> "report-status" capability to updated "receive-pack", and the new
> versions of "receive-pack", after receiving and processing the pack
> data stream, can send new messages the original protocol did not
> allow it to send to "git push".
>
> An important point to understand is that there is one more thing
> that is needed.  The updated "git push" that asks for "report-status"
> needed to learn how to interpret the new message and act on it.
>
> Another example in the same protocol was the addition of "sideband"
> capability.  Before that happened, there was no way to send the
> error stream from the pushee to the pusher.  Unlike "report-status"
> that happens at the very end, this actually changes the way how the
> remainder of the protocol exchange proceeds once it is activated.
>
> You could add a new capability that says "when in effect, both ends
> can write to and read from new sidebands 4 and 5 to communicate out
> of line", but that is not all that useful.  You need to make the
> updated programs to agree on what should happen to the main
> interaction when a new kind of communication is made out of band.
> For example, you may ask "do you really mean it [Y/n]?" to the
> client, pause the entire transaction until you hear Yes or No from
> the client, and you may even choose to do something different from
> the usual when the client says No, but you also need to update the
> client to behave differently after that, perhaps by defining a new
> conversation path in the protocol.  At that point, you would need to
> handle real and concrete definition of the extended protocol and the
> code on the both sides to support it _anyway_.
>
> For example, imagine that you want to let a new user interaction to
> verify the repository it is pushing to.  How would we do that with
> your "adding random interaction between the server and client"?
>
> The server end notices that the user is trying to push into one
> repository.  throws "Do you really mean to push into repository
> foo?"  Updated client that understands your "random interaction"
> extention is capable of show this message, pauses until the user
> says "Yes" or "No", and sends it back to the server end.  If the
> answer is "No", the client may want to say "No, the repository I
> really wanted to push into was not foo, but bar".
>
> How did the client _know_ the "Do you really mean" message from the
> server requires a Yes or No response in the above scenario?
>
> How did the client know, when saying "No", to ask the real URL of
> the repository to the end user, disconnect the current session with
> the server, and restart itself using the real URL of the repository?
>
> All of that need clearly defined new protocol exchange steps, state
> transitions, and new conditional and new codepaths to implement
> them.  It cannot be just "let's allow the server and the client to
> randomly exchange packets out of band".
>
> In short, I am not fundamentally opposed to protocol extension, as I
> said in my very first response.
>
> I however do not think allowing the server 'to communicate directly
> to the git user' (which was the phrase used in the original message
> that expressed the idea) is a good idea, at least for two reasons:
>
>  - because we already have a proven-to-work-well way to extend the
>    protocol via "capability"; and
>
>  - because doing so is not useful for the purpose of extending the
>    feature of the protocol,
>

As I may have said earlier, this interaction is far too site-specific
to be rolled into git itself.

How about a new hook instead?  A pre-pack-protocol hook that acts as
if it was called by the remote user as a command, and if it exit's
with 0, then the real pack protocol starts else it gets aborted.  Let
him do whatever he wants in there.  Arguments to the hook will be repo
name and command (git-upload-pack mainly).

>From the ssh stream point of view, he'd have full STDIN/OUT/ERR
available in this hook, but when the hook exits, the normal protocol
takes over (or the whole thing aborts).

Of course this will only work with ssh.  None of what Fredrik has so
far suggested would possibly work on smart http without even more
hacks, I think.

And even then, all we are doing is rolling into git something that he
can very easily do outside right now on his own environment if he
wishes to, using a shell script on each side.  (And therein lies a
more general rant for the future, but not now...!)

-- 
Sitaram

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  1:04           ` Sitaram Chamarty
@ 2012-07-30  1:21             ` Shawn Pearce
  2012-07-30  1:33               ` Sitaram Chamarty
  2012-07-30  2:38               ` Junio C Hamano
  2012-07-30  1:28             ` Junio C Hamano
  1 sibling, 2 replies; 25+ messages in thread
From: Shawn Pearce @ 2012-07-30  1:21 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Junio C Hamano, Fredrik Gustafsson, git

On Sun, Jul 29, 2012 at 6:04 PM, Sitaram Chamarty <sitaramc@gmail.com> wrote:
> Of course this will only work with ssh.  None of what Fredrik has so
> far suggested would possibly work on smart http without even more
> hacks, I think.

Now that we have smart HTTP, and its somewhat popular for sites to
deploy with, we need to carefully consider all future protocol changes
to make sure they are compatible with the HTTP one. Since the HTTP is
single request/response model, its hard to implement a conversation
with the end-user.

One thing I would like to do with the protocol is add custom site
specific extensions to the protocol, where hooks are able to advertise
in the initial capability list something like a namespace prefix that
`git push` can use to offer site specific command line flags from:

  ....00000000... HEAD^{} report-status delete-refs side-band-64k
ofs-delta hook=gitolite

and the client seeing this would recognize a push command like:

  git push -Lgitolite,create-repository URL master

passing the string "gitolite,create-repository" as data in the header
of the push request. gitolite would need to scan more than just the
git receive-pack command line from SSH to see this data, but you can
use it to implement an "are you sure you want to create this
repository" exchange by failing a push with sideband information
telling the user to reinvoke push with the create-repository flag if
they really mean to create it.

We sort of want this in Gerrit Code Review to pass reviewer names on
the command line of git push, making it easier for users to upload a
code review. The idea is similar to what happens with gcc accepting
linker flags that are just passed onto the linker. From what I
understand, Mercurial already has something like this in their push
system for hooks to accept additional data one-way from the client.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  1:04           ` Sitaram Chamarty
  2012-07-30  1:21             ` Shawn Pearce
@ 2012-07-30  1:28             ` Junio C Hamano
  2012-07-30  1:45               ` Sitaram Chamarty
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2012-07-30  1:28 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Fredrik Gustafsson, git

Sitaram Chamarty <sitaramc@gmail.com> writes:

> As I may have said earlier, this interaction is far too site-specific
> to be rolled into git itself.
>
> How about a new hook instead?  A pre-pack-protocol hook that acts as
> if it was called by the remote user as a command, and if it exit's
> with 0, then the real pack protocol starts else it gets aborted.  Let
> him do whatever he wants in there.  Arguments to the hook will be repo
> name and command (git-upload-pack mainly).

Not very interested.  If it is _known_ to happen before Git protocol
proper happens, why not give the user something like the gitolite
command "D", that is interpreted by gitolite-shell without bothering
Git at all?

After all, at least you and I share the understanding that once the
conversation in Git protocol proper starts, there is no place for
such a random hook to affect further behaviour of the protocol, so
the approach "hook" can only solve narrow "before gitolite-shell (or
git-shell or whatever-shell) decides it is time to call Git" cases,
and the only way it can affect the outcome of the main conversation
is to abort it without graceful degradation, or let the main
conversation continue as if nothing happened.  I think we agree
between us that "a new hook", while it may be _a_ way to do
something new, is _not_ a good way to do so.  Why add such a wart?

> And even then, all we are doing is rolling into git something that he
> can very easily do outside right now on his own environment ...

Yes, that is a repeat of what you told him already, to which I said
"thanks for sanity", I think ;-).

I am not opposed to protocol enhancements, but a new feature that
has to change what either end does should be added properly at the
protocol level as a protocol capability, not added out of band with
band-aid, leaving the main conversation oblivious to what is going
on ouside.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  1:21             ` Shawn Pearce
@ 2012-07-30  1:33               ` Sitaram Chamarty
  2012-07-30  2:38               ` Junio C Hamano
  1 sibling, 0 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-30  1:33 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Junio C Hamano, Fredrik Gustafsson, git

On Mon, Jul 30, 2012 at 6:51 AM, Shawn Pearce <spearce@spearce.org> wrote:
> On Sun, Jul 29, 2012 at 6:04 PM, Sitaram Chamarty <sitaramc@gmail.com> wrote:
>> Of course this will only work with ssh.  None of what Fredrik has so
>> far suggested would possibly work on smart http without even more
>> hacks, I think.
>
> Now that we have smart HTTP, and its somewhat popular for sites to
> deploy with, we need to carefully consider all future protocol changes
> to make sure they are compatible with the HTTP one. Since the HTTP is
> single request/response model, its hard to implement a conversation
> with the end-user.
>
> One thing I would like to do with the protocol is add custom site
> specific extensions to the protocol, where hooks are able to advertise
> in the initial capability list something like a namespace prefix that
> `git push` can use to offer site specific command line flags from:
>
>   ....00000000... HEAD^{} report-status delete-refs side-band-64k
> ofs-delta hook=gitolite
>
> and the client seeing this would recognize a push command like:
>
>   git push -Lgitolite,create-repository URL master
>
> passing the string "gitolite,create-repository" as data in the header
> of the push request. gitolite would need to scan more than just the
> git receive-pack command line from SSH to see this data, but you can

I would avoid anything that requires gitolite to even *know* the pack
protocol.  They should be orthogonal.

But if you mean this extra info will be added to the command line
itself in some way (or in http terms to the REQUEST_URI, PATH_INFO,
QUERY_STRING, etc) then that's fine.

People who use gitolite already should try "git ls-remote
git@host:testing.git1" instead of the usual "...testing" or
"...testing.git" to see how a trace capability is currently hacked
into gitolite.  Also change the 1 to a 2 then a 3 if you wish.

I'd certainly love a cleaner way of doing this, and what you suggest
seems it will satisfy.

> use it to implement an "are you sure you want to create this
> repository" exchange by failing a push with sideband information
> telling the user to reinvoke push with the create-repository flag if
> they really mean to create it.
>
> We sort of want this in Gerrit Code Review to pass reviewer names on
> the command line of git push, making it easier for users to upload a
> code review. The idea is similar to what happens with gcc accepting
> linker flags that are just passed onto the linker. From what I
> understand, Mercurial already has something like this in their push
> system for hooks to accept additional data one-way from the client.



-- 
Sitaram

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  1:28             ` Junio C Hamano
@ 2012-07-30  1:45               ` Sitaram Chamarty
  0 siblings, 0 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-30  1:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Gustafsson, git

On Mon, Jul 30, 2012 at 6:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> writes:
>
>> As I may have said earlier, this interaction is far too site-specific
>> to be rolled into git itself.
>>
>> How about a new hook instead?  A pre-pack-protocol hook that acts as
>> if it was called by the remote user as a command, and if it exit's
>> with 0, then the real pack protocol starts else it gets aborted.  Let
>> him do whatever he wants in there.  Arguments to the hook will be repo
>> name and command (git-upload-pack mainly).
>
> Not very interested.  If it is _known_ to happen before Git protocol
> proper happens, why not give the user something like the gitolite
> command "D", that is interpreted by gitolite-shell without bothering
> Git at all?

Indeed.  In one of the earlier messages I already told him he should
copy and munge the fork command in gitolite to do precisely this.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  1:21             ` Shawn Pearce
  2012-07-30  1:33               ` Sitaram Chamarty
@ 2012-07-30  2:38               ` Junio C Hamano
  2012-07-30  5:20                 ` Shawn Pearce
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2012-07-30  2:38 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Sitaram Chamarty, Fredrik Gustafsson, git

Shawn Pearce <spearce@spearce.org> writes:

> We sort of want this in Gerrit Code Review to pass reviewer names on
> the command line of git push, making it easier for users to upload a
> code review. The idea is similar to what happens with gcc accepting
> linker flags that are just passed onto the linker.

For reviewer names, authentication cookies and things of that nature
where the extra pieces of information affect the outcome in a way
that does not have to change how the underlying protocol exchange
works, such an additional one-way channel from the pusher to pushee
to carry auxiliary information would be sufficient.  The server may
decide to accept otherwise forbidden, or reject otherwise permitted,
push based on the extra information given, for example, and that is
an example of an enhancement that does not have to change how the
underlying protocol exchange works.

The way to expose the extra information parsed by Git to the server
side could be made into calling out to hooks, and at that point,
gitolite would not even have to know about the pack protocol.
Perhaps the interface to such a hook may be "hook can tell Git to
abort the communication by exiting non-zero, after giving a message
to its standard output".

It is a separate matter if it makes sense to add another channel
that goes the other way on demand (i.e.  taking the end-user
response from the pusher and giving it to the pushee, and then
allowing the pushee change its behaviour in a way more than just
simply aborting the connection but performing a useful alternative
operation)---I doubt it is.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  2:38               ` Junio C Hamano
@ 2012-07-30  5:20                 ` Shawn Pearce
  2012-07-30  6:28                   ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: Shawn Pearce @ 2012-07-30  5:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sitaram Chamarty, Fredrik Gustafsson, git

On Sun, Jul 29, 2012 at 7:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
>> We sort of want this in Gerrit Code Review to pass reviewer names on
>> the command line of git push, making it easier for users to upload a
>> code review. The idea is similar to what happens with gcc accepting
>> linker flags that are just passed onto the linker.
>
> For reviewer names, authentication cookies and things of that nature
> where the extra pieces of information affect the outcome in a way
> that does not have to change how the underlying protocol exchange
> works, such an additional one-way channel from the pusher to pushee
> to carry auxiliary information would be sufficient.

Yes, that is what I was trying to argue. :-)

I agree that authentication information is outside of the Git protocol
itself. We rely on SSH authentication for SSH and HTTP native
authentication methods for HTTP transport. But at least in the HTTP
case, the Git client has learned how to set up the authentication data
for the user to make it easier to use HTTP authentication. We don't
yet support native OAuth 2.0 (ick!) or HTTP cookies as well as we do
client side SSL certificates or basic username/password pair.

If we want to support additional information from pusher to pushee,
this is a "native feature" of Git and should be supported on all
native push type transports, with roughly the same semantics
everywhere. I don't want to add additional data into X-Git-Foo HTTP
headers in HTTP, and as environment variables in SSH, for example.
Additional HTTP headers will *probably* transit an HTTP proxy
correctly (but there are a lot of broken proxy servers so I don't put
it past someone to strip an X-* header they don't think is "safe").
SSH environment variables are icky to set from the client, and server
side Git would need to know how it was invoked to decode the correct
data and make it available uniformly to repository owner authored
hooks.

>  The server may
> decide to accept otherwise forbidden, or reject otherwise permitted,
> push based on the extra information given, for example, and that is
> an example of an enhancement that does not have to change how the
> underlying protocol exchange works.

Yes.

> The way to expose the extra information parsed by Git to the server
> side could be made into calling out to hooks, and at that point,
> gitolite would not even have to know about the pack protocol.

Good point. The case that spawned this thread however still has a
problem with this approach. gitolite would need to create a repository
to invoke the receive-pack process within, and install that new hook
script into... when the hook was trying to prevent the creation of
that repository in the first place.

Maybe I am jaded by the way JGit handles the protocol, it is easy for
application code to glue into and see things going on in the protocol
in ways that are hard to do from git-core.

> Perhaps the interface to such a hook may be "hook can tell Git to
> abort the communication by exiting non-zero, after giving a message
> to its standard output".

Perhaps this new channel data is simply passed as arguments to
receive-pack on the remote side?

An ancient Git would abort hard if passed this flag. An updated Git
could set environment variables before calling hooks, making the
arguments visible that way. And gitolite can still scrape what it
needs from the command line without having to muck about inside of the
protocol, but only if it needs to observe this new data from pusher to
pushee?

`git push -Rfoo=baz host:dest.git master` on the client would turn
into `git-receive-pack -Rfoo=baz dest.git` in the SSH and git://
command line, and cause GIT_PUSH_ARG_FOO=baz to appear in the
environment of hooks. Over smart HTTP requests would get an additional
query parameter of foo=baz.


The other hacky idea I had was to use a fake reference and have the
client push a structured blob to that ref. The server would decode the
blob, and deny the creation of the fake reference, but be able to get
additional data from that blob. Its hacky, and I don't like making a
new blob on the server just to transport a few small bits of data from
the client.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  5:20                 ` Shawn Pearce
@ 2012-07-30  6:28                   ` Junio C Hamano
  2012-07-30  8:12                     ` Sitaram Chamarty
  2012-07-30  8:31                     ` Sitaram Chamarty
  0 siblings, 2 replies; 25+ messages in thread
From: Junio C Hamano @ 2012-07-30  6:28 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Sitaram Chamarty, Fredrik Gustafsson, git

Shawn Pearce <spearce@spearce.org> writes:

>> The way to expose the extra information parsed by Git to the server
>> side could be made into calling out to hooks, and at that point,
>> gitolite would not even have to know about the pack protocol.
>
> Good point. The case that spawned this thread however still has a
> problem with this approach. gitolite would need to create a repository
> to invoke the receive-pack process within, and install that new hook
> script into... when the hook was trying to prevent the creation of
> that repository in the first place.

Heh.  While I do not particularly consider auto-creation-upon-push a
useful thing to begin with (after all, once you created a
repository, you would want ways to manage it, setting up ACL for it
and stuff like that, so adding a "create" command to the management
interface suite would be a more natural direction to go), as long as
we are discussing a hack that involves hooks, I do not think your
observation is a show-stopper downside.

The hook can interact with the end user over the back channel and
decide to abort the transaction, while leaving some clue in the
repository that is pre-agreed between the gitlite server and the
hook.  When gitolite culls the process with wait4(2), it could
notice that "clue", read the wish of the hook that the repository
needs to be removed from it, and remove the repository. Up to that
point, there is no real data transferred, so there isn't much wasted
time or network resource anyway.

> An ancient Git would abort hard if passed this flag. An updated Git
> could set environment variables before calling hooks, making the
> arguments visible that way. And gitolite can still scrape what it
> needs from the command line without having to muck about inside of the
> protocol, but only if it needs to observe this new data from pusher to
> pushee?

I do not think the details of how the extra information is passed
via the Git at the receiving end to its surrounding matters that
much.  It would even work fine if we let the hook to talk with the
end user sitting at the "git push" end, by using two extra sidebands
to throw bits between them, while the Git process that spawned the
hook acts as a relay, to establish a custom bi-di conversation (but
again, I do not think it is useful, because such an out of band
conversation cannot affect the outcome of the main protocol exchange
in a meaningful way other than aborting).

Or you could export environment variables, which would be far more
limiting with respect to the nature of the data (i.e. needs to be
free of NUL) and the size of data you can pass.  The limitation may
actually be a feature to discourage people from doing wacky things,
though.

> `git push -Rfoo=baz host:dest.git master` on the client would turn
> into `git-receive-pack -Rfoo=baz dest.git` in the SSH and git://
> command line, and cause GIT_PUSH_ARG_FOO=baz to appear in the
> environment of hooks. Over smart HTTP requests would get an additional
> query parameter of foo=baz.

I think using the same "extra args on the command line" would be a
good way to upgrade the protocol version in a way the current
"capability" system does not allow us to (namely, stop the party
that accepts the connection from immediately advertising its refs).

> The other hacky idea I had was to use a fake reference and have the
> client push a structured blob to that ref. The server would decode the
> blob, and deny the creation of the fake reference, but be able to get
> additional data from that blob. Its hacky, and I don't like making a
> new blob on the server just to transport a few small bits of data from
> the client.

That way lies madness, and at that point, you are better off doing a
proper protocol extension by registering a new capability and defining
the semantics for it.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  6:28                   ` Junio C Hamano
@ 2012-07-30  8:12                     ` Sitaram Chamarty
  2012-07-30  8:31                     ` Sitaram Chamarty
  1 sibling, 0 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-30  8:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn Pearce, Fredrik Gustafsson, git

On Mon, Jul 30, 2012 at 11:58 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Heh.  While I do not particularly consider auto-creation-upon-push a
> useful thing to begin with (after all, once you created a
> repository, you would want ways to manage it, setting up ACL for it

[side point] these things are managed with templates of ACL rules that
map access to "roles", and the owner (the guy who created it) defines
users for each role.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Enhancements to git-protocoll
  2012-07-30  6:28                   ` Junio C Hamano
  2012-07-30  8:12                     ` Sitaram Chamarty
@ 2012-07-30  8:31                     ` Sitaram Chamarty
  1 sibling, 0 replies; 25+ messages in thread
From: Sitaram Chamarty @ 2012-07-30  8:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn Pearce, Fredrik Gustafsson, git

On Mon, Jul 30, 2012 at 11:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
>>> The way to expose the extra information parsed by Git to the server
>>> side could be made into calling out to hooks, and at that point,
>>> gitolite would not even have to know about the pack protocol.
>>
>> Good point. The case that spawned this thread however still has a
>> problem with this approach. gitolite would need to create a repository
>> to invoke the receive-pack process within, and install that new hook
>> script into... when the hook was trying to prevent the creation of
>> that repository in the first place.
>
> Heh.  While I do not particularly consider auto-creation-upon-push a
> useful thing to begin with (after all, once you created a
> repository, you would want ways to manage it, setting up ACL for it
> and stuff like that, so adding a "create" command to the management
> interface suite would be a more natural direction to go), as long as
> we are discussing a hack that involves hooks, I do not think your
> observation is a show-stopper downside.
>
> The hook can interact with the end user over the back channel and
> decide to abort the transaction, while leaving some clue in the
> repository that is pre-agreed between the gitlite server and the
> hook.  When gitolite culls the process with wait4(2), it could
> notice that "clue", read the wish of the hook that the repository
> needs to be removed from it, and remove the repository. Up to that
> point, there is no real data transferred, so there isn't much wasted
> time or network resource anyway.
>
>> An ancient Git would abort hard if passed this flag. An updated Git
>> could set environment variables before calling hooks, making the
>> arguments visible that way. And gitolite can still scrape what it
>> needs from the command line without having to muck about inside of the
>> protocol, but only if it needs to observe this new data from pusher to
>> pushee?
>
> I do not think the details of how the extra information is passed
> via the Git at the receiving end to its surrounding matters that
> much.  It would even work fine if we let the hook to talk with the
> end user sitting at the "git push" end, by using two extra sidebands
> to throw bits between them, while the Git process that spawned the
> hook acts as a relay, to establish a custom bi-di conversation (but
> again, I do not think it is useful, because such an out of band
> conversation cannot affect the outcome of the main protocol exchange
> in a meaningful way other than aborting).
>
> Or you could export environment variables, which would be far more
> limiting with respect to the nature of the data (i.e. needs to be
> free of NUL) and the size of data you can pass.  The limitation may
> actually be a feature to discourage people from doing wacky things,
> though.
>
>> `git push -Rfoo=baz host:dest.git master` on the client would turn
>> into `git-receive-pack -Rfoo=baz dest.git` in the SSH and git://
>> command line, and cause GIT_PUSH_ARG_FOO=baz to appear in the
>> environment of hooks. Over smart HTTP requests would get an additional
>> query parameter of foo=baz.
>
> I think using the same "extra args on the command line" would be a
> good way to upgrade the protocol version in a way the current
> "capability" system does not allow us to (namely, stop the party
> that accepts the connection from immediately advertising its refs).

More importantly from gitolite's point of view, this is the only way
gitolite can see those variables in some situations, because gitolite
runs *before* git, (and then again later via the update hook for
pushes).

>> The other hacky idea I had was to use a fake reference and have the
>> client push a structured blob to that ref. The server would decode the
>> blob, and deny the creation of the fake reference, but be able to get
>> additional data from that blob. Its hacky, and I don't like making a
>> new blob on the server just to transport a few small bits of data from
>> the client.
>
> That way lies madness, and at that point, you are better off doing a
> proper protocol extension by registering a new capability and defining
> the semantics for it.



-- 
Sitaram

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2012-07-30  8:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-28 21:41 Enhancements to git-protocoll Fredrik Gustafsson
2012-07-29  6:58 ` Junio C Hamano
2012-07-29 14:24   ` Fredrik Gustafsson
2012-07-29 14:39     ` Sitaram Chamarty
2012-07-29 20:51       ` Junio C Hamano
2012-07-29 21:26         ` Fredrik Gustafsson
2012-07-30  0:37           ` Sitaram Chamarty
2012-07-29 21:38         ` Junio C Hamano
2012-07-30  1:04           ` Sitaram Chamarty
2012-07-30  1:21             ` Shawn Pearce
2012-07-30  1:33               ` Sitaram Chamarty
2012-07-30  2:38               ` Junio C Hamano
2012-07-30  5:20                 ` Shawn Pearce
2012-07-30  6:28                   ` Junio C Hamano
2012-07-30  8:12                     ` Sitaram Chamarty
2012-07-30  8:31                     ` Sitaram Chamarty
2012-07-30  1:28             ` Junio C Hamano
2012-07-30  1:45               ` Sitaram Chamarty
2012-07-29 10:37 ` Sitaram Chamarty
2012-07-29 14:13   ` Fredrik Gustafsson
2012-07-29 14:25     ` Sitaram Chamarty
2012-07-29 15:05       ` Fredrik Gustafsson
2012-07-29 15:15         ` Sitaram Chamarty
2012-07-29 15:41           ` Fredrik Gustafsson
2012-07-29 18:22             ` Sitaram Chamarty

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).