ref_transaction_commit(): provide better error messages
Now that lock_ref_sha1_basic() gives us back its error messages via a
strbuf, incorporate its error message into our error message rather
than emitting one error messages to stderr immediately and returning a
second to our caller.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
rename_ref(): integrate lock_ref_sha1_basic() errors into ours
Now that lock_ref_sha1_basic() gives us back its error messages via a
strbuf, incorporate its error message into our error message rather
than emitting two separate error messages.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
lock_ref_sha1_basic(): improve diagnostics for ref D/F conflicts
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>
lock_ref_sha1_basic(): report errors via a "struct strbuf *err"
For now, change the callers to spew the error to stderr like before.
But soon we will change them to incorporate the reason for the failure
into their own error messages.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Rename is_refname_available() to verify_refname_available() and change
its return value from 1 for success to 0 for success, to be consistent
with our error-handling convention. In a moment it will also get a
"struct strbuf *err" parameter.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
refs: check for D/F conflicts among refs created in a transaction
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>
ref_transaction_commit(): use a string_list for detecting duplicates
Detect duplicates by storing the reference names in a string_list and
sorting that, instead of sorting the ref_updates directly.
* In a moment the string_list will be used for another purpose, too.
* This removes the need for the custom comparison function
ref_update_compare().
* This means that we can carry out the updates in the order that the
user specified them instead of reordering them. This might be handy
someday if, we want to permit multiple updates to a single reference
as long as they are compatible with each other.
Note: we can't use string_list_remove_duplicates() to check for
duplicates, because we need to know the name of the reference that
appeared multiple times, to be used in the error message.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
In the first loop (over prefixes of refname), use dirname to keep
track of the current prefix. This is not an improvement in itself, but
in a moment we will start using dirname for a role where a
NUL-terminated string is needed.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
It wasn't pulling its weight. And we are about to need code similar to
this where no ref_entry is available and with more diverse error
messages. Rather than try to generalize the function, just inline it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
It wasn't pulling its weight. And in a moment we will need similar
tests that take a refname rather than a ref_entry as parameter, which
would have made entry_matches() even less useful.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
The function had a "dir" parameter that was shadowed by a local "dir"
variable within a code block. Use the former in place of the latter.
(This is consistent with "dir"'s use elsewhere in the function.)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
t1404: new tests of ref D/F conflicts within transactions
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>
Merge branch 'tb/connect-ipv6-parse-fix' into maint
An earlier update to the parser that disects a URL broke an
address, followed by a colon, followed by an empty string (instead
of the port number), e.g. ssh://example.com:/path/to/repo.
* tb/connect-ipv6-parse-fix:
connect.c: ignore extra colon after hostname
The "git push --signed" protocol extension did not limit what the
"nonce" that is a server-chosen string can contain or how long it
can be, which was unnecessarily lax. Limit both the length and the
alphabet to a reasonably small space that can still have enough
entropy.
* jc/push-cert:
push --signed: tighten what the receiving end can ask to sign
"diff-highlight" (in contrib/) used to show byte-by-byte
differences, which meant that multi-byte characters can be chopped
in the middle. It learned to pay attention to character boundaries
(assuming the UTF-8 payload).
* jk/colors:
diff-highlight: do not split multibyte characters
* 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
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
* 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
The "git push --signed" protocol extension did not limit what the
"nonce" that is a server-chosen string can contain or how long it
can be, which was unnecessarily lax. Limit both the length and the
alphabet to a reasonably small space that can still have enough
entropy.
* jc/push-cert:
push --signed: tighten what the receiving end can ask to sign
* tag 'gitgui-0.20.0' of http://repo.or.cz/r/git-gui:
git-gui: set version 0.20
git-gui: sv.po: Update Swedish translation (547t0f0u)
git-gui i18n: Updated Bulgarian translation (547t,0f,0u)
git-gui: Makes chooser set 'gitdir' to the resolved path
git-gui: Fixes chooser not accepting gitfiles
git-gui: reinstate support for Tcl 8.4
git-gui: fix problem with gui.maxfilesdisplayed
git-gui: fix verbose loading when git path contains spaces.
git-gui/gitk: Do not depend on Cygwin's "kill" command on Windows
git-gui: add configurable tab size to the diff view
git-gui: Make git-gui lib dir configurable at runime
git-gui i18n: Updated Bulgarian translation (520t,0f,0u)
L10n: vi.po (543t): Init translation for Vietnamese
git-gui: align the new recursive checkbox with the radiobuttons.
git-gui: Add a 'recursive' checkbox in the clone menu.
Revert "merge: pass verbosity flag down to merge-recursive"
This reverts commit 2bf15a3330a26183adc8563dbeeacc11294b8a01, whose
intention was good, but the verbosity levels used in merge-recursive
turns out to be rather uneven. For example, a merge of two branches
with conflicting submodule updates used to report CONFLICT: output
with --quiet but no longer (which *is* desired), while the final
"Automatic merge failed; fix conflicts and then commit" message is
still shown even with --quiet (which *is* inconsistent).
Originally reported by Bryan Turner; it is too early to declare what
the concensus is, but it seems that we would need to level the
verbosity levels used in merge strategy backends before we can go
forward. In the meantime, we'd revert to the old behaviour until
that happens.
"diff-highlight" (in contrib/) used to show byte-by-byte
differences, which meant that multi-byte characters can be chopped
in the middle. It learned to pay attention to character boundaries
(assuming the UTF-8 payload).
* jk/colors:
diff-highlight: do not split multibyte characters
Changed inaccurate count of "rough rules" from three to the more
generic 'a few'.
Signed-off-by: Julian Gindi <juliangindi@gmail.com> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "help-all" option is being initialized with a wrong value.
While being semantically wrong this can also cause a segmentation
fault in gcc on ARMv7 hardfloat platforms with a hardened
toolchain. Fix this by initializing with a NULL value.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Reviewed-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t9814: guarantee only one source exists in git-p4 copy tests
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>
When the input is UTF-8 and Perl is operating on bytes instead of
characters, a diff that changes one multibyte character to another
that shares an initial byte sequence will result in a broken diff
display as the common byte sequence prefix will be separated from
the rest of the bytes in the multibyte character.
For example, if a single line contains only the unicode character
U+C9C4 (encoded as UTF-8 0xEC, 0xA7, 0x84) and that line is then
changed to the unicode character U+C9C0 (encoded as UTF-8 0xEC,
0xA7, 0x80), when operating on bytes diff-highlight will show only
the single byte change from 0x84 to 0x80 thus creating invalid UTF-8
and a broken diff display.
Fix this by putting Perl into character mode when splitting the line
and then back into byte mode after the split is finished.
The utf8::xxx functions require Perl 5.8 so we require that as well.
Also, since we are mucking with code in the split_line function, we
change a '*' quantifier to a '+' quantifier when matching the $COLOR
expression which has the side effect of speeding everything up while
eliminating useless '' elements in the returned array.
Reported-by: Yi EungJun <semtlenori@gmail.com> Signed-off-by: Kyle J. McKay <mackyle@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge: pass verbosity flag down to merge-recursive
This makes "git merge --quiet" really quiet when we call
into merge-recursive.
Note that we can't just pass our flag down as-is; the two
parts of the code use different scales. We center at "0" as
normal for git-merge (with "--quiet" giving a negative
value), but merge-recursive uses "2" as its center. This
patch passes a negative value to merge-recursive rather than
"1", though, as otherwise the user would have to use "-qqq"
to squelch all messages (but the downside is that the user
cannot distinguish between levels 0-2 if without resorting
to the GIT_MERGE_VERBOSITY variable).
We may want to review and renormalize the message severities
in merge-recursive, but that does not have to happen now.
This is at least in improvement in the sense that we are
respecting "--quiet" at all.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
push --signed: tighten what the receiving end can ask to sign
Instead of blindly trusting the receiving side to give us a sensible
nonce to sign, limit the length (max 256 bytes) and the alphabet
(alnum and a few selected punctuations, enough to encode in base64)
that can be used in nonce.
send-pack: unify error messages for unsupported capabilities
If --signed is not supported, the error message names the remote
"receiving end". If --atomic is not supported, the error message
names the remote "server". Unify the naming to "receiving end"
as we're in the context of "push".
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
howto: document more tools for recovery corruption
Long ago, I documented a corruption recovery I did and gave
some C code that I used to help find a flipped bit. I had
to fix a similar case recently, and I ended up writing a few
more tools. I hope nobody ever has to use these, but it
does not hurt to share them, just in case.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
push-to-deploy: allow pushing into an unborn branch and updating it
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>