send-email: don't call methods on undefined values
If SSL verification is enabled in git send-email, we could attempt to call a
method on an undefined value if the verification failed, since $smtp would end
up being undef. Look up the error string in a way that will produce a helpful
error message and not cause further errors.
Signed-off-by: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no reason not to turn on keepalives by default.
They take very little bandwidth, and significantly less than
the progress reporting they are replacing. And in the case
that progress reporting is on, we should never need to send
a keepalive anyway, as we will constantly be showing
progress and resetting the keepalive timer.
We do not necessarily know what the client's idea of a
reasonable timeout is, so let's keep this on the low side of
5 seconds. That is high enough that we will always prefer
our normal 1-second progress reports to sending a keepalive
packet, but low enough that no sane client should consider
the connection hung.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack: send keepalive packets during pack computation
When upload-pack has started pack-objects, there may be a quiet
period while pack-objects prepares the pack (i.e., counting objects
and delta compression). Normally we would see (and send to the
client) progress information, but if "--quiet" is in effect,
pack-objects will produce nothing at all until the pack data is
ready. On a large repository, this can take tens of seconds (or even
minutes if the system is loaded or the repository is badly packed).
Clients or intermediate proxies can sometimes give up in this
situation, assuming that the server or connection has hung.
This patch introduces a "keepalive" option; if upload-pack sees no
data from pack-objects for a certain number of seconds, it will send
an empty sideband data packet to let the other side know that we are
still working on it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
branch.c: Relax unnecessary requirement on upstream's remote ref name
When creating an upstream relationship, we use the configured remotes and
their refspecs to determine the upstream configuration settings
branch.<name>.remote and branch.<name>.merge. However, if the matching
refspec does not have refs/heads/<something> on the remote side, we end
up rejecting the match, and failing the upstream configuration.
It could be argued that when we set up an branch's upstream, we want that
upstream to also be a proper branch in the remote repo. Although this is
typically the common case, there are cases (as demonstrated by the previous
patch in this series) where this requirement prevents a useful upstream
relationship from being formed. Furthermore:
- We have fundamentally no say in how the remote repo have organized its
branches. The remote repo may put branches (or branch-like constructs
that are insteresting for downstreams to track) outside refs/heads/*.
- The user may intentionally want to track a non-branch from a remote
repo, by using a branch and configured upstream in the local repo.
Relaxing the checking to only require a matching remote/refspec allows the
testcase introduced in the previous patch to succeed, and has no negative
effect on the rest of the test suite.
This patch fixes a behavior (arguably a regression) first introduced in 41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*) on 2013-04-21 (released in >= v1.8.3.2).
Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t3200: Add test demonstrating minor regression in 41c21f2
In 41c21f2 (branch.c: Validate tracking branches with refspecs instead of
refs/remotes/*), we changed the rules for what is considered a valid tracking
branch (a.k.a. upstream branch). We now use the configured remotes and their
refspecs to determine whether a proposed tracking branch is in fact within
the domain of a remote, and we then use that information to deduce the
upstream configuration (branch.<name>.remote and branch.<name>.merge).
However, with that change, we also check that - in addition to a matching
refspec - the result of mapping the tracking branch through that refspec
(i.e. the corresponding ref name in the remote repo) happens to start with
"refs/heads/". In other words, we require that a tracking branch refers to
a _branch_ in the remote repo.
Now, consider that you are e.g. setting up an automated building/testing
infrastructure for a group of similar "source" repositories. The build/test
infrastructure consists of a central scheduler, and a number of build/test
"slave" machines that perform the actual build/test work. The scheduler
monitors the group of similar repos for changes (e.g. with a periodic
"git fetch"), and triggers builds/tests to be run on one or more slaves.
Graphically the changes flow between the repos like this:
The scheduler maintains a single Git repo with each of the source repos set
up as distinct remotes. The slaves also need access to all the changes from
all of the source repos, so they pull from the scheduler repo, but using the
following custom refspec:
Git now looks at the configured remotes (in this case there is only "origin",
pointing to the scheduler's repo) and sees refs/remotes/source-1/some_branch
matching origin's refspec. Mapping through that refspec we find that the
corresponding remote ref name is "refs/remotes/source-1/some_branch".
However, since this remote ref name does not start with "refs/heads/", we
discard it as a suitable upstream, and the whole command fails.
This patch adds a testcase demonstrating this failure by creating two
source repos ("a" and "b") that are forwarded through a scheduler ("c")
to a slave repo ("d"), that then tries create a local branch with an
upstream. See the next patch in this series for the exciting conclusion
to this story...
Reported-by: Per Cederqvist <cederp@opera.com> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t3200: Minor fix when preparing for tracking failure
We're testing that trying to --track a ref that is not covered by any remote
refspec should fail. For that, we want to have refs/remotes/local/master
present, but we also want the remote.local.fetch refspec to NOT match
refs/remotes/local/master (so that the tracking setup will fail, as intended).
However, when doing "git fetch local" to ensure the existence of
refs/remotes/local/master, we must not already have changed remote.local.fetch
so as to cause refs/remotes/local/master not to be fetched. Therefore, set
remote.local.fetch to refs/heads/*:refs/remotes/local/* BEFORE we fetch, and
then reset it to refs/heads/s:refs/remotes/local/s AFTER we have fetched
(but before we test --track).
Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rebase: fix run_specific_rebase's use of "return" on FreeBSD
Since a1549e10, git-rebase--am.sh uses the shell's "return" statement, to
mean "return from the current file inclusion", which is POSIXly correct,
but badly interpreted on FreeBSD, which returns from the current
function, hence skips the finish_rebase statement that follows the file
inclusion.
Make the use of "return" portable by using the file inclusion as the last
statement of a function.
Reported-by: Christoph Mallon <christoph.mallon@gmx.de> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
add--interactive: fix external command invocation on Windows
Back in 21e9757e (Hack git-add--interactive to make it work with
ActiveState Perl, 2007-08-01), the invocation of external commands was
changed to use qx{} on Windows. The rationale was that the command
interpreter on Windows is not a POSIX shell, but rather Windows's CMD.
That patch was wrong to include 'msys' in the check whether to use qx{}
or not: 'msys' identifies MSYS perl as shipped with Git for Windows,
which does not need the special treatment; qx{} should be used only with
ActiveState perl, which is identified by 'MSWin32'.
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make setup_git_env() resolve .git file when $GIT_DIR is not specified
This makes reinitializing on a .git file repository work.
This is probably the only case that setup_git_env() (via
set_git_dir()) is called on a .git file. Other cases in
setup_git_dir_gently() and enter_repo() both cover .git file case
explicitly because they need to verify the target repo is valid.
Reported-by: Ximin Luo <infinity0@gmx.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
has_sha1_file: re-check pack directory before giving up
When we read a sha1 file, we first look for a packed
version, then a loose version, and then re-check the pack
directory again before concluding that we cannot find it.
This lets us handle a process that is writing to the
repository simultaneously (e.g., receive-pack writing a new
pack followed by a ref update, or git-repack packing
existing loose objects into a new pack).
However, we do not do the same trick with has_sha1_file; we
only check the packed objects once, followed by loose
objects. This means that we might incorrectly report that we
do not have an object, even though we could find it if we
simply re-checked the pack directory.
By itself, this is usually not a big deal. The other process
is running simultaneously, so we may run has_sha1_file
before it writes, anyway. It is a race whether we see the
object or not. However, we may also see other things
the writing process has done (like updating refs); and in
that case, we must be able to also see the new objects.
For example, imagine we are doing a for_each_ref iteration,
and somebody simultaneously pushes. Receive-pack may write
the pack and update a ref after we have examined the
objects/pack directory, but before the iteration gets to the
updated ref. When we do finally see the updated ref,
for_each_ref will call has_sha1_file to check whether the
ref is broken. If has_sha1_file returns the wrong answer, we
erroneously will think that the ref is broken.
For a normal iteration without DO_FOR_EACH_INCLUDE_BROKEN,
this means that the caller does not see the ref at all
(neither the old nor the new value). So not only will we
fail to see the new value of the ref (which is acceptable,
since we are running simultaneously with the writer, and we
might well read the ref before the writer commits its
write), but we will not see the old value either. For
programs that act on reachability like pack-objects or
prune, this can cause data loss, as we may see the objects
referenced by the original ref value as dangling (and either
omit them from the pack, or delete them via prune).
There's no test included here, because the success case is
two processes running simultaneously forever. But you can
replicate the issue with:
# base.sh
# run this in one terminal; it creates and pushes
# repeatedly to a repository
git init parent &&
(cd parent &&
# create a base commit that will trigger us looking at
# the objects/pack directory before we hit the updated ref
echo content >file &&
git add file &&
git commit -m base &&
# set the unpack limit abnormally low, which
# lets us simulate full-size pushes using tiny ones
git config receive.unpackLimit 1
) &&
git clone parent child &&
cd child &&
n=0 &&
while true; do
echo $n >file && git add file && git commit -m $n &&
git push origin HEAD:refs/remotes/child/master &&
n=$(($n + 1))
done
# fsck.sh
# now run this simultaneously in another terminal; it
# repeatedly fscks, looking for us to consider the
# newly-pushed ref broken. We cannot use for-each-ref
# here, as it uses DO_FOR_EACH_INCLUDE_BROKEN, which
# skips the has_sha1_file check (and if it wants
# more information on the object, it will actually read
# the object, which does the proper two-step lookup)
cd parent &&
while true; do
broken=`git fsck 2>&1 | grep remotes/child`
if test -n "$broken"; then
echo $broken
exit 1
fi
done
Without this patch, the fsck loop fails within a few seconds
(and almost instantly if the test repository actually has a
large number of refs). With it, the two can run
indefinitely.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sparse issues an "'prepare_transport' was not declared. Should it
be static?" warning. In order to suppress the warning, since this
symbol only requires file scope, we simply add the static modifier
to it's declaration.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
mailmap: handle mailmap blobs without trailing newlines
The read_mailmap_buf function reads each line of the mailmap
using strchrnul, like:
const char *end = strchrnul(buf, '\n');
unsigned long linelen = end - buf + 1;
But that's off-by-one when we actually hit the NUL byte; our
line does not have a terminator, and so is only "end - buf"
bytes long. As a result, when we subtract the linelen from
the total len, we end up with (unsigned long)-1 bytes left
in the buffer, and we start reading random junk from memory.
We could fix it with:
unsigned long linelen = end - buf + !!*end;
but let's take a step back for a moment. It's questionable
in the first place for a function that takes a buffer and
length to be using strchrnul. But it works because we only
have one caller (and are only likely to ever have this one),
which is handing us data from read_sha1_file. Which means
that it's always NUL-terminated.
Instead of tightening the assumptions to make the
buffer/length pair work for a caller that doesn't actually
exist, let's let loosen the assumptions to what the real
caller has: a modifiable, NUL-terminated string.
This makes the code simpler and shorter (because we don't
have to correlate strchrnul with the length calculation),
correct (because the code with the off-by-one just goes
away), and more efficient (we can drop the extra allocation
we needed to create NUL-terminated strings for each line,
and just terminate in place).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add testcase for needless objects during a shallow fetch
This is a testcase that checks for a problem where, during a specific
shallow fetch where the client does not have any commits that are a
successor of the new shallow root (i.e., the fetch creates a new
detached piece of history), the server would simply send over _all_
objects, instead of taking into account the objects already present in
the client.
The actual problem was fixed by a recent patch series by Nguyễn Thái
Ngọc Duy already.
Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
list-objects: mark more commits as edges in mark_edges_uninteresting
The purpose of edge commits is to let pack-objects know what objects
it can use as base, but does not need to include in the thin pack
because the other side is supposed to already have them. So far we
mark uninteresting parents of interesting commits as edges. But even
an unrelated uninteresting commit (that the other side has) may
become a good base for pack-objects and help produce more efficient
packs.
This is especially true for shallow clone, when the client issues a
fetch with a depth smaller or equal to the number of commits the
server is ahead of the client. For example, in this commit history
the client has up to "A" and the server has up to "B":
-------A---B
have--^ ^
/
want--+
If depth 1 is requested, the commit list to send to the client
includes only B. The way m_e_u is working, it checks if parent
commits of B are uninteresting, if so mark them as edges. Due to
shallow effect, commit B is grafted to have no parents and the
revision walker never sees A as the parent of B. In fact it marks no
edges at all in this simple case and sends everything B has to the
client even if it could have excluded what A and also the client
already have.
In a slightly different case where A is not a direct parent of B
(iow there are commits in between A and B), marking A as an edge can
still save some because B may still have stuff from the far ancestor
A.
There is another case from the earlier patch, when we deepen a ref
from C->E to A->E:
In this case we need to send A and B to the client, and C (i.e. the
current shallow point that the client informs the server) is a very
good base because it's closet to A and B. Normal m_e_u won't recognize
C as an edge because it only looks back to parents (i.e. A<-B) not the
opposite way B->C even if C is already marked as uninteresting commit
by the previous patch.
This patch includes all uninteresting commits from command line as
edges and lets pack-objects decide what's best to do. The upside is we
have better chance of producing better packs in certain cases. The
downside is we may need to process some extra objects on the server
side.
For the shallow case on git.git, when the client is 5 commits behind
and does "fetch --depth=3", the result pack is 99.26 KiB instead of
4.92 MiB.
Reported-and-analyzed-by: Matthijs Kooijman <matthijs@stdin.nl> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the first argument and let mark_edges_uninteresting figure that
out by itself. It helps answer the question "are this commit list and
revs related in any way?" when looking at mark_edges_uninteresting
implementation.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-pack: delegate rev walking in shallow fetch to pack-objects
upload-pack has a special revision walking code for shallow
recipients. It works almost like the similar code in pack-objects
except:
1. in upload-pack, graft points could be added for deepening;
2. also when the repository is deepened, the shallow point will be
moved further away from the tip, but the old shallow point will be
marked as edge to produce more efficient packs. See 6523078 (make
shallow repository deepening more network efficient - 2009-09-03).
Pass the file to pack-objects via --shallow-file. This will override
$GIT_DIR/shallow and give pack-objects the exact repository shape
that upload-pack has.
mark edge commits by revision command arguments. Even if old shallow
points are passed as "--not" revisions as in this patch, they will not
be picked up by mark_edges_uninteresting() because this function looks
up to parents for edges, while in this case the edge is the children,
in the opposite direction. This will be fixed in an later patch when
all given uninteresting commits are marked as edges.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function is like setup_alternate_shallow() except that it does
not lock $GIT_DIR/shallow. It is supposed to be used when a program
generates temporary shallow for use by another program, then throw
the shallow file away.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
shallow: only add shallow graft points to new shallow file
for_each_commit_graft() goes through all graft points, and shallow
boundaries are just one special kind of grafting.
If $GIT_DIR/shallow and $GIT_DIR/info/grafts are both present,
write_shallow_commits() may catch both sets, accidentally turning
some graft points to shallow boundaries. Don't do that.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
config: do not use C function names as struct members
According to C99, section 7.1.4:
Any function declared in a header may be additionally
implemented as a function-like macro defined in the
header.
Therefore calling our struct member function pointer "fgetc"
may run afoul of unwanted macro expansion when we call:
char c = cf->fgetc(cf);
This turned out to be a problem on uclibc, which defines
fgetc as a macro and causes compilation failure.
The standard suggests fixing this in a few ways:
1. Using extra parentheses to inhibit the function-like
macro expansion. E.g., "(cf->fgetc)(cf)". This is
undesirable as it's ugly, and each call site needs to
remember to use it (and on systems without the macro,
forgetting will compile just fine).
2. Using #undef (because a conforming implementation must
also be providing fgetc as a function). This is
undesirable because presumably the implementation was
using the macro for a performance benefit, and we are
dropping that optimization.
Instead, we can simply use non-colliding names.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'todo' sheet for interactive rebase shows abbreviated SHA-1's and
then performs its operations upon those shortened values. This can lead
to an abort if the SHA-1 of a reworded or edited commit is no longer
unique within the abbreviated SHA-1 space and a subsequent SHA-1 in the
todo list has the same abbreviated value.
If, after editing, the new SHA-1 of "first" also has prefix badbeef,
then the subsequent 'pick badbeef second' will fail since badbeef is no
longer a unique SHA-1 abbreviation:
error: short SHA1 badbeef is ambiguous.
fatal: Needed a single revision
Invalid commit name: badbeef
Fix this problem by expanding the SHA-1's in the todo list before
performing the operations.
[es: also collapse & expand SHA-1's for --edit-todo; respect
core.commentchar in transform_todo_ids(); compose commit message]
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t3404: rebase -i: demonstrate short SHA-1 collision
The 'todo' sheet for interactive rebase shows abbreviated SHA-1's and
then performs its operations upon those shortened values. This can lead
to an abort if the SHA-1 of a reworded or edited commit is no longer
unique within the abbreviated SHA-1 space and a subsequent SHA-1 in the
todo list has the same abbreviated value.
If, after editing, the new SHA-1 of "first" also has prefix badbeef,
then the subsequent 'pick badbeef second' will fail since badbeef is no
longer a unique SHA-1 abbreviation:
error: short SHA1 badbeef is ambiguous.
fatal: Needed a single revision
Invalid commit name: badbeef
Demonstrate this problem with a couple of specially crafted commits
which initially have distinct abbreviated SHA-1's, but for which the
abbreviated SHA-1's collide after a simple rewording of the first
commit's message.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
As its very first action, t3404 installs (via set_fake_editor) a
specialized $EDITOR which simplifies automated 'rebase -i' testing. Many
tests rely upon this setting, thus tests which need a different editor
must take extra care upon completion to restore $EDITOR in order to
avoid breaking following tests. This places extra burden upon such tests
and requires that they undesirably have extra knowledge about
surrounding tests. Ease this burden by having each test install the
$EDITOR it requires, rather than relying upon a global setting.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch-pack: do not remove .git/shallow file when --depth is not specified
fetch_pack() can remove .git/shallow file when a shallow repository
becomes a full one again. This behavior is triggered incorrectly when
tags are also fetched because fetch_pack() will be called twice. At
the first fetch_pack() call:
- shallow_lock is set up
- alternate_shallow_file points to shallow_lock.filename, which is
"shallow.lock"
- commit_lock_file is called, which sets shallow_lock.filename to "".
alternate_shallow_file also becomes "" because it points to the
same memory.
At the second call, setup_alternate_shallow() is not called and
alternate_shallow_file remains "". It's mistaken as unshallow case and
.git/shallow is removed. The end result is a broken repository.
Fix this by always initializing alternate_shallow_file when
fetch_pack() is called. As an extra measure, check if args->depth > 0
before commit/rollback shallow file.
Reported-by: Kacper Kornet <kornet@camk.edu.pl> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git commit --author=$name" sets the author to one whose name
matches the given string from existing commits, when $name is not in
the "Name <e-mail>" format. However, it does not honor the mailmap
to use the canonical name for the author found this way.
Fix it by telling the logic to find a matching existing author to
honor the mailmap, and use the name and email after applying the
mailmap.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
dir.c::test_one_path(): work around directory_exists_in_index_icase() breakage
directory_exists_in_index() takes pathname and its length, but its
helper function directory_exists_in_index_icase() reads one byte
beyond the end of the pathname and expects there to be a '/'.
This needs to be fixed, as that one-byte-beyond-the-end location may
not even be readable, possibly by not registering directories to
name hashes with trailing slashes. In the meantime, update the new
caller added recently to treat_one_path() to make sure that the path
buffer it gives the function is one byte longer than the path it is
asking the function about by appending a slash to it.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
contrib/git-prompt.sh: handle missing 'printf -v' more gracefully
Old Bash (3.0) which is distributed with RHEL 4.X and other ancient
platforms that are still in wide use, do not have a printf that
supports -v. Neither does Zsh (which is already handled in the code).
As suggested by Junio, let's test whether printf supports the -v
option and store the result. Then later, we can use it to
determine whether 'printf -v' can be used, or whether printf
must be called in a subshell.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t9902-completion.sh: old Bash still does not support array+=('') notation
Old Bash (3.0) which is distributed with RHEL 4.X and other ancient
platforms that are still in wide use, does not understand the
array+=() notation. Let's use an explicit assignment to the new array
element which works everywhere, like:
array[${#array[@]}+1]=''
The right-hand side '' is not strictly necessary, but in this case
I think it is more clear.
Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rebase --preserve-merges: ignore "merge.log" config
When "merge.log" config is set, "rebase --preserve-merges" will add
the log lines to the message of the rebased merge commit. A rebase
should not modify a commit message automatically.
Teach "git-rebase" to ignore that configuration by passing
"--no-log" to the git-merge call.
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
stream_to_pack: xread does not guarantee to read all requested bytes
The deflate loop in bulk-checkin::stream_to_pack expects to get all bytes
from a file that it requests to read in a single function call. But it
used xread(), which does not give that guarantee. Replace it by
read_in_full().
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Checking out 2GB or more through an external filter (see test) fails
on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:
error: read from external filter cat failed
error: cannot feed the input to external filter cat
error: cat died of signal 13
error: external filter cat failed 141
error: external filter cat failed
The reason is that read() immediately returns with EINVAL when asked
to read more than 2GB. According to POSIX [1], if the value of
nbyte passed to read() is greater than SSIZE_MAX, the result is
implementation-defined. The write function has the same restriction
[2]. Since OS X still supports running 32-bit executables, the
32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also
imposed on 64-bit executables under certain conditions. For write,
the problem has been addressed earlier [6c642a].
Address the problem for read() and write() differently, by limiting
size of IO chunks unconditionally on all platforms in xread() and
xwrite(). Large chunks only cause problems, like causing latencies
when killing the process, even if OS X was not buggy. Doing IO in
reasonably sized smaller chunks should have no negative impact on
performance.
The compat wrapper clipped_write() introduced earlier [6c642a] is
not needed anymore. It will be reverted in a separate commit. The
new test catches read and write problems.
Note that 'git add' exits with 0 even if it prints filtering errors
to stderr. The test, therefore, checks stderr. 'git add' should
probably be changed (sometime in another commit) to exit with
nonzero if filtering fails. The test could then be changed to use
test_must_fail.
Thanks to the following people for suggestions and testing:
Johannes Sixt <j6t@kdbg.org>
John Keeping <john@keeping.me.uk>
Jonathan Nieder <jrnieder@gmail.com>
Kyle J. McKay <mackyle@gmail.com>
Linus Torvalds <torvalds@linux-foundation.org>
Torsten Bögershausen <tboegi@web.de>
This is because the parsing of "submodule.*.path" is not prepared to
see a value-less "true" and assumes that the value is always
non-NULL (parsing of "ignore" has the same problem).
Fix it by checking the NULL-ness of value and complain with
config_error_nonbool().
Signed-off-by: Jharrod LaFon <jlafon@eyesopen.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
180bad3d (rebase -i: respect core.commentchar, 2013-02-11) updated
"rebase -i" to honor core.commentchar but missed one instance of
hard-coded '#' comment character in skip_unnecessary_picks().
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t3010: update to demonstrate "ls-files -k" optimization pitfalls
An earlier draft of the previous step used cache_name_exists() to
check the directory we were looking at, which missed the second case
described in its log message. Demonstrate why it is not sufficient.
ls-files -k: a directory only can be killed if the index has a non-directory
"ls-files -o" and "ls-files -k" both traverse the working tree down
to find either all untracked paths or those that will be "killed"
(removed from the working tree to make room) when the paths recorded
in the index are checked out. It is necessary to traverse the
working tree fully when enumerating all the "other" paths, but when
we are only interested in "killed" paths, we can take advantage of
the fact that paths that do not overlap with entries in the index
can never be killed.
The treat_one_path() helper function, which is called during the
recursive traversal, is the ideal place to implement an
optimization.
When we are looking at a directory P in the working tree, there are
three cases:
(1) P exists in the index. Everything inside the directory P in
the working tree needs to go when P is checked out from the
index.
(2) P does not exist in the index, but there is P/Q in the index.
We know P will stay a directory when we check out the contents
of the index, but we do not know yet if there is a directory
P/Q in the working tree to be killed, so we need to recurse.
(3) P does not exist in the index, and there is no P/Q in the index
to require P to be a directory, either. Only in this case, we
know that everything inside P will not be killed without
recursing.
Note that this helper is called by treat_leading_path() that decides
if we need to traverse only subdirectories of a single common
leading directory, which is essential for this optimization to be
correct. This caller checks each level of the leading path
component from shallower directory to deeper ones, and that is what
allows us to only check if the path appears in the index. If the
call to treat_one_path() weren't there, given a path P/Q/R, the real
traversal may start from directory P/Q/R, even when the index
records P as a regular file, and we would end up having to check if
any leading subpath in P/Q/R, e.g. P, appears in the index.
dir.c: use the cache_* macro to access the current index
These codepaths always start from the_index and use index_*
functions, but there is no reason to do so. Use the compatibility
cache_* macro to access the current in-core index like everybody
else.
While at it, fix typo in the comment for a function to check if a
path within a directory appears in the index.
This reverts commit cdfd94837b27c220f70f032b596ea993d195488f, as it
does not just apply to "@" (and forms with modifiers like @{u}
applied to it), but also affects e.g. "refs/heads/@/foo", which it
shouldn't.
The basic idea of giving a short-hand might be good, and the topic
can be retried later, but let's revert to avoid affecting existing
use cases for now for the upcoming release.
Revert "git stash: avoid data loss when "git stash save" kills a directory"
This reverts commit a73653130edd6a8977106d45a8092c09040f9132, as it
has been reported that "ls-files --killed" is too time-consuming in
a deep directory with too many untracked crufts (e.g. $HOME/.git
tracking only a few files).
We'd need to revisit it later but "ls-files --killed" needs to be
optimized before it happens.
.mailmap: update long-lost friends with multiple defunct addresses
A handful of past contributors are recorded with multiple e-mail
addresses, all of which are undeliverable. With a lot of help from
Jonathan, we located all of them except for one person, and a pair
of addresses we suspect belong to a single person but we are not
certain.
Update the found ones with their currently preferred address, and
use the last known address to consolidate contributions by the lost
one.
Helped-by: Stefan Beller <stefanbeller@googlemail.com> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch adds no new names, but fixes the mistakes I made in the previous
commits. (94b410bba8, f4f49e225, c07a6bc57, 2013-07-12, .mailmap: Map
email addresses to names).
These mistakes are double white spaces between name and surname,
different capitalization in email address, or just the email address set
as name.
Also I forgot to include James Knight to the mailmap file.
Signed-off-by: Stefan Beller <stefanbeller@googlemail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In t/t7407-submodule-foreach.sh there is a typo in one of the
path names given for a test step. The correct path is
nested1/nested2/.git, but nested1/nested1/nested2/.git is
given instead. The typo is hidden because this line also
accidentally omits the && chain operator. The omitted chain
also means the return values of all the previous commands in
this test are also being ignored.
Fix the path and add the chain operator so the entire test
sequence can be properly validated.
Signed-off-by: Phil Hord <hordp@cisco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
a469a1019352b8ef (silence some -Wuninitialized false positives;
2012-12-15) triggered "unused value" warnings when the return value of
opterror() and several other error-related functions was not used. 5ded807f7c0be10e (fix clang -Wunused-value warnings for error functions;
2013-01-16) applied a fix by adding #if !defined(__clang__) in cache.h
and git-compat-util.h, but misspelled it as #if !defined(clang) in
parse-options.h. Fix this.
This mistake went unnoticed because existing callers of opterror()
utilize its return value. 1158826394e162c5 (parse-options: add
OPT_CMDMODE(); 2013-07-30), however, adds a new invocation of opterror()
which ignores the return value, thus triggering the "unused value"
warning.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A Git-aware "connect" transport allows the "transport_take_over" to
redirect generic transport requests like fetch(), push_refs() and
get_refs_list() to the native Git transport handling methods. The
take-over process replaces transport->data with a fake data that
these method implementations understand.
While this hack works OK for a single request, it breaks when the
transport needs to make more than one requests. transport->data
that used to hold necessary information for the specific helper to
work correctly is destroyed during the take-over process.
One codepath that this matters is "git fetch" in auto-follow mode;
when it does not get all the tags that ought to point at the history
it got (which can be determined by looking at the peeled tags in the
initial advertisement) from the primary transfer, it internally
makes a second request to complete the fetch. Because "take-over"
hack has already destroyed the data necessary to talk to the
transport helper by the time this happens, the second request cannot
make a request to the helper to make another connection to fetch
these additional tags.
Mark such a transport as "cannot_reuse", and use a separate
transport to perform the backfill fetch in order to work around
this breakage.
Note that this problem does not manifest itself when running t5802,
because our upload-pack gives you all the necessary auto-followed
tags during the primary transfer. You would need to step through
"git fetch" in a debugger, stop immediately after the primary
transfer finishes and writes these auto-followed tags, remove the
tag references and repack/prune the repository to convince the
"find-non-local-tags" procedure that the primary transfer failed to
give us all the necessary tags, and then let it continue, in order
to trigger the bug in the secondary transfer this patch fixes.
Usually the upload-pack process running on the other side will give
us all the reachable tags we need during the primary object transfer
in do_fetch(). If that does not happen (e.g. the other side may be
running a third-party implementation of upload-pack), we will run
another fetch to pick up leftover tags that we know point at the
commits reachable from our updated tips.
Separate out the code to run this second fetch into a helper
function.
fetch: rename file-scope global "transport" to "gtransport"
Although many functions in this file take a "struct transport" as a
parameter, "fetch_one()" assigns to the global singleton instance
which is a file-scope static, in order to allow a parameterless
signal handler unlock_pack() to access it.
Rename the variable to gtransport to make sure these uses stand out.
This is an attempt to reproduce a problem reported for a third-party
custom "connect" remote helper. The conjecture is that sometimes
"git fetch" wants to make two connections (one for the primary
transfer with 'follow-tags' option set, and then after noticing that
some tags are not packed because the primary transfer did not have
to send any commit that is pointed by them, another to explicitly
ask for the missing tags), and their "connect" helper is not called
in the second request, breaking the "fetch" as a whole.
Unfortunately this test script does not trigger the alleged failure
and happily passes when talking to upload-pack from git-core (see
patch 5/5 for details).
* maint:
fix typo in documentation of git-svn
Documentation/rev-list-options: add missing word in --*-parents
log doc: the argument to --encoding is not optional
t8001, t8002: fix "blame -L :literal" test on NetBSD
Sub-test 42 of t8001 and t8002 ("blame -L :literal") fails on NetBSD
with the following verbose output:
git annotate -L:main hello.c
Author F (expected 4, attributed 3) bad
Author G (expected 1, attributed 1) good
This is not caused by different behaviour of git blame or annotate on
that platform, but by different test input, in turn caused by a sed
command that forgets to add a newline on NetBSD. Here's the diff of the
commit that adds "goodbye" to hello.c, for Linux:
It also adds an extra TAB, but it is missing the newline character
after the semicolon.
The following patch gets rid of the extra TAB at the beginning, but
more importantly adds the missing newline at the end in a (hopefully)
portable way, mentioned in http://sed.sourceforge.net/sedfaq4.html.
The diff becomes this, on both Linux and NetBSD:
t3900: test rejecting log message with NULs correctly
It is not like that our longer term desire is to someday start
accept log messages with NULs in them, so it is wrong to mark a test
that demonstrates "git commit" that correctly fails given such an
input as "expect-failure". "git commit" should fail today, and it
should fail the same way in the future given a message with NUL in it.
The test file that the UTF-16 rejection test looks for is missing, but this went
unnoticed because the test is expected to fail anyway; as a consequence, the
test fails because the file containing the commit message is missing, and not
because the test file contains a NUL byte. Fix this by including a sample text
file containing a commit message encoded in UTF-16.
Signed-off-by: Brian M. Carlson <sandals@crustytoothpaste.net> Tested-by: Duy Nguyen <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
.mailmap: Multiple addresses of Michael S. Tsirkin
Acked-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
log: use true parents for diff when walking reflogs
The reflog walking logic (git log -g) replaces the true parent list
with the preceding commit in the reflog. This results in bogus commit
diffs when combined with options such as -p; the diff is against the
reflog predecessor, not the parent of the commit.
Save the true parents on the side, extending the functions from the
previous commit. The diff logic picks them up and uses them to show
the correct diffs.
We do have to be somewhat careful about repeated calling of
save_parents(), since the reflog may list a commit more than once. We
now store (commit_list*)-1 to distinguish the "not saved yet" and
"root commit" cases. This lets us preserve an empty parent list even
if save_parents() is repeatedly called.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>