Makefile: detect compiler and enable more warnings in DEVELOPER=1
The set of extra warnings we enable when DEVELOPER has to be
conservative because we can't assume any compiler version the
developer may use. Detect the compiler version so we know when it's
safe to enable -Wextra and maybe more.
These warning settings are mostly from my custom config.mak a long
time ago when I tried to enable as many warnings as possible that can
still build without showing warnings. Some of those warnings are
probably worth fixing instead of just suppressing in future.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is a series running in parallel with this one that adds code
like this
switch (...) {
case ...:
die_initial_contact();
case ...:
There is nothing wrong with this. There is no actual falling
through. But since gcc is not that smart and gcc 7.x introduces
-Wimplicit-fallthrough, it raises a false alarm in this case.
This class of warnings may be useful elsewhere, so instead of
suppressing the whole class, let's try to fix just this code. gcc is
smart enough to realize that no execution can continue after a
NORETURN function call and no longer raises the warning.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 22 Mar 2018 21:24:16 +0000 (14:24 -0700)]
Merge branch 'nd/shared-index-fix' into maint
Code clean-up.
* nd/shared-index-fix:
read-cache: don't write index twice if we can't write shared index
read-cache.c: move tempfile creation/cleanup out of write_shared_index
read-cache.c: change type of "temp" in write_shared_index()
Junio C Hamano [Thu, 22 Mar 2018 21:24:13 +0000 (14:24 -0700)]
Merge branch 'ks/submodule-doc-updates' into maint
Doc updates.
* ks/submodule-doc-updates:
Doc/git-submodule: improve readability and grammar of a sentence
Doc/gitsubmodules: make some changes to improve readability and syntax
Junio C Hamano [Thu, 22 Mar 2018 21:24:12 +0000 (14:24 -0700)]
Merge branch 'sg/test-i18ngrep' into maint
Test fixes.
* sg/test-i18ngrep:
t: make 'test_i18ngrep' more informative on failure
t: validate 'test_i18ngrep's parameters
t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'
t5536: let 'test_i18ngrep' read the file without redirection
t5510: consolidate 'grep' and 'test_i18ngrep' patterns
t4001: don't run 'git status' upstream of a pipe
t6022: don't run 'git merge' upstream of a pipe
t5812: add 'test_i18ngrep's missing filename parameter
t5541: add 'test_i18ngrep's missing filename parameter
Junio C Hamano [Thu, 22 Mar 2018 21:24:10 +0000 (14:24 -0700)]
Merge branch 'tg/split-index-fixes' into maint
The split-index mode had a few corner case bugs fixed.
* tg/split-index-fixes:
travis: run tests with GIT_TEST_SPLIT_INDEX
split-index: don't write cache tree with null oid entries
read-cache: fix reading the shared index for other repos
Junio C Hamano [Thu, 22 Mar 2018 21:24:09 +0000 (14:24 -0700)]
Merge branch 'jt/http-redact-cookies' into maint
The http tracing code, often used to debug connection issues,
learned to redact potentially sensitive information from its output
so that it can be more safely sharable.
* jt/http-redact-cookies:
http: support omitting data from traces
http: support cookie redaction when tracing
Junio C Hamano [Thu, 22 Mar 2018 21:24:08 +0000 (14:24 -0700)]
Merge branch 'sg/travis-build-during-script-phase' into maint
Build the executable in 'script' phase in Travis CI integration, to
follow the established practice, rather than during 'before_script'
phase. This allows the CI categorize the failures better ('failed'
is project's fault, 'errored' is build environment's).
* sg/travis-build-during-script-phase:
travis-ci: build Git during the 'script' phase
Junio C Hamano [Tue, 27 Feb 2018 18:43:54 +0000 (10:43 -0800)]
Merge branch 'sb/submodule-update-reset-fix' into maint
When resetting the working tree files recursively, the working tree
of submodules are now also reset to match.
* sb/submodule-update-reset-fix:
submodule: submodule_move_head omits old argument in forced case
unpack-trees: oneway_merge to update submodules
t/lib-submodule-update.sh: fix test ignoring ignored files in submodules
t/lib-submodule-update.sh: clarify test
Junio C Hamano [Tue, 27 Feb 2018 18:39:34 +0000 (10:39 -0800)]
Merge branch 'nd/ita-wt-renames-in-status' into maint
"git status" after moving a path in the working tree (hence making
it appear "removed") and then adding with the -N option (hence
making that appear "added") detected it as a rename, but did not
report the old and new pathnames correctly.
* nd/ita-wt-renames-in-status:
wt-status.c: handle worktree renames
wt-status.c: rename rename-related fields in wt_status_change_data
wt-status.c: catch unhandled diff status codes
wt-status.c: coding style fix
Use DIFF_DETECT_RENAME for detect_rename assignments
t2203: test status output with porcelain v2 format
In gitsubmodules.txt, a few non-ASCII apostrophes are used to spell
possessive, e.g. "submodule's". These unfortunately are not
rendered at https://git-scm.com/docs/gitsubmodules correctly by the
renderer used there.
Use ASCII apostrophes instead to work around the problem. It also
is good to be consistent, as there are possessives spelled with
ASCII apostrophes.
Signed-off-by: Motoki Seki <marmot.motoki@gmail.com> Acked-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 21 Feb 2018 23:27:24 +0000 (18:27 -0500)]
revision: drop --show-all option
This was an undocumented debugging aid that does not seem to
have come in handy in the past decade, judging from its lack
of mentions on the mailing list.
Let's drop it in the name of simplicity. This is morally a
revert of 3131b71301 (Add "--show-all" revision walker flag
for debugging, 2008-02-09), but note that I did leave in the
mapping of UNINTERESTING to "^" in get_revision_mark(). I
don't think this would be possible to trigger with the
current code, but it's the only sensible marker.
We'll skip the usual deprecation period because this was
explicitly a debugging aid that was never documented.
Signed-off-by: Jeff King <peff@peff.net> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 21 Feb 2018 23:13:38 +0000 (18:13 -0500)]
commit: drop uses of get_cached_commit_buffer()
The "--show-all" revision option shows UNINTERESTING
commits. Some of these commits may be unparsed when we try
to show them (since we may or may not need to walk their
parents to fulfill the request).
Commit 3131b71301 (Add "--show-all" revision walker flag for
debugging, 2008-02-09) resolved this by just skipping
pretty-printing for commits without their object contents
cached, saying:
Because we now end up listing commits we may not even have been parsed
at all "show_log" and "show_commit" need to protect against commits
that don't have a commit buffer entry.
That was the easy fix to avoid the pretty-printer segfaulting,
but:
1. It doesn't work for all formats. E.g., --oneline
prints the oid for each such commit but not a trailing
newline, leading to jumbled output.
2. It only affects some commits, depending on whether we
happened to parse them or not (so if they were at the
tip of an UNINTERESTING starting point, or if we
happened to traverse over them, you'd see more data).
3. It unncessarily ties the decision to show the verbose
header to whether the commit buffer was cached. That
makes it harder to change the logic around caching
(e.g., if we could traverse without actually loading
the full commit objects).
These days it's safe to feed such a commit to the
pretty-print code. Since be5c9fb904 (logmsg_reencode: lazily
load missing commit buffers, 2013-01-26), we'll load it on
demand in such a case. So let's just always show the verbose
headers.
This does change the behavior of plumbing, but:
a. The --show-all option was explicitly introduced as a
debugging aid, and was never documented (and has rarely
even been mentioned on the list by git devs).
b. Avoiding the commits was already not deterministic due
to (2) above. So the caller might have seen full
headers for these commits anyway, and would need to be
prepared for it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Fri, 16 Feb 2018 20:44:52 +0000 (15:44 -0500)]
git-worktree.txt: fix indentation of example and text of 'add' command
When 4e85333197 (worktree: make add <path> <branch> dwim, 2017-11-26)
added an example command in a literal code block, it neglected to
insert a mandatory "+" line before the block. This omission resulted
in both the literal code block and the (existing) paragraph following
the block to be outdented, even though they should be indented under
the 'add' sub-command along with the rest of the text pertaining to
that command. Furthermore, the mandatory "+" line separating the code
block from the following text got rendered as a leading character on
the line ("+ If <commit-ish>...") rather than being treated as a
formatting directive.
Fix these problems by adding the missing "+" line before the example
code block.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Fri, 16 Feb 2018 20:44:51 +0000 (15:44 -0500)]
git-worktree.txt: fix missing ")" typo
Add the closing ")" to a parenthetical phrase introduced by 4e85333197
(worktree: make add <path> <branch> dwim, 2017-11-26).
While at it, add a missing ":" at the end of the same sentence since
it precedes an example literal command block.
Reported-by: Mike Nordell <tamlin.thefirst@gmail.com> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Genki Sky [Fri, 16 Feb 2018 02:46:04 +0000 (21:46 -0500)]
test-lib.sh: unset XDG_CACHE_HOME
git respects XDG_CACHE_HOME for the credential cache. So, we should
unset XDG_CACHE_HOME for the test environment, lest a user's custom one
cause failure in the test.
For example, t/t0301-credential-cache.sh expects a default directory
to be used if it hasn't explicitly set XDG_CACHE_HOME.
Signed-off-by: Genki Sky <sky@genki.is> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 15 Feb 2018 23:18:12 +0000 (15:18 -0800)]
Merge branch 'jk/abort-clone-with-existing-dest' into maint
"git clone $there $here" is allowed even when here directory exists
as long as it is an empty directory, but the command incorrectly
removed it upon a failure of the operation.
* jk/abort-clone-with-existing-dest:
clone: do not clean up directories we didn't create
clone: factor out dir_exists() helper
t5600: modernize style
t5600: fix outdated comment about unborn HEAD
Junio C Hamano [Thu, 15 Feb 2018 23:18:11 +0000 (15:18 -0800)]
Merge branch 'rs/lose-leak-pending' into maint
API clean-up around revision traversal.
* rs/lose-leak-pending:
commit: remove unused function clear_commit_marks_for_object_array()
revision: remove the unused flag leak_pending
checkout: avoid using the rev_info flag leak_pending
bundle: avoid using the rev_info flag leak_pending
bisect: avoid using the rev_info flag leak_pending
object: add clear_commit_marks_all()
ref-filter: use clear_commit_marks_many() in do_merge_filter()
commit: use clear_commit_marks_many() in remove_redundant()
commit: avoid allocation in clear_commit_marks_many()
Junio C Hamano [Thu, 15 Feb 2018 23:18:11 +0000 (15:18 -0800)]
Merge branch 'jm/svn-pushmergeinfo-fix' into maint
"git svn dcommit" did not take into account the fact that a
svn+ssh:// URL with a username@ (typically used for pushing) refers
to the same SVN repository without the username@ and failed when
svn.pushmergeinfo option is set.
* jm/svn-pushmergeinfo-fix:
git-svn: fix svn.pushmergeinfo handling of svn+ssh usernames.
Stefan Beller [Thu, 15 Feb 2018 00:18:12 +0000 (16:18 -0800)]
Documentation/git-status: clarify status table for porcelain mode
It is possible to have the output ' A' from 'git status --porcelain'
by adding a file using the '--intend-to-add' flag. Make this clear by
adding the pattern in the table of the documentation.
However the mode 'DM' (deleted in the index, modified in the working tree)
is not possible in the non-merge case in which the file only shows
as 'D ' (and adding it back to the worktree would show an additional line
of an '??' untracked file). It is also not possible in the merge case as
then the mode involves a 'U' on one side of the merge.
Remove that pattern.
Reported-by: Ross Light <light@google.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Tue, 13 Feb 2018 02:23:52 +0000 (02:23 +0000)]
docs/interpret-trailers: fix agreement error
In the description of git interpret-trailers, we describe "a group…of
lines" that have certain characteristics. Ensure both options
describing this group use a singular verb for parallelism.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ramsay Jones [Mon, 12 Feb 2018 00:21:02 +0000 (00:21 +0000)]
Makefile: suppress a sparse warning for pack-revindex.c
Sparse has, for a long time, been issuing the following warning against
the pack-revindex.c file:
SP pack-revindex.c
pack-revindex.c:64:23: warning: memset with byte count of 262144
This results from a unconditional check, with a hard-coded limit, which
is really only appropriate for the kernel source code. (The check is for
a 'large' byte count in a call to memcpy(), memset(), copy_from_user()
and copy_to_user() functions).
A recent release of sparse (v0.5.1) has introduced some options to allow
this check to be turned off (-Wno-memcpy-max-count) or to specify the
actual limit used (-fmemcpy-max-count=COUNT), rather than a hard-coded
limit of 100000.
In order to suppress the warning, add a target for pack-revindex.sp that
adds the '-Wno-memcpy-max-count' option to the SPARSE_FLAGS variable.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ramsay Jones [Mon, 12 Feb 2018 00:20:08 +0000 (00:20 +0000)]
config.mak.uname: remove SPARSE_FLAGS setting for cygwin
Since commit f66450ae9 ("cygwin: Remove the Win32 l/stat() implementation",
2013-06-22), the cygwin build has not used the WIN32 API/header files.
This means that the '-isystem /usr/include/w32api' option to sparse is
no longer necessary (to allow sparse to find the WIN32 header files).
In addition, the '-Wno-one-bit-signed-bitfield' option can be removed,
since the warning suppressed by that option was only provoked by a WIN32
header file.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 8 Feb 2018 15:56:56 +0000 (16:56 +0100)]
t: make 'test_i18ngrep' more informative on failure
When 'test_i18ngrep' can't find the expected pattern, it exits
completely silently; when its negated form does find the pattern that
shouldn't be there, it prints the matching line(s) but otherwise exits
without any error message. This leaves the developer puzzled about
what could have gone wrong.
Make 'test_i18ngrep' more informative on failure by printing an error
message including the invoked 'grep' command and the contents of the
file it had to scan through.
Note that this "dump the scanned file" part is not quite perfect, as
it dumps only the file specified as the function's last positional
parameter, thus assuming that there is only a single file parameter.
I think that's a reasonable assumption to make, one that holds true in
the current code base. And even if someone were to scan multiple
files at once in the future, the worst thing that could happen is that
the verbose error message won't include the contents of all those
files, only the last one. Alas, we can't really do any better than
this, because checking whether the other positional parameters match a
filename can result in false positives: 't3400-rebase.sh' and
't3404-rebase-interactive.sh' contain one test each, where the
'test_i18ngrep's pattern verbatimly matches a file in the trash
directory.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 8 Feb 2018 15:56:55 +0000 (16:56 +0100)]
t: validate 'test_i18ngrep's parameters
Some of the previous patches in this series fixed bogus
'test_i18ngrep' invocations:
- Two invocations where the tested git command's standard output is
directly piped into 'test_i18ngrep'. While convenient, this is an
antipattern, because the pipe hides the git command's exit code,
and the test could continue even if the command exited with error.
- Two invocations that had neither a filename parameter nor anything
piped into their standard input, yet both managed to remain
unnoticed for years. A third similarly bogus invocation is
currently lurking in 'pu' for a couple of weeks now.
Prevent similar mistakes in the future by validating 'test_i18ngrep's
parameters requiring that
- The last parameter names an existing file to be read, effectively
forbidding piping into 'test_i18ngrep'.
Note that this change will also forbid cases where 'test_i18ngrep'
would legitimately read its standard input, e.g. when its standard
input is redirected from a file, or when a git command's standard
output is first written to an intermediate file, which is then
preprocessed by a non-git command before the results are piped
into 'test_i18ngrep'. See two of the previous patches for the
only such cases we had in our test suite. However, reliably
preventing the piping antipattern is arguably more important than
supporting these cases, which can be easily worked around by
opening the file directly or using an intermediate file anyway.
- There are at least two parameters, not including the optional '!'
to negate the pattern. This ought to catch corner cases when
'test_i18ngrep' looks for the name of an existing file on its
standard input; the above check would miss this case becase the
filename as pattern would be the last parameter.
Note that this is not quite perfect, as it doesn't account for any
'grep --options' given as parameters. However, doing so would be
far too complicated, considering that patterns can start with
dashes as well, and in the majority of the cases we don't use any
such options anyway.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 8 Feb 2018 15:56:54 +0000 (16:56 +0100)]
t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'
Both 'test_i18ncmp' and 'test_i18ngrep' helper functions are supposed
to be called from our test scripts, so they should be in
'test-lib-functions.sh'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 8 Feb 2018 15:56:52 +0000 (16:56 +0100)]
t5510: consolidate 'grep' and 'test_i18ngrep' patterns
One of the tests in 't5510-fetch.sh' checks the output of 'git fetch'
using 'test_i18ngrep', and while doing so it prefilters the output
with 'grep' before piping the result into 'test_i18ngrep'.
This prefiltering is unnecessary, with the appropriate pattern
'test_i18ngrep' can do it all by itself. Furthermore, piping data
into 'test_i18ngrep' will interfere with the linting that will be
added in a later patch.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 8 Feb 2018 15:56:51 +0000 (16:56 +0100)]
t4001: don't run 'git status' upstream of a pipe
The primary purpose of three tests in 't4001-diff-rename.sh' is to
check rename detection in 'git status', but all three do so by running
'git status' upstream of a pipe, hiding its exit code. Consequently,
the test could continue even if 'git status' exited with error.
Use an intermediate file between 'git status' and 'test_i18ngrep' to
catch a potential failure of the former.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Thu, 8 Feb 2018 15:56:50 +0000 (16:56 +0100)]
t6022: don't run 'git merge' upstream of a pipe
The primary purpose of 't6022-merge-rename.sh' is to test 'git merge',
but one of the tests runs it upstream of a pipe, hiding its exit code.
Consequently, the test could continue even if 'git merge' exited with
error.
Use an intermediate file between 'git merge' and 'test_i18ngrep' to
catch a potential failure of the former.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The second 'test_i18ngrep' invocation in the test 'curl redirects
respect whitelist' is missing its filename parameter. This has
remained unnoticed since its introduction in f4113cac0 (http: limit
redirection to protocol-whitelist, 2015-09-22), because it would only
cause the test to fail if Git was built with a sufficiently old
libcurl version. The test's two ||-chained 'test_i18ngrep'
invocations are supposed to check that either one of the two patterns
is present in 'git clone's error message. As it happens, the first
invocation covers the error message from any reasonably up-to-date
libcurl, thus the second invocation, the one without the filename
parameter, isn't executed at all. Apparently no one has run the test
suite's httpd tests with such an old libcurl in the last 2+ years, or
at least they haven't bothered to notify us about the failed test.
Fix this by consolidating the two patterns into a single extended
regexp, eliminating the need for an ||-chained second 'test_i18ngrep'
invocation.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The test 'push --no-progress silences progress but not status' runs
'test_i18ngrep' without specifying a filename parameter. This has
remained unnoticed since its introduction in e304aeba2 (t5541: test
more combinations of --progress, 2012-05-01), because that
'test_i18ngrep' is supposed to check that the given pattern is not
present in its input, and of course it won't find that pattern if its
input is empty (as it comes from /dev/null). This also means that
this test could miss a potential breakage of 'git push --no-progress'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 6 Feb 2018 08:44:56 +0000 (03:44 -0500)]
git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME
Running "make NO_GETTEXT=1 GETTEXT_POISON=1" currently fails
t0205.
While it might seem nonsensical at first glance to both
poison and disable gettext, it's useful to be able to do a
poison test-run on a system that doesn't have gettext at
all. And it works fine for C programs; the problem is only
with the shell code.
The issue is that we check the baked-in USE_GETTEXT_SCHEME
value before GETTEXT_POISON. And when NO_GETTEXT is set, the
Makefile sets USE_GETTEXT_SCHEME to "fallthrough".
So one fix would be to have the Makefile just set
USE_GETTEXT_SCHEME to "poison" if GETTEXT_POISON is set.
But there are two problems with that:
1. USE_GETTEXT_SCHEME is actually a user-facing knob, so
conceivably somebody could override it with:
make USE_GETTEXT_SCHEME=gnu GETTEXT_POISON=1
which would do the wrong thing (though that's much less
likely than them having the variable set in their
config.mak and just overriding GETTEXT_POISON on the
command-line for a one-off test).
2. We don't actually bake GETTEXT_POISON in to the shell
library like we do for the C code. It checks
$GIT_GETTEXT_POISON at runtime, which is set up by the
test suite. So it makes sense to put the fix in the
runtime code, too, which would cover something like:
GIT_GETTEXT_POISON=foo git foo
It's not likely that people use the poison code outside
of running the test suite, but it's easy enough to make
this case work.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Thu, 1 Feb 2018 18:56:34 +0000 (19:56 +0100)]
cocci: simplify check for trivial format strings
353d84c537 (coccicheck: make transformation for strbuf_addf(sb, "...")
more precise) added a check to avoid transforming calls with format
strings which contain percent signs, as that would change the result.
It uses embedded Python code for that. Simplify this rule by using the
regular expression matching operator instead.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Duy Nguyen [Thu, 1 Feb 2018 09:59:15 +0000 (16:59 +0700)]
gitignore.txt: elaborate shell glob syntax
`fnmatch(3)` is a great mention if the intended audience is
programmers. For normal users it's probably better to spell out what
a shell glob is.
This paragraph is updated to roughly tell (or remind) what the main
wildcards are supposed to do. All the details are still hidden away
behind the `fnmatch(3)` wall because bringing the whole specification
here may be too much.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Wong [Mon, 29 Jan 2018 23:11:07 +0000 (23:11 +0000)]
git-svn: control destruction order to avoid segfault
It seems necessary to control destruction ordering to avoid a
segfault with SVN 1.9.5 when using "git svn branch". I've also
reported the problem against libsvn-perl to Debian [Bug #888791],
but releasing the SVN::Client instance can be beneficial anyways to
save memory.
ref: https://bugs.debian.org/888791 Tested-by: Todd Zullinger <tmz@pobox.com> Reported-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Todd Zullinger [Sun, 28 Jan 2018 19:16:07 +0000 (14:16 -0500)]
doc: mention 'git show' defaults to HEAD
When 'git show' is called without any object it defaults to HEAD. This
has been true since d4ed9793fd ("Simplify common default options setup
for built-in log family.", 2006-04-16).
The SYNOPSIS suggests that the object argument is required. Clarify
that it is not required and note the default.
Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Duy Nguyen [Fri, 26 Jan 2018 01:31:42 +0000 (08:31 +0700)]
completion: fix completing merge strategies on non-C locales
The anchor string "Available strategies are:" is translatable so
__git_list_merge_strategies may fail to collect available strategies
from 'git merge' on non-C locales. Force C locale on this command.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Jan 2018 00:58:54 +0000 (19:58 -0500)]
daemon: fix length computation in newline stripping
When git-daemon gets a pktline request, we strip off any
trailing newline, replacing it with a NUL. Clients prior to 5ad312bede (in git v1.4.0) would send:
git-upload-pack repo.git\n
and we need to strip it off to understand their request.
After 5ad312bede, we send the host attribute but no newline,
like:
git-upload-pack repo.git\0host=example.com\0
Both of these are parsed correctly by git-daemon. But if
some client were to combine the two:
git-upload-pack repo.git\n\0host=example.com\0
we don't parse it correctly. The problem is that we use the
"len" variable to record the position of the NUL separator,
but then decrement it when we strip the newline. So we start
with:
git-upload-pack repo.git\n\0host=example.com\0
^-- len
and end up with:
git-upload-pack repo.git\0\0host=example.com\0
^-- len
This is arguably correct, since "len" tells us the length of
the initial string, but we don't actually use it for that.
What we do use it for is finding the offset of the extended
attributes; they used to be at len+1, but are now at len+2.
We can solve that by just leaving "len" where it is. We
don't have to care about the length of the shortened string,
since we just treat it like a C string.
No version of Git ever produced such a string, but it seems
like the daemon code meant to handle this case (and it seems
like a reasonable thing for somebody to do in a 3rd-party
implementation).
Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Jan 2018 00:58:19 +0000 (19:58 -0500)]
t/lib-git-daemon: add network-protocol helpers
All of our git-protocol tests rely on invoking the client
and having it make a request of a server. That gives a nice
real-world test of how the two behave together, but it
doesn't leave any room for testing how a server might react
to _other_ clients.
Let's add a few test helper functions which can be used to
manually conduct a git-protocol conversation with a remote
git-daemon:
1. To connect to a remote git-daemon, we need something
like "netcat". But not everybody will have netcat. And
even if they do, the behavior with respect to
half-duplex shutdowns is not portable (openbsd netcat
has "-N", with others you must rely on "-q 1", which is
racy).
Here we provide a "fake_nc" that is capable of doing
a client-side netcat, with sane half-duplex semantics.
It relies on perl's IO::Socket::INET. That's been in
the base distribution since 5.6.0, so it's probably
available everywhere. But just to be on the safe side,
we'll add a prereq.
2. To help tests speak and read pktline, this patch adds
packetize() and depacketize() functions.
I've put fake_nc() into lib-git-daemon.sh, since that's
really the only server where we'd need to use a network
socket. Whereas the pktline helpers may be of more general
use, so I've added them to test-lib-functions.sh. Programs
like upload-pack speak pktline, but can talk directly over
stdio without a network socket.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Jan 2018 00:56:20 +0000 (19:56 -0500)]
daemon: handle NULs in extended attribute string
If we receive a request with extended attributes after the
NUL, we try to write those attributes to the log. We do so
with a "%s" format specifier, which will only show
characters up to the first NUL.
That's enough for printing a "host=" specifier. But since dfe422d04d (daemon: recognize hidden request arguments,
2017-10-16) we may have another NUL, followed by protocol
parameters, and those are not logged at all.
Let's cut out the attempt to show the whole string, and
instead log when we parse individual attributes. We could
leave the "extended attributes (%d bytes) exist" part of the
log, which in theory could alert us to attributes that fail
to parse. But anything we don't parse as a "host=" parameter
gets blindly added to the "protocol" attribute, so we'd see
it in that part of the log.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Jan 2018 00:56:07 +0000 (19:56 -0500)]
daemon: fix off-by-one in logging extended attributes
If receive a request like:
git-upload-pack /foo.git\0host=localhost
we mark the offset of the NUL byte as "len", and then log
the bytes after the NUL with a "%.*s" placeholder, using
"pktlen - len" as the length, and "line + len + 1" as the
start of the string.
This is off-by-one, since the start of the string skips past
the separating NUL byte, but the adjusted length includes
it. Fortunately this doesn't actually read past the end of
the buffer, since "%.*s" will stop when it hits a NUL. And
regardless of what is in the buffer, packet_read() will
always add an extra NUL terminator for safety.
As an aside, the git.git client sends an extra NUL after a
"host" field, too, so we'd generally hit that one first, not
the one added by packet_read(). You can see this in the test
output which reports 15 bytes, even though the string has
only 14 bytes of visible data. But the point is that even a
client sending unusual data could not get us to read past
the end of the buffer, so this is purely a cosmetic fix.
Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Jan 2018 19:16:41 +0000 (14:16 -0500)]
t/lib-git-daemon: record daemon log
When we start git-daemon for our tests, we send its stderr
log stream to a named pipe. We synchronously read the first
line to make sure that the daemon started, and then dump the
rest to descriptor 4. This is handy for debugging test
output with "--verbose", but the tests themselves can't
access the log data.
Let's dump the log into a file, as well, so that future
tests can check the log. There are a few subtleties worth
calling out here:
- we'll continue to send output to descriptor 4 for
viewing/debugging, which would imply swapping out "cat"
for "tee". But we want to ensure that there's no
buffering, and "tee" doesn't have a standard way to
ask for that. So we'll use a shell loop around "read"
and "printf" instead. That ensures that after a request
has been served, the matching log entries will have made
it to the file.
- the existing first-line shell loop used read/echo. We'll
switch to consistently using "read -r" and "printf" to
relay data as faithfully as possible.
- we open the logfile for append, rather than just output.
That makes it OK for tests to truncate the logfile
without restarting the daemon (the OS will atomically
seek to the end of the file when outputting each line).
That allows tests to look at the log without worrying
about pollution from earlier tests.
Helped-by: Lucas Werkmeister <mail@lucaswerkmeister.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Jan 2018 00:55:05 +0000 (19:55 -0500)]
t5570: use ls-remote instead of clone for interp tests
We don't actually care about the clone operation here; we
just want to know if we were able to actually contact the
remote repository. Using ls-remote does that more
efficiently, and without us having to worry about managing
the tmp.git directory.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If <msg> or <patch> files can't be opened, then mailinfo() returns an
error before it even initializes mi->p_hdr_data or mi->s_hdr_data.
When cmd_mailinfo() then calls clear_mailinfo(), we dereference the
NULL pointers trying to free their contents.
Signed-off-by: Juan F. Codagnone <jcodagnone@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
read-cache: don't write index twice if we can't write shared index
In a0a967568e ("update-index --split-index: do not split if $GIT_DIR is
read only", 2014-06-13), we tried to make sure we can still write an
index, even if the shared index can not be written.
We did so by just calling 'do_write_locked_index()' just before
'write_shared_index()'. 'do_write_locked_index()' always at least
closes the tempfile nowadays, and used to close or commit the lockfile
if COMMIT_LOCK or CLOSE_LOCK were given at the time this feature was
introduced. COMMIT_LOCK or CLOSE_LOCK is passed in by most callers of
'write_locked_index()'.
After calling 'write_shared_index()', we call 'write_split_index()',
which calls 'do_write_locked_index()' again, which then tries to use the
closed lockfile again, but in fact fails to do so as it's already
closed. This eventually leads to a segfault.
Make sure to write the main index only once.
[nd: most of the commit message and investigation done by Thomas, I only
tweaked the solution a bit]
Helped-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Wong [Sat, 20 Jan 2018 07:43:51 +0000 (07:43 +0000)]
fsck: fix leak when traversing trees
While fsck_walk/fsck_walk_tree/parse_tree populates "struct tree"
idempotently, it is still up to the fsck_walk caller to call
free_tree_buffer.
Fixes: ad2db4030e42890e ("fsck: remove redundant parse_tree() invocation") Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Sun, 21 Jan 2018 08:07:28 +0000 (03:07 -0500)]
t5601-clone: test case-conflicting files on case-insensitive filesystem
A recently introduced regression caused a segfault at clone time on
case-insensitive filesystems when filenames differing only in case are
present. This bug has already been fixed (repository: pre-initialize
hash algo pointer, 2018-01-18), but it's not the first time similar
problems have arisen. Therefore, introduce a test to catch this case and
protect against future regressions.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Fri, 19 Jan 2018 04:18:25 +0000 (04:18 +0000)]
repository: pre-initialize hash algo pointer
There are various git subcommands (among them, clone) which don't set up
the repository (that is, they lack RUN_SETUP or RUN_SETUP_GENTLY) but
end up needing to have information about the hash algorithm in use.
Because the hash algorithm is part of struct repository and it's only
initialized in repository setup, we can end up dereferencing a NULL
pointer in some cases if we call one of these subcommands and look up
the empty blob or empty tree values.
A "git clone" of a project that has two paths that differ only in
case suffers from this if it is run on a case insensitive platform.
When the command attempts to check out one of these two paths after
checking out the other one, the checkout codepath needs to see if
the version that is already on the filesystem (which should not
happen if the FS were case sensitive) is dirty, and it needs to
exercise the hashing code at that point.
In the future, we can add a command line option for this or read it
from the configuration, but until we're ready to expose that
functionality to the user, simply initialize the repository
structure to use the current hash algorithm, SHA-1.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mathias Rav [Thu, 18 Jan 2018 13:38:41 +0000 (14:38 +0100)]
files_initial_transaction_commit(): only unlock if locked
Running git clone --single-branch --mirror -b TAGNAME previously
triggered the following error message:
fatal: multiple updates for ref 'refs/tags/TAGNAME' not allowed.
This error condition is handled in files_initial_transaction_commit().
42c7f7ff9 ("commit_packed_refs(): remove call to `packed_refs_unlock()`", 2017-06-23)
introduced incorrect unlocking in the error path of this function,
which changes the error message to
fatal: BUG: packed_refs_unlock() called when not locked
Move the call to packed_refs_unlock() above the "cleanup:" label
since the unlocking should only be done in the last error path.
Signed-off-by: Mathias Rav <m@git.strova.dk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan [Fri, 19 Jan 2018 00:28:02 +0000 (16:28 -0800)]
http: support omitting data from traces
GIT_TRACE_CURL provides a way to debug what is being sent and received
over HTTP, with automatic redaction of sensitive information. But it
also logs data transmissions, which significantly increases the log file
size, sometimes unnecessarily. Add an option "GIT_TRACE_CURL_NO_DATA" to
allow the user to omit such data transmissions.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan [Fri, 19 Jan 2018 00:28:01 +0000 (16:28 -0800)]
http: support cookie redaction when tracing
When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and
"Proxy-Authorization:" HTTP headers. Extend this redaction to a
user-specified list of cookies, specified through the
"GIT_REDACT_COOKIES" environment variable.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thomas Gummerer [Sun, 7 Jan 2018 22:30:15 +0000 (22:30 +0000)]
travis: run tests with GIT_TEST_SPLIT_INDEX
Split index mode only has a few dedicated tests, but as the index is
involved in nearly every git operation, this doesn't quite cover all the
ways repositories with split index can break. To use split index mode
throughout the test suite a GIT_TEST_SPLIT_INDEX environment variable
can be set, which makes git split the index at random and thus
excercises the functionality much more thoroughly.
As this is not turned on by default, it is not executed nearly as often
as the test suite is run, so occationally breakages slip through. Try
to counteract that by running the test suite with GIT_TEST_SPLIT_INDEX
mode turned on on travis.
To avoid using too many cycles on travis only run split index mode in
the linux-gcc target only. The Linux build was chosen over the Mac OS
builds because it tends to be much faster to complete.
The linux gcc build was chosen over the linux clang build because the
linux clang build is the fastest build, so it can serve as an early
indicator if something is broken and we want to avoid spending the extra
cycles of running the test suite twice for that.
Helped-by: Lars Schneider <larsxschneider@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
