push: mention "push.default=tracking" in the documentation
Change the documentation for push.tracking=* to re-include a mention
of what "tracking" does.
The "tracking" option was renamed to "upstream" back in 53c4031 ("push.default: Rename 'tracking' to 'upstream'", 2011-02-16),
this section was then subsequently rewritten in 87a70e4 ("config doc:
rewrite push.default section", 2013-06-19) to remove any mention of
"tracking".
Maybe we should just warn or die nowadays if this option is in the
config, but I had some old config of mine use this option, I'd
forgotten that it was a synonym, and nothing in git's documentation
mentioned that.
That's bad, either we shouldn't support it at all, or we should
document what it does. This patch does the latter.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-commit.txt: list post-rewrite in HOOKS section
The hook was added in a86ed83cce (Merge branch 'tr/notes-display' -
2010-03-24), which updated githooks.txt but not git-commit.txt.
git-commit.txt was later updated in e858af6d50 (commit: document a
couple of options - 2012-06-08). Since this commit focused on command
line options, this section was probably forgotten.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pickaxe: fix segfault with '-S<...> --pickaxe-regex'
'git {log,diff,...} -S<...> --pickaxe-regex' can segfault as a result
of out-of-bounds memory reads.
diffcore-pickaxe.c:contains() looks for all matches of the given regex
in a buffer in a loop, advancing the buffer pointer to the end of the
last match in each iteration. When we switched to REG_STARTEND in b7d36ffca (regex: use regexec_buf(), 2016-09-21), we started passing
the size of that buffer to the regexp engine, too. Unfortunately,
this buffer size is never updated on subsequent iterations, and as the
buffer pointer advances on each iteration, this "bufptr+bufsize"
points past the end of the buffer. This results in segmentation
fault, if that memory can't be accessed. In case of 'git log' it can
also result in erroneously listed commits, if the memory past the end
of buffer is accessible and happens to contain data matching the
regex.
Reduce the buffer size on each iteration as the buffer pointer is
advanced, thus maintaining the correct end of buffer location.
Furthermore, make sure that the buffer pointer is not dereferenced in
the control flow statements when we already reached the end of the
buffer.
The new test is flaky, I've never seen it fail on my Linux box even
without the fix, but this is expected according to db5dfa3 (regex:
-G<pattern> feeds a non NUL-terminated string to regexec() and fails,
2016-09-21). However, it did fail on Travis CI with the first (and
incomplete) version of the fix, and based on that commit message I
would expect the new test without the fix to fail most of the time on
Windows.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command: fix segfault when cleaning forked async process
Callers of the run-command API may mark a child as
"clean_on_exit"; it gets added to a list and killed when the
main process dies. Since commit 46df6906f
(execv_dashed_external: wait for child on signal death,
2017-01-06), we respect an extra "wait_after_clean" flag,
which we expect to find in the child_process struct.
When Git is built with NO_PTHREADS, we start "struct
async" processes by forking rather than spawning a thread.
The resulting processes get added to the cleanup list but
they don't have a child_process struct, and the cleanup
function ends up dereferencing NULL.
We should notice this case and assume that the processes do
not need to be waited for (i.e., the same behavior they had
before 46df6906f).
Reported-by: Brandon Williams <bmwill@google.com> Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
shortlog: don't set after_subject to an empty string
The string after_subject is added to a strbuf by pp_title_line() if
it's not NULL. Adding an empty string has the same effect as not
adding anything, but the latter is easier, so don't bother changing
the context member from NULL to "".
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tests: make the 'test_pause' helper work in non-verbose mode
When the 'test_pause' helper function invokes the shell mid-test, it
explicitly redirects the shell's stdout and stderr to file descriptors
3 and 4, which are the stdout and stderr of the tests (i.e. where they
would be connected anyway without those redirections). These file
descriptors are only attached to the terminal in verbose mode, hence
the restriction of 'test_pause' to work only with '-v'.
Redirect the shell's stdout and stderr to the test environment's
original stdout and stderr, allowing it to work properly even in
non-verbose mode, and the restriction can be lifted.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tests: create an interactive gdb session with the 'debug' helper
The 'debug' test helper is supposed to facilitate debugging by running
a command of the test suite under gdb. Unfortunately, its usefulness
is severely limited, because that gdb session is not interactive,
since the test's, and thus gdb's standard input is redirected from
/dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin
of tests, 2011-12-15)).
Redirect gdb's standard file descriptors from/to the test
environment's stdin, stdout and stderr in the 'debug' helper, thus
creating an interactive gdb session (even in non-verbose mode), which
is much, much more useful.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of counting the arguments to see if there are any and then
building the full command use a single loop and add the hook command
just before the first argument. This reduces duplication and overall
code size.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git v2.12 was shipped with an embarrassing breakage where various
operations that verify paths given from the user stopped dying when
seeing an issue, and instead later triggering segfault.
... and then to down to 'maint'.
* js/realpath-pathdup-fix:
real_pathdup(): fix callsites that wanted it to die on error
t1501: demonstrate NULL pointer access with invalid GIT_WORK_TREE
Merge branch 'jk/add-i-patch-do-prompt' into maint
The patch subcommand of "git add -i" was meant to have paths
selection prompt just like other subcommand, unlike "git add -p"
directly jumps to hunk selection. Recently, this was broken and
"add -i" lost the paths selection dialog, but it now has been
fixed.
* jk/add-i-patch-do-prompt:
add--interactive: fix missing file prompt for patch mode with "-i"
Merge branch 'jt/http-base-url-update-upon-redirect' into maint
When a redirected http transport gets an error during the
redirected request, we ignored the error we got from the server,
and ended up giving a not-so-useful error message.
* jt/http-base-url-update-upon-redirect:
http: attempt updating base URL only if no error
The final() function accepts a NULL value for certain
parameters, and falls back to writing into a reusable "name"
buffer, and then either:
1. For "keep_name", requiring all uses to do "keep_name ?
keep_name : name.buf". This is awkward, and it's easy
to accidentally look at the maybe-NULL keep_name.
2. For "final_index_name" and "final_pack_name", aliasing
those pointers to the "name" buffer. This is easier to
use, but the aliased pointers become invalid after the
buffer is reused (this isn't a bug now, but it's a
potential pitfall).
One way to make this safer would be to introduce an extra
pointer to do the aliasing, and have its lifetime match the
validity of the "name" buffer. But it's still easy to
accidentally use the wrong name (i.e., to use
"final_pack_name" instead of the aliased pointer).
Instead, let's use three separate buffers that will remain
valid through the function. That makes it safe to alias the
pointers and use them consistently. The extra allocations
shouldn't matter, as this function is not performance
sensitive.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In several places we write the name of the pack filename
into a fixed-size buffer using snprintf(), but do not check
the return value. As a result, a very long object directory
could cause us to quietly truncate the pack filename
(potentially leading to a corrupted repository, as a newly
written packfile could be missing its .pack extension).
We can use odb_pack_name() to do this with a strbuf (and
shorten the code, as well).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The odb_pack_keep() function generates the name of a .keep
file and opens it. This has two problems:
1. It requires a fixed-size buffer to create the filename
and doesn't notice when the result is truncated.
2. Of the two callers, one sometimes wants to open a
filename it already has, which makes things awkward (it
has to do so manually, and skips the leading-directory
creation).
Instead, let's have odb_pack_keep() just open the file.
Generating the name isn't hard, and a future patch will
switch callers over to odb_pack_name() anyway.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sha1_file.c: make pack-name helper globally accessible
We provide sha1_pack_name() and sha1_pack_index_name(), but
the more generic form (which takes its own strbuf and an
arbitrary extension) is only used to implement the other
two. Let's make it available, but clean up a few things:
1. Name it odb_pack_name(), as the original
sha1_get_pack_name() is long but not all that
descriptive.
2. Switch the strbuf argument to the beginning, so that it
matches similar path-building functions like
git_path_buf().
3. Clean up the out-dated docstring and move it to the
public declaration.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
These functions were originally conceived as wrapper
functions similar to xmkstemp(). They were later moved by 463db9b10 (wrapper: move odb_* to environment.c,
2010-11-06). The more appropriate place for a declaration is
in cache.h.
While we're at it, let's add some basic docstrings.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
All of these options do the same thing "--foo" iterates over
the "foo" refs, and "--foo=<glob>" does the same with a
glob. We can factor this into its own function to avoid
repeating ourselves.
There are two subtleties to note:
- the original called for_each_branch_ref(), etc, in the
non-glob case. Now we will call for_each_ref_in("refs/heads/")
which is exactly what for_each_branch_ref() did under
the hood.
- for --glob, we'll call for_each_glob_ref_in() with a
NULL "prefix" argument. Which is exactly what
for_each_glob_ref() was doing already.
So both cases should behave identically, and it seems
reasonable to assume that this will remain the same. The
functions we are calling now are the more-generic ones, and
the ones we are dropping are just convenience wrappers.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We can't just use a bare skip_prefix() for these cases,
because we need to match both the "--foo" form and the
"--foo=<value>" form (and tell the difference between the
two in the caller).
We can wrap this in a simple helper which has two obvious
callsites, and will gain some more in the next patch.
Note that the error output for abbrev-ref changes slightly,
as we don't keep our original "arg" pointer. However, the
new output should hopefully be more clear:
[before]
fatal: unknown mode for --abbrev-ref=foo
[after]
fatal: unknown mode for --abbrev-ref: foo
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git status provides a porcelain mode for porcelain writers with a
supposedly stable (plumbing) interface. 7a76c28ff2 ("status: disable translation when --porcelain is used", 2014-03-20)
made sure that ahead/behind info is not translated (i.e. is stable).
Make sure that the remaining two strings (initial commit, detached head)
are stable, too.
These changes are for the v1 porcelain interface. While we do have a perfectly
stable v2 porcelain interface now, some tools (such as
powerline-gitstatus) are written against v1 and profit from fixing v1
without any changes on their side.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we parse a remote alternates (or http-alternates), we
expect relative lines like:
../../foo.git/objects
which we convert into "$URL/../foo.git/" (and then use that
as a base for fetching more objects).
But if the remote feeds us nonsense like just:
../
we will try to blindly strip the last 7 characters, assuming
they contain the string "objects". Since we don't _have_ 7
characters at all, this results in feeding a small negative
value to strbuf_add(), which converts it to a size_t,
resulting in a big positive value. This should consistently
fail (since we can't generall allocate the max size_t minus
7 bytes), so there shouldn't be any security implications.
Let's fix this by using strbuf_strip_suffix() to drop the
characters we want. If they're not present, we'll ignore the
alternate (in theory we could use it as-is, but the rest of
the http-walker code unconditionally tacks "objects/" back
on, so it is it not prepared to handle such a case).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/interop: add test of old clients against modern git-daemon
This test just checks that old clients can clone and fetch
from a newer git-daemon. The opposite should also be true,
but it's hard to test ancient versions of git-daemon because
they lack basic options like "--listen".
Note that we have to make a slight tweak to the
lib-git-daemon helper from the regular tests, so that it
starts the daemon with our correct git.a version.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current test suite is good at letting you test a
particular version of Git. But it's not very good at letting
you test _two_ versions and seeing how they interact (e.g.,
one cloning from the other).
This commit adds a test harness that will build two
arbitrary versions of git and make it easy to call them from
inside your tests. See the README and the example script for
details.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
real_pathdup(): fix callsites that wanted it to die on error
In 4ac9006f832 (real_path: have callers use real_pathdup and
strbuf_realpath, 2016-12-12), we changed the xstrdup(real_path())
pattern to use real_pathdup() directly.
The problem with this change is that real_path() calls
strbuf_realpath() with die_on_error = 1 while real_pathdup() calls
it with die_on_error = 0. Meaning that in cases where real_path()
causes Git to die() with an error message, real_pathdup() is silent
and returns NULL instead.
The callers, however, are ill-prepared for that change, as they expect
the return value to be non-NULL (and otherwise the function died
with an appropriate error message).
Fix this by extending real_pathdup()'s signature to accept the
die_on_error flag and simply pass it through to strbuf_realpath(),
and then adjust all callers after a careful audit whether they would
handle NULLs well.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If our pack-objects sub-process dies of a signal, then it
likely didn't have a chance to write anything useful to
stderr. The user may be left scratching their head why the
push failed. Let's detect this situation and write something
to stderr.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
send-pack: read "unpack" status even on pack-objects failure
If the local pack-objects of a push fails, we'll tell the
user about it. But one likely cause is that the remote
index-pack stopped reading for some reason (because it
didn't like our input, or encountered another error). In
that case we'd expect the remote to report more details to
us via the "unpack ..." status line. However, the current
code just hangs up completely, and the user never sees it.
Instead, let's call receive_unpack_status(), which will
complain on stderr with whatever reason the remote told us.
Note that if our pack-objects fails because the connection
was severed or the remote just crashed entirely, then our
packet_read_line() call may fail with "the remote end hung
up unexpectedly". That's OK. It's a more accurate
description than what we get now (which is just "some refs
failed to push").
This should be safe from any deadlocks. At the point we make
this call we'll have closed the writing end of the
connection to the server (either by handing it off to
a pack-objects which exited, explicitly in the stateless_rpc
case, or by doing a half-duplex shutdown for a socket). So
there should be no chance that the other side is waiting
for the rest of our pack-objects input.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the remote tells us that the "unpack" step failed, we
show an error message. However, unless you are familiar with
the internals of send-pack and receive-pack, it was not
clear that this represented an error on the remote side.
Let's re-word to make that more obvious.
Likewise, when we got an unexpected packet from the other
end, we complained with a vague message but did not actually
show the packet. Let's fix that.
And finally, neither message was marked for translation. The
message from the remote probably won't be translated, but
there's no reason we can't do better for the local half.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
After sending the pack, we call receive_status() which gets
both the "unpack" line and the ref status. Let's break these
into two functions so we can call the first part
independently.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
receive-pack: fix deadlock when we cannot create tmpdir
The err_fd descriptor passed to the unpack() function is
intended to be handed off to the child index-pack, and our
async muxer will read until it gets EOF. However, if we
encounter an error before handing off the descriptor, we
must manually close(err_fd). Otherwise we will be waiting
for our muxer to finish, while the muxer is waiting for EOF
on err_fd.
We fixed an identical deadlock already in 49ecfa13f
(receive-pack: close sideband fd on early pack errors,
2013-04-19). But since then, the function grew a new
early-return in 722ff7f87 (receive-pack: quarantine objects
until pre-receive accepts, 2016-10-03), when we fail to
create a temporary directory. This return needs the same
treatment.
Reported-by: Horst Schirmeier <horst@schirmeier.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ewah subsystem typedefs eword_t to be uint64_t, but some
code uses a bare uint64_t. This isn't a bug now, but it's a
potential maintenance problem if the definition of eword_t
ever changes. Let's use the correct type.
Note that we can't use COPY_ARRAY() here because the source
and destination point to objects of different sizes. For
that reason we'll also skip the usual "sizeof(*dst)" and use
the real type, which should make it more clear that there's
something tricky going on.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This memcpy meant to get the sizeof a "struct range", not a
"range_set", as the former is what our array holds. Rather
than swap out the types, let's convert this site to
COPY_ARRAY, which avoids the problem entirely (and confirms
that the src and dst types match).
Note for curiosity's sake that this bug doesn't trigger on
I32LP64 systems, but does on ILP32 systems. The mistaken
"struct range_set" has two ints and a pointer. That's 16
bytes on LP64, or 12 on ILP32. The correct "struct range"
type has two longs, which is also 16 on LP64, but only 8 on
ILP32.
Likewise an IL32P64 system would experience the bug.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When Git v2.9.1 was released, it had a bug that showed only on Windows
and on 32-bit systems: our assumption that `unsigned long` can hold
64-bit values turned out to be wrong.
This could have been caught earlier if we had a Continuous Testing
set up that includes a build and test run on 32-bit Linux.
Let's do this (and take care of the Windows build later). This patch
asks Travis CI to install a Docker image with 32-bit libraries and then
goes on to build and test Git using this 32-bit setup.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This likely has no real-world impact on memory usage,
but it is cleaner for future readers.
Fixes: abcbdc03895f ("http: respect protocol.*.allow=user for http-alternates") Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
http: inform about alternates-as-redirects behavior
It is disconcerting for users to not notice the behavior
change in handling alternates from commit cb4d2d35c4622ec2
("http: treat http-alternates like redirects")
Give the user a hint about the config option so they can
see the URL and decide whether or not they want to enable
http.followRedirects in their config.
Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
filter-branch: fix --prune-empty on parentless commits
Previously, the git_commit_non_empty_tree function would always pass any
commit with no parents to git-commit-tree, regardless of whether the
tree was nonempty. The new commit would then be recorded in the
filter-branch revision map, and subsequent commits which leave the tree
untouched would be correctly filtered.
With this change, parentless commits with an empty tree are correctly
pruned, and an empty file is recorded in the revision map, signifying
that it was rewritten to "no commits." This works naturally with the
parent mapping for subsequent commits.
Signed-off-by: Devin J. Pohly <djpohly@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
line-log.c: prevent crash during union of too many ranges
The existing implementation of range_set_union does not correctly
reallocate memory, leading to a heap overflow when it attempts to union
more than 24 separate line ranges.
For struct range_set *out to grow correctly it must have out->nr set to
the current size of the buffer when it is passed to range_set_grow.
However, the existing implementation of range_set_union only updates
out->nr at the end of the function, meaning that it is always zero
before this. This results in range_set_grow never growing the buffer, as
well as some of the union logic itself being incorrect as !out->nr is
always true.
The reason why 24 is the limit is that the first allocation of size 1
ends up allocating a buffer of size 24 (due to the call to alloc_nr in
ALLOC_GROW). This goes some way to explain why this hasn't been
caught before.
Fix the problem by correctly updating out->nr after reallocating the
range_set. As this results in out->nr containing the same value as the
variable o, replace o with out->nr as well.
Finally, add a new test to help prevent the problem reoccurring in the
future. Thanks to Vegard Nossum for writing the test.
Signed-off-by: Allan Xavier <allan.x.xavier@oracle.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
contrib: git-remote-{bzr,hg} placeholders don't need Python
It does not make sense for these placeholder scripts to depend on Python
just because the real scripts do. At the example of Git for Windows, we
would not even be able to see those warnings as it does not ship with
Python. So just use plain shell scripts instead.
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/perf: add fallback for pre-bin-wrappers versions of git
It's tempting to say:
./run v1.0.0 HEAD
to see how we've sped up Git over the years. Unfortunately,
this doesn't quite work because versions of Git prior to
v1.7.0 lack bin-wrappers, so our "run" script doesn't
correctly put them in the PATH.
Worse, it means we silently find whatever other "git" is in
the PATH, and produce test results that have no bearing on
what we asked for.
Let's fallback to the main git directory when bin-wrappers
isn't present. Many modern perf scripts won't run with such
an antique version of Git, of course, but at least those
failures are detected and reported (and you're free to write
a limited perf script that works across many versions).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/perf: use $MODERN_GIT for all repo-copying steps
Since 1a0962dee (t/perf: fix regression in testing older
versions of git, 2016-06-22), we point "$MODERN_GIT" to a
copy of git that matches the t/perf script itself, and which
can be used for tasks outside of the actual timings. This is
needed because the setup done by perf scripts keeps moving
forward in time, and may use features that the older
versions of git we are testing do not have.
That commit used $MODERN_GIT to fix a case where we relied
on the relatively recent --git-path option. But if you go
back further still, there are more problems.
Since 7501b5921 (perf: make the tests work in worktrees,
2016-05-13), we use "git -C", but versions of git older than 44e1e4d67 (git: run in a directory given with -C option,
2013-09-09) don't know about "-C". So testing an old version
of git with a new version of t/perf will fail the setup
step.
We can fix this by using $MODERN_GIT during the setup;
there's no need to use the antique version, since it doesn't
affect the timings. Likewise, we'll adjust the "init"
invocation; antique versions of git called this "init-db".
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In p0001, a variable was created in a test_expect_success block to be
used in later test_perf blocks, but was not exported. This caused the
variable to not appear in those blocks (this can be verified by writing
'test -n "$commit"' in those blocks), resulting in a slightly different
invocation than what was intended. Export that variable.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch-pack: add specific error for fetching an unadvertised object
Enhance filter_refs (which decides whether a request for an unadvertised
object should be sent to the server) to record a new match status on the
"struct ref" when a request is not allowed, and have
report_unmatched_refs check for this status and print a special error
message, "Server does not allow request for unadvertised object".
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git fetch" currently doesn't bother to check that it got all refs it
sought, because the common case of requesting a nonexistent ref triggers
a die() in get_fetch_map. However, there's at least one case that
slipped through: "git fetch REMOTE SHA1" if the server doesn't allow
requests for unadvertised objects. Make fetch_refs_via_pack (which is
on the "git fetch" code path) call report_unmatched_refs so that we at
least get an error message in that case.
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
checkout: restrict @-expansions when finding branch
When we parse "git checkout $NAME", we try to interpret
$NAME as a local branch-name. If it is, then we point HEAD
to that branch. Otherwise, we detach the HEAD at whatever
commit $NAME points to.
We do the interpretation by calling strbuf_branchname(), and
then blindly sticking "refs/heads/" on the front. This leads
to nonsense results when expansions like "@{upstream}" or
"@" point to something besides a local branch. We end up
with a local branch name like "refs/heads/origin/master" or
"refs/heads/HEAD".
Normally this has no user-visible effect because those
branches don't exist, and so we fallback to feeding the
result to get_sha1(), which resolves them correctly.
But as the new test in t3204 shows, there are corner cases
where the effect is observable, and we check out the wrong
local branch rather than detaching to the correct one.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
strbuf_check_ref_format(): expand only local branches
This function asks strbuf_branchname() to expand any @-marks
in the branchname, and then we blindly stick refs/heads/ in
front of the result. This is obviously nonsense if the
expansion is "HEAD" or a ref in refs/remotes/.
The most obvious end-user effect is that creating or
renaming a branch with an expansion may have confusing
results (e.g., creating refs/heads/origin/master from
"@{upstream}" when the operation should be disallowed).
We can fix this by telling strbuf_branchname() that we are
only interested in local expansions. Any unexpanded bits are
then fed to check_ref_format(), which either disallows them
(in the case of "@{upstream}") or lets them through
("refs/heads/@" is technically valid, if a bit silly).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We use strbuf_branchname() to expand the branch name from
the command line, so you can delete the branch given by
@{-1}, for example. However, we allow other nonsense like
"@", and we do not respect our "-r" flag (so we may end up
deleting an oddly-named local ref instead of a remote one).
We can fix this by passing the appropriate "allowed" flag to
strbuf_branchname().
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-branch feeds the branch names from the command line to
strbuf_branchname(), but we do not yet tell that function
which kinds of expansions should be allowed. Let's create a
set of tests that cover both the allowed and disallowed
cases.
That shows off some breakages where we currently create or
delete the wrong ref (and will make sure that we do not
break any cases that _should_ be working when we do add more
restrictions).
Note that we check branch creation and deletion, but do not
bother with renames. Those follow the same code path as
creation.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
interpret_branch_name: allow callers to restrict expansions
The interpret_branch_name() function converts names like
@{-1} and @{upstream} into branch names. The expanded ref
names are not fully qualified, and may be outside of the
refs/heads/ namespace (e.g., "@" expands to "HEAD", and
"@{upstream}" is likely to be in "refs/remotes/").
This is OK for callers like dwim_ref() which are primarily
interested in resolving the resulting name, no matter where
it is. But callers like "git branch" treat the result as a
branch name in refs/heads/. When we expand to a ref outside
that namespace, the results are very confusing (e.g., "git
branch @" tries to create refs/heads/HEAD, which is
nonsense).
Callers can't know from the returned string how the
expansion happened (e.g., did the user really ask for a
branch named "HEAD", or did we do a bogus expansion?). One
fix would be to return some out-parameters describing the
types of expansion that occurred. This has the benefit that
the caller can generate precise error messages ("I
understood @{upstream} to mean origin/master, but that is a
remote tracking branch, so you cannot create it as a local
name").
However, out-parameters make the function interface somewhat
cumbersome. Instead, let's do the opposite: let the caller
tell us which elements to expand. That's easier to pass in,
and none of the callers give more precise error messages
than "@{upstream} isn't a valid branch name" anyway (which
should be sufficient).
The strbuf_branchname() function needs a similar parameter,
as most of the callers access interpret_branch_name()
through it.
We can break the callers down into two groups:
1. Callers that are happy with any kind of ref in the
result. We pass "0" here, so they continue to work
without restrictions. This includes merge_name(),
the reflog handling in add_pending_object_with_path(),
and substitute_branch_name(). This last is what powers
dwim_ref().
2. Callers that have funny corner cases (mostly in
git-branch and git-checkout). These need to make use of
the new parameter, but I've left them as "0" in this
patch, and will address them individually in follow-on
patches.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The return value from strbuf_branchname() is confusing and
useless: it's 0 if the whole name was consumed by an @-mark,
but otherwise is the length of the original name we fed.
No callers actually look at the return value, so let's just
get rid of it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
interpret_branch_name: move docstring to header file
We generally put docstrings with function declarations,
because it's the callers who need to know how the function
works. Let's do so for interpret_branch_name().
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
interpret_branch_name(): handle auto-namelen for @{-1}
The interpret_branch_name() function takes a ptr/len pair
for the name, but you can pass "0" for "namelen", which will
cause it to check the length with strlen().
However, before we do that auto-namelen magic, we call
interpret_nth_prior_checkout(), which gets fed the bogus
"0". This was broken by 8cd4249c4 (interpret_branch_name:
always respect "namelen" parameter, 2014-01-15). Though to
be fair to that commit, it was broken in the _opposite_
direction before, where we would always treat "name" as a
string even if a length was passed.
You can see the bug with "git log -g @{-1}". That code path
always passes "0", and without this patch it cannot figure
out which branch's reflog to show.
We can fix it by a small reordering of the code.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff: do not short-cut CHECK_SIZE_ONLY check in diff_populate_filespec()
Callers of diff_populate_filespec() can choose to ask only for the
size of the blob without grabbing the blob data, and the function,
after running lstat() when the filespec points at a working tree
file, returns by copying the value in size field of the stat
structure into the size field of the filespec when this is the case.
However, this short-cut cannot be taken if the contents from the
path needs to go through convert_to_git(), whose resulting real blob
data may be different from what is in the working tree file.
As "git diff --quiet" compares the .size fields of filespec
structures to skip content comparison, this bug manifests as a
false "there are differences" for a file that needs eol conversion,
for example.
Reported-by: Mike Crowe <mac@mcrowe.com> Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
add--interactive: fix missing file prompt for patch mode with "-i"
When invoked as "git add -i", each menu interactive menu
option prompts the user to select a list of files. This
includes the "patch" option, which gets the list before
starting the hunk-selection loop.
As "git add -p", it behaves differently, and jumps straight
to the hunk selection loop.
Since 0539d5e6d (i18n: add--interactive: mark patch prompt
for translation, 2016-12-14), the "add -i" case mistakenly
jumps to straight to the hunk-selection loop. Prior to that
commit the distinction between the two cases was managed by
the $patch_mode variable. That commit used $patch_mode for
something else, and moved the old meaning to the "$cmd"
variable. But it forgot to update the $patch_mode check
inside patch_update_cmd() which controls the file-list
behavior.
The simplest fix would be to change that line to check $cmd.
But while we're here, let's use a less obscure name for this
flag: $patch_mode_only, a boolean which tells whether we are
in full-interactive mode or only in patch-mode.
Reported-by: Henrik Grubbström <grubba@grubba.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
README: create HTTP/HTTPS links from URLs in Markdown
Markdown supports automatic links by surrounding URLs with
angle brackets, as documented in
<https://daringfireball.net/projects/markdown/syntax#autolink>
While we're at it, update URLs to avoid redirecting clients for
git-scm.com (by using HTTPS) and public-inbox.org (by adding a
trailing slash).
Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
gitweb tests: skip tests when we don't have Time::HiRes
Change the gitweb tests to skip when we can't load the Time::HiRes
module.
Gitweb needs this module to work. It has been in perl core since v5.8,
which is the oldest version we support. However CentOS (and perhaps
some other distributions) carve it into its own non-core-perl package
that's not installed along with /usr/bin/perl by default. Without this
we'll hard fail the gitweb tests when trying to load the module.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The last call to the mkstemps() function was removed in commit 659488326
("wrapper.c: delete dead function git_mkstemps()", 22-04-2016). In order
to support platforms without mkstemps(), this functionality was provided,
along with a Makefile build variable (NO_MKSTEMPS), by the gitmkstemps()
function. Remove the dead code, along with the defunct build machinery.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The last caller of git_mkstemp() was removed in commit 6fec0a89
("verify_signed_buffer: use tempfile object", 16-06-2016). Since
the introduction of the 'tempfile' APIs, along with git_mkstemp_mode,
it is unlikely that new callers will materialize. Remove the dead
code.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
(that is to say, as long as the Git part of the path and the query
string is preserved in the final redirect destination, the intermediate
steps can have any URL). However, if one of the intermediate steps
results in an HTTP exception, a confusing "unable to update url base
from redirection" message is printed instead of a Curl error message
with the HTTP exception code.
This was introduced by 2 commits. Commit c93c92f ("http: update base
URLs when we see redirects", 2013-09-28) introduced a best-effort
optimization that required checking if only the "base" part of the URL
differed between the initial request and the final redirect destination,
but it performed the check before any HTTP status checking was done. If
something went wrong, the normal code path was still followed, so this
did not cause any confusing error messages until commit 6628eb4 ("http:
always update the base URL for redirects", 2016-12-06), which taught
http to die if the non-"base" part of the URL differed.
Therefore, teach http to check the HTTP status before attempting to
check if only the "base" part of the URL differed. This commit teaches
http_request_reauth to return early without updating options->base_url
upon an error; the only invoker of this function that passes a non-NULL
"options" is remote-curl.c (through "http_get_strbuf"), which only uses
options->base_url for an informational message in the situations that
this commit cares about (that is, when the return value is not HTTP_OK).
The included test checks that the redirect scheme at the beginning of
this commit message works, and that returning a 502 in the middle of the
redirect scheme produces the correct result. Note that this is different
from the test in commit 6628eb4 ("http: always update the base URL for
redirects", 2016-12-06) in that this commit tests that a Git-shaped URL
(http://.../info/refs?service=git-upload-pack) works, whereas commit 6628eb4 tests that a non-Git-shaped URL
(http://.../info/refs/foo?service=git-upload-pack) does not work (even
though Git is processing that URL) and is an error that is fatal, not
silently swallowed.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
docs/diffcore: unquote "Complete Rewrites" in headers
The gitdiffcore documentation quotes the term "Complete Rewrites" in
headers for no real gain. This would make sense if the term could be
easily confused if not properly grouped together. But actually, the term
is quite obvious and thus does not really need any quoting, especially
regarding that it is not used anywhere else.
But more importanly, this brings up a bug when rendering man pages: when
trying to render quotes inside of a section header, we end up with
quotes which have been misaligned to the end of line. E.g.
diffcore-break: For Splitting Up Complete Rewrites
--------------------------------------------------
renders as
DIFFCORE-BREAK: FOR SPLITTING UP COMPLETE REWRITES""
, which is obviously wrong. While this is fixable for the man pages by
using double-quotes (e.g. ""COMPLETE REWRITES""), this again breaks it
for our generated HTML pages.
So fix the issue by simply dropping quotes inside of section headers,
which is currently only done for the term "Complete Rewrites".
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cvs tests: skip tests that call "cvs commit" when running as root
Change the tests that fail to when we run the test suite as root, due
to calling "cvs commit".
The GNU cvs package has an optional compile-time CVS_BADROOT
flag. When compiled with this flag "cvs commit" will refuse to commit
anything as root. On my Debian box this isn't compiled in[1] in, but
on CentOS it is.
I've run all the t/t*cvs*.sh tests, and these are the only two that
fail. For some reason e.g. t9402-git-cvsserver-refs.sh still works as
root despite doing "cvs commit", I haven't dug into why.
This commit is technically being overzealous, we could do better by
making a mock cvs commit as root and run the tests if that works, but
I don't see any compelling reason to bend over backwards to run these
tests in all cases, just skipping them as root seems good enough.
1. Per: strings /usr/bin/cvs|grep 'is not allowed to commit'
Using cvs 1.11.23 on CentOS, 1.12.13-MirDebian-18 on Debian.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In one test, we use "git checkout --orphan HEAD" to create
an unborn branch. Confusingly, the resulting branch is named
"refs/heads/HEAD". The original probably meant something
like:
git checkout --orphan orphaned-branch HEAD
Let's just use "orphaned-branch" here to make this less
confusing. Putting HEAD in the second argument is already
implied.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit: don't check for space twice when looking for header
Both standard_header_field() and excluded_header_field() check if
there's a space after the buffer that's handed to them. We already
check in the caller if that space is present. Don't bother calling
the functions if it's missing, as they are guaranteed to return 0 in
that case, and remove the now redundant checks from them.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit: be more precise when searching for headers
Search for a space character only within the current line in
read_commit_extra_header_lines() instead of searching in the whole
buffer (and possibly beyond, if it's not NUL-terminated) and then
discarding any results after the end of the current line.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a function for appending the canonized absolute pathname of a given
path to a strbuf. It keeps the existing contents intact, as expected of
a function of the strbuf_add() family, while avoiding copying the result
if the given strbuf is empty. It's more consistent with the rest of the
strbuf API than strbuf_realpath(), which it's wrapping.
Also add a semantic patch demonstrating its intended usage and apply it
to the current tree. Using strbuf_add_real_path() instead of calling
strbuf_addstr() and real_path() avoids an extra copy to a static buffer.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a semantic patch for using ALLOC_ARRAY to allocate arrays and apply
the transformation on the current source tree. The macro checks for
multiplication overflow and infers the element size automatically; the
result is shorter and safer code.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sha1_file: release fallback base's memory in unpack_entry()
If a pack entry that's used as a delta base is corrupt, unpack_entry()
marks it as unusable and then searches the object again in the hope that
it can be found in another pack or in a loose file. The memory for this
external base object is never released. Free it after use.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This variable needs to be specified to make some types of
non-basic authentication work, but ideally this would just
work out of the box for everyone.
However, simply setting it to "1" by default introduces an
extra round-trip for cases where it _isn't_ useful. We end
up sending a bogus empty credential that the server rejects.
Instead, let's introduce an automatic mode, that works like
this:
1. We won't try to send the bogus credential on the first
request. We'll wait to get an HTTP 401, as usual.
2. After seeing an HTTP 401, the empty-auth hack will kick
in only when we know there is an auth method available
that might make use of it (i.e., something besides
"Basic" or "Digest").
That should make it work out of the box, without incurring
any extra round-trips for people hitting Basic-only servers.
This _does_ incur an extra round-trip if you really want to
use "Basic" but your server advertises other methods (the
emptyauth hack will kick in but fail, and then Git will
actually ask for a password).
The auto mode may incur an extra round-trip over setting
http.emptyauth=true, because part of the emptyauth hack is
to feed this blank password to curl even before we've made a
single request.
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adding comments after a tag in the body is a common practise (e.g. in
the Linux kernel) and git-send-email has been supporting this for years
by removing any trailing cruft after the address.
After some recent changes, any trailing comment is now instead appended
to the recipient name (with some random white space inserted) resulting
in undesirable noise in the headers, for example:
CC: "# 3 . 3 . x : 1b9508f : sched : Rate-limit newidle" <stable@vger.kernel.org>
Revert to the earlier behaviour of discarding anything after the (first)
address in a tag while parsing the body.
Note that multiple addresses after are still allowed after a command
line switch (and in a CC header field).
Also note that --suppress-cc=self was never honoured when using multiple
addresses in a tag.
Signed-off-by: Johan Hovold <johan@kernel.org> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse_config_key() function was introduced to make it
easier to match "section.subsection.key" variables. It also
handles the simpler "section.key", and the caller is
responsible for distinguishing the two from its
out-parameters.
Most callers who _only_ want "section.key" would just use a
strcmp(var, "section.key"), since there is no parsing
required. However, they may still use parse_config_key() if
their "section" variable isn't a constant (an example of
this is in parse_hide_refs_config).
Using the parse_config_key is a bit clunky, though:
const char *subsection;
int subsection_len;
const char *key;
parse_config_key: use skip_prefix instead of starts_with
This saves us having to repeatedly add in "section_len" (and
also avoids walking over the first part of the string
multiple times for a strlen() and strrchr()).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs: parse_hide_refs_config to use parse_config_key
parse_config_key was introduced in 1b86bbb0ade (config: add helper
function for parsing key names, 2013-01-22), the NEEDSWORK that is removed
in this patch was introduced at daebaa7813 (upload/receive-pack: allow
hiding ref hierarchies, 2013-01-18), which is only a couple days apart,
so presumably the code replaced in this patch was only introduced due
to not wanting to wait on the proper helper function being available.
Make the condition easier to read by using parse_config_key.
Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>