git.git
7 years agovscode: add a dictionary for cSpell
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>
7 years agovscode: use 8-space tabs, no trailing ws, etc for Git's source code
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>
7 years agovscode: wrap commit messages at column 72 by default
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>
7 years agovscode: only overwrite C/C++ settings
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>
7 years agomingw: define WIN32 explicitly
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>
7 years agocache.h: extract enum declaration from inside a struct declaration
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>
7 years agovscode: hard-code a couple defines
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>
7 years agocontrib: add a script to initialize VS Code configuration
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>
7 years agopack-objects: document about thread synchronization
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>
7 years agot5562: avoid non-portable "export FOO=bar" construct
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>
7 years agodoc: fix want-capability separator
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>
7 years agotests: make use of the test_must_be_empty function
Æ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>
7 years agotests: make use of the test_must_be_empty function
Æ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>
7 years agofsck: test and document unknown fsck.<msg-id> values
Æ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>
7 years agofsck: add stress tests for fsck.skipList
Æ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>
7 years agofsck: test & document {fetch,receive}.fsck.* config fallback
Æ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>
7 years agofetch: implement fetch.fsck.*
Æ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>
7 years agotransfer.fsckObjects tests: untangle confusing setup
Æ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>
7 years agoconfig doc: elaborate on fetch.fsckObjects security
Æ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>
7 years agoconfig doc: elaborate on what transfer.fsckObjects does
Æ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>
7 years agoconfig doc: unify the description of fsck.* and receive.fsck.*
Æ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>
7 years agoconfig doc: don't describe *.fetchObjects twice
Æ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>
7 years agoreceive.fsck.<msg-id> tests: remove dead code
Æ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>
7 years agomerge-recursive: preserve skip_worktree bit when necessary
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>
7 years agot3507: add a testcase showing failure with sparse checkout
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>
7 years agohttp-backend: respect CONTENT_LENGTH for receive-pack
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>
7 years agopackfile: ensure that enum object_type is defined
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>
7 years agobanned.h: mark strncpy() as banned
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>
7 years agobanned.h: mark sprintf() as banned
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>
7 years agobanned.h: mark strcat() as banned
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>
7 years agoautomatically ban strcpy()
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>
7 years agol10n: de.po: translate 108 new messages
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>
7 years agodoc hash-function-transition: note the lack of a changelog
Æ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>
7 years agodiff: --color-moved: rename "dimmed_zebra" to "dimmed-zebra"
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>
7 years agoMakefile: add a DEVOPTS flag to get pedantic compilation
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>
7 years agoFourth batch for 2.19 cycle
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>
7 years agoMerge branch 'as/sequencer-customizable-comment-char'
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

7 years agoMerge branch 'sb/blame-color'
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

7 years agoMerge branch 'nd/command-list'
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

7 years agoMerge branch 'es/test-lint-one-shot-export'
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

7 years agoMerge branch 'wc/find-commit-with-pattern-on-detached-head'
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

7 years agoMerge branch 'jc/t3404-one-shot-export-fix'
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

7 years agoMerge branch 'mk/merge-in-sparse-checkout'
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

7 years agoMerge branch 'hs/push-cert-check-cleanup'
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

7 years agoMerge branch 'jk/empty-pick-fix'
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

7 years agoMerge branch 'bp/log-ref-write-fd-with-strbuf'
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

7 years agoMerge branch 'jt/partial-clone-fsck-connectivity'
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"

7 years agoMerge branch 'bc/send-email-auto-cte'
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

7 years agoMerge branch 'bb/unicode-11-width'
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

7 years agoMerge branch 'bb/pedantic'
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

7 years agoMerge branch 'tb/config-default'
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

7 years agoMerge branch 'mh/fast-import-no-diff-delta-empty'
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

7 years agoMerge branch 'kn/userdiff-php'
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

7 years agoMerge branch 'jk/fetch-all-peeled-fix'
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

7 years agoMerge branch 'ag/rebase-p'
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

7 years agoMerge branch 'jt/connectivity-check-after-unshallow'
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

7 years agoMerge branch 'jk/for-each-ref-icase'
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

7 years agoMerge branch 'en/t5407-rebase-m-fix'
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

7 years agoMerge branch 'en/apply-comment-fix'
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

7 years agoMerge branch 'en/rebase-consistency'
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

7 years agoMerge branch 'sb/submodule-move-head-error-msg'
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

7 years agoMerge branch 'rj/submodule-fsck-skip'
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()

7 years agopack-protocol: mention and point to docs for protocol v2
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>
7 years agostrbuf_humanise: use unsigned variables
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>
7 years agopass st.st_size as hint for strbuf_readlink()
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>
7 years agostrbuf_readlink: use ssize_t
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>
7 years agostrbuf: use size_t for length in intermediate variables
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>
7 years agoreencode_string: use size_t for string lengths
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>
7 years agoreencode_string: use st_add/st_mult helpers
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>
7 years agoMerge branch 'sb/blame-color' into jk/banned-function
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

7 years agofetch: send "refs/tags/" prefix upon CLI refspecs
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>
7 years agot5702: test fetch with multiple refspecs at a time
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>
7 years agofetch-pack: mark die strings for translation
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>
7 years agoformat-patch: allow --interdiff to apply to a lone-patch
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>
7 years agolog-tree: show_log: make commentary block delimiting reusable
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>
7 years agointerdiff: teach show_interdiff() to indent interdiff
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>
7 years agoformat-patch: teach --interdiff to respect -v/--reroll-count
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>
7 years agoformat-patch: add --interdiff option to embed diff in cover letter
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>
7 years agoformat-patch: allow additional generated content in make_cover_letter()
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>
7 years agococcinelle: extract dedicated make target to clean Coccinelle's results
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>
7 years agococcinelle: put sane filenames into output patches
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>
7 years agococcinelle: exclude sha1dc source files from static analysis
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>
7 years agococcinelle: use $(addsuffix) in 'coccicheck' make target
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>
7 years agococcinelle: mark the 'coccicheck' make target as .PHONY
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>
7 years agot7406: avoid failures solely due to timing issues
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>
7 years agotravis-ci: fail if Coccinelle static analysis found something to transform
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>
7 years agotravis-ci: run Coccinelle static analysis with two parallel jobs
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>
7 years agotransport-helper.c: mark more strings for translation
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>
7 years agotransport.c: mark more strings for translation
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>
7 years agosha1-file.c: mark more strings for translation
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>
7 years agosequencer.c: mark more strings for translation
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>
7 years agoreplace-object.c: mark more strings for translation
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>
7 years agorefspec.c: mark more strings for translation
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>
7 years agorefs.c: mark more strings for translation
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>
7 years agopkt-line.c: mark more strings for translation
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>
7 years agoobject.c: mark more strings for translation
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>
7 years agoexec-cmd.c: mark more strings for translation
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>
7 years agoenvironment.c: mark more strings for translation
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>
7 years agodir.c: mark more strings for translation
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>
7 years agoconvert.c: mark more strings for translation
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>
git clone https://git.99rst.org/PROJECT