path: root/t/
AgeCommit message (Collapse)Author
2021-09-27refs: turn on GIT_REF_PARANOIA by defaultJeff King
The original point of the GIT_REF_PARANOIA flag was to include broken refs in iterations, so that possibly-destructive operations would not silently ignore them (and would generally instead try to operate on the oids and fail when the objects could not be accessed). We already turned this on by default for some dangerous operations, like "repack -ad" (where missing a reachability tip would mean dropping the associated history). But it was not on for general use, even though it could easily result in the spreading of corruption (e.g., imagine cloning a repository which simply omits some of its refs because their objects are missing; the result quietly succeeds even though you did not clone everything!). This patch turns on GIT_REF_PARANOIA by default. So a clone as mentioned above would actually fail (upload-pack tells us about the broken ref, and when we ask for the objects, pack-objects fails to deliver them). This may be inconvenient when working with a corrupted repository, but: - we are better off to err on the side of complaining about corruption, and then provide mechanisms for explicitly loosening safety. - this is only one type of corruption anyway. If we are missing any other objects in the history that _aren't_ ref tips, then we'd behave similarly (happily show the ref, but then barf when we started traversing). We retain the GIT_REF_PARANOIA variable, but simply default it to "1" instead of "0". That gives the user an escape hatch for loosening this when working with a corrupt repository. It won't work across a remote connection to upload-pack (because we can't necessarily set environment variables on the remote), but there the client has other options (e.g., choosing which refs to fetch). As a bonus, this also makes ref iteration faster in general (because we don't have to call has_object_file() for each ref), though probably not noticeably so in the general case. In a repo with a million refs, it shaved a few hundred milliseconds off of upload-pack's advertisement; that's noticeable, but most repos are not nearly that large. The possible downside here is that any operation which iterates refs but doesn't ever open their objects may now quietly claim to have X when the object is corrupted (e.g., "git rev-list new-branch --not --all" will treat a broken ref as uninteresting). But again, that's not really any different than corruption below the ref level. We might have refs/heads/old-branch as non-corrupt, but we are not actively checking that we have the entire reachable history. Or the pointed-to object could even be corrupted on-disk (but our "do we have it" check would still succeed). In that sense, this is merely bringing ref-corruption in line with general object corruption. One alternative implementation would be to actually check for broken refs, and then _immediately die_ if we see any. That would cause the "rev-list --not --all" case above to abort immediately. But in many ways that's the worst of all worlds: - it still spends time looking up the objects an extra time - it still doesn't catch corruption below the ref level - it's even more inconvenient; with the current implementation of GIT_REF_PARANOIA for something like upload-pack, we can make the advertisement and let the client choose a non-broken piece of history. If we bail as soon as we see a broken ref, they cannot even see the advertisement. The test changes here show some of the fallout. A non-destructive "git repack -adk" now fails by default (but we can override it). Deleting a broken ref now actually tells the hooks the correct "before" state, rather than a confusing null oid. Signed-off-by: Jeff King <> Reviewed-by: Jonathan Tan <> Signed-off-by: Junio C Hamano <>
2021-09-27t5516: don't use HEAD ref for invalid ref-deletion testsJeff King
A few tests in t5516 want to assert that we can delete a corrupted ref whose pointed-to object is missing. They do so by using the "main" branch, which is also pointed to by HEAD. This does work, but only because of a subtle assumption about the implementation. We do not block the deletion because of the invalid ref, but we _also_ do not notice that the deleted branch is pointed to by HEAD. And so the safety rule of "do not allow HEAD to be deleted in a non-bare repository" does not kick in, and the test passes. Let's instead use a non-HEAD branch. That still tests what we care about here (deleting a corrupt ref), but without implicitly depending on our failure to notice that we're deleting HEAD. That will future proof the test against that behavior changing. Signed-off-by: Jeff King <> Reviewed-by: Jonathan Tan <> Signed-off-by: Junio C Hamano <>
2021-07-15send-pack: fix push nego. when remote has refsJonathan Tan
Commit 477673d6f3 ("send-pack: support push negotiation", 2021-05-05) did not test the case in which a remote advertises at least one ref. In such a case, "remote_refs" in get_commons_through_negotiation() in send-pack.c would also contain those refs with a zero ref->new_oid (in addition to the refs being pushed with a nonzero ref->new_oid). Passing them as negotiation tips to "git fetch" causes an error, so filter them out. (The exact error that would happen in "git fetch" in this case is a segmentation fault, which is unwanted. This will be fixed in the subsequent commit.) Signed-off-by: Jonathan Tan <> Signed-off-by: Junio C Hamano <>
2021-05-05send-pack: support push negotiationJonathan Tan
Teach Git the push.negotiate config variable. Signed-off-by: Jonathan Tan <> Signed-off-by: Junio C Hamano <>
2021-02-26Merge branch 'jc/push-delete-nothing'Junio C Hamano
"git push $there --delete ''" should have been diagnosed as an error, but instead turned into a matching push, which has been corrected. * jc/push-delete-nothing: push: do not turn --delete '' into a matching push
2021-02-23push: do not turn --delete '' into a matching pushJunio C Hamano
When we added a syntax sugar "git push remote --delete <ref>" to "git push" as a synonym to the canonical "git push remote :<ref>" syntax at f517f1f2 (builtin-push: add --delete as syntactic sugar for :foo, 2009-12-30), we weren't careful enough to make sure that <ref> is not empty. Blindly rewriting "--delete <ref>" to ":<ref>" means that an empty string <ref> results in refspec ":", which is the syntax to ask for "matching" push that does not delete anything. Worse yet, if there were matching refs that can be fast-forwarded, they would have been published prematurely, even if the user feels that they are not ready yet to be pushed out, which would be a real disaster. Noticed-by: Tilman Vogel <> Signed-off-by: Junio C Hamano <>
2021-01-25Merge branch 'js/default-branch-name-tests-final-stretch'Junio C Hamano
Prepare tests not to be affected by the name of the default branch "git init" creates. * js/default-branch-name-tests-final-stretch: (28 commits) tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed t99*: adjust the references to the default branch name "main" tests(git-p4): transition to the default branch name `main` t9[5-7]*: adjust the references to the default branch name "main" t9[0-4]*: adjust the references to the default branch name "main" t8*: adjust the references to the default branch name "main" t7[5-9]*: adjust the references to the default branch name "main" t7[0-4]*: adjust the references to the default branch name "main" t6[4-9]*: adjust the references to the default branch name "main" t64*: preemptively adjust alignment to prepare for `master` -> `main` t6[0-3]*: adjust the references to the default branch name "main" t5[6-9]*: adjust the references to the default branch name "main" t55[4-9]*: adjust the references to the default branch name "main" t55[23]*: adjust the references to the default branch name "main" t551*: adjust the references to the default branch name "main" t550*: adjust the references to the default branch name "main" t5503: prepare aligned comment for replacing `master` with `main` t5[0-4]*: adjust the references to the default branch name "main" t5323: prepare centered comment for `master` -> `main` t4*: adjust the references to the default branch name "main" ...
2021-01-10t5516: loosen "not our ref" error checkJeff King
Commit 014ade7484 (upload-pack: send ERR packet for non-tip objects, 2019-04-13) added a test that greps the output of a failed fetch to make sure that upload-pack sent us the ERR packet we expected. But checking this is racy; despite the argument in that commit, the client may still be sending a "done" line after the server exits, causing it to die() on a failed write() and never see the ERR packet at all. This fails quite rarely on Linux, but more often on macOS. However, it can be triggered reliably with: diff --git a/fetch-pack.c b/fetch-pack.c index 876f90c759..cf40de9092 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -489,6 +489,7 @@ static int find_common(struct fetch_negotiator *negotiator, done: trace2_region_leave("fetch-pack", "negotiation_v0_v1", the_repository); if (!got_ready || !no_done) { + sleep(1); packet_buf_write(&req_buf, "done\n"); send_request(args, fd[1], &req_buf); } This is a real user-visible race that it would be nice to fix, but it's tricky to do so: the client would have to speculatively try to read an ERR packet after hitting a write() error. And at least for this error, it's specific to v0 (since v2 does not enforce reachability at all). So let's loosen the test to avoid annoying racy failures. If we eventually do the read-after-failed-write thing, we can tighten it. And if not, v0 will grow increasingly obsolete as servers support v2, so the utility of this test will decrease over time anyway. Note that we can still check stderr to make sure upload-pack bailed for the reason we expected. It writes a similar message to stderr, and because the server side is just another process connected by pipes, we'll reliably see it. This would not be the case for git://, or for ssh servers that do not relay stderr (e.g., GitHub's custom endpoint does not). Helped-by: SZEDER Gábor <> Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2020-11-30refspec: make @ a synonym of HEADFelipe Contreras
Since commit 9ba89f484e git learned how to push to a remote branch using the source @, for example: git push origin @:master However, if the right-hand side is missing, the push fails: git push origin @ It is obvious what is the desired behavior, and allowing the push makes things more consistent. Additionally, @:master now has the same semantics as HEAD:master. Signed-off-by: Felipe Contreras <> Signed-off-by: Junio C Hamano <>
2020-11-30tests: push: trivial cleanupFelipe Contreras
No need to do two checkouts. Signed-off-by: Felipe Contreras <> Signed-off-by: Junio C Hamano <>
2020-11-30tests: push: improve cleanup of HEAD testsFelipe Contreras
So that we are not left in an inconsistent state between them. Signed-off-by: Felipe Contreras <> Signed-off-by: Junio C Hamano <>
2020-11-19t551*: adjust the references to the default branch name "main"Johannes Schindelin
This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -- t551*.sh) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Signed-off-by: Johannes Schindelin <> Signed-off-by: Junio C Hamano <>
2020-11-19tests: mark tests relying on the current default for `init.defaultBranch`Johannes Schindelin
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/` 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/ t/ \ t/ \ t/ t/ \ t/ t/ \ t/ \ t/ \ t/ t/ \ t/ t/ \ t/ \ t/ t/ \ t/ t/ \ t/ t/ \ t/ t/ \ t/ \ t/ \ t/ t/ \ t/ \ t/ t/ \ t/ 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 <> Signed-off-by: Junio C Hamano <>
2020-09-25Merge branch 'jx/proc-receive-hook'Junio C Hamano
"git receive-pack" that accepts requests by "git push" learned to outsource most of the ref updates to the new "proc-receive" hook. * jx/proc-receive-hook: doc: add documentation for the proc-receive hook transport: parse report options for tracking refs t5411: test updates of remote-tracking branches receive-pack: new config receive.procReceiveRefs doc: add document for capability report-status-v2 New capability "report-status-v2" for git-push receive-pack: feed report options to post-receive receive-pack: add new proc-receive hook t5411: add basic test cases for proc-receive hook transport: not report a non-head push as a branch
2020-08-27transport: not report a non-head push as a branchJiang Xin
When pushing a new reference (not a head or tag), report it as a new reference instead of a new branch. Signed-off-by: Jiang Xin <> Signed-off-by: Junio C Hamano <>
2020-07-07Merge branch 'js/pu-to-seen'Junio C Hamano
The documentation and some tests have been adjusted for the recent renaming of "pu" branch to "seen". * js/pu-to-seen: tests: reference `seen` wherever `pu` was referenced docs: adjust the technical overview for the rename `pu` -> `seen` docs: adjust for the recent rename of `pu` to `seen`
2020-06-25tests: reference `seen` wherever `pu` was referencedJohannes Schindelin
As our test suite partially reflects how we work in the Git project, it is natural that the branch name `pu` was used in a couple places. Since that branch was renamed to `seen`, let's use the new name consistently. Signed-off-by: Johannes Schindelin <> Signed-off-by: Junio C Hamano <>
2020-04-17send-pack: fix inconsistent porcelain outputJiang Xin
The porcelain output of a failed `git-push` command is inconsistent for different protocols. For example, the following `git-push` command may fail due to the failure of the `pre-receive` hook. git push --porcelain origin HEAD:refs/heads/master For SSH protocol, the porcelain output does not end with a "Done" message: To <URL/of/upstream.git> ! HEAD:refs/heads/master [remote rejected] (pre-receive hook declined) While for HTTP protocol, the porcelain output does end with a "Done" message: To <URL/of/upstream.git> ! HEAD:refs/heads/master [remote rejected] (pre-receive hook declined) Done The following code at the end of function `send_pack()` indicates that `send_pack()` should not return an error if some references are rejected in porcelain mode. int send_pack(...) ... ... if (args->porcelain) return 0; for (ref = remote_refs; ref; ref = ref->next) { switch (ref->status) { case REF_STATUS_NONE: case REF_STATUS_UPTODATE: case REF_STATUS_OK: break; default: return -1; } } return 0; } So if atomic push failed, must check the porcelain mode before return an error. And `receive_status()` should not return an error for a failed updated reference, because `send_pack()` will check them instead. Signed-off-by: Jiang Xin <> Signed-off-by: Junio C Hamano <>
2020-03-05Merge branch 'hv/receive-denycurrent-everywhere'Junio C Hamano
"git push" should stop from updating a branch that is checked out when receive.denyCurrentBranch configuration is set, but it failed to pay attention to checkouts in secondary worktrees. This has been corrected. * hv/receive-denycurrent-everywhere: t2402: test worktree path when called in .git directory receive.denyCurrentBranch: respect all worktrees t5509: use a bare repository for test push target get_main_worktree(): allow it to be called in the Git directory
2020-02-24receive.denyCurrentBranch: respect all worktreesHariom Verma
The receive.denyCurrentBranch config option controls what happens if you push to a branch that is checked out into a non-bare repository. By default, it rejects it. It can be disabled via `ignore` or `warn`. Another yet trickier option is `updateInstead`. However, this setting was forgotten when the git worktree command was introduced: only the main worktree's current branch is respected. With this change, all worktrees are respected. That change also leads to revealing another bug, i.e. `receive.denyCurrentBranch = true` was ignored when pushing into a non-bare repository's unborn current branch using ref namespaces. As `is_ref_checked_out()` returns 0 which means `receive-pack` does not get into conditional statement to switch `deny_current_branch` accordingly (ignore, warn, refuse, unconfigured, updateInstead). receive.denyCurrentBranch uses the function `refs_resolve_ref_unsafe()` (called via `resolve_refdup()`) to resolve the symbolic ref HEAD, but that function fails when HEAD does not point at a valid commit. As we replace the call to `refs_resolve_ref_unsafe()` with `find_shared_symref()`, which has no problem finding the worktree for a given branch even if it is unborn yet, this bug is fixed at the same time: receive.denyCurrentBranch now also handles worktrees with unborn branches as intended even while using ref namespaces. Helped-by: Johannes Schindelin <> Signed-off-by: Hariom Verma <> Signed-off-by: Junio C Hamano <>
2020-01-15test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriateJonathan Nieder
Since 8cbeba0632 (tests: define GIT_TEST_PROTOCOL_VERSION, 2019-02-25), it has been possible to run tests with a newer protocol version by setting the GIT_TEST_PROTOCOL_VERSION envvar to a version number. Tests that assume protocol v0 handle this by explicitly setting GIT_TEST_PROTOCOL_VERSION= or similar constructs like 'test -z "$GIT_TEST_PROTOCOL_VERSION" || return 0' to declare that they only handle the default (v0) protocol. The emphasis there is a bit off: it would be clearer to specify GIT_TEST_PROTOCOL_VERSION=0 to inform the reader that these tests are specifically testing and relying on details of protocol v0. Do so. This way, a reader does not need to know what the default protocol version is, and the tests can continue to work when the default protocol version used by Git advances past v0. Signed-off-by: Jonathan Nieder <> Signed-off-by: Junio C Hamano <>
2019-04-25Merge branch 'jk/fetch-reachability-error-fix'Junio C Hamano
Code clean-up and a fix for "git fetch" by an explicit object name (as opposed to fetching refs by name). * jk/fetch-reachability-error-fix: fetch: do not consider peeled tags as advertised tips remote.c: make singular free_ref() public fetch: use free_refs() pkt-line: prepare buffer before handling ERR packets upload-pack: send ERR packet for non-tip objects t5530: check protocol response for "not our ref" t5516: drop ok=sigpipe from unreachable-want tests
2019-04-22Merge branch 'js/spell-out-options-in-tests'Junio C Hamano
The tests have been updated not to rely on the abbreviated option names the parse-options API offers, to protect us from an abbreviated form of an option that used to be unique within the command getting non-unique when a new option that share the same prefix is added. * js/spell-out-options-in-tests: tests: disallow the use of abbreviated options (by default) tests (pack-objects): use the full, unabbreviated `--revs` option tests (status): spell out the `--find-renames` option in full tests (push): do not abbreviate the `--follow-tags` option t5531: avoid using an abbreviated option t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match` tests (rebase): spell out the `--force-rebase` option tests (rebase): spell out the `--keep-empty` option
2019-04-15fetch: do not consider peeled tags as advertised tipsJeff King
Our filter_refs() function accidentally considers the target of a peeled tag to be advertised by the server, even though upload-pack on the server side does not consider it so. This can result in the client making a bogus fetch to the server, which will end with the server complaining "not our ref". Whereas the correct behavior is for the client to notice that the server will not allow the request and error out immediately. So as bugs go, this is not very serious (the outcome is the same either way -- the fetch fails). But it's worth making the logic here correct and consistent with other related cases (e.g., fetching an oid that the server did not mention at all). The crux of the issue comes from fdb69d33c4 (fetch-pack: always allow fetching of literal SHA1s, 2017-05-15). After that, the strategy of filter_refs() is basically: - for each advertised ref, try to match it with a "sought" ref provided by the user. Skip any malformed refs (which includes peeled values like "refs/tags/foo^{}"), and place any unmatched items onto the unmatched list. - if there are unmatched sought refs, then put all of the advertised tips into an oidset, including the unmatched ones. - for each sought ref, see if it's in the oidset, in which case it's legal for us to ask the server for it The problem is in the second step. Our list of unmatched refs includes the peeled refs, even though upload-pack does not allow them to be directly fetched. So the simplest fix would be to exclude them during that step. However, we can observe that the unmatched list isn't used for anything else, and is freed at the end. We can just free those malformed refs immediately. That saves us having to check each ref a second time to see if it's malformed. Note that this code only kicks in when "strict" is in effect. I.e., if we are using the v0 protocol and uploadpack.allowReachableSHA1InWant is not in effect. With v2, all oids are allowed, and we do not bother creating or consulting the oidset at all. To future-proof our test against the upcoming GIT_TEST_PROTOCOL_VERSION flag, we'll manually mark it as a v0-only test. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-04-15pkt-line: prepare buffer before handling ERR packetsJeff King
Since 2d103c31c2 (pack-protocol.txt: accept error packets in any context, 2018-12-29), the pktline code will detect an ERR packet and die automatically, saving the caller from dealing with it. But we do so too early in the function, before we have terminated the buffer with a NUL. As a result, passing the ERR message to die() may result in us printing random cruft from a previous packet. This doesn't trigger memory tools like ASan because we reuse the same buffer over and over (so the contents are valid and initialized; they're just stale). We can see demonstrate this by tightening the regex we use to match the error message in t5516; without this patch, git-fetch will accidentally print the capabilities from the (much longer) initial packet we received. By moving the ERR code later in the function we get a few other benefits, too: - we'll now chomp any newline sent by the other side (which is what we want, since die() will add its own newline) - we'll now mention the ERR packet with GIT_TRACE_PACKET Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-04-15upload-pack: send ERR packet for non-tip objectsJeff King
Commit bdb31eada7 (upload-pack: report "not our ref" to client, 2017-02-23) catches the case where a client asks for an object we don't have, and issues a message that the client can show to the user (in addition to dying and writing to stderr). There's a similar case (with the same message) when the client asks for an object which we _do_ have, but which isn't a ref tip (or isn't reachable, when uploadpack.allowReachableSHA1InWant is true). Let's give that one the same treatment, for the same reason (namely that it's more informative to the client than just hanging up, since they won't see our stderr over some protocols). There are two tests here. We cover it most directly in t5530 by invoking upload-pack, which matches the existing "not our ref" test. But a more end-to-end check is that "git fetch" actually shows the message to the client. We're already checking in t5516 that this case fails, so we can just check stderr there, too. Note that even after we started ignoring SIGPIPE in 8bf4becf0c, this could in theory still be racy as described in that commit (because we die() on write failures before pumping the connection for any ERR packets). In practice this should be OK for this case. The server will not actually check reachability until it has received our whole group of "want" lines. And since we have no objects in the repository, we won't send any "have" lines, meaning we're always waiting to read the server response. Note also that this case cannot happen in the v2 protocol, since it allows any available object to be requested. However, we don't have to take any steps to protect against the upcoming GIT_TEST_PROTOCOL_VERSION in our tests: - the tests in t5516 would already need to be skipped under v2, and that is covered by ab0c5f5096 (tests: always test fetch of unreachable with v0, 2019-02-25) - the tests in t5530 invoke upload-pack directly, which will continue to default to v0. Eventually we may have a test setting which uses v2 even for bare upload-pack calls, but we can't override it here until we know what the setting looks like. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-04-15t5516: drop ok=sigpipe from unreachable-want testsJeff King
We annotated our test_must_fail calls in 8bf4becf0c (add "ok=sigpipe" to test_must_fail and use it to fix flaky tests, 2015-11-27) because the abrupt hangup of the server meant that we'd sometimes fail on read() and sometimes get SIGPIPE on write(). But since 143588949c (fetch: ignore SIGPIPE during network operation, 2019-03-03), we make sure that we end up with a real die(), and our tests no longer need to work around the race. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-04-02tests (push): do not abbreviate the `--follow-tags` optionJohannes Schindelin
We really want to spell out the option in the full form, to avoid any ambiguity that might be introduced by future patches. Signed-off-by: Johannes Schindelin <> Signed-off-by: Junio C Hamano <>
2019-03-07tests: always test fetch of unreachable with v0Jonathan Tan
Some tests check that fetching an unreachable object fails, but protocol v2 allows such fetches. Unset GIT_TEST_PROTOCOL_VERSION so that these tests are always run using protocol v0. Signed-off-by: Jonathan Tan <> Signed-off-by: Junio C Hamano <>
2018-11-20tests: send "bug in the test script" errors to the script's stderrSZEDER Gábor
Some of the functions in our test library check that they were invoked properly with conditions like this: test "$#" = 2 || error "bug in the test script: not 2 parameters to test-expect-success" If this particular condition is triggered, then 'error' will abort the whole test script with a bold red error message [1] right away. However, under certain circumstances the test script will be aborted completely silently, namely if: - a similar condition in a test helper function like 'test_line_count' is triggered, - which is invoked from the test script's "main" shell [2], - and the test script is run manually (i.e. './' as opposed to 'make' or 'make test') [3] - and without the '--verbose' option, because the error message is printed from within 'test_eval_', where standard output is redirected either to /dev/null or to a log file. The only indication that something is wrong is that not all tests in the script are executed and at the end of the test script's output there is no "# passed all N tests" message, which are subtle and can easily go unnoticed, as I had to experience myself. Send these "bug in the test script" error messages directly to the test scripts standard error and thus to the terminal, so those bugs will be much harder to overlook. Instead of updating all ~20 such 'error' calls with a redirection, let's add a BUG() function to '', wrapping an 'error' call with the proper redirection and also including the common prefix of those error messages, and convert all those call sites [4] to use this new BUG() function instead. [1] That particular error message from 'test_expect_success' is printed in color only when running with or without '--verbose'; with '--tee' or '--verbose-log' the error is printed without color, but it is printed to the terminal nonetheless. [2] If such a condition is triggered in a subshell of a test, then 'error' won't be able to abort the whole test script, but only the subshell, which in turn causes the test to fail in the usual way, indicating loudly and clearly that something is wrong. [3] Well, 'error' aborts the test script the same way when run manually or by 'make' or 'prove', but both 'make' and 'prove' pay attention to the test script's exit status, and even a silently aborted test script would then trigger those tools' usual noticable error messages. [4] Strictly speaking, not all those 'error' calls need that redirection to send their output to the terminal, see e.g. 'test_expect_success' in the opening example, but I think it's better to be consistent. Signed-off-by: SZEDER Gábor <> Signed-off-by: Junio C Hamano <>
2018-10-30Merge branch 'jc/receive-deny-current-branch-fix'Junio C Hamano
The receive.denyCurrentBranch=updateInstead codepath kicked in even when the push should have been rejected due to other reasons, such as it does not fast-forward or the update-hook rejects it, which has been corrected. * jc/receive-deny-current-branch-fix: receive: denyCurrentBranch=updateinstead should not blindly update
2018-10-19receive: denyCurrentBranch=updateinstead should not blindly updateJunio C Hamano
The handling of receive.denyCurrentBranch=updateInstead was added to a switch statement that handles other values of the variable, but all the other case arms only checked a condition to reject the attempted push, or let later logic in the same function to still intervene, so that a push that does not fast-forward (which is checked after the switch statement in question) is still rejected. But the handling of updateInstead incorrectly took immediate effect, without giving other checks a chance to intervene. Instead of calling update_worktree() that causes the side effect immediately, just note the fact that we will need to call the function later, and first give other checks a chance to reject the request. After the update-hook gets a chance to reject the push (which happens as the last step in a series of checks), call update_worktree() when we earlier detected the need to. Reported-by: Rajesh Madamanchi Signed-off-by: Junio C Hamano <>
2018-09-17Merge branch 'ab/fetch-tags-noclobber'Junio C Hamano
The rules used by "git push" and "git fetch" to determine if a ref can or cannot be updated were inconsistent; specifically, fetching to update existing tags were allowed even though tags are supposed to be unmoving anchoring points. "git fetch" was taught to forbid updates to existing tags without the "--force" option. * ab/fetch-tags-noclobber: fetch: stop clobbering existing tags without --force fetch: document local ref updates with/without --force push doc: correct lies about how push refspecs work push doc: move mention of "tag <tag>" later in the prose push doc: remove confusing mention of remote merger fetch tests: add a test for clobbering tag behavior push tests: use spaces in interpolated string push tests: make use of unused $1 in test description fetch: change "branch" to "reference" in --force -h output
2018-08-31fetch: stop clobbering existing tags without --forceÆvar Arnfjörð Bjarmason
Change "fetch" to treat "+" in refspecs (aka --force) to mean we should clobber a local tag of the same name. This changes the long-standing behavior of "fetch" added in 853a3697dc ("[PATCH] Multi-head fetch.", 2005-08-20). Before this change, all tag fetches effectively had --force enabled. See the git-fetch-script code in fast_forward_local() with the comment: > Tags need not be pointing at commits so there is no way to > guarantee "fast-forward" anyway. That commit and the rest of the history of "fetch" shows that the "+" (--force) part of refpecs was only conceived for branch updates, while tags have accepted any changes from upstream unconditionally and clobbered the local tag object. Changing this behavior has been discussed as early as 2011[1]. The current behavior doesn't make sense to me, it easily results in local tags accidentally being clobbered. We could namespace our tags per-remote and not locally populate refs/tags/*, but as with my 97716d217c ("fetch: add a --prune-tags option and fetch.pruneTags config", 2018-02-09) it's easier to work around the current implementation than to fix the root cause. So this change implements suggestion #1 from Jeff's 2011 E-Mail[1], "fetch" now only clobbers the tag if either "+" is provided as part of the refspec, or if "--force" is provided on the command-line. This also makes it nicely symmetrical with how "tag" itself works when creating tags. I.e. we refuse to clobber any existing tags unless "--force" is supplied. Now we can refuse all such clobbering, whether it would happen by clobbering a local tag with "tag", or by fetching it from the remote with "fetch". Ref updates outside refs/{tags,heads/* are still still not symmetrical with how "git push" works, as discussed in the recently changed pull-fetch-param.txt documentation. This change brings the two divergent behaviors more into line with one another. I don't think there's any reason "fetch" couldn't fully converge with the behavior used by "push", but that's a topic for another change. One of the tests added in 31b808a032 ("clone --single: limit the fetch refspec to fetched branch", 2012-09-20) is being changed to use --force where a clone would clobber a tag. This changes nothing about the existing behavior of the test. 1. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-08-31fetch tests: add a test for clobbering tag behaviorÆvar Arnfjörð Bjarmason
The test suite only incidentally (and unintentionally) tested for the current behavior of eager tag clobbering on "fetch". This is a followup to 380efb65df ("push tests: assert re-pushing annotated tags", 2018-07-31) which tests for it explicitly. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-08-31push tests: use spaces in interpolated stringÆvar Arnfjörð Bjarmason
The quoted -m'msg' option would mean the same as -mmsg when passed through the test_force_push_tag helper. Let's instead use a string with spaces in it, to have a working example in case we need to pass other whitespace-delimited arguments to git-tag. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-08-31push tests: make use of unused $1 in test descriptionÆvar Arnfjörð Bjarmason
Fix up a logic error in 380efb65df ("push tests: assert re-pushing annotated tags", 2018-07-31), where the $tag_type_description variable was assigned to but never used, unlike in the subsequently added companion test for fetches in 2d216a7ef6 ("fetch tests: add a test for clobbering tag behavior", 2018-04-29). Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-08-20Merge branch 'ab/fetch-tags-noclobber'Junio C Hamano
Test and doc clean-ups. * ab/fetch-tags-noclobber: pull doc: fix a long-standing grammar error fetch tests: correct a comment "remove it" -> "remove them" push tests: assert re-pushing annotated tags push tests: add more testing for forced tag pushing push tests: fix logic error in "push" test assertion push tests: remove redundant 'git push' invocation fetch tests: change "Tag" test tag to "testTag"
2018-07-31push tests: assert re-pushing annotated tagsÆvar Arnfjörð Bjarmason
Change the test that asserts that lightweight tags can only be clobbered by a force-push to check do the same tests for annotated tags. There used to be less exhaustive tests for this with the code added in 40eff17999 ("push: require force for annotated tags", 2012-11-29), but Junio removed them in 256b9d70a4 ("push: fix "refs/tags/ hierarchy cannot be updated without --force"", 2013-01-16) while fixing some of the behavior around tag pushing. That change left us without any coverage asserting that pushing and clobbering annotated tags worked as intended. There was no reason to suspect that the receive machinery wouldn't behave the same way with annotated tags, but now we know for sure. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-07-31push tests: add more testing for forced tag pushingÆvar Arnfjörð Bjarmason
Improve the tests added in dbfeddb12e ("push: require force for refs under refs/tags/", 2012-11-29) to assert that the same behavior applies various other combinations of command-line option and refspecs. Supplying either "+" in refspec or "--force" is sufficient to clobber the reference. With --no-force we still pay attention to "+" in the refspec, and vice-versa with clobbering kicking in if there's no "+" in the refspec but "+" is given. This is consistent with how refspecs work for branches, where either "+" or "--force" will enable clobbering, with neither taking priority over the other. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-07-31push tests: fix logic error in "push" test assertionÆvar Arnfjörð Bjarmason
Fix a logic error that's been here since this test was added in dbfeddb12e ("push: require force for refs under refs/tags/", 2012-11-29). The intent of this test is to force-create a new tag pointing to HEAD~, and then assert that pushing it doesn't work without --force. Instead, the code was not creating a new tag at all, and then failing to push the previous tag for the unrelated reason of providing a refspec that doesn't make any sense. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-07-31push tests: remove redundant 'git push' invocationÆvar Arnfjörð Bjarmason
Remove an invocation of 'git push' that's exactly the same as the one on the preceding line. This was seemingly added by mistake in dbfeddb12e ("push: require force for refs under refs/tags/", 2012-11-29) and doesn't affect the result of the test, the second "push" was a no-op as there was nothing new to push. Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-07-31fetch tests: change "Tag" test tag to "testTag"Ævar Arnfjörð Bjarmason
Calling the test tag "Tag" will make for confusing reading later in this series when making use of the "git push tag <name>" feature. Let's call the tag testTag instead. Changes code initially added in dbfeddb12e ("push: require force for refs under refs/tags/", 2012-11-29). Signed-off-by: Ævar Arnfjörð Bjarmason <> Signed-off-by: Junio C Hamano <>
2018-07-16t5000-t5999: fix broken &&-chainsEric Sunshine
Signed-off-by: Eric Sunshine <> Signed-off-by: Junio C Hamano <>
2018-06-01Merge branch 'bw/ref-prefix-for-configured-refspec'Junio C Hamano
* bw/ref-prefix-for-configured-refspec: fetch: do not pass ref-prefixes for fetch by exact SHA1
2018-06-01fetch: do not pass ref-prefixes for fetch by exact SHA1Jonathan Nieder
When v2.18.0-rc0~10^2~1 (refspec: consolidate ref-prefix generation logic, 2018-05-16) factored out the ref-prefix generation code for reuse, it left out the 'if (!item->exact_sha1)' test in the original ref-prefix generation code. As a result, fetches by SHA-1 generate ref-prefixes as though the SHA-1 being fetched were an abbreviated ref name: $ GIT_TRACE_PACKET=1 bin-wrappers/git -c protocol.version=2 \ fetch origin 12039e008f9a4e3394f3f94f8ea897785cb09448 [...] packet: fetch> ref-prefix 12039e008f9a4e3394f3f94f8ea897785cb09448 packet: fetch> ref-prefix refs/12039e008f9a4e3394f3f94f8ea897785cb09448 packet: fetch> ref-prefix refs/tags/12039e008f9a4e3394f3f94f8ea897785cb09448 packet: fetch> ref-prefix refs/heads/12039e008f9a4e3394f3f94f8ea897785cb09448 packet: fetch> ref-prefix refs/remotes/12039e008f9a4e3394f3f94f8ea897785cb09448 packet: fetch> ref-prefix refs/remotes/12039e008f9a4e3394f3f94f8ea897785cb09448/HEAD packet: fetch> 0000 If there is another ref name on the command line or the object being fetched is already available locally, then that's mostly harmless. But otherwise, we error out with fatal: no matching remote head since the server did not send any refs we are interested in. Filter out the exact_sha1 refspecs to avoid this. This patch adds a test to check this behavior that notices another behavior difference between protocol v0 and v2 in the process. Add a NEEDSWORK comment to clear it up. Signed-off-by: Jonathan Nieder <> Signed-off-by: Junio C Hamano <>
2018-05-30Merge branch 'bc/hash-independent-tests'Junio C Hamano
Many tests hardcode the raw object names, which would change once we migrate away from SHA-1. While some of them must test against exact object names, most of them do not have to use hardcoded constants in the test. The latter kind of tests have been updated to test the moral equivalent of the original without hardcoding the actual object names. * bc/hash-independent-tests: (28 commits) t5300: abstract away SHA-1-specific constants t4208: abstract away SHA-1-specific constants t4045: abstract away SHA-1-specific constants t4042: abstract away SHA-1-specific constants t4205: sort log output in a hash-independent way t/lib-diff-alternative: abstract away SHA-1-specific constants t4030: abstract away SHA-1-specific constants t4029: abstract away SHA-1-specific constants t4029: fix test indentation t4022: abstract away SHA-1-specific constants t4020: abstract away SHA-1-specific constants t4014: abstract away SHA-1-specific constants t4008: abstract away SHA-1-specific constants t4007: abstract away SHA-1-specific constants t3905: abstract away SHA-1-specific constants t3702: abstract away SHA-1-specific constants t3103: abstract away SHA-1-specific constants t2203: abstract away SHA-1-specific constants t: skip pack tests if not using SHA-1 t4044: skip test if not using SHA-1 ...
2018-05-23Merge branch 'sg/t5516-fixes'Junio C Hamano
Test fixes. * sg/t5516-fixes: t5516-fetch-push: fix broken &&-chain t5516-fetch-push: fix 'push with dry-run' test
2018-05-14t: switch $_z40 to $ZERO_OIDbrian m. carlson
Switch all uses of $_z40 to $ZERO_OID so that they work correctly with larger hashes. This commit was created by using the following sed command to modify all files in the t directory except t/ sed -i 's/\$_z40/$ZERO_OID/g' Signed-off-by: brian m. carlson <> Signed-off-by: Junio C Hamano <>
2018-05-11t5516-fetch-push: fix broken &&-chainSZEDER Gábor
b2dc968e60 (t5516: refactor oddball tests, 2008-11-07) accidentaly broke the &&-chain in the test 'push does not update local refs on failure', but since it was in a subshell, chain-lint couldn't notice it. Signed-off-by: SZEDER Gábor <> Signed-off-by: Junio C Hamano <>