If there is a failure to lock a reference that is likely caused by a
D/F conflict (e.g., trying to lock "refs/foo/bar" when reference
"refs/foo" already exists), invoke verify_refname_available() to try
to generate a more helpful error message.
That function might not detect an error. For example, some
non-reference file might be blocking the deletion of an
otherwise-empty directory tree, or there might be a race with another
process that just deleted the offending reference. In such cases,
generate the strerror-based error message like before.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
If two references that D/F conflict (e.g., "refs/foo" and
"refs/foo/bar") are created in a single transaction, the old code
discovered the problem only after the "commit" phase of
ref_transaction_commit() had already begun. This could leave some
references updated and others not, which violates the promise of
atomicity.
Instead, check for such conflicts during the "locking" phase:
* Teach is_refname_available() to take an "extras" parameter that can
contain extra reference names with which the specified refname must
not conflict.
* Change lock_ref_sha1_basic() to take an "extras" parameter, which it
passes through to is_refname_available().
* Change ref_transaction_commit() to pass "affected_refnames" to
lock_ref_sha1_basic() as its "extras" argument.
This change fixes a test case in t1404.
This code is a bit stricter than it needs to be. We could conceivably
allow reference "refs/foo/bar" to be created in the same transaction
as "refs/foo" is deleted (or vice versa). But that would be
complicated to implement, because it is not possible to lock
"refs/foo/bar" while "refs/foo" exists as a loose reference, but on
the other hand we don't want to delete some references before adding
others (because that could leave a gap during which required objects
are unreachable). There is also a complication that reflog files'
paths can conflict.
Any less-strict implementation would probably require tricks like the
packing of all references before the start of the real transaction, or
the use of temporary intermediate reference names.
So for now let's accept too-strict checks. Some reference update
transactions will be rejected unnecessarily, but they will be rejected
in their entirety rather than leaving the repository in an
intermediate state, as would happen now.
Please note that there is still one kind of D/F conflict that is *not*
handled correctly. If two processes are running at the same time, and
one tries to create "refs/foo" at the same time that the other tries
to create "refs/foo/bar", then they can race with each other. Both
processes can obtain their respective locks ("refs/foo.lock" and
"refs/foo/bar.lock"), proceed to the "commit" phase of
ref_transaction_commit(), and then the slower process will discover
that it cannot rename its lockfile into place (after possibly having
committed changes to other references). There appears to be no way to
fix this race without changing the locking policy, which in turn would
require a change to *all* Git clients.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Add some tests of reference D/F conflicts (by which I mean the fact
that references like "refs/foo" and "refs/foo/bar" are not allowed to
coexist) in the context of reference transactions.
The test of creating two conflicting references in the same
transaction fails, leaving the transaction half-completed. This will
be fixed later in this patch series.
Please note that the error messages emitted in the case of conflicts
are not very user-friendly. In particular, when the conflicts involve
loose references, then the errors are reported as
error: there are still refs under 'refs/foo'
fatal: Cannot lock the ref 'refs/foo'.
or
error: unable to resolve reference refs/foo/bar: Not a directory
fatal: Cannot lock the ref 'refs/foo/bar'.
This is because lock_ref_sha1_basic() fails while trying to lock the
new reference, before it even gets to the is_refname_available()
check. This situation will also be improved later in this patch
series.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
An earlier update to the parser that disects an address broke an
address, followed by a colon, followed by an empty string (instead
of the port number).
* tb/connect-ipv6-parse-fix:
connect.c: ignore extra colon after hostname
Test fixes for git-p4.
* va/fix-git-p4-tests:
t9814: guarantee only one source exists in git-p4 copy tests
git-p4: fix copy detection test
t9814: fix broken shell syntax in git-p4 rename test
A push into an unborn branch, with "receive.denyCurrentBranch" set
to "updateInstead", did not check out the working tree as expected.
* jc/update-instead-into-void:
push-to-deploy: allow pushing into an unborn branch and updating it
By using a tree with multiple identical files and allowing copy detection to
choose any one of them, the check in the test is unnecessarily complex. We can
simplify by:
* Modify source file (file2) before copying the file.
* Check that only file2 is the source in the output of "p4 filelog".
* Remove all "case" statements and replace them with simple tests to check
that source is "file2".
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ignore an extra ':' at the end of the hostname in URL's like
"ssh://example.com:/path/to/repo"
The colon is meant to separate a port number from the hostname.
If the port is empty, the colon should be ignored, see RFC 3986.
It had been working for URLs with ssh:// scheme, but was unintentionally
broken in 86ceb3, "allow ssh://user@[2001:db8::1]/repo.git"
Reported-by: Reid Woodbury Jr. <reidw@rawsound.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Setting receive.denycurrentbranch to updateinstead and pushing into
the current branch, when the working tree and the index is truly
clean, is supposed to reset the working tree and the index to match
the tree of the pushed commit. This did not work when pushing into
an unborn branch.
The code that drives push-to-checkout hook needs no change, as the
interface is defined so that hook can decide what to do when the
push is coming to an unborn branch and take an appropriate action
since the beginning.
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
File file11 is copied from file2 and diff-tree correctly reports
this file as its the source. But it is possible that the diff-tree
algorithm detects file10, which was also copied from file2, as the
origin of the new file.
This fix uses a case statement to support both files as the source
of file11, as was done in other tests in this file.
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An update to the tests in 2.1 era introduced a broken case statements
that lack closing esac.
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
People often forget to chain the commands in their test together
with &&, leaving a failure from an earlier command in the test go
unnoticed. The new GIT_TEST_CHAIN_LINT mechanism allows you to
catch such a mistake more easily.
* jk/test-chain-lint: (36 commits)
t9001: drop save_confirm helper
t0020: use test_* helpers instead of hand-rolled messages
t: simplify loop exit-code status variables
t: fix some trivial cases of ignored exit codes in loops
t7701: fix ignored exit code inside loop
t3305: fix ignored exit code inside loop
t0020: fix ignored exit code inside loops
perf-lib: fix ignored exit code inside loop
t6039: fix broken && chain
t9158, t9161: fix broken &&-chain in git-svn tests
t9104: fix test for following larger parents
t4104: drop hand-rolled error reporting
t0005: fix broken &&-chains
t7004: fix embedded single-quotes
t0050: appease --chain-lint
t9001: use test_when_finished
t4117: use modern test_* helpers
t6034: use modern test_* helpers
t1301: use modern test_* helpers
t0020: use modern test_* helpers
...
"git prune" used to largely ignore broken refs when deciding which
objects are still being used, which could spread an existing small
damage and make it a larger one.
* jk/prune-with-corrupt-refs:
refs.c: drop curate_packed_refs
repack: turn on "ref paranoia" when doing a destructive repack
prune: turn on ref_paranoia flag
refs: introduce a "ref paranoia" flag
t5312: test object deletion code paths in a corrupted repository
"git fetch" that fetches a commit using the allow-tip-sha1-in-want
extension could have failed to fetch all the requested refs.
* jk/fetch-pack:
fetch-pack: remove dead assignment to ref->new_sha1
fetch_refs_via_pack: free extra copy of refs
filter_ref: make a copy of extra "sought" entries
filter_ref: avoid overwriting ref->old_sha1 with garbage
"git log --graph --no-walk A B..." is a otcnflicting request that
asks nonsense; no-walk tells us show discrete points in the
history, while graph asks to draw connections between these
discrete points. Forbid the combination.
* dj/log-graph-with-no-walk:
revision: forbid combining --graph and --no-walk
"git rev-list --bisect --first-parent" does not work (yet) and can
even cause SEGV; forbid it. "git log --bisect --first-parent"
would not be useful until "git bisect --first-parent" materializes,
so it is also forbidden for now.
* kd/rev-list-bisect-first-parent:
rev-list: refuse --first-parent combined with --bisect
The prompt script (in contrib/) did not show the untracked sign
when working in a subdirectory without any untracked files.
* ct/prompt-untracked-fix:
git prompt: use toplevel to find untracked files
The idea of this helper is that we want to save the current
value of a config variable and then restore it again after
the test completes. However, there's no point in actually
saving the value; it should always be restored to the string
"never" (which you can confirm by instrumenting
save_confirm to print the value it finds).
Let's just replace it with a single test_when_finished call.
Suggested-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These tests are not wrong, but it is much shorter and more
idiomatic to say "verbose" or "test_must_fail" rather than
printing our own messages on failure. Likewise, there is no
need to say "happy" at the end of a test; the test suite
takes care of that.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since shell loops may drop the exit code of failed commands
inside the loop, some tests try to keep track of the status
by setting a variable. This can end up cumbersome and hard
to read; it is much simpler to just exit directly from the
loop using "return 1" (since each case is either in a helper
function or inside a test snippet).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These are all cases where we do a setup step of the form:
for i in $foo; do
set_up $i || break
done &&
more_setup
would not notice a failure in set_up (because break always
returns a 0 exit code). These are just setup steps that we
do not expect to fail, but it does not hurt to be defensive.
Most can be fixed by converting the "break" to a "return 1"
(since we eval our tests inside a function for just this
purpose). A few of the loops are inside subshells, so we can
use just "exit 1" to break out of the subshell. And a few
can actually be made shorter by just unrolling the loop.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When checking a list of file mtimes, we use a loop and break
out early from the loop if any entry does not match.
However, the exit code of a loop exited via break is always
0, meaning that the test will fail to notice we had a
mismatch. Since the loop is inside a function, we can fix
this by doing an early "return 1".
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we test deleting notes, we run "git notes remove" in a
loop. However, the exit value of the loop will only reflect
the final note we process. We should break out of the loop
with a failing exit code as soon as we see a problem.
Note that we can call "exit 1" here without explicitly
creating a subshell, because the while loop on the
right-hand side of a pipe executes in its own implicit
subshell.
Note also that the "break" above does not suffer the same
problem; it is meant to exit the loop early at a certain
number of iterations. We can bump it into the conditional of
the loop to make this more obvious.
Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A loop like:
for f in one two; do
something $f ||
break
done
will correctly break out of the loop when we see a failure
of one item, but the resulting exit code will always be
zero. We can fix that by putting the loop into a function or
subshell, but in this case it is simpler still to just
unroll the loop. We do add a helper function, which
hopefully makes the end result even more readable (in
addition to being shorter).
Reported-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When copying the test repository, we try to detect whether
the copy succeeded. However, most of the heavy lifting is
done inside a for loop, where our "break" will lose the exit
code of the failing "cp". We can take advantage of the fact
that we are in a subshell, and just "exit 1" to break out
with a code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test fixes.
* jk/test-annoyances:
t5551: make EXPENSIVE test cheaper
t5541: move run_with_cmdline_limit to test-lib.sh
t: pass GIT_TRACE through Apache
t: redirect stderr GIT_TRACE to descriptor 4
t: translate SIGINT to an exit
The transfer.hiderefs support did not quite work for smart-http
transport.
* jk/smart-http-hide-refs:
upload-pack: do not check NULL return of lookup_unknown_object
upload-pack: fix transfer.hiderefs over smart-http
Add missing &&, detected by the --chain-lint option
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Output from "git log --decorate" mentions HEAD when it points at a
tip of an branch differently from a detached HEAD.
This is a potentially backward-incompatible change.
* mg/log-decorate-HEAD:
log: decorate HEAD with branch name
"git log --decorate" did not reset colors correctly around the
branch names.
* jc/decorate-leaky-separator-color:
log --decorate: do not leak "commit" color into the next item
Documentation/config.txt: simplify boolean description in the syntax section
Documentation/config.txt: describe 'color' value type in the "Values" section
Documentation/config.txt: have a separate "Values" section
Documentation/config.txt: describe the structure first and then meaning
Documentation/config.txt: explain multi-valued variables once
Documentation/config.txt: avoid unnecessary negation
Workarounds for certain build of GPG that triggered false breakage
in a test.
* mg/verify-commit:
t7510: do not fail when gpg warns about insecure memory
Portability fixes and workarounds for shell scripts have been added
to help BSD-derived systems.
* km/bsd-shells:
t5528: do not fail with FreeBSD shell
help.c: use SHELL_PATH instead of hard-coded "/bin/sh"
git-compat-util.h: move SHELL_PATH default into header
git-instaweb: use @SHELL_PATH@ instead of /bin/sh
git-instaweb: allow running in a working tree subdirectory
"git branch" on a detached HEAD always said "(detached from xyz)",
even when "git status" would report "detached at xyz". The HEAD is
actually at xyz and haven't been moved since it was detached in
such a case, but the user cannot read what the current value of
HEAD is when "detached from" is used.
* mg/detached-head-report:
branch: name detached HEAD analogous to status
wt-status: refactor detached HEAD analysis
"git -C '' subcmd" refused to work in the current directory, unlike
"cd ''" which silently behaves as a no-op.
* kn/git-cd-to-empty:
git: treat "git -C '<path>'" as a no-op when <path> is empty
The versionsort.prerelease configuration variable can be used to
specify that v1.0-pre1 comes before v1.0.
* nd/versioncmp-prereleases:
config.txt: update versioncmp.prereleaseSuffix
versionsort: support reorder prerelease suffixes
When we delete a ref, we have to rewrite the entire
packed-refs file. We take this opportunity to "curate" the
packed-refs file and drop any entries that are crufty or
broken.
Dropping broken entries (e.g., with bogus names, or ones
that point to missing objects) is actively a bad idea, as it
means that we lose any notion that the data was there in the
first place. Aside from the general hackiness that we might
lose any information about ref "foo" while deleting an
unrelated ref "bar", this may seriously hamper any attempts
by the user at recovering from the corruption in "foo".
They will lose the sha1 and name of "foo"; the exact pointer
may still be useful even if they recover missing objects
from a different copy of the repository. But worse, once the
ref is gone, there is no trace of the corruption. A
follow-up "git prune" may delete objects, even though it
would otherwise bail when seeing corruption.
We could just drop the "broken" bits from
curate_packed_refs, and continue to drop the "crufty" bits:
refs whose loose counterpart exists in the filesystem. This
is not wrong to do, and it does have the advantage that we
may write out a slightly smaller packed-refs file. But it
has two disadvantages:
1. It is a potential source of races or mistakes with
respect to these refs that are otherwise unrelated to
the operation. To my knowledge, there aren't any active
problems in this area, but it seems like an unnecessary
risk.
2. We have to spend time looking up the matching loose
refs for every item in the packed-refs file. If you
have a large number of packed refs that do not change,
that outweighs the benefit from writing out a smaller
packed-refs file (it doesn't get smaller, and you do a
bunch of directory traversal to find that out).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we are repacking with "-ad", we will drop any unreachable
objects. Likewise, using "-Ad --unpack-unreachable=<time>"
will drop any old, unreachable objects. In these cases, we
want to make sure the reachability we compute with "--all"
is complete. We can do this by passing GIT_REF_PARANOIA=1 in
the environment to pack-objects.
Note that "-Ad" is safe already, because it only loosens
unreachable objects. It is up to "git prune" to avoid
deleting them.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Prune should know about broken objects at the tips of refs,
so that we can feed them to our traversal rather than
ignoring them. It's better for us to abort the operation on
the broken object than it is to start deleting objects with
an incomplete view of the reachability namespace.
Note that for missing objects, aborting is the best we can
do. For a badly-named ref, we technically could use its sha1
as a reachability tip. However, the iteration code just
feeds us a null sha1, so there would be a reasonable amount
of code involved to pass down our wishes. It's not really
worth trying to do better, because this is a case that
should happen extremely rarely, and the message we provide:
fatal: unable to parse object: refs/heads/bogus:name
is probably enough to point the user in the right direction.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we are doing a destructive operation like "git prune",
we want to be extra careful that the set of reachable tips
we compute is valid. If there is any corruption or oddity,
we are better off aborting the operation and letting the
user figure things out rather than plowing ahead and
possibly deleting some data that cannot be recovered.
The tests here include:
1. Pruning objects mentioned only be refs with invalid
names. This used to abort prior to d0f810f (refs.c:
allow listing and deleting badly named refs,
2014-09-03), but since then we silently ignore the tip.
Likewise, we test repacking that can drop objects
(either "-ad", which drops anything unreachable,
or "-Ad --unpack-unreachable=<time>", which tries to
optimize out a loose object write that would be
directly pruned).
2. Pruning objects when some refs point to missing
objects. We don't know whether any dangling objects
would have been reachable from the missing objects. We
are better to keep them around, as they are better than
nothing for helping the user recover history.
3. Packed refs that point to missing objects can sometimes
be dropped. By itself, this is more of an annoyance
(you do not have the object anyway; even if you can
recover it from elsewhere, all you are losing is a
placeholder for your state at the time of corruption).
But coupled with (2), if we drop the ref and then go
on to prune, we may lose unrecoverable objects.
Note that we use test_might_fail for some of the operations.
In some cases, it would be appropriate to abort the
operation, and in others, it might be acceptable to continue
but taking the information into account. The tests don't
care either way, and check only for data loss.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The different index versions have different sha-1 checksums. Those
checksums are checked in t1700, which makes it fail when the test suite
is run with TEST_GIT_INDEX_VERSION=4. Fix it.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All of these cases are moderate since they would most probably not
lead to missed failing tests; either they would fail otherwise, or
fail a rm in test_when_finished only.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test is special for several reasons:
It ends with a "true" statement, which should be a no-op.
It is not because the &&-chain is broken right before it.
Also, looking at what the test intended to test according to
7f578c5 (git-svn: --follow-parent now works on sub-directories of larger
branches, 2007-01-24)
it is not clear how it would achieve that with the given steps.
Amend the test to include the second svn id to be tested for, and
change the tested refs to the ones which are to be expected, and which
make the test pass.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This use of "||" fools --chain-lint into thinking the
&&-chain is broken (and indeed, it is somewhat broken; a
failure of update-index in these tests would show the patch
file, even if we never got to the part of the test where we
fed the patch to git-apply).
The extra blocks were there to include more debugging
output, but it hardly seems worth it; the user should know
which command failed (because git-apply will produce error
messages) and can look in the trash directory themselves.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ":" noop command always returns true, so it is fine to
include these lines in an &&-chain (and it appeases
--chain-lint).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test uses single quotes inside the single-quoted test
snippet, which effectively makes the contents unquoted.
Since they don't need quoted anyway, this isn't a problem,
but let's switch them to double-quotes to make it more
obviously correct.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some of the symlink tests check an either-or case using the
"||". This is not wrong, but fools --chain-lint into
thinking the &&-chain is broken (in fact, there is no &&
chain here).
We can solve this by wrapping the "||" inside a {} block.
This is a bit more verbose, but this construct is rare, and
the {} block helps call attention to it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The confirmation tests in t9001 all save the value of
sendemail.confirm, do something to it, then restore it at
the end, in a way that breaks the &&-chain (they are not
wrong, because they save the $? value, but it fools
--chain-lint).
Instead, they can all use test_when_finished, and we can
even make the code simpler by factoring out the shared
lines.
Note that we can _almost_ use test_config here, except that:
1. We do not restore the config with test_unconfig, but by
setting it back to some prior value.
2. We are not always setting a config variable. Sometimes
the change to be undone is unsetting it entirely.
We could teach test_config to handle these cases, but it's
not worth the complexity for a single call-site.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We can use test_must_fail and test_path_* to avoid some
hand-rolled if statements. This makes the code shorter, and
makes it more obvious when we are breaking the &&-chain.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These say roughly the same thing as the hand-rolled
messages. We do lose the "merge did not complete" debug
message, but merge and write-tree are prefectly capable of
writing useful error messages when they fail.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
