Junio C Hamano [Sun, 4 Oct 2020 19:49:04 +0000 (12:49 -0700)]
Merge branch 'js/no-builtins-on-disk-option'
The installation procedure learned to optionally omit "git-foo"
executable files for each 'foo' built-in subcommand, which are only
required by old timers that still rely on the age old promise that
prepending "git --exec-path" output to PATH early in their script
will keep the "git-foo" calls they wrote working.
The old attempt to remove these executables from the disk failed in
the 1.6 era; it may be worth attempting again, but I think it is
worth to keep this topic separate from such a policy change to help
it graduate early.
* js/no-builtins-on-disk-option:
ci: stop linking built-ins to the dashed versions
Optionally skip linking/copying the built-ins
msvc: copy the correct `.pdb` files in the Makefile target `install`
Junio C Hamano [Sun, 4 Oct 2020 19:49:04 +0000 (12:49 -0700)]
Merge branch 'ab/mediawiki-fixes'
Modernization and fixes to MediaWiki remote backend.
* ab/mediawiki-fixes:
remote-mediawiki: use "sh" to eliminate unquoted commands
remote-mediawiki: annotate unquoted uses of run_git()
remote-mediawiki: convert to quoted run_git() invocation
remote-mediawiki: provide a list form of run_git()
remote-mediawiki tests: annotate failing tests
remote-mediawiki: fix duplicate revisions being imported
remote-mediawiki tests: use CLI installer
remote-mediawiki tests: use inline PerlIO for readability
remote-mediawiki tests: replace deprecated Perl construct
remote-mediawiki tests: use a more idiomatic dispatch table
remote-mediawiki tests: use "$dir/" instead of "$dir."
remote-mediawiki tests: change `[]` to `test`
remote-mediawiki tests: use test_cmp in tests
remote-mediawiki tests: use a 10 character password
remote-mediawiki tests: use the login/password variables
remote-mediawiki doc: don't hardcode Debian PHP versions
remote-mediawiki doc: link to MediaWiki's current version
remote-mediawiki doc: correct link to GitHub project
Nikita Leonov [Sat, 3 Oct 2020 13:29:12 +0000 (13:29 +0000)]
credential: treat CR/LF as line endings in the credential protocol
This fix makes using Git credentials more friendly to Windows users: it
allows a credential helper to communicate using CR/LF line endings ("DOS
line endings" commonly found on Windows) instead of LF-only line endings
("Unix line endings").
Note that this changes the behavior a bit: if a credential helper
produces, say, a password with a trailing Carriage Return character,
that will now be culled even when the rest of the lines end only in Line
Feed characters, indicating that the Carriage Return was not meant to be
part of the line ending.
In practice, it seems _very_ unlikely that something like this happens.
Passwords usually need to consist of non-control characters, URLs need
to have special characters URL-encoded, and user names, well, are names.
However, it _does_ help on Windows, where CR/LF line endings are common:
as unrecognized commands are simply ignored by the credential machinery,
even a command like `quit\r` (which is clearly intended to abort) would
simply be ignored (silently) by Git.
So let's change the credential machinery to accept both CR/LF and LF
line endings.
While we do this for the credential helper protocol, we do _not_ adjust
`git credential-cache--daemon` (which won't work on Windows, anyway,
because it requires Unix sockets) nor `git credential-store` (which
writes the file `~/.git-credentials` which we consider an implementation
detail that should be opaque to the user, read: we do expect users _not_
to edit this file manually).
Signed-off-by: Nikita Leonov <nykyta.leonov@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Sat, 3 Oct 2020 17:06:27 +0000 (10:06 -0700)]
Merge remote-tracking branch 'paulus/master' into pm/gitk-update
* paulus/master:
gitk: Resize panes correctly when reducing window size
gitk: replace tabs with spaces
gitk: fix the context menu not appearing in the presence of submodule diffs
gitk: Un-hide selection in areas with non-default background color
gitk: add diff lines background colors
gitk: be prepared to be run in a bare repository
gitk: Preserve window dimensions on exit when not using ttk themes
gitk: don't highlight files after submodules as submodules
gitk: fix branch name encoding error
gitk: rename "commit summary" to "commit reference"
t, doc: update tests, reference for "--force-if-includes"
Update test cases for the new option, and document its usage
and update related references.
Update test cases for the new option, and document its usage
and update related references.
- t/t5533-push-cas.sh:
Update test cases for "compare-and-swap" when used along with
"--force-if-includes" helps mitigate overwrites when remote
refs are updated in the background; allows forced updates when
changes from remote are integrated locally.
- Documentation:
Add reference for the new option, configuration setting
("push.useForceIfIncludes") and advise messages.
Signed-off-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
push: parse and set flag for "--force-if-includes"
The previous commit added the necessary machinery to implement the
"--force-if-includes" protection, when "--force-with-lease" is used
without giving exact object the remote still ought to have. Surface
the feature by adding a command line option and a configuration
variable to enable it.
- Add a flag: "TRANSPORT_PUSH_FORCE_IF_INCLUDES" to indicate that the
new option was passed from the command line of via configuration
settings; update command line and configuration parsers to set the
new flag accordingly.
- Introduce a new configuration option "push.useForceIfIncludes", which
is equivalent to setting "--force-if-includes" in the command line.
- Update "remote-curl" to recognize and pass this option to "send-pack"
when enabled.
- Update "advise" to catch the reject reason "REJECT_REF_NEEDS_UPDATE",
set when the ref status is "REF_STATUS_REJECT_REMOTE_UPDATED" and
(optionally) print a help message when the push fails.
- The new option is a "no-op" in the following scenarios:
* When used without "--force-with-lease".
* When used with "--force-with-lease", and if the expected commit
on the remote side is specified as an argument.
Signed-off-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a check to verify if the remote-tracking ref of the local branch
is reachable from one of its "reflog" entries.
The check iterates through the local ref's reflog to see if there
is an entry for the remote-tracking ref and collecting any commits
that are seen, into a list; the iteration stops if an entry in the
reflog matches the remote ref or if the entry timestamp is older
the latest entry of the remote ref's "reflog". If there wasn't an
entry found for the remote ref, "in_merge_bases_many()" is called
to check if it is reachable from the list of collected commits.
When a local branch that is based on a remote ref, has been rewound
and is to be force pushed on the remote, "--force-if-includes" runs
a check that ensures any updates to the remote-tracking ref that may
have happened (by push from another repository) in-between the time
of the last update to the local branch (via "git-pull", for instance)
and right before the time of push, have been integrated locally
before allowing a forced update.
If the new option is passed without specifying "--force-with-lease",
or specified along with "--force-with-lease=<refname>:<expect>" it
is a "no-op".
Signed-off-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Paul Mackerras [Sat, 3 Oct 2020 05:20:33 +0000 (15:20 +1000)]
gitk: Resize panes correctly when reducing window size
The resizeclistpanes and resizecdetpanes procedures attempt to keep
the horizontal proportions of the panes of the gitk window
approximately constant when the gitk window is resized. However, if
the size is reduced enough that an existing sash position would go
outside the window, Tk moves the sash to the left to keep it inside
the window (without moving other sash positions to keep the
proportions). This happens before these resize procedures get
control, and so they work with incorrect proportions.
To fix this, we record the sash positions we set previously and use
those previously-set sash positions rather than the current sash
positions when computing the proportions.
Denton Liu [Fri, 11 Sep 2020 04:36:33 +0000 (21:36 -0700)]
gitk: replace tabs with spaces
The source code is a mix of tabs and spaces. The indentation style
currently is four spaces per indent level but uses tabs every other
level (at eight spaces). Fix this inconsistent spacing and tabbing by
just using a space-indent for everything.
This was done mechanically by running:
$ expand -i gitk >gitk.new
$ mv gitk.new gitk
This patch should be empty with `--ignore-all-space`.
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Derrick Stolee [Fri, 2 Oct 2020 14:58:56 +0000 (14:58 +0000)]
commit-reach: fix in_merge_bases_many bug
Way back in f9b8908b (commit.c: use generation numbers for
in_merge_bases(), 2018-05-01), a heuristic was used to short-circuit
the in_merge_bases() walk. This works just fine as long as the
caller is checking only two commits, but when there are multiple,
there is a possibility that this heuristic is _very wrong_.
Some code moves since then has changed this method to
repo_in_merge_bases_many() inside commit-reach.c. The heuristic
computes the minimum generation number of the "reference" list, then
compares this number to the generation number of the "commit".
In a recent topic, a test was added that used in_merge_bases_many()
to test if a commit was reachable from a number of commits pulled
from a reflog. However, this highlighted the problem: if any of the
reference commits have a smaller generation number than the given
commit, then the walk is skipped _even if there exist some with
higher generation number_.
This heuristic is wrong! It must check the MAXIMUM generation number
of the reference commits, not the MINIMUM.
This highlights a testing gap. t6600-test-reach.sh covers many
methods in commit-reach.c, including in_merge_bases() and
get_merge_bases_many(), but since these methods either restrict to
two input commits or actually look for the full list of merge bases,
they don't check this heuristic!
Add a possible input to "test-tool reach" that tests
in_merge_bases_many() and add tests to t6600-test-reach.sh that
cover this heuristic. This includes cases for the reference commits
having generation above and below the generation of the input commit,
but also having maximum generation below the generation of the input
commit.
The fix itself is to swap min_generation with a max_generation in
repo_in_merge_bases_many().
Reported-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The format.useAutoBase configuration option exists to allow users to
enable '--base=auto' for format-patch by default.
This can sometimes lead to poor workflow, due to unexpected failures
when attempting to format an ancient patch:
$ git format-patch -1 <an old commit>
fatal: base commit shouldn't be in revision list
This can be very confusing, as it is not necessarily immediately obvious
that the user requested a --base (since this was in the configuration,
not on the command line).
We do want --base=auto to fail when it cannot provide a suitable base,
as it would be equally confusing if a formatted patch did not include
the base information when it was requested.
Teach format.useAutoBase a new mode, "whenAble". This mode will cause
format-patch to attempt to include a base commit when it can. However,
if no valid base commit can be found, then format-patch will continue
formatting the patch without a base commit.
In order to avoid making yet another branch name unusable with --base,
do not teach --base=whenAble or --base=whenable.
Instead, refactor the base_commit option to use a callback, and rely on
the global configuration variable auto_base.
This does mean that a user cannot request this optional base commit
generation from the command line. However, this is likely not too
valuable. If the user requests base information manually, they will be
immediately informed of the failure to acquire a suitable base commit.
This allows the user to make an informed choice about whether to
continue the format.
Add tests to cover the new mode of operation for --base.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
submodule update: silence underlying merge/rebase with "--quiet"
Commands such as
$ git pull --rebase --recurse-submodules --quiet
produce non-quiet output from the merge or rebase. Pass the --quiet
option down when invoking "rebase" and "merge".
Also fix the parsing of git submodule update -v.
When e84c3cf3 (git-submodule.sh: accept verbose flag in cmd_update
to be non-quiet, 2018-08-14) taught "git submodule update" to take
"--quiet", it apparently did not know how ${GIT_QUIET:+--quiet}
works, and reviewers seem to have missed that setting the variable
to "0", rather than unsetting it, still results in "--quiet" being
passed to underlying commands.
Signed-off-by: Theodore Dubois <tbodt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sean Barag [Thu, 1 Oct 2020 03:46:16 +0000 (03:46 +0000)]
clone: allow configurable default for `-o`/`--origin`
While the default remote name of "origin" can be changed at clone-time
with `git clone`'s `--origin` option, it was previously not possible
to specify a default value for the name of that remote. Add support for
a new `clone.defaultRemoteName` config, with the newly-created remote
name resolved in priority order:
1. (Highest priority) A remote name passed directly to `git clone -o`
2. A `clone.defaultRemoteName=new_name` in config `git clone -c`
3. A `clone.defaultRemoteName` value set in `/path/to/template/config`,
where `--template=/path/to/template` is provided
4. A `clone.defaultRemoteName` value set in a non-template config file
5. The default value of `origin`
Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Derrick Stolee <stolee@gmail.com> Helped-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sean Barag [Thu, 1 Oct 2020 03:46:15 +0000 (03:46 +0000)]
clone: read new remote name from remote_name instead of option_origin
In a future patch, the name of the remote created by `git clone` may
come from multiple sources. To avoid confusion, convert most uses of
option_origin to remote_name, leaving option_origin to exclusively
represent the -o/--origin option.
Helped-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sean Barag [Thu, 1 Oct 2020 03:46:14 +0000 (03:46 +0000)]
clone: validate --origin option before use
Providing a bad origin name to `git clone` currently reports an
'invalid refspec' error instead of a more explicit message explaining
that the `--origin` option was malformed. This behavior dates back to
since 8434c2f1 (Build in clone, 2008-04-27). Reintroduce
validation for the provided `--origin` option, but notably _don't_
include a multi-level check (e.g. "foo/bar") that was present in the
original `git-clone.sh`. `git remote` allows multi-level remote names
since at least 46220ca100 (remote.c: Fix overtight refspec validation,
2008-03-20), so that appears to be the desired behavior.
Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Derrick Stolee <stolee@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sean Barag [Thu, 1 Oct 2020 03:46:13 +0000 (03:46 +0000)]
refs: consolidate remote name validation
In preparation for a future patch, extract from remote.c a function that
validates possible remote names so that its rules can be used
consistently in other places.
Helped-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sean Barag [Thu, 1 Oct 2020 03:46:12 +0000 (03:46 +0000)]
remote: add tests for add and rename with invalid names
In preparation for a future patch that moves `builtin/remote.c`'s
remote-name validation, ensure `git remote add` and `git remote rename`
report errors when the new name isn't valid.
Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sean Barag [Thu, 1 Oct 2020 03:46:11 +0000 (03:46 +0000)]
clone: use more conventional config/option layering
Parsing command-line options before reading from config required careful
handling to ensure CLI options were treated with higher priority. Read
config first to let parsed CLI naively overwrite matching config values.
Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller [Wed, 30 Sep 2020 21:25:29 +0000 (14:25 -0700)]
refspec: add support for negative refspecs
Both fetch and push support pattern refspecs which allow fetching or
pushing references that match a specific pattern. Because these patterns
are globs, they have somewhat limited ability to express more complex
situations.
For example, suppose you wish to fetch all branches from a remote except
for a specific one. To allow this, you must setup a set of refspecs
which match only the branches you want. Because refspecs are either
explicit name matches, or simple globs, many patterns cannot be
expressed.
Add support for a new type of refspec, referred to as "negative"
refspecs. These are prefixed with a '^' and mean "exclude any ref
matching this refspec". They can only have one "side" which always
refers to the source. During a fetch, this refers to the name of the ref
on the remote. During a push, this refers to the name of the ref on the
local side.
With negative refspecs, users can express more complex patterns. For
example:
will fetch all branches on origin into remotes/origin, but will exclude
fetching the branch named dontwant.
Refspecs today are commutative, meaning that order doesn't expressly
matter. Rather than forcing an implied order, negative refspecs will
always be applied last. That is, in order to match, a ref must match at
least one positive refspec, and match none of the negative refspecs.
This is similar to how negative pathspecs work.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 11:52:40 +0000 (07:52 -0400)]
combine-diff: handle --find-object in multitree code path
When doing combined diffs, we have two possible code paths:
- a slower one which independently diffs against each parent, applies
any filters, and then intersects the resulting paths
- a faster one which walks all trees simultaneously
When the diff options specify that we must do certain filters, like
pickaxe, then we always use the slow path, since the pickaxe code only
knows how to handle filepairs, not the n-parent entries generated for
combined diffs.
But there are two problems with the slow path:
1. It's slow. Running:
git rev-list HEAD | git diff-tree --stdin -r -c
in git.git takes ~3s on my machine. But adding "--find-object" to
that increases it to ~6s, even though find-object itself should
incur only a few extra oid comparisons. On linux.git, it's even
worse: 35s versus 215s.
2. It doesn't catch all cases where a particular path is interesting.
Consider a merge with parent blobs X and Y for a particular path,
and end result Z. That should be interesting according to "-c",
because the result doesn't match either parent. And it should be
interesting even with "--find-object=X", because "X" went away in
the merge.
But because we perform each pairwise diff independently, this
confuses the intersection code. The change from X to Z is still
interesting according to --find-object. But in the other parent we
went from Y to Z, so the diff appears empty! That causes the
intersection code to think that parent didn't change the path, and
thus it's not interesting for "-c".
This patch fixes both by implementing --find-object for the multitree
code. It's a bit unfortunate that we have to duplicate some logic from
diffcore-pickaxe, but this is the best we can do for now. In an ideal
world, all of the diffcore code would stop thinking about filepairs and
start thinking about n-parent sets, and we could use the multitree walk
with all of it.
Until then, there are some leftover warts:
- other pickaxe operations, like -S or -G, still suffer from both
problems. These would be hard to adapt because they rely on having
a diff_filespec() for each path to look at content. And we'd need to
define what an n-way "change" means in each case (probably easy for
"-S", which can compare counts, but not so clear for -G, which is
about grepping diffs).
- other options besides --find-object may cause us to use the slow
pairwise path, in which case we'll go back to producing a different
(wrong) answer for the X/Y/Z case above.
We may be able to hack around these, but I think the ultimate solution
will be a larger rewrite of the diffcore code. For now, this patch
improves one specific case but leaves the rest.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The OFFSETOF_VAR(var, member) macro is implemented in terms of
offsetof(typeof(*var), member) with compilers that know typeof(),
but its fallback implemenation compares &(var->member) and (var) and
count the distance in bytes, i.e.
((uintptr_t)&(var)->member - (uintptr_t)(var))
MSVC's runtime check, when fed an uninitialized 'var', flags this as
a use of an uninitialized variable (and that is legit---uninitialized
contents of 'var' is subtracted) in a debug build.
After auditing all 6 uses of OFFSETOF_VAR(), 1 of them does feed a
potentially uninitialized 'var' to the macro in the beginning of the
for() loop:
#define hashmap_for_each_entry(map, iter, var, member) \
for (var = hashmap_iter_first_entry_offset(map, iter, \
OFFSETOF_VAR(var, member)); \
var; \
var = hashmap_iter_next_entry_offset(iter, \
OFFSETOF_VAR(var, member)))
We can work around this by making sure that var has _some_ value
when OFFSETOF_VAR() is called. Strictly speaking, it invites
undefined behaviour to use NULL here if we end up with pointer
comparison, but MSVC runtime seems to be happy with it, and most
other systems have typeof() and don't even need pointer comparison
fallback code.
Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cmake (Windows): recommend using Visual Studio's built-in CMake support
It is a lot more convenient to use than having to specify the
configuration in CMake manually (does not matter whether using the
command-line or CMake's GUI).
While at it, recommend using `contrib/buildsystems/out/` as build
directory also in the part that talks about running CMake manually.
Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The idea of having CMake support in Git's source tree is to enable
contributors on Windows to start contributing with little effort. To
that end, we just added some sensible defaults that will let users open
the worktree in Visual Studio and start building.
This expects the dependencies (such as zlib) to be available already,
though. If they are not available, we expect the user to run
`compat/vcbuild/vcpkg_install.bat`.
Rather than requiring this step to be manual, detect the situation and
run it as part of the CMake configuration step.
Note that this obviously only applies to the scenario when we want to
compile in Visual Studio (i.e. with MS Visual C), not with GCC.
Therefore, we guard this new code block behind the `MSVC` conditional.
This concludes our journey to make it as effortless as possible to start
developing Git in Visual Studio: all the developer needs to do is to
clone Git's repository, open the worktree via `File>Open>Folder...` and
wait for CMake to finish configuring.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cmake (Windows): let the `.dll` files be found when running the tests
Contrary to Unix-ish platforms, the dependencies' shared libraries are
not usually found in one central place. In our case, since we use
`vcpkg`, they are to be found inside the `compat/vcbuild/vcpkg/` tree.
Let's make sure that they are in the search path when running the tests.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cmake: quote the path accurately when editing `test-lib.sh`
By default, the build directory will be called something like
`contrib/buildsystems/out/build/x64-Debug (default)` (note the space and
the parentheses). We need to make sure that such a path is quoted
properly when editing the assignment of the `GIT_BUILD_DIR` variable.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:35:00 +0000 (08:35 -0400)]
dir.c: drop unused "untracked" from treat_path_fast()
We don't use the untracked_cache_dir parameter that is passed in, but
instead look at the untracked_cache_dir inside the cached_dir struct we
are passed. It's been this way since the introduction of
treat_path_fast() in 91a2288b5f (untracked cache: record/validate dir
mtime and reuse cached output, 2015-03-08).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:34:11 +0000 (08:34 -0400)]
sequencer: handle ignore_footer when parsing trailers
The append_signoff() function takes an "ignore_footer"
argument, which specifies a number of bytes at the end of
the message buffer which should not be considered (they
cannot contain trailers, and the trailer is spliced in
before them).
But to find the existing trailers, it calls into
has_conforming_trailer(). That function takes an
ignore_footer parameter, but since 967dfd4d56 (sequencer:
use trailer's trailer layout, 2016-11-02) the parameter is
completely ignored.
The trailer interface we're using takes a single string,
with no option to tell it to use part of the string.
However, since we have a mutable strbuf, we can work around
this by simply overwriting (and later restoring) the
boundary with a NUL.
I'm not sure if this can actually trigger a bug in practice.
It's easy to get a non-zero ignore_footer by doing something
like this:
git commit -F - --cleanup=verbatim <<-EOF
subject
body
Signed-off-by: me
# this looks like a comment, but is actually in the
# message! That makes the earlier s-o-b fake.
EOF
git commit --amend -s
There git-commit calls ignore_non_trailer() to count up the
"#" cruft, which becomes the ignore_footer header. But it
works even without this patch! That's because the trailer
code _also_ calls ignore_non_trailer() and skips the cruft,
too. So it happens to work because the only callers with a
non-zero ignore_footer are using the exact same function
that the trailer parser uses internally.
And that seems true for all of the current callers, but
there's nothing guaranteeing it. We're better off only
feeding the correct buffer to the trailer code in the first
place.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:30:27 +0000 (08:30 -0400)]
test-advise: check argument count with argc instead of argv
We complain if "test-tool advise" is not given an argument, but we
quietly ignore any additional arguments it receives. Let's instead check
that we got the expected number. As a bonus, this silences
-Wunused-parameter, which notes that we don't ever look at argc.
While we're here, we can also fix the indentation in the conditional.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:30:10 +0000 (08:30 -0400)]
sparse-checkout: fill in some options boilerplate
The sparse-checkout passes along argv and argc to its sub-command helper
functions. Many of these sub-commands do not yet take any command-line
options, and ignore those parameters.
Let's instead add empty option lists and make sure we call
parse_options(). That will give a useful error message for something
like:
git sparse-checkout list --nonsense
which currently just silently ignores the unknown option.
As a bonus, it also silences some -Wunused-parameter warnings.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:29:31 +0000 (08:29 -0400)]
sequencer: drop repository argument from run_git_commit()
When we switched to using an external git-commit call in b0a3186140
(sequencer: simplify root commit creation, 2019-08-19), this function
didn't need to care about the repository object any more.
Arguably we could be passing along the repository path to the external
git-commit by using "--git-dir=r->path" here. But for the most part the
sequencer code relies on sub-process finding the same repository we're
already in (using the same environment variables or discovery process we
did). But we don't have a convenient interface for doing so, and there's
no indication that we need to. Let's just drop the unused parameter for
now.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:29:09 +0000 (08:29 -0400)]
push: drop unused repo argument to do_push()
We stopped using the "repo" argument in 8e4c8af058 (push: disallow --all
and refspecs when remote.<name>.mirror is set, 2019-09-02), which moved
the pushremote handling to its caller.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:29:02 +0000 (08:29 -0400)]
assert PARSE_OPT_NONEG in parse-options callbacks
In the spirit of 517fe807d6 (assert NOARG/NONEG behavior of
parse-options callbacks, 2018-11-05), let's cover some parse-options
callbacks which expect to be used with PARSE_OPT_NONEG but don't
explicitly assert that this is the case. These callbacks are all used
correctly in the current code, but this will help document their
expectations and future-proof the code.
As a bonus, it also silences -Wunused-parameters (these were added since
the initial sweep of 517fe807d6, and we can't yet turn on
-Wunused-parameters to remind people because it has too many existing
false positives).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:28:32 +0000 (08:28 -0400)]
env--helper: write to opt->value in parseopt helper
We use OPT_CALLBACK_F() to call the option_parse_type() callback,
passing it the address of "cmdmode" as the value to write to. But the
callback doesn't look at opt->value at all, and instead writes to a
global variable.
This works out because that's the same global variable we happen to pass
in, but it's rather confusing. Let's use the passed-in value instead.
We'll also make "cmdmode" a local variable of the main function,
ensuring we can't make the same mistake again.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:28:18 +0000 (08:28 -0400)]
drop unused argc parameters
Many functions take an argv/argc pair, but never actually look at argc.
This makes it useless at best (we use the NULL sentinel in argv to find
the end of the array), and misleading at worst (what happens if the argc
count does not match the argv NULL?).
In each of these instances, the argv NULL does match the argc count, so
there are no bugs here. But let's tighten the interfaces to make it
harder to get wrong (and to reduce some -Wunused-parameter complaints).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 30 Sep 2020 12:27:53 +0000 (08:27 -0400)]
convert: drop unused crlf_action from check_global_conv_flags_eol()
The crlf_action parameter hasn't been used since a0ad53c181 (convert:
Correct NNO tests and missing `LF will be replaced by CRLF`,
2016-08-13), where that part of the function was hoisted out to a
separate will_convert_lf_to_crlf() helper. Let's drop the useless
parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 29 Sep 2020 21:01:22 +0000 (14:01 -0700)]
Merge branch 'ah/pull'
Earlier we taught "git pull" to warn when the user does not say the
histories need to be merged, rebased or accepts only fast-
forwarding, but the warning triggered for those who have set the
pull.ff configuration variable.
* ah/pull:
pull: don't warn if pull.ff has been set
Junio C Hamano [Tue, 29 Sep 2020 21:01:21 +0000 (14:01 -0700)]
Merge branch 'dl/zero-oid-in-hooks'
Adjust sample hooks for hash algorithm other than SHA-1.
* dl/zero-oid-in-hooks:
hooks--update.sample: use hash-agnostic zero OID
hooks--pre-push.sample: use hash-agnostic zero OID
hooks--pre-push.sample: modernize script
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
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()'
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
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 <peff@peff.net> Helped-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <phil.hord@gmail.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <evan.gates@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <oystwa@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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:
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <uzonyi.akos@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
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
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
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.
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.
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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:
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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:
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <chris.torek@gmail.com> Reported-by: Son Luong Ngoc <sluongng@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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.
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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.
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).
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 <congdanhqx@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
