Hi Junio, On Mon, 1 Dec 2014, Junio C Hamano wrote:
> When receive.denyCurrentBranch is set to updateInstead, this hook > can be used to override the built-in "push-to-deploy" logic, which > insists that the working tree and the index must be unchanged > relative to HEAD. The hook receives the commit with which the > tip of the current is going to be updated, and is responsible to > make any necessary changes to the working tree and to the index to > bring them to the desired state when the tip of the current branch > is updated to the new commit. > > For example, the hook can simply run "git read-tree -u -m HEAD $1" > to the workflow to emulate "'git fetch' going in the reverse > direction with 'git push'" better than the push-to-deploy logic, as > the two-tree form of "read-tree -u -m" is essentially the same as > "git checkout" that switches branches while keeping the local > changes in the working tree that do not interfere with the > difference between the branches. I like it. The only sad part is that the already huge test suite is enlarged by yet another extensive set of test cases (and those tests might not really need to be that extensive because they essentially only need to make sure that the hook is run successfully *instead* of trying to update the working directory, i.e. a simple 'touch yep' hook would have been enough). It starts to be painful to run the complete test suite, not only on Windows (where this has been a multi-hour endeavor for me for ages already). BuildHive (CloudBees' very kind offer of Jenkins CI for Open Source, integrated conveniently with GitHub) already takes over an hour to run the Git test suite – and BuildHive runs on Linux, not Windows! That means that everytime I push into a GitHub Pull Request, I have to wait for a full hour to know whether everything is groovy. Worse: when Git for Windows contributors (yes! they exist!) push into their Pull Requests, I have to wait for a full hour before I can merge, unless I want to merge something that the test suite did not validate! So maybe it is time to start thinking about conciser tests that verify the bare minimum, especially for rarely exercised functionality? I mean, testing is always a balance between too much and too little. And at this point, I am afraid that several well-intended, but overly extensive tests increase the overall runtime of "make test" to a point where developers *avoid* running it, costing more time in the long run than necessary. In this particular case, I think that we really, really *just* need to verify that the presence of the hook switches off the default behavior of updateInstead. *Nothing* else is needed to verify that this particular functionality hasn't regressed. I.e. something like: +test_expect_success 'updateInstead with push-to-checkout hook' ' + rm -fr testrepo && + git clone . testrepo && + ( + cd testrepo && + echo unclean > path1 && + git config receive.denyCurrentBranch updateInstead && + echo 'touch yep' | write_script .git/hooks/push-to-checkout + ) && + git push testrepo HEAD^:refs/heads/master && + test unclean = $(cat testrepo/path1) && + test -f testrepo/yep +' would be more appropriate (although it probably has one or three bugs, given that I wrote this in the mailer). Ciao, Johannes
