f8da6801 (am --skip: support skipping while on unborn branch,
2015-06-06) introduced a performance regression to "git am --skip",
where it used "read-tree" to reconstruct the index from scratch
without reusing the cached stat information.
This is a backport of the corresponding patch to the builtin am in 2.6:
3ecc704 (am --skip/--abort: merge HEAD/ORIG_HEAD tree into index,
2015-08-19).
Reportedly, it can make a huge difference on Windows, in one case a `git
rebase --skip` took 1m40s without, and 5s with, this patch.
cf. https://github.com/git-for-windows/git/issues/365
Reported-and-suggested-by: Kim Gybels <kgybels@infogroep.be>
Acked-by: Paul Tan <pyokagan@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Recent "git am" had regression when adding a Signed-off-by line
with its "-s" option by an unintended tightening of how an existing
trailer block is detected.
* jc/builtin-am-signoff-regression-fix:
am: match --signoff to the original scripted version
When executing user-specified programs, we generally always
want to use a shell, for flexibility and consistency. One
big exception is executing $GIT_SSH, which for historical
reasons must not use a shell.
Once upon a time the logic in git_connect looked like:
if (protocol == PROTO_SSH) {
... setup ssh ...
} else {
... setup local connection ...
conn->use_shell = 1;
}
But over time the PROTO_SSH block has grown, and the "local"
block has shrunk so that it contains only conn->use_shell;
it's easy to miss at the end of the large block. Moreover,
PROTO_SSH now also sometimes sets use_shell, when the new
GIT_SSH_COMMAND is used.
Let's just set conn->use_shell when we're setting up the "conn"
struct, and unset it (with a comment) in the historical GIT_SSH
case. This will make the flow easier to follow.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus noticed that the recently reimplemented "git am -s" defines
the trailer block too rigidly, resulting in an unnecessary blank
line between the existing sign-offs and his new sign-off. An e-mail
submission sent to Linus in real life ends with mixture of sign-offs
and commentaries, e.g.
title here
message here
Signed-off-by: Original Author <original@auth.or>
[rv: tweaked frotz and nitfol]
Signed-off-by: Re Viewer <rv@ew.er>
Signed-off-by: Other Reviewer <other@rev.ewer>
---
patch here
Because the reimplementation reused append_signoff() helper that is
used by other codepaths, which is unaware that people intermix such
comments with their sign-offs in the trailer block, such a message
was judged to end with a non-trailer, resulting in an extra blank
line before adding a new sign-off.
The original scripted version of "git am" used a lot looser
definition, i.e. "if and only if there is no line that begins with
Signed-off-by:, add a blank line before adding a new sign-off". For
the upcoming release, stop using the append_signoff() in "git am"
and reimplement the looser definition used by the scripted version
to use only in "git am" to fix this regression in "am" while
avoiding new regressions to other users of append_signoff().
In the longer term, we should look into loosening append_signoff()
so that other codepaths that add a new sign-off behave the same way
as "git am -s", but that is a task for post-release.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we "switch" to another local repository to run the server
side of a fetch or push, we must clear the variables in
local_repo_env so that our local $GIT_DIR, etc, do not
pollute the upload-pack or receive-pack that is executing in
the "remote" repository.
We have never done so for ssh connections. For the most
part, nobody has noticed because ssh will not pass unknown
environment variables by default. However, it is not out of
the question for a user to configure ssh to pass along GIT_*
variables using SendEnv/AcceptEnv.
We can demonstrate the problem by using "git -c" on a local
command and seeing its impact on a remote repository. This
config ends up in $GIT_CONFIG_PARAMETERS. In the local case,
the config has no impact, but in the ssh transport, it does
(our test script has a fake ssh that passes through all
environment variables; this isn't normal, but does simulate
one possible setup).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since the commit a3da882120 (pager: do wait_for_pager on signal
death), we call wait_for_pager() in the pager's signal handler. The
recent bug report revealed that this causes a deadlock in glibc at
aborting "git log" [*1*]. When this happens, git process is left
unterminated, and it can't be killed by SIGTERM but only by SIGKILL.
The problem is that wait_for_pager() function does more than waiting
for pager process's termination, but it does cleanups and printing
errors. Unfortunately, the functions that may be used in a signal
handler are very limited [*2*]. Particularly, malloc(), free() and the
variants can't be used in a signal handler because they take a mutex
internally in glibc. This was the cause of the deadlock above. Other
than the direct calls of malloc/free, many functions calling
malloc/free can't be used. strerror() is such one, either.
Also the usage of fflush() and printf() in a signal handler is bad,
although it seems working so far. In a safer side, we should avoid
them, too.
This patch tries to reduce the calls of such functions in signal
handlers. wait_for_signal() takes a flag and avoids the unsafe
calls. Also, finish_command_in_signal() is introduced for the
same reason. There the free() calls are removed, and only waits for
the children without whining at errors.
[*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297
[*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we show "branch@{0}", we format into a fixed-size
buffer using sprintf. This can overflow if you have long
branch names. We can fix it by using a temporary strbuf.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function assumes that the relative_base path passed
into it is no larger than PATH_MAX, and writes into a
fixed-size buffer. However, this path may not have actually
come from the filesystem; for example, add_submodule_odb
generates a path using a strbuf and passes it in. This is
hard to trigger in practice, though, because the long
submodule directory would have to exist on disk before we
would try to open its info/alternates file.
We can easily avoid the bug, though, by simply creating the
filename on the heap.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we are loading a notes tree into our internal hash
table, we also collect any files that are clearly non-notes.
We format the name of the file into a PATH_MAX buffer, but
unlike true notes (which cannot be larger than a fanned-out
sha1 hash), these tree entries can be arbitrarily long,
overflowing our buffer.
We can fix this by switching to a strbuf. It doesn't even
cost us an extra allocation, as we can simply hand ownership
of the buffer over to the non-note struct.
This is of moderate security interest, as you might fetch
notes trees from an untrusted remote. However, we do not do
so by default, so you would have to manually fetch into the
notes namespace.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When unpack-trees wants to know whether a path will
overwrite anything in the working tree, we use lstat() to
see if there is anything there. But if we are going to write
"foo/bar", we can't just lstat("foo/bar"); we need to look
for leading prefixes (e.g., "foo"). So we use the lstat cache
to find the length of the leading prefix, and copy the
filename up to that length into a temporary buffer (since
the original name is const, we cannot just stick a NUL in
it).
The copy we make goes into a PATH_MAX-sized buffer, which
will overflow if the prefix is longer than PATH_MAX. How
this happens is a little tricky, since in theory PATH_MAX is
the biggest path we will have read from the filesystem. But
this can happen if:
- the compiled-in PATH_MAX does not accurately reflect
what the filesystem is capable of
- the leading prefix is not _quite_ what is on disk; it
contains the next element from the name we are checking.
So if we want to write "aaa/bbb/ccc/ddd" and "aaa/bbb"
exists, the prefix of interest is "aaa/bbb/ccc". If
"aaa/bbb" approaches PATH_MAX, then "ccc" can overflow
it.
So this can be triggered, but it's hard to do. In
particular, you cannot just "git clone" a bogus repo. The
verify_absent checks happen before unpack-trees writes
anything to the filesystem, so there are never any leading
prefixes during the initial checkout, and the bug doesn't
trigger. And by definition, these files are larger than
PATH_MAX, so writing them will fail, and clone will
complain (though it may write a partial path, which will
cause a subsequent "git checkout" to hit the bug).
We can fix it by creating the temporary path on the heap.
The extra malloc overhead is not important, as we are
already making at least one stat() call (and probably more
for the prefix discovery).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git init empty && git -C empty log" said "bad default revision 'HEAD'",
which was found to be a bit confusing to new users.
* jk/log-missing-default-HEAD:
log: diagnose empty HEAD more clearly
The "interpret-trailers" helper mistook a multi-paragraph title of
a commit log message with a colon in it as the end of the trailer
block.
* cc/trailers-corner-case-fix:
trailer: support multiline title
trailer: retitle a test and correct an in-comment message
trailer: ignore first line of message
When re-priming the cache-tree opportunistically while committing
the in-core index as-is, we mistakenly invalidated the in-core
index too aggressively, causing the experimental split-index code
to unnecessarily rewrite the on-disk index file(s).
* dt/commit-preserve-base-index-upon-opportunistic-cache-tree-update:
commit: don't rewrite shared index unnecessarily
"git archive" did not use zip64 extension when creating an archive
with more than 64k entries, which nobody should need, right ;-)?
* rs/archive-zip-many:
archive-zip: support more than 65535 entries
archive-zip: use a local variable to store the creator version
t5004: test ZIP archives with many entries
The code in "multiple-worktree" support that attempted to recover
from an inconsistent state updated an incorrect file.
* nd/fixup-linked-gitdir:
setup: update the right file in multiple checkouts
"git rev-list" does not take "--notes" option, but did not complain
when one is given.
* jk/rev-list-has-no-notes:
rev-list: make it obvious that we do not support notes
Because the configuration system does not allow "alias.0foo" and
"pager.0foo" as the configuration key, the user cannot use '0foo'
as a custom command name anyway, but "git 0foo" tried to look these
keys up and emitted useless warnings before saying '0foo is not a
git command'. These warning messages have been squelched.
* jk/fix-alias-pager-config-key-warnings:
config: silence warnings for command names with invalid keys
t1509 test that requires a dedicated VM environment had some
bitrot, which has been corrected.
* ps/t1509-chroot-test-fixup:
tests: fix cleanup after tests in t1509-root-worktree
tests: fix broken && chains in t1509-root-worktree
strbuf_read() used to have one extra iteration (and an unnecessary
strbuf_grow() of 8kB), which was eliminated.
* jh/strbuf-read-use-read-in-full:
strbuf_read(): skip unnecessary strbuf_grow() at eof
The codepath to produce error messages had a hard-coded limit to
the size of the message, primarily to avoid memory allocation while
calling die().
* jk/long-error-messages:
vreportf: avoid intermediate buffer
vreportf: report to arbitrary filehandles
When trying to see that an object does not exist, a state errno
leaked from our "first try to open a packfile with O_NOATIME and
then if it fails retry without it" logic on a system that refuses
O_NOATIME. This confused us and caused us to die, saying that the
packfile is unreadable, when we should have just reported that the
object does not exist in that packfile to the caller.
* cb/open-noatime-clear-errno:
git_open_noatime: return with errno=0 on success
An off-by-one error made "git remote" to mishandle a remote with a
single letter nickname.
* mh/get-remote-group-fix:
get_remote_group(): use skip_prefix()
get_remote_group(): eliminate superfluous call to strcspn()
get_remote_group(): rename local variable "space" to "wordlen"
get_remote_group(): handle remotes with single-character names
