summaryrefslogtreecommitdiff
path: root/t/t1416-ref-transaction-hooks.sh
AgeCommit message (Collapse)AuthorFilesLines
2022-04-13Revert "Merge branch 'ps/avoid-unnecessary-hook-invocation-with-packed-refs'"Libravatar Junio C Hamano1-50/+0
This reverts commit 991b4d47f0accd3955d05927d5ce434e03ffbdb6, reversing changes made to bcd020f88e1e22f38422ac3f73ab06b34ec4bef1.
2022-03-17tests: assume the hooks are disabled by defaultLibravatar Ævar Arnfjörð Bjarmason1-1/+0
Stop moving the .git/hooks directory out of the way, or creating it during test setup. Instead assume that it will contain harmless *.sample files. That we can assume that is discussed in point #4 of f0d4d398e28 (test-lib: split up and deprecate test_create_repo(), 2021-05-10), those parts of this could and should have been done in that change. Removing the "mkdir -p" here will then validate that our templates are being used, since we'd subsequently fail to create a hook in that directory if it didn't exist. Subsequent commits will have those hooks created by a "test_hook" wrapper, which will then being doing that same validation. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-17test-lib-functions: add and use a "test_hook" wrapperLibravatar Ævar Arnfjörð Bjarmason1-14/+12
Add a "test_hook" wrapper similar to the existing "test_config" wrapper added in d960c47a881 (test-lib: add helper functions for config, 2011-08-17). This wrapper: - Will clean up the hook with "test_when_finished", unless --setup is provided. - Will error if we clobber a hook, unless --clobber is provided. - Takes a name like "update" instead of ".git/hooks/update". - Accepts -C <dir>, like "test_config" and "test_commit". By using a wrapper we'll be able to easily change all the hook-related code that assumes that the template-created ".git/hooks" directory is created by "init", "clone" etc. once another topic follows-up and changes the test suite to stop creating trash directories using those templates. In addition this will make it easy to have the hooks configured using the "configuration-based hooks" topic, once we get around to integrating that. I.e. we'll be able to run the tests in a mode where we sometimes create a .git/hooks/<name>, and other times create a script in another location, and point the relevant configuration snippet to it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: skip hooks when deleting uncovered packed refsLibravatar Patrick Steinhardt1-6/+1
When deleting refs from the loose-files refs backend, then we need to be careful to also delete the same ref from the packed refs backend, if it exists. If we don't, then deleting the loose ref would "uncover" the packed ref. We thus always have to queue up deletions of refs for both the loose and the packed refs backend. This is done in two separate transactions, where the end result is that the reference-transaction hook is executed twice for the deleted refs. This behaviour is quite misleading: it's exposing implementation details of how the files backend works to the user, in contrast to the logical updates that we'd really want to expose via the hook. Worse yet, whether the hook gets executed once or twice depends on how well-packed the repository is: if the ref only exists as a loose ref, then we execute it once, otherwise if it is also packed then we execute it twice. Fix this behaviour and don't execute the reference-transaction hook at all when refs in the packed-refs backend if it's driven by the files backend. This works as expected even in case the refs to be deleted only exist in the packed-refs backend because the loose-backend always queues refs in its own transaction even if they don't exist such that they can be locked for concurrent creation. And it also does the right thing in case neither of the backends has the ref because that would cause the transaction to fail completely. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: do not execute reference-transaction hook on packing refsLibravatar Patrick Steinhardt1-10/+1
The reference-transaction hook is supposed to track logical changes to references, but it currently also gets executed when packing refs in a repository. This is unexpected and ultimately not all that useful: packing refs is not supposed to result in any user-visible change to the refs' state, and it ultimately is an implementation detail of how refs stores work. Fix this excessive execution of the hook when packing refs. Reported-by: Waleed Khan <me@waleedkhan.name> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-17refs: demonstrate excessive execution of the reference-transaction hookLibravatar Patrick Steinhardt1-0/+64
Add tests which demonstate that we're executing the reference-transaction hook too often in some cases, which thus leaks implementation details about the reference store's implementation itself. Behaviour will be fixed in follow-up commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19t[01]*: adjust the references to the default branch name "main"Libravatar Johannes Schindelin1-6/+6
Carefully excluding t1309, which sees independent development elsewhere at the time of writing, we transition above-mentioned tests to the default branch name `main`. This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/naster/nain/g' -- t[01]*.sh && git checkout HEAD -- t1309\*) Note that t5533 contains a variation of the name `master` (`naster`) that we rename here, too. This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19tests: mark tests relying on the current default for `init.defaultBranch`Libravatar Johannes Schindelin1-0/+3
In addition to the manual adjustment to let the `linux-gcc` CI job run the test suite with `master` and then with `main`, this patch makes sure that GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME is set in all test scripts that currently rely on the initial branch name being `master by default. To determine which test scripts to mark up, the first step was to force-set the default branch name to `master` in - all test scripts that contain the keyword `master`, - t4211, which expects `t/t4211/history.export` with a hard-coded ref to initialize the default branch, - t5560 because it sources `t/t556x_common` which uses `master`, - t8002 and t8012 because both source `t/annotate-tests.sh` which also uses `master`) This trick was performed by this command: $ sed -i '/^ *\. \.\/\(test-lib\|lib-\(bash\|cvs\|git-svn\)\|gitweb-lib\)\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' $(git grep -l master t/t[0-9]*.sh) \ t/t4211*.sh t/t5560*.sh t/t8002*.sh t/t8012*.sh After that, careful, manual inspection revealed that some of the test scripts containing the needle `master` do not actually rely on a specific default branch name: either they mention `master` only in a comment, or they initialize that branch specificially, or they do not actually refer to the current default branch. Therefore, the aforementioned modification was undone in those test scripts thusly: $ git checkout HEAD -- \ t/t0027-auto-crlf.sh t/t0060-path-utils.sh \ t/t1011-read-tree-sparse-checkout.sh \ t/t1305-config-include.sh t/t1309-early-config.sh \ t/t1402-check-ref-format.sh t/t1450-fsck.sh \ t/t2024-checkout-dwim.sh \ t/t2106-update-index-assume-unchanged.sh \ t/t3040-subprojects-basic.sh t/t3301-notes.sh \ t/t3308-notes-merge.sh t/t3423-rebase-reword.sh \ t/t3436-rebase-more-options.sh \ t/t4015-diff-whitespace.sh t/t4257-am-interactive.sh \ t/t5323-pack-redundant.sh t/t5401-update-hooks.sh \ t/t5511-refspec.sh t/t5526-fetch-submodules.sh \ t/t5529-push-errors.sh t/t5530-upload-pack-error.sh \ t/t5548-push-porcelain.sh \ t/t5552-skipping-fetch-negotiator.sh \ t/t5572-pull-submodule.sh t/t5608-clone-2gb.sh \ t/t5614-clone-submodules-shallow.sh \ t/t7508-status.sh t/t7606-merge-custom.sh \ t/t9302-fast-import-unpack-limit.sh We excluded one set of test scripts in these commands, though: the range of `git p4` tests. The reason? `git p4` stores the (foreign) remote branch in the branch called `p4/master`, which is obviously not the default branch. Manual analysis revealed that only five of these tests actually require a specific default branch name to pass; They were modified thusly: $ sed -i '/^ *\. \.\/lib-git-p4\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' t/t980[0167]*.sh t/t9811*.sh Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-11t1416: avoid hard-coded sha1 idsLibravatar Jeff King1-2/+3
The test added by e5256c82e5 (refs: fix interleaving hook calls with reference-transaction hook, 2020-08-07) uses hard-coded sha1 object ids in its expected output. This causes it to fail when run with GIT_TEST_DEFAULT_HASH=sha256. Let's make use of the oid variables we define earlier, as the rest of the nearby tests do. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-07refs: fix interleaving hook calls with reference-transaction hookLibravatar Patrick Steinhardt1-0/+26
In order to not repeatedly search for the reference-transaction hook in case it's getting called multiple times, we use a caching mechanism to only call `find_hook()` once. What was missed though is that the return value of `find_hook()` actually comes from a static strbuf, which means it will get overwritten when calling `find_hook()` again. As a result, we may call the wrong hook with parameters of the reference-transaction hook. This scenario was spotted in the wild when executing a git-push(1) with multiple references, where there are interleaving calls to both the update and the reference-transaction hook. While initial calls to the reference-transaction hook work as expected, it will stop working after the next invocation of the update hook. The result is that we now start calling the update hook with parameters and stdin of the reference-transaction hook. This commit fixes the issue by storing a copy of `find_hook()`'s return value in the cache. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-19refs: implement reference transaction hookLibravatar Patrick Steinhardt1-0/+109
The low-level reference transactions used to update references are currently completely opaque to the user. While certainly desirable in most usecases, there are some which might want to hook into the transaction to observe all queued reference updates as well as observing the abortion or commit of a prepared transaction. One such usecase would be to have a set of replicas of a given Git repository, where we perform Git operations on all of the repositories at once and expect the outcome to be the same in all of them. While there exist hooks already for a certain subset of Git commands that could be used to implement a voting mechanism for this, many others currently don't have any mechanism for this. The above scenario is the motivation for the new "reference-transaction" hook that reaches directly into Git's reference transaction mechanism. The hook receives as parameter the current state the transaction was moved to ("prepared", "committed" or "aborted") and gets via its standard input all queued reference updates. While the exit code gets ignored in the "committed" and "aborted" states, a non-zero exit code in the "prepared" state will cause the transaction to be aborted prematurely. Given the usecase described above, a voting mechanism can now be implemented via this hook: as soon as it gets called, it will take all of stdin and use it to cast a vote to a central service. When all replicas of the repository agree, the hook will exit with zero, otherwise it will abort the transaction by returning non-zero. The most important upside is that this will catch _all_ commands writing references at once, allowing to implement strong consistency for reference updates via a single mechanism. In order to test the impact on the case where we don't have any "reference-transaction" hook installed in the repository, this commit introduce two new performance tests for git-update-refs(1). Run against an empty repository, it produces the following results: Test origin/master HEAD -------------------------------------------------------------------- 1400.2: update-ref 2.70(2.10+0.71) 2.71(2.10+0.73) +0.4% 1400.3: update-ref --stdin 0.21(0.09+0.11) 0.21(0.07+0.14) +0.0% The performance test p1400.2 creates, updates and deletes a branch a thousand times, thus averaging runtime of git-update-refs over 3000 invocations. p1400.3 instead calls `git-update-refs --stdin` three times and queues a thousand creations, updates and deletes respectively. As expected, p1400.3 consistently shows no noticeable impact, as for each batch of updates there's a single call to access(3P) for the negative hook lookup. On the other hand, for p1400.2, one can see an impact caused by this patchset. But doing five runs of the performance tests where each one was run with GIT_PERF_REPEAT_COUNT=10, the overhead ranged from -1.5% to +1.1%. These inconsistent performance numbers can be explained by the overhead of spawning 3000 processes. This shows that the overhead of assembling the hook path and executing access(3P) once to check if it's there is mostly outweighed by the operating system's overhead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>