summaryrefslogtreecommitdiff
path: root/t/t5516-fetch-push.sh
AgeCommit message (Collapse)AuthorFilesLines
2022-02-09Merge branch 'gc/fetch-negotiate-only-early-return'Libravatar Junio C Hamano1-0/+12
"git fetch --negotiate-only" is an internal command used by "git push" to figure out which part of our history is missing from the other side. It should never recurse into submodules even when fetch.recursesubmodules configuration variable is set, nor it should trigger "gc". The code has been tightened up to ensure it only does common ancestry discovery and nothing else. * gc/fetch-negotiate-only-early-return: fetch: help translators by reusing the same message template fetch --negotiate-only: do not update submodules fetch: skip tasks related to fetching objects fetch: use goto cleanup in cmd_fetch()
2022-01-18fetch --negotiate-only: do not update submodulesLibravatar Glen Choo1-0/+12
`git fetch --negotiate-only` is an implementation detail of push negotiation and, unlike most `git fetch` invocations, does not actually update the main repository. Thus it should not update submodules even if submodule recursion is enabled. This is not just slow, it is wrong e.g. push negotiation with "submodule.recurse=true" will cause submodules to be updated because it invokes `git fetch --negotiate-only`. Fix this by disabling submodule recursion if --negotiate-only was given. Since this makes --negotiate-only and --recurse-submodules incompatible, check for this invalid combination and die. This does not use the "goto cleanup" introduced in the previous commit because we want to recurse through submodules whenever a ref is fetched, and this can happen without introducing new objects. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-03Merge branch 'es/test-chain-lint'Libravatar Junio C Hamano1-4/+1
Broken &&-chains in the test scripts have been corrected. * es/test-chain-lint: t6000-t9999: detect and signal failure within loop t5000-t5999: detect and signal failure within loop t4000-t4999: detect and signal failure within loop t0000-t3999: detect and signal failure within loop tests: simplify by dropping unnecessary `for` loops tests: apply modern idiom for exiting loop upon failure tests: apply modern idiom for signaling test failure tests: fix broken &&-chains in `{...}` groups tests: fix broken &&-chains in `$(...)` command substitutions tests: fix broken &&-chains in compound statements tests: use test_write_lines() to generate line-oriented output tests: simplify construction of large blocks of text t9107: use shell parameter expansion to avoid breaking &&-chain t6300: make `%(raw:size) --shell` test more robust t5516: drop unnecessary subshell and command invocation t4202: clarify intent by creating expected content less cleverly t1020: avoid aborting entire test script when one test fails t1010: fix unnoticed failure on Windows t/lib-pager: use sane_unset() to avoid breaking &&-chain
2021-12-21Merge branch 'ak/protect-any-current-branch'Libravatar Junio C Hamano1-0/+32
"git fetch" without the "--update-head-ok" option ought to protect a checked out branch from getting updated, to prevent the working tree that checks it out to go out of sync. The code was written before the use of "git worktree" got widespread, and only checked the branch that was checked out in the current worktree, which has been updated. (originally called ak/fetch-not-overwrite-any-current-branch) * ak/protect-any-current-branch: branch: protect branches checked out in all worktrees receive-pack: protect current branch for bare repository worktree receive-pack: clean dead code from update_worktree() fetch: protect branches checked out in all worktrees worktree: simplify find_shared_symref() memory ownership model branch: lowercase error messages receive-pack: lowercase error messages fetch: lowercase error messages
2021-12-13t5516: drop unnecessary subshell and command invocationLibravatar Eric Sunshine1-4/+1
To create its "expect" file, this test pipes into `sort` the output of `git for-each-ref` and a copy of that same output but with a minor textual transformation applied. To do so, it employs a subshell and commands `cat` and `sed` even though the same result can be accomplished by `sed` alone (without a subshell). Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01receive-pack: protect current branch for bare repository worktreeLibravatar Anders Kaseorg1-0/+14
A bare repository won’t have a working tree at "..", but it may still have separate working trees created with git worktree. We should protect the current branch of such working trees from being updated or deleted, according to receive.denyCurrentBranch. Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01fetch: protect branches checked out in all worktreesLibravatar Anders Kaseorg1-0/+18
Refuse to fetch into the currently checked out branch of any working tree, not just the current one. Fixes this previously reported bug: https://lore.kernel.org/git/cb957174-5e9a-5603-ea9e-ac9b58a2eaad@mathema.de/ As a side effect of using find_shared_symref, we’ll also refuse the fetch when we’re on a detached HEAD because we’re rebasing or bisecting on the branch in question. This seems like a sensible change. Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-18t5516: add test case for pushing remote refspecsLibravatar Glen Choo1-0/+9
"git push remote-name" (that is, with no refspec given on the command line) should push the refspecs in remote.remote-name.push. There is no test case that checks this behavior in detached HEAD, so add one. Signed-off-by: Glen Choo <chooglen@google.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27refs: turn on GIT_REF_PARANOIA by defaultLibravatar Jeff King1-3/+4
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 <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27t5516: don't use HEAD ref for invalid ref-deletion testsLibravatar Jeff King1-9/+9
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 <peff@peff.net> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-15send-pack: fix push nego. when remote has refsLibravatar Jonathan Tan1-1/+3
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 <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-05send-pack: support push negotiationLibravatar Jonathan Tan1-0/+35
Teach Git the push.negotiate config variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-25Merge branch 'jc/push-delete-nothing'Libravatar Junio C Hamano1-0/+5
"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 pushLibravatar Junio C Hamano1-0/+5
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 <tilman.vogel@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-25Merge branch 'js/default-branch-name-tests-final-stretch'Libravatar Junio C Hamano1-259/+255
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-09t5516: loosen "not our ref" error checkLibravatar Jeff King1-1/+3
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 <szeder.dev@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-30refspec: make @ a synonym of HEADLibravatar Felipe Contreras1-51/+56
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 <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-30tests: push: trivial cleanupLibravatar Felipe Contreras1-2/+1
No need to do two checkouts. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-30tests: push: improve cleanup of HEAD testsLibravatar Felipe Contreras1-1/+3
So that we are not left in an inconsistent state between them. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19t551*: adjust the references to the default branch name "main"Libravatar Johannes Schindelin1-251/+251
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 <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-09-25Merge branch 'jx/proc-receive-hook'Libravatar Junio C Hamano1-1/+1
"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 branchLibravatar Jiang Xin1-1/+1
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 <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-06Merge branch 'js/pu-to-seen'Libravatar Junio C Hamano1-8/+8
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 referencedLibravatar Johannes Schindelin1-8/+8
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-17send-pack: fix inconsistent porcelain outputLibravatar Jiang Xin1-0/+1
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 <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-05Merge branch 'hv/receive-denycurrent-everywhere'Libravatar Junio C Hamano1-0/+11
"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 worktreesLibravatar Hariom Verma1-0/+11
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 <johannes.schindelin@gmx.de> Signed-off-by: Hariom Verma <hariom18599@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-15test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriateLibravatar Jonathan Nieder1-6/+6
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 <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-25Merge branch 'jk/fetch-reachability-error-fix'Libravatar Junio C Hamano1-4/+16
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'Libravatar Junio C Hamano1-2/+2
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 tipsLibravatar Jeff King1-0/+11
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-15pkt-line: prepare buffer before handling ERR packetsLibravatar Jeff King1-1/+1
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-15upload-pack: send ERR packet for non-tip objectsLibravatar Jeff King1-1/+2
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-15t5516: drop ok=sigpipe from unreachable-want testsLibravatar Jeff King1-3/+3
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-02tests (push): do not abbreviate the `--follow-tags` optionLibravatar Johannes Schindelin1-2/+2
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07tests: always test fetch of unreachable with v0Libravatar Jonathan Tan1-5/+17
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 <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-20tests: send "bug in the test script" errors to the script's stderrLibravatar SZEDER Gábor1-1/+1
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. './t1234-foo.sh' as opposed to 'make t1234-foo.sh' 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 'test-lib.sh', 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 <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-30Merge branch 'jc/receive-deny-current-branch-fix'Libravatar Junio C Hamano1-1/+7
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 updateLibravatar Junio C Hamano1-1/+7
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 <gitster@pobox.com>
2018-09-17Merge branch 'ab/fetch-tags-noclobber'Libravatar Junio C Hamano1-2/+27
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 --forceLibravatar Ævar Arnfjörð Bjarmason1-2/+3
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. https://public-inbox.org/git/20111123221658.GA22313@sigill.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-31fetch tests: add a test for clobbering tag behaviorLibravatar Ævar Arnfjörð Bjarmason1-0/+24
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-31push tests: use spaces in interpolated stringLibravatar Ævar Arnfjörð Bjarmason1-1/+1
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-31push tests: make use of unused $1 in test descriptionLibravatar Ævar Arnfjörð Bjarmason1-1/+1
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-20Merge branch 'ab/fetch-tags-noclobber'Libravatar Junio C Hamano1-20/+45
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 tagsLibravatar Ævar Arnfjörð Bjarmason1-37/+45
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31push tests: add more testing for forced tag pushingLibravatar Ævar Arnfjörð Bjarmason1-2/+20
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31push tests: fix logic error in "push" test assertionLibravatar Ævar Arnfjörð Bjarmason1-2/+2
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31push tests: remove redundant 'git push' invocationLibravatar Ævar Arnfjörð Bjarmason1-1/+0
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 <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>