Junio C Hamano [off-list ref] writes:
Sitaram Chamarty [off-list ref] writes:
quoted
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,
On Mon, Jul 30, 2012 at 3:08 AM, Junio C Hamano [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
Sitaram Chamarty [off-list ref] writes:
quoted
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
On Sun, Jul 29, 2012 at 6:04 PM, Sitaram Chamarty [off-list ref] 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.
On Mon, Jul 30, 2012 at 6:51 AM, Shawn Pearce [off-list ref] wrote:
On Sun, Jul 29, 2012 at 6:04 PM, Sitaram Chamarty [off-list ref] wrote:
quoted
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