Re: Enhancements to git-protocoll
Shawn Pearce spea...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Mon, Jul 30, 2012 at 11:58 AM, Junio C Hamano gits...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Mon, Jul 30, 2012 at 11:58 AM, Junio C Hamano gits...@pobox.com wrote: Shawn Pearce spea...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
Fredrik Gustafsson iv...@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).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sun, Jul 29, 2012 at 3:11 AM, Fredrik Gustafsson iv...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sun, Jul 29, 2012 at 04:07:13PM +0530, Sitaram Chamarty wrote: On Sun, Jul 29, 2012 at 3:11 AM, Fredrik Gustafsson iv...@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: iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sat, Jul 28, 2012 at 11:58:09PM -0700, Junio C Hamano wrote:
Fredrik Gustafsson iv...@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: iv...@iveqy.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sun, Jul 29, 2012 at 7:43 PM, Fredrik Gustafsson iv...@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 iv...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
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: iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sun, Jul 29, 2012 at 8:35 PM, Fredrik Gustafsson iv...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
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: iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sun, Jul 29, 2012 at 9:11 PM, Fredrik Gustafsson iv...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
Sitaram Chamarty sitar...@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 ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
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 sitar...@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: iv...@iveqy.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
Junio C Hamano gits...@pobox.com writes: Sitaram Chamarty sitar...@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
Re: Enhancements to git-protocoll
On Mon, Jul 30, 2012 at 2:56 AM, Fredrik Gustafsson iv...@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 sitar...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Mon, Jul 30, 2012 at 3:08 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Sitaram Chamarty sitar...@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
Re: Enhancements to git-protocoll
Sitaram Chamarty sitar...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Mon, Jul 30, 2012 at 6:51 AM, Shawn Pearce spea...@spearce.org wrote:
On Sun, Jul 29, 2012 at 6:04 PM, Sitaram Chamarty sitar...@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:
... 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Mon, Jul 30, 2012 at 6:58 AM, Junio C Hamano gits...@pobox.com wrote: Sitaram Chamarty sitar...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
Shawn Pearce spea...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Enhancements to git-protocoll
On Sun, Jul 29, 2012 at 7:38 PM, Junio C Hamano gits...@pobox.com wrote: Shawn Pearce spea...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
