Jiang Xin <worldhello.net@xxxxxxxxx> writes: > It would be more convenient to work in a centralized workflow like what > Gerrit provided for some cases. For example, a read-only user may run > the following `git push` command to push commits to a special reference > to create a code review, instead of updating a reference directly. > > git push -o reviewers=user1,user2 \ > -o oldoid=89c082363ac950d224a7259bfba3ccfbf4c560c4 \ > origin \ > HEAD:refs/for/<branch-name>/<session> > > The `<branch-name>` in the above example can be as simple as "master", > or a more complicated branch name like "foo/bar". The `<session>` in > the above example command can be the local branch name of the clien- > side, such as "my/topic". > > To support this kind of workflow in CGit, add a filter and a new > handler. The filter will check the prefix of the reference name, and > if the command has a special reference name, the filter will add a > specific tag (`exec_by_hook`) to the command. Commands with this > specific tag will be executed by a new handler (an external hook named > "execute-commands") instead of the internal `execute_commands` function. I do not claim to be great at naming, but you are worse ;-) - Any hook is about executing command(s), so "execute-commands" hook does not give any information to users. - IIUC, this is only about what happens when accepting a push and is not called at any other time. Naming your hook without "receive" anywhere in its name would mean other people won't be able to add hook that "executes" commands upon cues other than receiving a push. I can guess why you chose that name, because I know there is a function called execute_commands() in "git receive-pack", but that is not somethhing you can expect your end users, who are not intimate to our codebase, to know. > We can use the external "execute-commands" hook to create pull requests > or send emails. You can create pull requests or send emails out of the post-receive hook, so that is not a convincing justification why we want a new hook. Now, I understand that Gerrit-style "notice a push to for/<target>, take over the whole operation that happens after receiving the pack data and do something entirely different, such as attempting to run a merge with refs/heads/<target> and update refs/heads/<target> instead, or fail the push if automerge fails" is not easy to arrange within the current "pre-receive" + "post-receive" framework (by the way, we should start considering to deprecate "update", and "post-update" hooks as these "*-receive" hooks were added to replace them, perhaps we should leave a #leftoverbits mark here). And I think it is reasonable to add a new hook that takes over the whole flow in "git receive-pack" to do so. I just do not think "the execute-commands hook" is a good name for it. Perhaps "divert-receive" (as it diverts large portion of what receive does) or something? I dunno. How do Gerrit folks deal with the "we pushed to the server, so let's pretend to have turned around and fetched from the same server immediately after doing so" client-side hack, by the way? A vanilla "git push" on the client side does not know a push to refs/for/master would result in an update to refs/heads/master on the server side, and it would not know the result of the munging done on the server side (whether it is to rebase what is received on top of 'master' or to merge it to 'master') anyway, the remote-tracking branch refs/remotes/origin/master on the client side would be left stale. If we wanted to help them pretend to have fetched immediately after, I think we need to extend the protocol. Right now, after accepting "git push", the server end will say, for each proposed update for a ref, if the push updated successfully or not, but to support the "push to for/<target>, get heads/<target> updated" interaction, the reporting of the result (done in the report() function in builtin/receive-pack.c) needs to be able to say what ref (it may be a ref that "git push" did not think it pushed to) got updated to what value (it may be an object the client does not yet have---and we may have to actually turn around and fetch from them internally if we want to keep the illusion).
