Johannes Schindelin [Mon, 30 Jul 2018 15:42:57 +0000 (08:42 -0700)]
vscode: add a dictionary for cSpell
The quite useful cSpell extension allows VS Code to have "squiggly"
lines under spelling mistakes. By default, this would add too much
clutter, though, because so much of Git's source code uses words that
would trigger cSpell.
Let's add a few words to make the spell checking more useful by reducing
the number of false positives.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:55 +0000 (08:42 -0700)]
vscode: use 8-space tabs, no trailing ws, etc for Git's source code
This adds a couple settings for the .c/.h files so that it is easier to
conform to Git's conventions while editing the source code.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:54 +0000 (08:42 -0700)]
vscode: wrap commit messages at column 72 by default
When configuring VS Code as core.editor (via `code --wait`), we really
want to adhere to the Git conventions of wrapping commit messages.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:52 +0000 (08:42 -0700)]
vscode: only overwrite C/C++ settings
The C/C++ settings are special, as they are the only generated VS Code
configurations that *will* change over the course of Git's development,
e.g. when a new constant is defined.
Therefore, let's only update the C/C++ settings, also to prevent user
modifications from being overwritten.
Ideally, we would keep user modifications in the C/C++ settings, but
that would require parsing JSON, a task for which a Unix shell script is
distinctly unsuited. So we write out .new files instead, and warn the
user if they may want to reconcile their changes.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:51 +0000 (08:42 -0700)]
mingw: define WIN32 explicitly
This helps VS Code's intellisense to figure out that we want to include
windows.h, and that we want to define the minimum target Windows version
as Windows Vista/2008R2.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:49 +0000 (08:42 -0700)]
cache.h: extract enum declaration from inside a struct declaration
While it is technically possible, it is confusing. Not only the user,
but also VS Code's intellisense.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:48 +0000 (08:42 -0700)]
vscode: hard-code a couple defines
Sadly, we do not get all of the definitions via ALL_CFLAGS. Some defines
are passed to GCC *only* when compiling specific files, such as git.o.
Let's just hard-code them into the script for the time being.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 30 Jul 2018 15:42:46 +0000 (08:42 -0700)]
contrib: add a script to initialize VS Code configuration
VS Code is a lightweight but powerful source code editor which runs on
your desktop and is available for Windows, macOS and Linux. Among other
languages, it has support for C/C++ via an extension, which offers to
not only build and debug the code, but also Intellisense, i.e.
code-aware completion and similar niceties.
This patch adds a script that helps set up the environment to work
effectively with VS Code: simply run the Unix shell script
contrib/vscode/init.sh, which creates the relevant files, and open the
top level folder of Git's source code in VS Code.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sun, 29 Jul 2018 15:36:05 +0000 (17:36 +0200)]
pack-objects: document about thread synchronization
These extra comments should be make it easier to understand how to use
locks in pack-objects delta search code. For reference, see
8ecce684a3 (basic threaded delta search - 2007-09-06)
384b32c09b (pack-objects: fix threaded load balancing - 2007-12-08)
50f22ada52 (threaded pack-objects: Use condition... - 2007-12-16)
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ramsay Jones [Sat, 28 Jul 2018 22:51:28 +0000 (23:51 +0100)]
t5562: avoid non-portable "export FOO=bar" construct
Commit
6c213e863a ("http-backend: respect CONTENT_LENGTH for
receive-pack", 2018-07-27) adds a test which uses the non-portable
export construct. Replace it with "FOO=bar && export FOO" instead.
Signed-off-by: Ramsay Jones <redacted>
Signed-off-by: Junio C Hamano <redacted>
Masaya Suzuki [Sat, 28 Jul 2018 21:16:30 +0000 (14:16 -0700)]
doc: fix want-capability separator
Unlike ref advertisement, client capabilities and the first want are
separated by SP, not NUL, in the implementation. Fix the documentation
to align with the implementation. pack-protocol.txt is already fixed.
Signed-off-by: Masaya Suzuki <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 17:48:11 +0000 (17:48 +0000)]
tests: make use of the test_must_be_empty function
Change various tests that use an idiom of the form:
>expect &&
test_cmp expect actual
To instead use:
test_must_be_empty actual
The test_must_be_empty() wrapper was introduced in
ca8d148daf ("test:
test_must_be_empty helper", 2013-06-09). Many of these tests have been
added after that time. This was mostly found with, and manually pruned
from:
git grep '^\s+>.*expect.* &&$' t
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 17:48:11 +0000 (17:48 +0000)]
tests: make use of the test_must_be_empty function
Change various tests that use an idiom of the form:
>expect &&
test_cmp expect actual
To instead use:
test_must_be_empty actual
The test_must_be_empty() wrapper was introduced in
ca8d148daf ("test:
test_must_be_empty helper", 2013-06-09). Many of these tests have been
added after that time. This was mostly found with, and manually pruned
from:
git grep '^\s+>.*expect.* &&$' t
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:20 +0000 (14:37 +0000)]
fsck: test and document unknown fsck.<msg-id> values
When fsck.<msg-id> is set to an unknown value it'll cause "fsck" to
die, but the same is not true of the "fetch" and "receive"
variants. Document this and test for it.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:19 +0000 (14:37 +0000)]
fsck: add stress tests for fsck.skipList
Stress test the parsing logic shared by fsck.skipList and
{fetch,receive}.fsck.skipList added in
cd94c6f91e ("fsck: git
receive-pack: support excluding objects from fsck'ing",
2015-06-22). There were no tests for the work done by the
init_skiplist() routine, e.g. how it dies on invalid input.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:18 +0000 (14:37 +0000)]
fsck: test & document {fetch,receive}.fsck.* config fallback
Test and document that the {fetch,receive}.fsck.* family of variables
doesn't fall back on the corresponding .fsck.* variables.
This was alluded to in the existing documentation by saying that
"receive" looks at receive.fsck.* and "fsck" looks at fsck.* etc., but
it wasn't explicitly stated that there was no fallback, and if you'd
e.g. like to configure the skipList you need to do that for all three.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:17 +0000 (14:37 +0000)]
fetch: implement fetch.fsck.*
Implement support for fetch.fsck.* corresponding with the existing
receive.fsck.*. This allows for pedantically cloning repositories with
specific issues without turning off fetch.fsckObjects.
One such repository is https://github.com/robbyrussell/oh-my-zsh.git
which before this change will emit this error when cloned with
fetch.fsckObjects:
error: object
2b7227859263b6aabcc28355b0b994995b7148b6: zeroPaddedFilemode: contains zero-padded file modes
fatal: Error in object
fatal: index-pack failed
Now with fetch.fsck.zeroPaddedFilemode=warn we'll warn about that
issue, but the clone will succeed:
warning: object
2b7227859263b6aabcc28355b0b994995b7148b6: zeroPaddedFilemode: contains zero-padded file modes
warning: object
a18c4d13c2a5fa2d4ecd5346c50e119b999b807d: zeroPaddedFilemode: contains zero-padded file modes
warning: object
84df066176c8da3fd59b13731a86d90f4f1e5c9d: zeroPaddedFilemode: contains zero-padded file modes
The motivation for this is to be able to turn on fetch.fsckObjects
globally across a fleet of computers but still be able to manually
clone various legacy repositories by either white-listing specific
issues, or better yet whitelist specific objects.
The use of --git-dir=* instead of -C in the tests could be considered
somewhat archaic, but the tests I'm adding here are duplicating the
corresponding receive.* tests with as few changes as possible.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:16 +0000 (14:37 +0000)]
transfer.fsckObjects tests: untangle confusing setup
The tests for transfer.fsckObjects have grown organically over time to
not make much sense.
Initially when these were added in
b10a53583f ("test: fetch/receive
with fsckobjects", 2011-09-04) they were only testing the "corrupt or
missing object" case, but later on in
70a4ae73d8 ("fsck: add a simple
test for receive.fsck.<msg-id>", 2015-06-22) they were expanded to
check for the fsck.<msg-id> feature.
The problem was that we still kept the same corrupt test repo, making
it harder to add new tests that check the entirety of the repository
between operations via "git fsck" to see whether only known issues
that can be ignored with fsck.<msg-id> have occurred.
The tests only did the right thing because such a full "git fsck" was
never done after a certain point, and instead we were only
manipulating specific refs. This makes it harder to add new tests, and
none of the fsck.<msg-id> tests relied on this.
So let's not confuse the two and repair the corrupt repository before
we run the fsck.<msg-id> tests.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:15 +0000 (14:37 +0000)]
config doc: elaborate on fetch.fsckObjects security
Change the transfer.fsckObjects documentation to explicitly note the
unique security and/or corruption issues fetch.fsckObjects suffers
from, since it doesn't have a quarantine environment.
This was already alluded to in the existing documentation, but let's
spell it out so there's no confusion here, and give a concrete example
of how to work around this limitation.
Let's also prominently note that this is considered to be a limitation
of the current implementation, rather than something that's intended
and by design, since we might change this in the future.
See
https://public-inbox.org/git/
20180531060259.GE17344@sigill.intra.peff.net/
for further details.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:14 +0000 (14:37 +0000)]
config doc: elaborate on what transfer.fsckObjects does
The existing documentation led the user to believe that all we were
doing were basic reachability sanity checks, but that hasn't been true
for a very long time. Update the description to match reality, and
note the caveat that there's a quarantine for accepting pushes, but
not for fetching.
Also mention that the fsck checks for security issues, which was my
initial motivation for writing this fetch.fsck.* series.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:13 +0000 (14:37 +0000)]
config doc: unify the description of fsck.* and receive.fsck.*
The documentation for the fsck.<msg-id> and receive.fsck.<msg-id>
variables was mostly duplicated in two places, with fsck.<msg-id>
making no mention of the corresponding receive.fsck.<msg-id>, and the
same for fsck.skipList.
I spent quite a lot of time today wondering why setting the
fsck.<msg-id> variant wasn't working to clone a legacy repository (not
that that would have worked anyway, but a subsequent patch implements
fetch.fsck.<msg-id>).
Rectify this situation by describing the feature in general terms
under the fsck.* documentation, and make the receive.fsck.*
documentation refer to those variables instead.
This documentation was initially added in
2becf00ff7 ("fsck: support
demoting errors to warnings", 2015-06-22) and
4b55b9b479 ("fsck:
document the new receive.fsck.<msg-id> options", 2015-06-22).
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:12 +0000 (14:37 +0000)]
config doc: don't describe *.fetchObjects twice
Refer readers of fetch.fsckObjects and receive.fsckObjects to
transfer.fsckObjects instead of repeating the description at each
location.
I don't think this description of them makes much sense, but for now
I'm just moving the existing documentation around. Making it better
will be done in a later patch.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ævar Arnfjörð Bjarmason [Fri, 27 Jul 2018 14:37:11 +0000 (14:37 +0000)]
receive.fsck.<msg-id> tests: remove dead code
Remove the setting of a receive.fsck.badDate config variable to
"ignore". This was added in
efaba7cc77 ("fsck: optionally ignore
specific fsck issues completely", 2015-06-22) but never did anything,
presumably it was part of some work-in-progress code that never made
it into git.git.
None of these tests will emit the "invalid author/committer line - bad
date" warning. The dates on the commit objects we're setting up are
not invalid.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Fri, 27 Jul 2018 12:59:44 +0000 (12:59 +0000)]
merge-recursive: preserve skip_worktree bit when necessary
merge-recursive takes any files marked as unmerged by unpack_trees,
tries to figure out whether they can be resolved (e.g. using renames
or a file-level merge), and then if they can be it will delete the old
cache entries and writes new ones. This means that any ce_flags for
those cache entries are essentially cleared when merging.
Unfortunately, if a file was marked as skip_worktree and it needs a
file-level merge but the merge results in the same version of the file
that was found in HEAD, we skip updating the worktree (because the
file was unchanged) but clear the skip_worktree bit (because of the
delete-cache-entry-and-write-new-one). This makes git treat the file
as having a local change in the working copy, namely a delete, when it
should appear as unchanged despite not being present. Avoid this
problem by copying the skip_worktree flag in this case.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ben Peart [Fri, 27 Jul 2018 12:59:42 +0000 (12:59 +0000)]
t3507: add a testcase showing failure with sparse checkout
Recent changes in merge_content() induced a bug when merging files that are
not present in the local working directory due to sparse-checkout. Add a
test case to demonstrate the bug so that we can ensure the fix resolves
it and to prevent future regressions.
Signed-off-by: Ben Peart <redacted>
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Max Kirillov [Fri, 27 Jul 2018 03:48:59 +0000 (06:48 +0300)]
http-backend: respect CONTENT_LENGTH for receive-pack
Push passes to another commands, as described in
https://public-inbox.org/git/
20171129032214.GB32345@sigill.intra.peff.net/
As it gets complicated to correctly track the data length, instead transfer
the data through parent process and cut the pipe as the specified length is
reached. Do it only when CONTENT_LENGTH is set, otherwise pass the input
directly to the forked commands.
Add tests for cases:
* CONTENT_LENGTH is set, script's stdin has more data, with all combinations
of variations: fetch or push, plain or compressed body, correct or truncated
input.
* CONTENT_LENGTH is specified to a value which does not fit into ssize_t.
Helped-by: Junio C Hamano <redacted>
Signed-off-by: Max Kirillov <redacted>
Signed-off-by: Junio C Hamano <redacted>
Beat Bolli [Wed, 25 Jul 2018 21:56:07 +0000 (23:56 +0200)]
packfile: ensure that enum object_type is defined
When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with
"make DEVELOPER=1 DEVOPTS=pedantic", the compiler says
error: redeclaration of already-defined enum 'object_type' is a GNU
extension [-Werror,-Wgnu-redeclared-enum]
According to https://en.cppreference.com/w/c/language/declarations
(section "Redeclaration"), a repeated declaration after the definition
is only legal for structs and unions, but not for enums.
Drop the belated declaration of enum object_type and include cache.h
instead to make sure the enum is defined.
Helped-by: Jonathan Nieder <redacted>
Signed-off-by: Beat Bolli <redacted>
Reviewed-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 09:28:28 +0000 (05:28 -0400)]
banned.h: mark strncpy() as banned
The strncpy() function is less horrible than strcpy(), but
is still pretty easy to misuse because of its funny
termination semantics. Namely, that if it truncates it omits
the NUL terminator, and you must remember to add it
yourself. Even if you use it correctly, it's sometimes hard
for a reader to verify this without hunting through the
code. If you're thinking about using it, consider instead:
- strlcpy() if you really just need a truncated but
NUL-terminated string (we provide a compat version, so
it's always available)
- xsnprintf() if you're sure that what you're copying
should fit
- strbuf or xstrfmt() if you need to handle
arbitrary-length heap-allocated strings
Note that there is one instance of strncpy in
compat/regex/regcomp.c, which is fine (it allocates a
sufficiently large string before copying). But this doesn't
trigger the ban-list even when compiling with NO_REGEX=1,
because:
1. we don't use git-compat-util.h when compiling it
(instead we rely on the system includes from the
upstream library); and
2. It's in an "#ifdef DEBUG" block
Since it's doesn't trigger the banned.h code, we're better
off leaving it to keep our divergence from upstream minimal.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 09:27:19 +0000 (05:27 -0400)]
banned.h: mark sprintf() as banned
The sprintf() function (and its variadic form vsprintf) make
it easy to accidentally introduce a buffer overflow. If
you're thinking of using them, you're better off either
using a dynamic string (strbuf or xstrfmt), or xsnprintf if
you really know that you won't overflow. The last sprintf()
call went away quite a while ago in
f0766bf94e (fsck: use
for_each_loose_file_in_objdir, 2015-09-24).
Note that we respect HAVE_VARIADIC_MACROS here, which some
ancient platforms lack. As a fallback, we can just "guess"
that the caller will provide 3 arguments. If they do, then
the macro will work as usual. If not, then they'll get a
slightly less useful error, like:
git.c:718:24: error: macro "sprintf" passed 3 arguments, but takes just 2
That's not ideal, but it at least alerts them to the problem
area. And anyway, we're primarily targeting people adding
new code. Most developers should be on modern enough
platforms to see the normal "good" error message.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 09:26:39 +0000 (05:26 -0400)]
banned.h: mark strcat() as banned
The strcat() function has all of the same overflow problems
as strcpy(). And as a bonus, it's easy to end up
accidentally quadratic, as each subsequent call has to walk
through the existing string.
The last strcat() call went away in
f063d38b80 (daemon: use
cld->env_array when re-spawning, 2015-09-24). In general,
strcat() can be replaced either with a dynamic string
(strbuf or xstrfmt), or with xsnprintf if you know the
length is bounded.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 26 Jul 2018 07:21:05 +0000 (03:21 -0400)]
automatically ban strcpy()
There are a few standard C functions (like strcpy) which are
easy to misuse. E.g.:
char path[PATH_MAX];
strcpy(path, arg);
may overflow the "path" buffer. Sometimes there's an earlier
constraint on the size of "arg", but even in such a case
it's hard to verify that the code is correct. If the size
really is unbounded, you're better off using a dynamic
helper like strbuf:
struct strbuf path = STRBUF_INIT;
strbuf_addstr(path, arg);
or if it really is bounded, then use xsnprintf to show your
expectation (and get a run-time assertion):
char path[PATH_MAX];
xsnprintf(path, sizeof(path), "%s", arg);
which makes further auditing easier.
We'd usually catch undesirable code like this in a review,
but there's no automated enforcement. Adding that
enforcement can help us be more consistent and save effort
(and a round-trip) during review.
This patch teaches the compiler to report an error when it
sees strcpy (and will become a model for banning a few other
functions). This has a few advantages over a separate
linting tool:
1. We know it's run as part of a build cycle, so it's
hard to ignore. Whereas an external linter is an extra
step the developer needs to remember to do.
2. Likewise, it's basically free since the compiler is
parsing the code anyway.
3. We know it's robust against false positives (unlike a
grep-based linter).
The two big disadvantages are:
1. We'll only check code that is actually compiled, so it
may miss code that isn't triggered on your particular
system. But since presumably people don't add new code
without compiling it (and if they do, the banned
function list is the least of their worries), we really
only care about failing to clean up old code when
adding new functions to the list. And that's easy
enough to address with a manual audit when adding a new
function (which is what I did for the functions here).
2. If this ends up generating false positives, it's going
to be harder to disable (as opposed to a separate
linter, which may have mechanisms for overriding a
particular case).
But the intent is to only ban functions which are
obviously bad, and for which we accept using an
alternative even when this particular use isn't buggy
(e.g., the xsnprintf alternative above).
The implementation here is simple: we'll define a macro for
the banned function which replaces it with a reference to a
descriptively named but undeclared identifier. Replacing it
with any invalid code would work (since we just want to
break compilation). But ideally we'd meet these goals:
- it should be portable; ideally this would trigger
everywhere, and does not need to be part of a DEVELOPER=1
setup (because unlike warnings which may depend on the
compiler or system, this is a clear indicator of
something wrong in the code).
- it should generate a readable error that gives the
developer a clue what happened
- it should avoid generating too much other cruft that
makes it hard to see the actual error
- it should mention the original callsite in the error
The output with this patch looks like this (using gcc 7, on
a checkout with
022d2ac1f3 reverted, which removed the final
strcpy from blame.c):
CC builtin/blame.o
In file included from ./git-compat-util.h:1246,
from ./cache.h:4,
from builtin/blame.c:8:
builtin/blame.c: In function ‘cmd_blame’:
./banned.h:11:22: error: ‘sorry_strcpy_is_a_banned_function’ undeclared (first use in this function)
#define BANNED(func) sorry_##func##_is_a_banned_function
^~~~~~
./banned.h:14:21: note: in expansion of macro ‘BANNED’
#define strcpy(x,y) BANNED(strcpy)
^~~~~~
builtin/blame.c:1074:4: note: in expansion of macro ‘strcpy’
strcpy(repeated_meta_color, GIT_COLOR_CYAN);
^~~~~~
./banned.h:11:22: note: each undeclared identifier is reported only once for each function it appears in
#define BANNED(func) sorry_##func##_is_a_banned_function
^~~~~~
./banned.h:14:21: note: in expansion of macro ‘BANNED’
#define strcpy(x,y) BANNED(strcpy)
^~~~~~
builtin/blame.c:1074:4: note: in expansion of macro ‘strcpy’
strcpy(repeated_meta_color, GIT_COLOR_CYAN);
^~~~~~
This prominently shows the phrase "strcpy is a banned
function", along with the original callsite in blame.c and
the location of the ban code in banned.h. Which should be
enough to get even a developer seeing this for the first
time pointed in the right direction.
This doesn't match our ideals perfectly, but it's a pretty
good balance. A few alternatives I tried:
1. Instead of using an undeclared variable, using an
undeclared function. This shortens the message, because
the "each undeclared identifier" message is not needed
(and as you can see above, it triggers a separate
mention of each of the expansion points).
But it doesn't actually stop compilation unless you use
-Werror=implicit-function-declaration in your CFLAGS.
This is the case for DEVELOPER=1, but not for a default
build (on the other hand, we'd eventually produce a
link error pointing to the correct source line with the
descriptive name).
2. The linux kernel uses a similar mechanism in its
BUILD_BUG_ON_MSG(), where they actually declare the
function but do so with gcc's error attribute. But
that's not portable to other compilers (and it also
runs afoul of our error() macro).
We could make a gcc-specific technique and fallback on
other compilers, but it's probably not worth the
complexity. It also isn't significantly shorter than
the error message shown above.
3. We could drop the BANNED() macro, which would shorten
the number of lines in the error. But curiously,
removing it (and just expanding strcpy directly to the
bogus identifier) causes gcc _not_ to report the
original line of code.
So this strategy seems to be an acceptable mix of
information, portability, simplicity, and robustness,
without _too_ much extra clutter. I also tested it with
clang, and it looks as good (actually, slightly less
cluttered than with gcc).
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Ralf Thielow [Thu, 26 Jul 2018 16:51:13 +0000 (18:51 +0200)]
l10n: de.po: translate 108 new messages
Translate 108 new messages came from git.pot update in
9b7388a85 (l10n:
git.pot: v2.18.0 round 1 (108 new, 14 removed)).
Signed-off-by: Ralf Thielow <redacted>
Acked-by: Matthias Rüster <redacted>
Ævar Arnfjörð Bjarmason [Wed, 25 Jul 2018 08:30:23 +0000 (08:30 +0000)]
doc hash-function-transition: note the lack of a changelog
The changelog embedded in the document pre-dates the addition of the
document to git.git (it used to be a Google Doc), so it only goes up
to
752414ae43 ("technical doc: add a design doc for hash function
transition", 2017-09-27).
Since then I made some small edits to it, which would have been worthy
of including in this changelog (but weren't). Instead of amending it
to include these, just note that future changes will be noted in the
log.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Tue, 24 Jul 2018 21:58:45 +0000 (17:58 -0400)]
diff: --color-moved: rename "dimmed_zebra" to "dimmed-zebra"
The --color-moved "dimmed_zebra" mode (with an underscore) is an
anachronism. Most options and modes are hyphenated. It is more difficult
to type and somewhat more difficult to read than those which are
hyphenated. Therefore, rename it to "dimmed-zebra", and nominally
deprecate "dimmed_zebra".
Signed-off-by: Eric Sunshine <redacted>
Reviewed-by: Jonathan Nieder <redacted>
Signed-off-by: Junio C Hamano <redacted>
Beat Bolli [Tue, 24 Jul 2018 19:26:43 +0000 (21:26 +0200)]
Makefile: add a DEVOPTS flag to get pedantic compilation
In the interest of code hygiene, make it easier to compile Git with the
flag -pedantic.
Pure pedantic compilation with GCC 7.3 results in one warning per use of
the translation macro `N_`:
warning: array initialized from parenthesized string constant [-Wpedantic]
Therefore also disable the parenthesising of i18n strings with
-DUSE_PARENS_AROUND_GETTEXT_N=0.
Signed-off-by: Beat Bolli <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Tue, 24 Jul 2018 21:59:49 +0000 (14:59 -0700)]
Fourth batch for 2.19 cycle
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Tue, 24 Jul 2018 21:50:51 +0000 (14:50 -0700)]
Merge branch 'as/sequencer-customizable-comment-char'
Honor core.commentchar when preparing the list of commits to replay
in "rebase -i".
* as/sequencer-customizable-comment-char:
sequencer: use configured comment character
Junio C Hamano [Tue, 24 Jul 2018 21:50:50 +0000 (14:50 -0700)]
Merge branch 'sb/blame-color'
Code clean-up.
* sb/blame-color:
blame: prefer xsnprintf to strcpy for colors
Junio C Hamano [Tue, 24 Jul 2018 21:50:50 +0000 (14:50 -0700)]
Merge branch 'nd/command-list'
Build doc update for Windows.
* nd/command-list:
vcbuild/README: update to accommodate for missing common-cmds.h
Junio C Hamano [Tue, 24 Jul 2018 21:50:50 +0000 (14:50 -0700)]
Merge branch 'es/test-lint-one-shot-export'
Look for broken use of "VAR=VAL shell_func" in test scripts as part
of test-lint.
* es/test-lint-one-shot-export:
t/check-non-portable-shell: detect "FOO=bar shell_func"
t/check-non-portable-shell: make error messages more compact
t/check-non-portable-shell: stop being so polite
t6046/t9833: fix use of "VAR=VAL cmd" with a shell function
Junio C Hamano [Tue, 24 Jul 2018 21:50:49 +0000 (14:50 -0700)]
Merge branch 'wc/find-commit-with-pattern-on-detached-head'
"git rev-parse ':/substring'" did not consider the history leading
only to HEAD when looking for a commit with the given substring,
when the HEAD is detached. This has been fixed.
* wc/find-commit-with-pattern-on-detached-head:
sha1-name.c: for ":/", find detached HEAD commits
Junio C Hamano [Tue, 24 Jul 2018 21:50:49 +0000 (14:50 -0700)]
Merge branch 'jc/t3404-one-shot-export-fix'
Correct a broken use of "VAR=VAL shell_func" in a test.
* jc/t3404-one-shot-export-fix:
t3404: fix use of "VAR=VAL cmd" with a shell function
Junio C Hamano [Tue, 24 Jul 2018 21:50:48 +0000 (14:50 -0700)]
Merge branch 'mk/merge-in-sparse-checkout'
"git reset --merge" (hence "git merge ---abort") and "git reset --hard"
had trouble working correctly in a sparsely checked out working
tree after a conflict, which has been corrected.
* mk/merge-in-sparse-checkout:
unpack-trees: do not fail reset because of unmerged skipped entry
Junio C Hamano [Tue, 24 Jul 2018 21:50:48 +0000 (14:50 -0700)]
Merge branch 'hs/push-cert-check-cleanup'
Code clean-up.
* hs/push-cert-check-cleanup:
gpg-interface: make parse_gpg_output static and remove from interface header
builtin/receive-pack: use check_signature from gpg-interface
Junio C Hamano [Tue, 24 Jul 2018 21:50:48 +0000 (14:50 -0700)]
Merge branch 'jk/empty-pick-fix'
Handling of an empty range by "git cherry-pick" was inconsistent
depending on how the range ended up to be empty, which has been
corrected.
* jk/empty-pick-fix:
sequencer: don't say BUG on bogus input
sequencer: handle empty-set cases consistently
Junio C Hamano [Tue, 24 Jul 2018 21:50:47 +0000 (14:50 -0700)]
Merge branch 'bp/log-ref-write-fd-with-strbuf'
Code clean-up.
* bp/log-ref-write-fd-with-strbuf:
convert log_ref_write_fd() to use strbuf
Junio C Hamano [Tue, 24 Jul 2018 21:50:47 +0000 (14:50 -0700)]
Merge branch 'jt/partial-clone-fsck-connectivity'
Partial clone support of "git clone" has been updated to correctly
validate the objects it receives from the other side. The server
side has been corrected to send objects that are directly
requested, even if they may match the filtering criteria (e.g. when
doing a "lazy blob" partial clone).
* jt/partial-clone-fsck-connectivity:
clone: check connectivity even if clone is partial
upload-pack: send refs' objects despite "filter"
Junio C Hamano [Tue, 24 Jul 2018 21:50:47 +0000 (14:50 -0700)]
Merge branch 'bc/send-email-auto-cte'
The content-transfer-encoding of the message "git send-email" sends
out by default was 8bit, which can cause trouble when there is an
overlong line to bust RFC 5322/2822 limit. A new option 'auto' to
automatically switch to quoted-printable when there is such a line
in the payload has been introduced and is made the default.
* bc/send-email-auto-cte:
docs: correct RFC specifying email line length
send-email: automatically determine transfer-encoding
send-email: accept long lines with suitable transfer encoding
send-email: add an auto option for transfer encoding
Junio C Hamano [Tue, 24 Jul 2018 21:50:47 +0000 (14:50 -0700)]
Merge branch 'bb/unicode-11-width'
The character display width table has been updated to match the
latest Unicode standard.
* bb/unicode-11-width:
unicode: update the width tables to Unicode 11
Junio C Hamano [Tue, 24 Jul 2018 21:50:47 +0000 (14:50 -0700)]
Merge branch 'bb/pedantic'
The codebase has been updated to compile cleanly with -pedantic
option.
* bb/pedantic:
utf8.c: avoid char overflow
string-list.c: avoid conversion from void * to function pointer
sequencer.c: avoid empty statements at top level
convert.c: replace "\e" escapes with "\033".
fixup! refs/refs-internal.h: avoid forward declaration of an enum
refs/refs-internal.h: avoid forward declaration of an enum
fixup! connect.h: avoid forward declaration of an enum
connect.h: avoid forward declaration of an enum
Junio C Hamano [Tue, 24 Jul 2018 21:50:46 +0000 (14:50 -0700)]
Merge branch 'tb/config-default'
Compilation fix.
* tb/config-default:
builtin/config: work around an unsized array forward declaration
Junio C Hamano [Tue, 24 Jul 2018 21:50:46 +0000 (14:50 -0700)]
Merge branch 'mh/fast-import-no-diff-delta-empty'
"git fast-import" has been updated to avoid attempting to create
delta against a zero-byte-long string, which is pointless.
* mh/fast-import-no-diff-delta-empty:
fast-import: do not call diff_delta() with empty buffer
Junio C Hamano [Tue, 24 Jul 2018 21:50:46 +0000 (14:50 -0700)]
Merge branch 'kn/userdiff-php'
The userdiff pattern for .php has been updated.
* kn/userdiff-php:
userdiff: support new keywords in PHP hunk header
t4018: add missing test cases for PHP
Junio C Hamano [Tue, 24 Jul 2018 21:50:45 +0000 (14:50 -0700)]
Merge branch 'jk/fetch-all-peeled-fix'
Test modernization.
* jk/fetch-all-peeled-fix:
t5500: prettify non-commit tag tests
Junio C Hamano [Tue, 24 Jul 2018 21:50:44 +0000 (14:50 -0700)]
Merge branch 'ag/rebase-p'
The help message shown in the editor to edit todo list in "rebase -p"
has regressed recently, which has been corrected.
* ag/rebase-p:
git-rebase--preserve-merges: fix formatting of todo help message
Junio C Hamano [Tue, 24 Jul 2018 21:50:44 +0000 (14:50 -0700)]
Merge branch 'jt/connectivity-check-after-unshallow'
"git fetch" failed to correctly validate the set of objects it
received when making a shallow history deeper, which has been
corrected.
* jt/connectivity-check-after-unshallow:
fetch-pack: write shallow, then check connectivity
fetch-pack: implement ref-in-want
fetch-pack: put shallow info in output parameter
fetch: refactor to make function args narrower
fetch: refactor fetch_refs into two functions
fetch: refactor the population of peer ref OIDs
upload-pack: test negotiation with changing repository
upload-pack: implement ref-in-want
test-pkt-line: add unpack-sideband subcommand
Junio C Hamano [Tue, 24 Jul 2018 21:50:44 +0000 (14:50 -0700)]
Merge branch 'jk/for-each-ref-icase'
The "--ignore-case" option of "git for-each-ref" (and its friends)
did not work correctly, which has been fixed.
* jk/for-each-ref-icase:
ref-filter: avoid backend filtering with --ignore-case
for-each-ref: consistently pass WM_IGNORECASE flag
t6300: add a test for --ignore-case
Junio C Hamano [Tue, 24 Jul 2018 21:50:43 +0000 (14:50 -0700)]
Merge branch 'en/t5407-rebase-m-fix'
* en/t5407-rebase-m-fix:
t5407: fix test to cover intended arguments
Junio C Hamano [Tue, 24 Jul 2018 21:50:43 +0000 (14:50 -0700)]
Merge branch 'en/apply-comment-fix'
* en/apply-comment-fix:
apply: fix grammar error in comment
Junio C Hamano [Tue, 24 Jul 2018 21:50:43 +0000 (14:50 -0700)]
Merge branch 'en/rebase-consistency'
"git rebase" behaved slightly differently depending on which one of
the three backends gets used; this has been documented and an
effort to make them more uniform has begun.
* en/rebase-consistency:
git-rebase: make --allow-empty-message the default
t3401: add directory rename testcases for rebase and am
git-rebase.txt: document behavioral differences between modes
directory-rename-detection.txt: technical docs on abilities and limitations
git-rebase.txt: address confusion between --no-ff vs --force-rebase
git-rebase: error out when incompatible options passed
t3422: new testcases for checking when incompatible options passed
git-rebase.sh: update help messages a bit
git-rebase.txt: document incompatible options
Junio C Hamano [Tue, 24 Jul 2018 21:50:43 +0000 (14:50 -0700)]
Merge branch 'sb/submodule-move-head-error-msg'
"git checkout --recurse-submodules another-branch" did not report
in which submodule it failed to update the working tree, which
resulted in an unhelpful error message.
* sb/submodule-move-head-error-msg:
submodule.c: report the submodule that an error occurs in
Junio C Hamano [Tue, 24 Jul 2018 21:50:42 +0000 (14:50 -0700)]
Merge branch 'rj/submodule-fsck-skip'
"fsck.skipList" did not prevent a blob object listed there from
being inspected for is contents (e.g. we recently started to
inspect the contents of ".gitmodules" for certain malicious
patterns), which has been corrected.
* rj/submodule-fsck-skip:
fsck: check skiplist for object in fsck_blob()
Brandon Williams [Mon, 23 Jul 2018 17:48:07 +0000 (10:48 -0700)]
pack-protocol: mention and point to docs for protocol v2
Signed-off-by: Brandon Williams <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 10:52:29 +0000 (06:52 -0400)]
strbuf_humanise: use unsigned variables
All of the numeric formatting done by this function uses
"%u", but we pass in a signed "int". The actual range
doesn't matter here, since the conditional makes sure we're
always showing reasonably small numbers. And even gcc's
format-checker does not seem to mind. But it's potentially
confusing to a reader of the code to see the mismatch.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 10:51:39 +0000 (06:51 -0400)]
pass st.st_size as hint for strbuf_readlink()
When we initially added the strbuf_readlink() function in
b11b7e13f4 (Add generic 'strbuf_readlink()' helper function,
2008-12-17), the point was that we generally have a _guess_
as to the correct size based on the stat information, but we
can't necessarily trust it.
Over the years, a few callers have grown up that simply pass
in 0, even though they have the stat information. Let's have
them pass in their hint for consistency (and in theory
efficiency, since it may avoid an extra resize/syscall loop,
but neither location is probably performance critical).
Note that st.st_size is actually an off_t, so in theory we
need xsize_t() here. But none of the other callsites use it,
and since this is just a hint, it doesn't matter either way
(if we wrap we'll simply start with a too-small hint and
then eventually complain when we cannot allocate the
memory).
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 10:51:25 +0000 (06:51 -0400)]
strbuf_readlink: use ssize_t
The return type of readlink() is ssize_t, not int. This
probably doesn't matter in practice, as it would require a
2GB symlink destination, but it doesn't hurt to be careful.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 10:51:08 +0000 (06:51 -0400)]
strbuf: use size_t for length in intermediate variables
A few strbuf functions store the length of a strbuf in a
temporary variable. We should always use size_t for this, as
it's possible for a strbuf to exceed an "int" (e.g., a 2GB
string on a 64-bit system). This is unlikely in practice,
but we should try to behave sensibly on silly or malicious
input.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 10:50:33 +0000 (06:50 -0400)]
reencode_string: use size_t for string lengths
The iconv interface takes a size_t, which is the appropriate
type for an in-memory buffer. But our reencode_string_*
functions use integers, meaning we may get confusing results
when the sizes exceed INT_MAX. Let's use size_t
consistently.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 24 Jul 2018 10:50:10 +0000 (06:50 -0400)]
reencode_string: use st_add/st_mult helpers
When converting a string with iconv, if the output buffer
isn't big enough, we grow it. But our growth is done without
any concern for integer overflow. So when we add:
outalloc = sofar + insz * 2 + 32;
we may end up wrapping outalloc (which is a size_t), and
allocating a too-small buffer. We then manipulate it
further:
outsz = outalloc - sofar - 1;
and feed outsz back to iconv. If outalloc is wrapped and
smaller than sofar, we'll end up with a small allocation but
feed a very large outsz to iconv, which could result in it
overflowing the buffer.
Can we use this to construct an attack wherein the victim
clones a repository with a very large commit object with an
encoding header, and running "git log" reencodes it into
utf8, causing an overflow?
An attack of this sort is likely impossible in practice.
"sofar" is how many output bytes we've written total, and
"insz" is the number of input bytes remaining. Imagine our
input doubles in size as we output it (which is easy to do
by converting latin1 to utf8, for example), and that we
start with N input bytes. Our initial output buffer also
starts at N bytes, so after the first call we'd have N/2
input bytes remaining (insz), and have written N bytes
(sofar). That means our next allocation will be
(N + N/2 * 2 + 32) bytes, or (2N + 32).
We can therefore overflow a 32-bit size_t with a commit
message that's just under 2^31 bytes, assuming it consists
mostly of "doubling" sequences (e.g., latin1 0xe1 which
becomes utf8 0xc3 0xa1).
But we'll never make it that far with such a message. We'll
be spending 2^31 bytes on the original string. And our
initial output buffer will also be 2^31 bytes. Which is not
going to succeed on a system with a 32-bit size_t, since
there will be other things using the address space, too. The
initial malloc will fail.
If we imagine instead that we can triple the size when
converting, then our second allocation becomes
(N + 2/3N * 2 + 32), or (7/3N + 32). That still requires two
allocations of 3/7 of our address space (6/7 of the total)
to succeed.
If we imagine we can quadruple, it becomes (5/2N + 32); we
need to be able to allocate 4/5 of the address space to
succeed.
This might start to get plausible. But is it possible to get
a 4-to-1 increase in size? Probably if you're converting to
some obscure encoding. But since git defaults to utf8 for
its output, that's the likely destination encoding for an
attack. And while there are 4-character utf8 sequences, it's
unlikely that you'd be able find a single-byte source
sequence in any encoding.
So this is certainly buggy code which should be fixed, but
it is probably not a useful attack vector.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Fri, 20 Jul 2018 21:42:53 +0000 (14:42 -0700)]
Merge branch 'sb/blame-color' into jk/banned-function
* sb/blame-color:
blame: prefer xsnprintf to strcpy for colors
Jonathan Tan [Tue, 5 Jun 2018 21:40:36 +0000 (14:40 -0700)]
fetch: send "refs/tags/" prefix upon CLI refspecs
When performing tag following, in addition to using the server's
"include-tag" capability to send tag objects (and emulating it if the
server does not support that capability), "git fetch" relies upon the
presence of refs/tags/* entries in the initial ref advertisement to
locally create refs pointing to the aforementioned tag objects. When
using protocol v2, refs/tags/* entries in the initial ref advertisement
may be suppressed by a ref-prefix argument, leading to the tag object
being downloaded, but the ref not being created.
Commit
dcc73cf7ff ("fetch: generate ref-prefixes when using a configured
refspec", 2018-05-18) ensured that "refs/tags/" is always sent as a ref
prefix when "git fetch" is invoked with no refspecs, but not when "git
fetch" is invoked with refspecs. Extend that functionality to make it
work in both situations.
This also necessitates a change another test which tested ref
advertisement filtering using tag refs - since tag refs are sent by
default now, the test has been switched to using branch refs instead.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jonathan Tan [Tue, 5 Jun 2018 21:40:35 +0000 (14:40 -0700)]
t5702: test fetch with multiple refspecs at a time
Extend the protocol v2 tests to also test fetches with multiple refspecs
specified. This also covers the previously uncovered cases of fetching
with prefix matching and fetching by SHA-1.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Brandon Williams [Mon, 23 Jul 2018 17:56:35 +0000 (10:56 -0700)]
fetch-pack: mark die strings for translation
Signed-off-by: Brandon Williams <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Sun, 22 Jul 2018 09:57:09 +0000 (05:57 -0400)]
format-patch: allow --interdiff to apply to a lone-patch
When submitting a revised version of a patch or series, it can be
helpful (to reviewers) to include a summary of changes since the
previous attempt in the form of an interdiff, typically in the cover
letter. However, it is occasionally useful, despite making for a noisy
read, to insert an interdiff into the commentary section of the lone
patch of a 1-patch series.
Therefore, extend "git format-patch --interdiff=<prev>" to insert an
interdiff into the commentary section of a lone patch rather than
requiring a cover letter. The interdiff is indented to avoid confusing
git-am and human readers into considering it part of the patch proper.
Implementation note: Generating an interdiff for insertion into the
commentary section of a patch which itself is currently being generated
requires invoking the diffing machinery recursively. However, the
machinery does not (presently) support this since it uses global state.
Consequently, we need to take care to stash away the state of the
in-progress operation while generating the interdiff, and restore it
after.
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Sun, 22 Jul 2018 09:57:08 +0000 (05:57 -0400)]
log-tree: show_log: make commentary block delimiting reusable
In patches generated by git-format-patch, the area below the "---" line
following the commit message and before the actual 'diff' can be used
for commentary which the patch author wants to convey to readers of the
patch itself but not include in the commit message proper.
By default, the commentary area is empty, however, the --notes option
causes it to be populated with notes associated with the commit. In the
future, other options may be added which also insert content into the
commentary section.
To accommodate this, factor out the logic which delimits commentary
blocks from the commit message so that it can be re-used for upcoming
optional inserted content.
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Sun, 22 Jul 2018 09:57:07 +0000 (05:57 -0400)]
interdiff: teach show_interdiff() to indent interdiff
A future change will allow "git format-patch --interdiff=<prev> -1" to
insert an interdiff into the commentary section of the lone patch of a
1-patch series. However, to prevent the inserted interdiff from
confusing git-am, as well as human readers, it needs to be indented.
Therefore, teach show_interdiff() how to indent.
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Sun, 22 Jul 2018 09:57:06 +0000 (05:57 -0400)]
format-patch: teach --interdiff to respect -v/--reroll-count
The --interdiff option introduces the embedded interdiff generically as
"Interdiff:", however, we can do better when --reroll-count is specified
by emitting "Interdiff against v{n}:" instead.
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Sun, 22 Jul 2018 09:57:05 +0000 (05:57 -0400)]
format-patch: add --interdiff option to embed diff in cover letter
When submitting a revised version of a patch series, it can be helpful
(to reviewers) to include a summary of changes since the previous
attempt in the form of an interdiff, however, doing so involves manually
copy/pasting the diff into the cover letter.
Add an --interdiff option to automate this process. The argument to
--interdiff specifies the tip of the previous attempt against which to
generate the interdiff. For example:
git format-patch --cover-letter --interdiff=v1 -3 v2
The previous attempt and the patch series being formatted must share a
common base.
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
Eric Sunshine [Sun, 22 Jul 2018 09:57:04 +0000 (05:57 -0400)]
format-patch: allow additional generated content in make_cover_letter()
make_cover_letter() returns early when it lacks sufficient state to emit
a diffstat, which makes it difficult to extend the function to reliably
emit additional generated content. Work around this shortcoming by
factoring diffstat-printing logic out to its own function and calling it
as needed without otherwise inhibiting normal control flow.
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:51:00 +0000 (15:51 +0200)]
coccinelle: extract dedicated make target to clean Coccinelle's results
Sometimes I want to remove only Coccinelle's results, but keep all
other build artifacts left after my usual 'make all man' build. This
new 'cocciclean' make target will allow just that.
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:50:59 +0000 (15:50 +0200)]
coccinelle: put sane filenames into output patches
Coccinelle outputs its suggested transformations as patches, whose
header looks something like this:
--- commit.c
+++ /tmp/cocci-output-19250-7ae78a-commit.c
Note the lack of 'diff --opts <old> <new>' line, the differing number
of path components on the --- and +++ lines, and the nonsensical
filename on the +++ line. 'patch -p0' can still apply these patches,
as it takes the filename to be modified from the --- line. Alas, 'git
apply' can't, because it takes the filename from the +++ line, and
then complains about the nonexisting file.
Pass the '--patch .' options to Coccinelle via the SPATCH_FLAGS 'make'
variable, as it seems to make it generate proper context diff patches,
with the header starting with a 'diff ...' line and containing sane
filenames. The resulting 'contrib/coccinelle/*.cocci.patch' files
then can be applied both with 'git apply' and 'patch' (even without
'-p0').
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:50:58 +0000 (15:50 +0200)]
coccinelle: exclude sha1dc source files from static analysis
sha1dc is an external library, that we carry in-tree for convenience
or grab as a submodule, so there is no use in applying our semantic
patches to its source files.
Therefore, exclude sha1dc's source files from Coccinelle's static
analysis.
This change also makes the static analysis somewhat faster: presumably
because of the heavy use of repetitive macro declarations, applying
the semantic patches 'array.cocci' and 'swap.cocci' to 'sha1dc/sha1.c'
takes over half a minute each on my machine, which amounts to about a
third of the runtime of applying these two semantic patches to the
whole git source tree.
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:50:57 +0000 (15:50 +0200)]
coccinelle: use $(addsuffix) in 'coccicheck' make target
The dependencies of the 'coccicheck' make target are listed with the
help of the $(patsubst) make function, which in this case doesn't do
any pattern substitution, but only adds the '.patch' suffix.
Use the shorter and more idiomatic $(addsuffix) make function instead.
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:50:56 +0000 (15:50 +0200)]
coccinelle: mark the 'coccicheck' make target as .PHONY
The 'coccicheck' target doesn't create a file with the same name, so
mark it as .PHONY.
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Schindelin [Mon, 23 Jul 2018 13:39:42 +0000 (06:39 -0700)]
t7406: avoid failures solely due to timing issues
Regression tests are automated tests which try to ensure a specific
behavior. The idea is: if the test case fails, the behavior indicated in
the test case's title regressed.
If a regression test that fails, even occasionally, for any reason other
than to indicate the particular regression(s) it tries to catch, it is
less useful than when it really only fails when there is a bug in the
(non-test) code that needs to be fixed.
In the instance of the test case "submodule update --init --recursive
from subdirectory" of the script t7406-submodule-update.sh, the exact
output of a recursive clone is compared with a pre-generated one. And
this is a racy test because the structure of the submodules only
guarantees a *partial* order. The 'none' and the 'rebasing' submodules
*can* be cloned in any order, which means that a mismatch with the
hard-coded order does not necessarily indicate a bug in the tested code.
See for example:
https://git-for-windows.visualstudio.com/git/_build/results?buildId=14035&view=logs
To prevent such false positives from unnecessarily costing time when
investigating test failures, let's take the exact order of the lines out
of the equation by sorting them before comparing them.
This test script seems not to have any more test cases that try to
verify any specific order in which recursive clones process the
submodules, therefore this is the only test case that is changed in this
manner.
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:02:30 +0000 (15:02 +0200)]
travis-ci: fail if Coccinelle static analysis found something to transform
Coccinelle's and in turn 'make coccicheck's exit code only indicates
that Coccinelle managed to finish its analysis without any errors
(e.g. no unknown --options, no missing files, no syntax errors in the
semantic patches, etc.), but it doesn't indicate whether it found any
undesired code patterns to transform or not. To find out the latter,
one has to look closer at 'make coccicheck's standard output and look
for lines like:
SPATCH result: contrib/coccinelle/<something>.cocci.patch
And this only indicates that there is something to transform, but to
see what the suggested transformations are one has to actually look
into those '*.cocci.patch' files.
This makes the automated static analysis build job on Travis CI not
particularly useful, because it neither draws our attention to
Coccinelle's findings, nor shows the actual findings. Consequently,
new topics introducing undesired code patterns graduated to master
on several occasions without anyone noticing.
The only way to draw attention in such an automated setting is to fail
the build job. Therefore, modify the 'ci/run-static-analysis.sh'
build script to check all the resulting '*.cocci.patch' files, and
fail the build job if any of them turns out to be not empty. Include
those files' contents, i.e. Coccinelle's suggested transformations, in
the build job's trace log, so we'll know why it failed.
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
SZEDER Gábor [Mon, 23 Jul 2018 13:02:29 +0000 (15:02 +0200)]
travis-ci: run Coccinelle static analysis with two parallel jobs
Currently the static analysis build job runs Coccinelle using a single
'make' job. Using two parallel jobs cuts down the build job's run
time from around 10-12mins to 6-7mins, sometimes even under 6mins
(there is quite large variation between build job runtimes). More
than two parallel jobs don't seem to bring further runtime benefits.
Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:41 +0000 (09:49 +0200)]
transport-helper.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:40 +0000 (09:49 +0200)]
transport.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:39 +0000 (09:49 +0200)]
sha1-file.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:38 +0000 (09:49 +0200)]
sequencer.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:37 +0000 (09:49 +0200)]
replace-object.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:36 +0000 (09:49 +0200)]
refspec.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:35 +0000 (09:49 +0200)]
refs.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:34 +0000 (09:49 +0200)]
pkt-line.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:33 +0000 (09:49 +0200)]
object.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:32 +0000 (09:49 +0200)]
exec-cmd.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:31 +0000 (09:49 +0200)]
environment.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:30 +0000 (09:49 +0200)]
dir.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
Nguyễn Thái Ngọc Duy [Sat, 21 Jul 2018 07:49:29 +0000 (09:49 +0200)]
convert.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>