Jonathan Tan [Tue, 18 Aug 2020 04:01:36 +0000 (21:01 -0700)]
promisor-remote: lazy-fetch objects in subprocess
Teach Git to lazy-fetch missing objects in a subprocess instead of doing
it in-process. This allows any fatal errors that occur during the fetch
to be isolated and converted into an error return value, instead of
causing the current command being run to terminate.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
René Scharfe [Tue, 18 Aug 2020 22:08:54 +0000 (00:08 +0200)]
patch-id: ignore newline at end of file in diff_flush_patch_id()
Whitespace is ignored when calculating patch IDs. This is done by
removing all whitespace from diff lines before hashing them, including
a newline at the end of a file. If that newline is missing, however,
diff reports that fact in a separate line containing "\ No newline at
end of file\n", and this marker is hashed like a context line.
This goes against our goal of making patch IDs independent of
whitespace. Use the same heuristic that
2485eab55cc (git-patch-id: do
not trip over "no newline" markers, 2011-02-17) added to git patch-id
instead and skip diff lines that start with a backslash and a space
and are longer than twelve characters.
Reported-by: Tilman Vogel <redacted>
Initial-test-by: Tilman Vogel <redacted>
Signed-off-by: René Scharfe <redacted>
Signed-off-by: Junio C Hamano <redacted>
Matheus Tavares [Tue, 18 Aug 2020 17:46:55 +0000 (14:46 -0300)]
checkout_entry(): remove unreachable error() call
This if statement never evaluates to true since we already check
state->force a few lines above, and immediately return when it is
false.
Signed-off-by: Matheus Tavares <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jonathan Tan [Tue, 18 Aug 2020 04:01:35 +0000 (21:01 -0700)]
fetch-pack: do not lazy-fetch during ref iteration
In order to determine negotiation tips, "fetch-pack" iterates over all
refs and dereferences all annotated tags found. This causes the
existence of targets of refs and annotated tags to be checked. Avoiding
this is especially important when we use "git fetch" (which invokes
"fetch-pack") to perform lazy fetches in a partial clone because a
target of such a ref or annotated tag may need to be itself lazy-fetched
(and otherwise causing an infinite loop).
Therefore, teach "fetch-pack" not to lazy fetch whenever iterating over
refs. This is done by using the raw form of ref iteration and by
dereferencing tags ourselves.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jonathan Tan [Tue, 18 Aug 2020 04:01:34 +0000 (21:01 -0700)]
fetch: only populate existing_refs if needed
In "fetch", get_ref_map() iterates over all refs to populate
"existing_refs" in order to populate peer_ref->old_oid in the returned
refmap, even if the refmap has no peer_ref set - which is the case when
only literal hashes (i.e. no refs by name) are fetched.
Iterating over refs causes the targets of those refs to be checked for
existence. Avoiding this is especially important when we use "git fetch"
to perform lazy fetches in a partial clone because a target of such a
ref may need to be itself lazy-fetched (and otherwise causing an
infinite loop).
Therefore, avoid populating "existing_refs" until necessary. With this
patch, because Git lazy-fetches objects by literal hashes (to be done in
a subsequent commit), it will then be able to guarantee avoiding reading
targets of refs.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jonathan Tan [Tue, 18 Aug 2020 04:01:33 +0000 (21:01 -0700)]
fetch: avoid reading submodule config until needed
In "fetch", there are two parameters submodule_fetch_jobs_config and
recurse_submodules that can be set in a variety of ways: through
.gitmodules, through .git/config, and through the command line.
Currently "fetch" handles this by first reading .gitmodules, then
reading .git/config (allowing it to overwrite existing values), then
reading the command line (allowing it to overwrite existing values).
Notice that we can avoid reading .gitmodules if .git/config and/or the
command line already provides us with what we need. In addition, if
recurse_submodules is found to be "no", we do not need the value of
submodule_fetch_jobs_config.
Avoiding reading .gitmodules is especially important when we use "git
fetch" to perform lazy fetches in a partial clone because the
.gitmodules file itself might need to be lazy fetched (and otherwise
causing an infinite loop).
In light of all this, avoid reading .gitmodules until necessary. When
reading it, we may only need one of the two parameters it provides, so
teach fetch_config_from_gitmodules() to support NULL arguments. With
this patch, users (including Git itself when invoking "git fetch" to
lazy-fetch) will be able to guarantee avoiding reading .gitmodules by
passing --recurse-submodules=no.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jonathan Tan [Tue, 18 Aug 2020 04:01:32 +0000 (21:01 -0700)]
fetch: allow refspecs specified through stdin
In a subsequent patch, partial clones will be taught to fetch missing
objects using a "git fetch" subprocess. Because the number of objects
fetched may be too numerous to fit on the command line, teach "fetch" to
accept refspecs passed through stdin.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jonathan Tan [Tue, 18 Aug 2020 04:01:31 +0000 (21:01 -0700)]
negotiator/noop: add noop fetch negotiator
Add a noop fetch negotiator. This is introduced to allow partial clones
to skip the unneeded negotiation step when fetching missing objects
using a "git fetch" subprocess. (The implementation of spawning a "git
fetch" subprocess will be done in a subsequent patch.) But this can also
be useful for end users, e.g. as a blunt fix for object corruption.
Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Tue, 18 Aug 2020 14:25:22 +0000 (14:25 +0000)]
fetch: optionally allow disabling FETCH_HEAD update
If you run fetch but record the result in remote-tracking branches,
and either if you do nothing with the fetched refs (e.g. you are
merely mirroring) or if you always work from the remote-tracking
refs (e.g. you fetch and then merge origin/branchname separately),
you can get away with having no FETCH_HEAD at all.
Teach "git fetch" a command line option "--[no-]write-fetch-head".
The default is to write FETCH_HEAD, and the option is primarily
meant to be used with the "--no-" prefix to override this default,
because there is no matching fetch.writeFetchHEAD configuration
variable to flip the default to off (in which case, the positive
form may become necessary to defeat it).
Note that under "--dry-run" mode, FETCH_HEAD is never written;
otherwise you'd see list of objects in the file that you do not
actually have. Passing `--write-fetch-head` does not force `git
fetch` to write the file.
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Sat, 15 Aug 2020 17:37:57 +0000 (17:37 +0000)]
mem-pool: use consistent pool variable name
About half the function declarations in mem-pool.h used 'struct mem_pool
*pool', while the other half used 'struct mem_pool *mem_pool'. Make the
code a bit more consistent by just using 'pool' in preference to
'mem_pool' everywhere.
No behavioral changes included; this is just a mechanical rename (though
a line or two was rewrapped as well).
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Sat, 15 Aug 2020 17:37:56 +0000 (17:37 +0000)]
mem-pool: use more standard initialization and finalization
A typical memory type, such as strbuf, hashmap, or string_list can be
stored on the stack or embedded within another structure. mem_pool
cannot be, because of how mem_pool_init() and mem_pool_discard() are
written. mem_pool_init() does essentially the following (simplified
for purposes of explanation here):
void mem_pool_init(struct mem_pool **pool...)
{
*pool = xcalloc(1, sizeof(*pool));
It seems weird to require that mem_pools can only be accessed through a
pointer. It also seems slightly dangerous: unlike strbuf_release() or
strbuf_reset() or string_list_clear(), all of which put the data
structure into a state where it can be re-used after the call,
mem_pool_discard(pool) will leave pool pointing at free'd memory.
read-cache (and split-index) are the only current users of mem_pools,
and they haven't fallen into a use-after-free mistake here, but it seems
likely to be problematic for future users especially since several of
the current callers of mem_pool_init() will only call it when the
mem_pool* is not already allocated (i.e. is NULL).
This type of mechanism also prevents finding synchronization
points where one can free existing memory and then resume more
operations. It would be natural at such points to run something like
mem_pool_discard(pool...);
and, if necessary,
mem_pool_init(&pool...);
and then carry on continuing to use the pool. However, this fails badly
if several objects had a copy of the value of pool from before these
commands; in such a case, those objects won't get the updated value of
pool that mem_pool_init() overwrites pool with and they'll all instead
be reading and writing from free'd memory.
Modify mem_pool_init()/mem_pool_discard() to behave more like
strbuf_init()/strbuf_release()
or
string_list_init()/string_list_clear()
In particular: (1) make mem_pool_init() just take a mem_pool* and have
it only worry about allocating struct mp_blocks, not the struct mem_pool
itself, (2) make mem_pool_discard() free the memory that the pool was
responsible for, but leave it in a state where it can be used to
allocate more memory afterward (without the need to call mem_pool_init()
again).
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Sat, 15 Aug 2020 17:37:55 +0000 (17:37 +0000)]
mem-pool: add convenience functions for strdup and strndup
fast-import had a special mem_pool_strdup() convenience function that I
want to be able to use from the new merge algorithm I am writing. Move
it from fast-import to mem-pool, and also add a mem_pool_strndup()
while at it that I also want to use.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Danny Lin [Tue, 18 Aug 2020 05:22:49 +0000 (13:22 +0800)]
contrib/subtree: document 'push' does not take '--squash'
git subtree push does not support --squash, as previously illustrated in
6ccc71a9 (contrib/subtree: there's no push --squash, 2015-05-07)
Signed-off-by: Danny Lin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Danny Lin [Tue, 18 Aug 2020 05:21:10 +0000 (13:21 +0800)]
contrib/subtree: fix "unsure" for --message in the document
Revise the documentation and remove previous "unsure" after making sure
that --message supports only 'add', 'merge', 'pull', and 'split --rejoin'.
Signed-off-by: Danny Lin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Tue, 18 Aug 2020 00:01:32 +0000 (17:01 -0700)]
Eighth batch
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Tue, 18 Aug 2020 00:02:49 +0000 (17:02 -0700)]
Merge branch 'so/log-diff-merges-opt'
Earlier, to countermand the implicit "-m" option when the
"--first-parent" option is used with "git log", we added the
"--[no-]diff-merges" option in the jk/log-fp-implies-m topic. To
leave the door open to allow the "--diff-merges" option to take
values that instructs how patches for merge commits should be
computed (e.g. "cc"? "-p against first parent?"), redefine
"--diff-merges" to take non-optional value, and implement "off"
that means the same thing as "--no-diff-merges".
* so/log-diff-merges-opt:
t/t4013: add test for --diff-merges=off
doc/git-log: describe --diff-merges=off
revision: change "--diff-merges" option to require parameter
Junio C Hamano [Tue, 18 Aug 2020 00:02:49 +0000 (17:02 -0700)]
Merge branch 'jk/log-fp-implies-m'
"git log --first-parent -p" showed patches only for single-parent
commits on the first-parent chain; the "--first-parent" option has
been made to imply "-m". Use "--no-diff-merges" to restore the
previous behaviour to omit patches for merge commits.
* jk/log-fp-implies-m:
doc/git-log: clarify handling of merge commit diffs
doc/git-log: move "-t" into diff-options list
doc/git-log: drop "-r" diff option
doc/git-log: move "Diff Formatting" from rev-list-options
log: enable "-m" automatically with "--first-parent"
revision: add "--no-diff-merges" option to counteract "-m"
log: drop "--cc implies -m" logic
Junio C Hamano [Tue, 18 Aug 2020 00:02:48 +0000 (17:02 -0700)]
Merge branch 'ma/stop-progress-null-fix'
NULL dereference fix.
* ma/stop-progress-null-fix:
progress: don't dereference before checking for NULL
Junio C Hamano [Tue, 18 Aug 2020 00:02:47 +0000 (17:02 -0700)]
Merge branch 'es/test-cmp-typocatcher'
Test framework update.
* es/test-cmp-typocatcher:
test_cmp: diagnose incorrect arguments
Junio C Hamano [Tue, 18 Aug 2020 00:02:46 +0000 (17:02 -0700)]
Merge branch 'rp/apply-cached-with-i-t-a'
Recent versions of "git diff-files" shows a diff between the index
and the working tree for "intent-to-add" paths as a "new file"
patch; "git apply --cached" should be able to take "git diff-files"
and should act as an equivalent to "git add" for the path, but the
command failed to do so for such a path.
* rp/apply-cached-with-i-t-a:
t4140: test apply with i-t-a paths
apply: make i-t-a entries never match worktree
apply: allow "new file" patches on i-t-a entries
Junio C Hamano [Tue, 18 Aug 2020 00:02:45 +0000 (17:02 -0700)]
Merge branch 'al/bisect-first-parent'
"git bisect" learns the "--first-parent" option to find the first
breakage along the first-parent chain.
* al/bisect-first-parent:
bisect: combine args passed to find_bisection()
bisect: introduce first-parent flag
cmd_bisect__helper: defer parsing no-checkout flag
rev-list: allow bisect and first-parent flags
t6030: modernize "git bisect run" tests
Junio C Hamano [Tue, 18 Aug 2020 00:02:45 +0000 (17:02 -0700)]
Merge branch 'jk/sideband-error-l10n'
Mark error message for i18n.
* jk/sideband-error-l10n:
sideband: mark "remote error:" prefix for translation
Junio C Hamano [Tue, 18 Aug 2020 00:02:44 +0000 (17:02 -0700)]
Merge branch 'jc/noop-with-static-inline'
A no-op replacement function implemented as a C preprocessor macro
does not perform as good a job as one implemented as a "static
inline" function in catching errors in parameters; replace the
former with the latter in <git-compat-util.h> header.
* jc/noop-with-static-inline:
compat-util: type-check parameters of no-op replacement functions
Junio C Hamano [Tue, 18 Aug 2020 00:02:43 +0000 (17:02 -0700)]
Merge branch 'pd/mergetool-nvimdiff'
The existing backends for "git mergetool" based on variants of vim
have been refactored and then support for "nvim" has been added.
* pd/mergetool-nvimdiff:
mergetools: add support for nvimdiff (neovim) family
mergetool--lib: improve support for vimdiff-style tool variants
Junio C Hamano [Tue, 18 Aug 2020 00:02:42 +0000 (17:02 -0700)]
Merge branch 'hn/reftable-prep-part-2'
Further preliminary change to refs API.
* hn/reftable-prep-part-2:
Make HEAD a PSEUDOREF rather than PER_WORKTREE.
Modify pseudo refs through ref backend storage
t1400: use git rev-parse for testing PSEUDOREF existence
Junio C Hamano [Tue, 18 Aug 2020 00:02:41 +0000 (17:02 -0700)]
Merge branch 'dd/send-email-config'
Stop when "sendmail.*" configuration variables are defined, which
could be a mistaken attempt to define "sendemail.*" variables.
* dd/send-email-config:
git-send-email: die if sendmail.* config is set
Junio C Hamano [Tue, 18 Aug 2020 00:02:40 +0000 (17:02 -0700)]
Merge branch 'ps/ref-transaction-hook'
The logic to find the ref transaction hook script attempted to
cache the path to the found hook without realizing that it needed
to keep a copied value, as the API it used returned a transitory
buffer space. This has been corrected.
* ps/ref-transaction-hook:
t1416: avoid hard-coded sha1 ids
refs: fix interleaving hook calls with reference-transaction hook
Derrick Stolee [Mon, 17 Aug 2020 14:04:48 +0000 (14:04 +0000)]
multi-pack-index: use hash version byte
Similar to the commit-graph format, the multi-pack-index format has a
byte in the header intended to track the hash version used to write the
file. This allows one to interpret the hash length without having the
context of the repository config specifying the hash length. This was
not modified as part of the SHA-256 work because the hash length was
automatically up-shifted due to that config.
Since we have this byte available, we can make the file formats more
obviously incompatible instead of relying on other context from the
repository.
Add a new oid_version() method in midx.c similar to the one in
commit-graph.c. This is specifically made separate from that
implementation to avoid artificially linking the formats.
The test impact requires a few more things than the corresponding change
in the commit-graph format. Specifically, 'test-tool read-midx' was not
writing anything about this header value to output. Since the value
available in 'struct multi_pack_index' is hash_len instead of a version
value, we output "20" or "32" instead of "1" or "2".
Since we want a user to not have their Git commands fail if their
multi-pack-index has the incorrect hash version compared to the
repository's hash version, we relax the die() to an error() in
load_multi_pack_index(). This has some effect on 'git multi-pack-index
verify' as we need to check that a failed parse of a file that exists is
actually a verify error. For that test that checks the hash version
matches, we change the corrupted byte from "2" to "3" to ensure the test
fails for both hash algorithms.
Helped-by: brian m. carlson <redacted>
Signed-off-by: Derrick Stolee <redacted>
Reviewed-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
Derrick Stolee [Mon, 17 Aug 2020 14:04:47 +0000 (14:04 +0000)]
commit-graph: use the "hash version" byte
The commit-graph format reserved a byte among the header of the file to
store a "hash version". During the SHA-256 work, this was not modified
because file formats are not necessarily intended to work across hash
versions. If a repository has SHA-256 as its hash algorithm, it
automatically up-shifts the lengths of object names in all necessary
formats.
However, since we have this byte available for adjusting the version, we
can make the file formats more obviously incompatible instead of relying
on other context from the repository.
Update the oid_version() method in commit-graph.c to add a new value, 2,
for sha-256. This automatically writes the new value in a SHA-256
repository _and_ verifies the value is correct. This is a breaking
change relative to the current 'master' branch since
092b677 (Merge
branch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking
relative to any released version of Git.
The test impact is relatively minor: the output of 'test-tool
read-graph' lists the header information, so those instances of '1' need
to be replaced with a variable determined by GIT_TEST_DEFAULT_HASH. A
more careful test is added that specifically creates a repository of
each type then swaps the commit-graph files. The important value here is
that the "git log" command succeeds while writing a message to stderr.
Helped-by: brian m. carlson <redacted>
Signed-off-by: Derrick Stolee <redacted>
Reviewed-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
Derrick Stolee [Mon, 17 Aug 2020 14:04:46 +0000 (14:04 +0000)]
t/README: document GIT_TEST_DEFAULT_HASH
Helped-by: Eric Sunshine <redacted>
Signed-off-by: Derrick Stolee <redacted>
Reviewed-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Mon, 17 Aug 2020 21:33:16 +0000 (17:33 -0400)]
submodule--helper: fix leak of core.worktree value
In the ensure_core_worktree() function, we load the core.worktree value
of the submodule repository using repo_config_get_string(). This
function copies the string, but we never free it, leaking the memory.
We can instead use the "tmp" version of that function to avoid the
allocation at all. We don't have to worry about lifetime issues, since
we never even look at the value (we just want to know if it's set).
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Mon, 17 Aug 2020 21:33:13 +0000 (17:33 -0400)]
config: fix leak in git_config_get_expiry_in_days()
We use git_config_get_string() to retrieve the expiry value in a newly
allocated string. But after parsing it, we never free it, leaking the
memory.
We could fix this with a free() obviously, but there's an even better
solution: we can use the non-allocating "tmp" variant of the function;
we only need it to be valid for the lifetime of our parse function.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Mon, 17 Aug 2020 21:33:11 +0000 (17:33 -0400)]
config: drop git_config_get_string_const()
As evidenced by the leak fixes in the previous commit, the "const" in
git_config_get_string_const() clearly misleads people into thinking that
it does not allocate a copy of the string. We can fix this by renaming
it, but it's easier still to just drop it. Of the four remaining
callers:
- The one in git_config_parse_expiry() still needs to allocate, since
that's what its callers expect. We can just use the non-const
version and cast our pointer. Slightly ugly, but the damage is
contained in one spot.
- The two in apply are writing to global "const char *" variables, and
need to continue allocating. We often mark these as const because we
assign default string literals to them. But in this case we don't do
that, so we can just declare them as real "char *" pointers and use
the non-const version.
- The call in checkout doesn't actually need a copy; it can just use
the non-allocating "tmp" version of the function.
The function is also mentioned in the MyFirstContribution document. We
can swap that call out for the non-allocating "tmp" variant, which fits
well in the example given.
We'll drop the "configset" and "repo" variants, as well (which are
unused).
Note that this frees up the "const" name, so we could rename the "tmp"
variant back to that. But let's give some time for topics in flight to
adapt to the new code before doing so (if we do it too soon, the
function semantics will change but the compiler won't alert us).
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Phillip Wood [Mon, 17 Aug 2020 17:40:02 +0000 (18:40 +0100)]
rebase -i: support --committer-date-is-author-date
Rebase is implemented with two different backends - 'apply' and
'merge' each of which support a different set of options. In
particular the apply backend supports a number of options implemented
by 'git am' that are not implemented in the merge backend. This means
that the available options are different depending on which backend is
used which is confusing. This patch adds support for the
--committer-date-is-author-date option to the merge backend. This
option uses the author date of the commit that is being rewritten as
the committer date when the new commit is created.
Original-patch-by: Rohit Ashiwal <redacted>
Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>
Phillip Wood [Mon, 17 Aug 2020 17:40:01 +0000 (18:40 +0100)]
am: stop exporting GIT_COMMITTER_DATE
The implementation of --committer-date-is-author-date exports
GIT_COMMITTER_DATE to override the default committer date but does not
reset GIT_COMMITTER_DATE in the environment after creating the commit
so it is set in the environment of any hooks that get run. We're about
to add the same functionality to the sequencer and do not want to have
GIT_COMMITTER_DATE set when running hooks or exec commands so lets
update commit_tree_extended() to take an explicit committer so we
override the default date without setting GIT_COMMITTER_DATE in the
environment.
Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>
Phillip Wood [Mon, 17 Aug 2020 13:23:08 +0000 (13:23 +0000)]
add -p: fix checking of user input
When a file has been deleted the C version of add -p allows the user
to edit a hunk even though 'e' is not in the list of allowed
responses. (I think 'e' is disallowed because if the file is edited it
is no longer a deletion and we're not set up to rewrite the diff
header).
The invalid response was allowed because the test that determines
whether to display 'e' was not duplicated correctly in the code that
processes the user's choice. Fix this by using flags that are set when
constructing the prompt and checked when processing the user's choice
rather than repeating the check itself.
Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>
Phillip Wood [Mon, 17 Aug 2020 13:23:07 +0000 (13:23 +0000)]
add -p: use ALLOC_GROW_BY instead of ALLOW_GROW
This simplifies the code slightly, especially the third case where
hunk_nr was incremented a few lines before ALLOC_GROW().
Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff Hostetler [Mon, 17 Aug 2020 10:37:02 +0000 (10:37 +0000)]
mingw: improve performance of mingw_unlink()
Update mingw_unlink() to first try to delete the file with existing
permissions before trying to force it.
Windows throws an error when trying to delete a read-only file. The
mingw_unlink() compatibility wrapper always tries to _wchmod(666) the
file before calling _wunlink() to avoid that error. However, since
most files in the worktree are already writable, this is usually
wasted effort.
Update mingw_unlink() to just call DeleteFileW() directly and if that
succeeds return. If that fails, fall back into the existing code path
to update the permissions and use _wunlink() to get the existing
error code mapping.
Signed-off-by: Jeff Hostetler <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Martin Ågren [Sun, 16 Aug 2020 10:01:18 +0000 (12:01 +0200)]
Documentation: mark `--object-format=sha256` as experimental
After
eff45daab8 ("repository: enable SHA-256 support by default",
2020-07-29), vanilla builds of Git enable the user to run, e.g.,
git init --object-format=sha256
and hack away. This can be a good way to gain experience with the
SHA-256 world, e.g., to find bugs that
GIT_TEST_DEFAULT_HASH=sha256 make test
doesn't spot.
But it really is a separate world: Such SHA-256 repos will live entirely
separate from the (by now fairly large) set of SHA-1 repos. Interacting
across the border is possible in principle, e.g., through "diff + apply"
(or "format-patch + am"), but even that has its limitations: Applying a
SHA-256 diff in a SHA-1 repo works in the simple case, but if you need
to resort to `-3`, you're out of luck.
Similarly, "push + pull" should work, but you really will be operating
mostly offset from the rest of the world. That might be ok by the time
you initialize your repository, and it might be ok for several months
after that, but there might come a day when you're starting to regret
your use of `git init --object-format=sha256` and have dug yourself into
a fairly deep hole.
There are currently topics in flight to document our data formats and
protocols regarding SHA-256 and in some cases (midx and commit-graph),
we're considering adjusting how the file formats indicate which object
format to use.
Wherever `--object-format` is mentioned in our documentation, let's make
it clear that using it with "sha256" is experimental. If we later need
to explain why we can't handle data we generated back in 2020, we can
always point to this paragraph we're adding here.
By "include::"-ing a small blurb, we should be able to be consistent
throughout the documentation and can eventually gradually tone down the
severity of this text. One day, we might even use it to start phasing
out `--object-format=sha1`, but let's not get ahead of ourselves...
There's also `extensions.objectFormat`, but it's only mentioned three
times. Twice where we're adding this new disclaimer and in the third
spot we already have a "do not edit" warning. From there, interested
readers should eventually find this new one that we're adding here.
Because `GIT_DEFAULT_HASH` provides another entry point to this
functionality, document the experimental nature of it too.
Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jacob Keller [Sat, 15 Aug 2020 00:25:08 +0000 (17:25 -0700)]
refspec: make sure stack refspec_item variables are zeroed
A couple of functions that used struct refspec_item did not zero out the
structure memory. This can result in unexpected behavior, especially if
additional parameters are ever added to refspec_item in the future. Use
memset to ensure that unset structure members are zero.
It may make sense to convert most of these uses of struct refspec_item
to use either struct initializers or refspec_item_init_or_die. However,
other similar code uses memset. Converting all of these uses has been
left as a future exercise.
Signed-off-by: Jacob Keller <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jacob Keller [Sat, 15 Aug 2020 00:25:07 +0000 (17:25 -0700)]
refspec: fix documentation referring to refspec_item
In commit
d27eb356bf25 ("remote: move doc to remote.h and refspec.h")
the documentation for the refspec structure was moved into refspec.h
This documentation refers to elements of the refspec_item, not the
struct refspec. Move the documentation slightly in order to align it
with the structure it is actually referring to.
Signed-off-by: Jacob Keller <redacted>
Signed-off-by: Junio C Hamano <redacted>
Martin Ågren [Sat, 15 Aug 2020 16:06:02 +0000 (18:06 +0200)]
shallow.txt: document SHA-256 shallow format
Similar to recent commits, document that we list object names rather
than SHA-1s.
Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Martin Ågren [Sat, 15 Aug 2020 16:06:01 +0000 (18:06 +0200)]
protocol-capabilities.txt: clarify "allow-x-sha1-in-want" re SHA-256
Two of our capabilities contain "sha1" in their names, but that's
historical. Clarify that object names are still to be given using
whatever object format has been negotiated using the "object-format"
capability.
Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Martin Ågren [Sat, 15 Aug 2020 16:06:00 +0000 (18:06 +0200)]
index-format.txt: document SHA-256 index format
Document that in SHA-1 repositories, we use SHA-1 and in SHA-256
repositories, we use SHA-256, then replace all other uses of "SHA-1"
with something more neutral. Avoid referring to "160-bit" hash values.
Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Martin Ågren [Sat, 15 Aug 2020 16:05:59 +0000 (18:05 +0200)]
http-protocol.txt: document SHA-256 "want"/"have" format
Document that rather than always naming objects using SHA-1, we should
use whatever has been negotiated using the object-format capability.
Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
René Scharfe [Wed, 12 Aug 2020 16:52:55 +0000 (18:52 +0200)]
upload-pack: use buffered I/O to talk to rev-list
Like
f0bca72dc77 (send-pack: use buffered I/O to talk to pack-objects,
2016-06-08), significantly reduce the number of system calls and
simplify the code for sending object IDs to rev-list by using stdio's
buffering.
Take care to handle errors immediately to get the correct error code,
and to flush the buffer explicitly before closing the stream in order to
catch any write errors for these last bytes.
Helped-by: Chris Torek <redacted>
Helped-by: Johannes Sixt <redacted>
Signed-off-by: Junio C Hamano <redacted>
René Scharfe [Wed, 12 Aug 2020 16:52:54 +0000 (18:52 +0200)]
midx: use buffered I/O to talk to pack-objects
Like
f0bca72dc77 (send-pack: use buffered I/O to talk to pack-objects,
2016-06-08), significantly reduce the number of system calls and
simplify the code for sending object IDs to pack-objects by using
stdio's buffering.
Helped-by: Chris Torek <redacted>
Helped-by: Johannes Sixt <redacted>
Encouraged-by: Derrick Stolee <redacted>
Signed-off-by: René Scharfe <redacted>
Signed-off-by: Junio C Hamano <redacted>
René Scharfe [Wed, 12 Aug 2020 16:52:49 +0000 (18:52 +0200)]
connected: use buffered I/O to talk to rev-list
Like
f0bca72dc77 (send-pack: use buffered I/O to talk to pack-objects,
2016-06-08), significantly reduce the number of system calls and
simplify the code for sending object IDs to rev-list by using stdio's
buffering.
Take care to handle errors immediately to get the correct error code,
and to flush the buffer explicitly before closing the stream in order to
catch any write errors for these last bytes.
Helped-by: Chris Torek <redacted>
Helped-by: Johannes Sixt <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Fri, 14 Aug 2020 16:17:36 +0000 (12:17 -0400)]
config: fix leaks from git_config_get_string_const()
There are two functions to get a single config string:
- git_config_get_string()
- git_config_get_string_const()
One might naively think that the first one allocates a new string and
the second one just points us to the internal configset storage. But
in fact they both allocate a new copy; the second one exists only to
avoid having to cast when using it with a const global which we never
intend to free.
The documentation for the function explains that clearly, but it seems
I'm not alone in being surprised by this. Of 17 calls to the function,
13 of them leak the resulting value.
We could obviously fix these by adding the appropriate free(). But it
would be simpler still if we actually had a non-allocating way to get
the string. There's git_config_get_value() but that doesn't quite do
what we want. If the config key is present but is a boolean with no
value (e.g., "[foo]bar" in the file), then we'll get NULL (whereas the
string versions will print an error and die).
So let's introduce a new variant, git_config_get_string_tmp(), that
behaves as these callers expect. We need a new name because we have new
semantics but the same function signature (so even if we converted the
four remaining callers, topics in flight might be surprised). The "tmp"
is because this value should only be held onto for a short time. In
practice it's rare for us to clear and refresh the configset,
invalidating the pointer, but hopefully the "tmp" makes callers think
about the lifetime. In each of the converted cases here the value only
needs to last within the local function or its immediate caller.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Fri, 14 Aug 2020 16:14:53 +0000 (12:14 -0400)]
checkout: fix leak of non-existent branch names
We unconditionally write a branch name into a newly allocated buffer in
new_branch_info->path, via setup_branch_path(). We then check to see if
the branch exists; if not, we set that field to NULL, leaking the
memory. We should take care to free() it when doing so.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Fri, 14 Aug 2020 16:14:14 +0000 (12:14 -0400)]
submodule--helper: use strbuf_release() to free strbufs
The prepare_to_clone_next_submodule() function has a few local-variable
strbufs. We use strbuf_reset() throughout the function to reuse the
buffers over and over. But at the end of the function we also use
strbuf_reset() as they go out of scope, which means we end up leaking
their heap buffers. This should be strbuf_release() instead.
These were introduced by
48308681b0 (git submodule update: have a
dedicated helper for cloning, 2016-02-29), but it doesn't seem to have
the same mistake elsewhere. Likewise, I looked for other instances of
the pattern in the submodule--helper file but couldn't find any.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Wed, 12 Aug 2020 14:40:04 +0000 (14:40 +0000)]
sequencer: avoid garbled merge machinery messages due to commit labels
sequencer's get_message() exists to provide good labels on conflict
hunks; see commits
d68565402a ("revert: clarify label on conflict hunks", 2010-03-20)
bf975d379d ("cherry-pick, revert: add a label for ancestor", 2010-03-20)
043a4492b3 ("sequencer: factor code out of revert builtin", 2012-01-11).
for background on this function. These labels are of the form
<commitID>... <commit summary>
or
parent of <commitID>... <commit summary>
These labels are then passed as branch names to the merge machinery.
However, these labels, as formatted, often also serve to confuse. For
example, if we have a rename involved in a content merge, then it
results in text such as the following:
<<<<<<<< HEAD:foo.c
int j;
========
int counter;
>>>>>>>>
b01dface... Removed unnecessary stuff:bar.c
Or in various conflict messages, it can make it very difficult to read:
CONFLICT (rename/delete): foo.c deleted in
b01dface... Removed
unnecessary stuff and renamed in HEAD. Version HEAD of foo.c left
in tree.
CONFLICT (file location): dir1/foo.c added in
b01dface... Removed
unnecessary stuff inside a directory that was renamed in HEAD,
suggesting it should perhaps be moved to dir2/foo.c.
Make a minor change to remove the ellipses and add parentheses around
the commit summary; this makes all three examples much easier to read:
<<<<<<<< HEAD:foo.c
int j;
========
int counter;
>>>>>>>>
b01dface (Removed unnecessary stuff):bar.c
CONFLICT (rename/delete): foo.c deleted in
b01dface (Removed
unnecessary stuff) and renamed in HEAD. Version HEAD of foo.c left
in tree.
CONFLICT (file location): dir1/foo.c added in
b01dface (Removed
unnecessary stuff) inside a directory that was renamed in HEAD,
suggesting it should perhaps be moved to dir2/foo.c.
Signed-off-by: Elijah Newren <redacted>
Reviewed-by: Taylor Blau <redacted>
Acked-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Fri, 14 Aug 2020 11:10:49 +0000 (07:10 -0400)]
clear_pattern_list(): clear embedded hashmaps
Commit
96cc8ab531 (sparse-checkout: use hashmaps for cone patterns,
2019-11-21) added some auxiliary hashmaps to the pattern_list struct,
but they're leaked when clear_pattern_list() is called.
Signed-off-by: Jeff King <redacted>
Acked-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Fri, 14 Aug 2020 01:07:12 +0000 (18:07 -0700)]
messages: avoid SHA-1 in end-user facing messages
There are still a handful mentions of SHA-1 when we meant the
(hexadecimal) object names in end-user facing messages. Rewrite
them.
I was hoping that this can mostly be s/SHA-1/object name/, but
a few messages needed rephrasing to keep the result readable.
Signed-off-by: Junio C Hamano <redacted>
brian m. carlson [Thu, 13 Aug 2020 22:49:01 +0000 (22:49 +0000)]
docs: fix step in transition plan
One of the required steps for the objectFormat extension is to implement
the loose object index. However, without support for
compatObjectFormat, we don't even know if the loose object index is
needed, so it makes sense to move that step to the compatObjectFormat
section. Do so.
Signed-off-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
brian m. carlson [Thu, 13 Aug 2020 22:49:00 +0000 (22:49 +0000)]
docs: document SHA-256 pack and indices
Now that we have SHA-256 support for packs and indices, let's document
that in SHA-256 repositories, we use SHA-256 instead of SHA-1 for object
names and checksums. Instead of duplicating this information throughout
the document, let's just document that in SHA-1 repositories, we use
SHA-1 for these purposes, and in SHA-256 repositories, we use SHA-256.
Signed-off-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Thu, 13 Aug 2020 21:13:59 +0000 (14:13 -0700)]
Seventh batch
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Thu, 13 Aug 2020 21:13:40 +0000 (14:13 -0700)]
Merge branch 'rp/blame-first-parent-doc'
The "git blame --first-parent" option was not documented, but now
it is.
* rp/blame-first-parent-doc:
blame-options.txt: document --first-parent option
Junio C Hamano [Thu, 13 Aug 2020 21:13:39 +0000 (14:13 -0700)]
Merge branch 'ma/test-quote-cleanup'
Test cleanup.
* ma/test-quote-cleanup:
t4104: modernize and simplify quoting
t: don't spuriously close and reopen quotes
Junio C Hamano [Thu, 13 Aug 2020 21:13:39 +0000 (14:13 -0700)]
Merge branch 'jt/has_object'
A new helper function has_object() has been introduced to make it
easier to mark object existence checks that do and don't want to
trigger lazy fetches, and a few such checks are converted using it.
* jt/has_object:
fsck: do not lazy fetch known non-promisor object
pack-objects: no fetch when allow-{any,promisor}
apply: do not lazy fetch when applying binary
sha1-file: introduce no-lazy-fetch has_object()
Junio C Hamano [Thu, 13 Aug 2020 21:13:39 +0000 (14:13 -0700)]
Merge branch 'bc/sha-256-cvs-svn-updates'
Portability fix.
* bc/sha-256-cvs-svn-updates:
git-cvsexportcommit: support Perl before 5.10.1
Antti Keränen [Thu, 13 Aug 2020 17:42:57 +0000 (20:42 +0300)]
rebase -i: fix possibly wrong onto hash in todo
'todo_list_write_to_file' may overwrite the static buffer, originating
from 'find_unique_abbrev', that was used to store the short commit hash
'c' for "# Rebase a..b onto c" message in the todo editor. This is
because the buffer that is returned from 'find_unique_abbrev' is valid
until 4 more calls to `find_unique_abbrev` are made.
As 'todo_list_write_to_file' calls 'find_unique_abbrev' for each rebased
commit, the hash for 'c' is overwritten if there are 4 or more commits
in the rebase. This behavior has been broken since its introduction.
Fix by storing the short onto commit hash in a different buffer that
remains valid, before calling 'todo_list_write_to_file'.
Found-by: Jussi Keränen <redacted>
Signed-off-by: Antti Keränen <redacted>
Acked-by: Alban Gruin <redacted>
Signed-off-by: Junio C Hamano <redacted>
Philippe Blain [Wed, 12 Aug 2020 22:30:29 +0000 (22:30 +0000)]
userdiff: improve Fortran xfuncname regex
The third part of the Fortran xfuncname regex wants to match the
beginning of a subroutine or function, so it allows for all characters
except `'`, `"` or whitespace before the keyword 'function' or
'subroutine'. This is meant to match the 'recursive', 'elemental' or
'pure' keywords, as well as function return types, and to prevent
matches inside strings.
However, the negated set does not contain the `!` comment character,
so a line with an end-of-line comment containing the keyword 'function' or
'subroutine' followed by another word is mistakenly chosen as a hunk header.
Improve the regex by adding `!` to the negated set.
Signed-off-by: Philippe Blain <redacted>
Signed-off-by: Junio C Hamano <redacted>
Philippe Blain [Wed, 12 Aug 2020 22:30:28 +0000 (22:30 +0000)]
userdiff: add tests for Fortran xfuncname regex
The Fortran userdiff patterns, introduced in
909a5494f8 (userdiff.c: add
builtin fortran regex patterns, 2010-09-10), predate the test
infrastructure for xfuncname patterns, introduced in
bfa7d01413 (t4018:
an infrastructure to test hunk headers, 2014-03-21).
Add tests for the Fortran xfuncname patterns. The test
't/t4018/fortran-comment-keyword' documents a shortcoming of the regex
that is fixed in a subsequent commit.
While at it, add descriptive comments for the different parts of the
regex.
Signed-off-by: Philippe Blain <redacted>
Signed-off-by: Junio C Hamano <redacted>
Philippe Blain [Wed, 12 Aug 2020 23:52:00 +0000 (23:52 +0000)]
fetch, pull doc: correct description of '--set-upstream'
The '--set-upstream' option to `git fetch` (which is also accepted by
`git pull` and passed through to the underlying `git fetch`) allows
setting the upstream configuration for the current branch. This was
added in
24bc1a1292 (pull, fetch: add --set-upstream option,
2019-08-19).
However, the documentation for that option describes its action as 'If
the remote is fetched successfully, pull and add upstream (tracking)
reference [...]', which is wrong because this option does not cause
neither `git fetch` nor `git pull` to pull: `git fetch` does not pull
and `git pull` always pulls.
Fix the description of that option.
Signed-off-by: Philippe Blain <redacted>
Signed-off-by: Junio C Hamano <redacted>
Johannes Berg [Thu, 13 Aug 2020 14:50:53 +0000 (16:50 +0200)]
docs: commit-graph: fix some whitespace in the diagram
In the merge diagram, some whitespace is missing which
makes it a bit confusing, fix that.
Signed-off-by: Johannes Berg <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 15:55:51 +0000 (11:55 -0400)]
ls-remote: simplify UNLEAK() usage
We UNLEAK() the "sorting" list created by parsing command-line options
(which is essentially used until the program exits). But we do so right
before leaving the cmd_ls_remote() function, which means we have to hit
all of the exits. But the point of UNLEAK() is that it's an annotation
which doesn't impact the variable itself. We can mark it as soon as
we're done writing its value, and then we only have to do so once.
This gives us a minor code reduction, and serves as a better example of
how UNLEAK() can be used.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 15:55:00 +0000 (11:55 -0400)]
stop calling UNLEAK() before die()
The point of UNLEAK() is to make a reference to a variable that is about
to go out of scope so that leak-checkers will consider it to be
not-leaked. Doing so right before die() is therefore pointless; even
though we are about to exit the program, the variable will still be on
the stack and accessible to leak-checkers.
These annotations aren't really hurting anything, but they clutter the
code and set a bad example of how to use UNLEAK().
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 15:00:17 +0000 (11:00 -0400)]
drop vcs-svn experiment
The code in vcs-svn was started in 2010 as an attempt to build a
remote-helper for interacting with svn repositories (as opposed to
git-svn). However, we never got as far as shipping a mature remote
helper, and the last substantive commit was
e99d012a6bc in 2012.
We do have a git-remote-testsvn, and it is even installed as part of
"make install". But given the name, it seems unlikely to be used by
anybody (you'd have to explicitly "git clone testsvn::$url", and there
have been zero mentions of that on the mailing list since 2013, and even
that includes the phrase "you might need to hack a bit to get it working
properly"[1]).
We also ship contrib/svn-fe, which builds on the vcs-svn work. However,
it does not seem to build out of the box for me, as the link step misses
some required libraries for using libgit.a. Curiously, the original
build breakage bisects for me to
eff80a9fd9 (Allow custom "comment
char", 2013-01-16), which seems unrelated. There was an attempt to fix
it in
da011cb0e7 (contrib/svn-fe: fix Makefile, 2014-08-28), but on my
system that only switches the error message.
So it seems like the result is not really usable by anybody in practice.
It would be wonderful if somebody wanted to pick up the topic again, and
potentially it's worth carrying around for that reason. But the flip
side is that people doing tree-wide operations have to deal with this
code. And you can see the list with (replace "HEAD" with this commit as
appropriate):
{
echo "--"
git diff-tree --diff-filter=D -r --name-only HEAD^ HEAD
} |
git log --no-merges --oneline
e99d012a6bc.. --stdin
which shows 58 times somebody had to deal with the code, generally due
to a compile or test failure, or a tree-wide style fix or API change.
Let's drop it and let anybody who wants to pick it up do so by
resurrecting it from the git history.
As a bonus, this also reduces the size of a stripped installation of Git
from 21MB to 19MB.
[1] https://lore.kernel.org/git/CALkWK0mPHzKfzFKKpZkfAus3YVC9NFYDbFnt+5JQYVKipk3bQQ@mail.gmail.com/
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 14:59:45 +0000 (10:59 -0400)]
make git-fast-import a builtin
There's no reason that git-fast-import benefits from being a separate
binary. And as it links against libgit.a, it has a non-trivial disk
footprint. Let's make it a builtin, which reduces the size of a stripped
installation from 22MB to 21MB.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 14:59:36 +0000 (10:59 -0400)]
make git-bugreport a builtin
There's no reason that bugreport has to be a separate binary. And since
it links against libgit.a, it has a rather large disk footprint. Let's
make it a builtin, which reduces the size of a stripped installation
from 24MB to 22MB.
This also simplifies our Makefile a bit. And we can take advantage of
builtin niceties like RUN_SETUP_GENTLY.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 14:58:55 +0000 (10:58 -0400)]
make credential helpers builtins
There's no real reason for credential helpers to be separate binaries. I
did them this way originally under the notion that helper don't _need_
to be part of Git, and so can be built totally separately (and indeed,
the ones in contrib/credential are). But the ones in our main Makefile
build on libgit.a, and the resulting binaries are reasonably large.
We can slim down our total disk footprint by just making them builtins.
This reduces the size of:
make strip install
from 29MB to 24MB on my Debian system.
Note that credential-cache can't operate without support for Unix
sockets. Currently we just don't build it at all when NO_UNIX_SOCKETS is
set. We could continue that with conditionals in the Makefile and our
list of builtins. But instead, let's build a dummy implementation that
dies with an informative message. That has two advantages:
- it's simpler, because the conditional bits are all kept inside
the credential-cache source
- a user who is expecting it to exist will be told _why_ they can't
use it, rather than getting the "credential-cache is not a git
command" error which makes it look like the Git install is broken.
Note that our dummy implementation does still respond to "-h" in order
to appease t0012 (and this may be a little friendlier for users, as
well).
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 14:57:19 +0000 (10:57 -0400)]
Makefile: drop builtins from MSVC pdb list
Over the years some more programs have become builtins, but nobody
updated this MSVC-specific section of the file (which specifically says
that it should not include builtins). Let's bring it up to date.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 05:25:31 +0000 (01:25 -0400)]
blame: only coalesce lines that are adjacent in result
After blame has finished but before we produce any output, we coalesce
groups of lines that were adjacent in the original suspect (which may
have been split apart by lines in intermediate commits which went away).
However, this can cause incorrect output if the lines are not also
adjacent in the result. For instance, the case in t8003 has:
ABC
DEF
which becomes
ABC
SPLIT
DEF
Blaming only lines 1 and 3 in the result yields two blame groups (one
for each line) that were adjacent in the original. That's enough for us
to coalesce them into a single group, but that loses information: our
output routines assume they're adjacent in the result as well, and we
output:
<oid> 1) ABC
<oid> 2) SPLIT
This is nonsense for two reasons:
- we were asked about line 3, not line 2; we should not output the
SPLIT line at all
- commit <oid> did not touch the SPLIT line at all! We found the
correct blame for line 3, but the bug is actually in the output
stage, which is showing the wrong line number and content from the
final file.
We can fix this by only coalescing when both the suspect and result
lines are adjacent. That fixes this bug, but keeps coalescing in cases
where want it (e.g., the existing test in t8003 where SPLIT goes away,
and the lines really are adjacent in the result).
Reported-by: Nuthan Munaiah <redacted>
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 05:23:41 +0000 (01:23 -0400)]
t8003: factor setup out of coalesce test
In preparation for adding more tests of blame's coalesce code, let's
split the setup out from the first test, and give each of the commits
a more meaningful name:
- $orig for the original source that added the lines
- $split for the version where they are split apart
- $final for the final version that re-joins them
That's not strictly necessary, but makes the follow-on tests less
brittle than relying on HEAD^, etc, to name the commits.
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Thu, 13 Aug 2020 05:23:05 +0000 (01:23 -0400)]
t8003: check output of coalesced blame
Commit
f0cbe742f4 (blame: add a test to cover blame_coalesce(),
2019-06-20) added a test case where blame can usefully coalesce two
groups of lines. But since it relies on the normal blame output, it only
exercises the code and can't tell whether the lines were actually
joined into a single group.
However, by using --porcelain output, we can see how git-blame considers
the groupings (and likewise how the coalescing might have a real
user-visible impact for a tool that uses the porcelain-output
groupings). This lets us confirm that we are indeed coalescing correctly
(and the fact that this test case requires coalescing can be verified by
dropping the call to blame_coalesce(), causing the test to fail).
Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
Prathamesh Chavan [Wed, 12 Aug 2020 19:44:04 +0000 (01:14 +0530)]
submodule: port submodule subcommand 'summary' from shell to C
Convert submodule subcommand 'summary' to a builtin and call it via
'git-submodule.sh'.
The shell version had to call $diff_cmd twice, once to find the modified
modules cared by the user and then again, with that list of modules
to do various operations for computing the summary of those modules.
On the other hand, the C version does not need a second call to
$diff_cmd since it reuses the module list from the first call to do the
aforementioned tasks.
In the C version, we use the combination of setting a child process'
working directory to the submodule path and then calling
'prepare_submodule_repo_env()' which also sets the 'GIT_DIR' to '.git',
so that we can be certain that those spawned processes will not access
the superproject's ODB by mistake.
A behavioural difference between the C and the shell version is that the
shell version outputs two line feeds after the 'git log' output when run
outside of the tests while the C version outputs one line feed in any
case. The reason for this is that the shell version calls log with
'--pretty=format:<fmt>' whose output is followed by two echo
calls; 'format' does not have "terminator" semantics like its 'tformat'
counterpart. So, the log output is terminated by a newline only when
invoked by the user and not when invoked from the scripts. This results
in the one & two line feed differences in the shell version.
On the other hand, the C version calls log with '--pretty=<fmt>'
which is equivalent to '--pretty:tformat:<fmt>' which is then
followed by a 'printf("\n")'. Due to its "terminator" semantics the
log output is always terminated by newline and hence one line feed in
any case.
Also, when we try to pass an option-like argument after a non-option
argument, for instance:
git submodule summary HEAD --foo-bar
(or)
git submodule summary HEAD --cached
That argument would be treated like a path to the submodule for which
the user is requesting a summary. So, the option ends up having no
effect. Though, passing '--quiet' is an exception to this:
git submodule summary HEAD --quiet
While 'summary' doesn't support '--quiet', we don't get an output for
the above command as '--quiet' is treated as a path which means we get
an output only if a submodule whose path is '--quiet' exists.
The error message in case of computing a summary for non-existent
submodules in the C version is different from that of the shell version.
Since the new error message is not marked for translation, change the
'test_i18ngrep' in t7421.4 to 'grep'.
Mentored-by: Christian Couder <redacted>
Mentored-by: Stefan Beller <redacted>
Mentored-by: Kaartic Sivaraam <redacted>
Helped-by: Johannes Schindelin <redacted>
Signed-off-by: Prathamesh Chavan <redacted>
Signed-off-by: Shourya Shukla <redacted>
Signed-off-by: Junio C Hamano <redacted>
Shourya Shukla [Wed, 12 Aug 2020 19:44:03 +0000 (01:14 +0530)]
t7421: introduce a test script for verifying 'summary' output
't7401-submodule-summary.sh' uses 'git add' to add submodules. Therefore,
some commands such as 'git submodule init' and 'git submodule deinit'
do not work as expected.
So, introduce a test script for verifying the 'summary' output for
submodules added using 'git submodule add' and notify regarding the
above mentioned behaviour in t7401 itself.
Mentored-by: Christian Couder <redacted>
Mentored-by: Kaartic Sivaraam <redacted>
Signed-off-by: Shourya Shukla <redacted>
Signed-off-by: Junio C Hamano <redacted>
Shourya Shukla [Wed, 12 Aug 2020 19:44:02 +0000 (01:14 +0530)]
submodule: rename helper functions to avoid ambiguity
The helper functions: show_submodule_summary(),
prepare_submodule_summary() and print_submodule_summary() are used by
the builtin_diff() function in diff.c to generate a summary of
submodules in the context of a diff. Functions with similar names are to
be introduced in the upcoming port of submodule's summary subcommand.
So, rename the helper functions to '*_diff_submodule_summary()' to avoid
ambiguity.
Mentored-by: Christian Couder <redacted>
Mentored-by: Kaartic Sivaraam <redacted>
Signed-off-by: Shourya Shukla <redacted>
Signed-off-by: Junio C Hamano <redacted>
Shourya Shukla [Wed, 12 Aug 2020 19:44:01 +0000 (01:14 +0530)]
submodule: remove extra line feeds between callback struct and macro
Many `submodule--helper` subcommands follow the convention that a struct
defines their callback data, and the declaration of that struct is
followed immediately by a macro to use in static initializers, without
any separating empty line.
Let's align the `init`, `status` and `sync` subcommands with that convention.
Mentored-by: Christian Couder <redacted>
Mentored-by: Kaartic Sivaraam <redacted>
Helped-by: Johannes Schindelin <redacted>
Helped-by: Philip Oakley <redacted>
Signed-off-by: Shourya Shukla <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Wed, 12 Aug 2020 07:12:36 +0000 (07:12 +0000)]
dir: avoid prematurely marking nonbare repositories as matches
Nonbare repositories are special directories. Unlike normal directories
that we might recurse into to list the files they contain, nonbare
repositories must themselves match and then we always report only on the
nonbare repository directory itself and not on any of its contents.
Separately, when traversing directories to try to find untracked or
excluded files, we often think in terms of paths either matching the
specified pathspec, or not matching them. However, there is a special
value that do_match_pathspec() uses named
MATCHED_RECURSIVELY_LEADING_PATHSPEC which means "this directory does
not match any pathspec BUT it is possible a file or directory underneath
it does." That special value prevents us from prematurely thinking that
some directory and everything under it is irrelevant, but also allows us
to differentiate from "this is a match".
The combination of these two special cases was previously uncovered.
Add a test to the testsuite to cover it, and make sure that we return a
nonbare repository as a non-match if the best match it got was
MATCHED_RECURSIVELY_LEADING_PATHSPEC.
Reported-by: christian w <redacted>
Simplified-testcase-and-bisection-by: Kyle Meyer <redacted>
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Wed, 12 Aug 2020 07:12:35 +0000 (07:12 +0000)]
t3000: fix some test description typos
There is no such flag as --o; it is either --others or -o.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
René Scharfe [Wed, 12 Aug 2020 13:45:47 +0000 (15:45 +0200)]
rebase: remove unused function reschedule_last_action
The only caller of reschedule_last_action was removed by
ef64bb328df
(rebase: strip unused code in git-rebase--preserve-merges.sh,
2018-05-28); remove this unused shell function as well.
Signed-off-by: René Scharfe <redacted>
Reviewed-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Wed, 12 Aug 2020 01:03:56 +0000 (18:03 -0700)]
Sixth batch
Signed-off-by: Junio C Hamano <redacted>
Junio C Hamano [Wed, 12 Aug 2020 01:04:13 +0000 (18:04 -0700)]
Merge branch 'ss/cmake-build'
CMake support to build with MSVC for Windows bypassing the Makefile.
* ss/cmake-build:
ci: modification of main.yml to use cmake for vs-build job
cmake: support for building git on windows with msvc and clang.
cmake: support for building git on windows with mingw
cmake: support for testing git when building out of the source tree
cmake: support for testing git with ctest
cmake: installation support for git
cmake: generate the shell/perl/python scripts and templates, translations
Introduce CMake support for configuring Git
Junio C Hamano [Wed, 12 Aug 2020 01:04:12 +0000 (18:04 -0700)]
Merge branch 'tb/upload-pack-filters'
The component to respond to "git fetch" request is made more
configurable to selectively allow or reject object filtering
specification used for partial cloning.
* tb/upload-pack-filters:
t5616: use test_i18ngrep for upload-pack errors
upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'
upload-pack.c: allow banning certain object filter(s)
list_objects_filter_options: introduce 'list_object_filter_config_name'
Junio C Hamano [Wed, 12 Aug 2020 01:04:12 +0000 (18:04 -0700)]
Merge branch 'es/worktree-doc-cleanups'
Doc cleanup around "worktree".
* es/worktree-doc-cleanups:
git-worktree.txt: link to man pages when citing other Git commands
git-worktree.txt: make start of new sentence more obvious
git-worktree.txt: fix minor grammatical issues
git-worktree.txt: consistently use term "working tree"
git-worktree.txt: employ fixed-width typeface consistently
Junio C Hamano [Wed, 12 Aug 2020 01:04:11 +0000 (18:04 -0700)]
Merge branch 'bc/sha-256-part-3'
The final leg of SHA-256 transition.
* bc/sha-256-part-3: (39 commits)
t: remove test_oid_init in tests
docs: add documentation for extensions.objectFormat
ci: run tests with SHA-256
t: make SHA1 prerequisite depend on default hash
t: allow testing different hash algorithms via environment
t: add test_oid option to select hash algorithm
repository: enable SHA-256 support by default
setup: add support for reading extensions.objectformat
bundle: add new version for use with SHA-256
builtin/verify-pack: implement an --object-format option
http-fetch: set up git directory before parsing pack hashes
t0410: mark test with SHA1 prerequisite
t5308: make test work with SHA-256
t9700: make hash size independent
t9500: ensure that algorithm info is preserved in config
t9350: make hash size independent
t9301: make hash size independent
t9300: use $ZERO_OID instead of hard-coded object ID
t9300: abstract away SHA-1-specific constants
t8011: make hash size independent
...
Sergey Organov [Wed, 5 Aug 2020 22:08:32 +0000 (01:08 +0300)]
t/t4013: add test for --diff-merges=off
Signed-off-by: Sergey Organov <redacted>
Signed-off-by: Junio C Hamano <redacted>
Sergey Organov [Wed, 5 Aug 2020 22:08:31 +0000 (01:08 +0300)]
doc/git-log: describe --diff-merges=off
Signed-off-by: Sergey Organov <redacted>
Signed-off-by: Junio C Hamano <redacted>
Sergey Organov [Wed, 5 Aug 2020 22:08:30 +0000 (01:08 +0300)]
revision: change "--diff-merges" option to require parameter
--diff-merges=off is the only accepted form for now, a synonym for
--no-diff-merges.
This patch is a preparation for adding more values, as well as supporting
--diff-merges=<parent>, where <parent> is single parent number to output diff
against.
Signed-off-by: Sergey Organov <redacted>
Signed-off-by: Junio C Hamano <redacted>
Jeff King [Tue, 11 Aug 2020 06:53:47 +0000 (02:53 -0400)]
t1416: avoid hard-coded sha1 ids
The test added by
e5256c82e5 (refs: fix interleaving hook calls with
reference-transaction hook, 2020-08-07) uses hard-coded sha1 object ids
in its expected output. This causes it to fail when run with
GIT_TEST_DEFAULT_HASH=sha256.
Let's make use of the oid variables we define earlier, as the rest of
the nearby tests do.
Signed-off-by: Jeff King <redacted>
Reviewed-by: Patrick Steinhardt <redacted>
Signed-off-by: Junio C Hamano <redacted>
Derrick Stolee [Tue, 11 Aug 2020 15:30:18 +0000 (15:30 +0000)]
multi-pack-index: repack batches below --batch-size
The --batch-size=<size> option of 'git multi-pack-index repack' is
intended to limit the amount of work done by the repack. In the case of
a large repository, this command should repack a number of small
pack-files but leave the large pack-files alone. Most often, the
repository has one large pack-file from a 'git clone' operation and
number of smaller pack-files from incremental 'git fetch' operations.
The issue with '--batch-size' is that it also _prevents_ the repack from
happening if the expected size of the resulting pack-file is too small.
This was intended as a way to avoid frequent churn of small pack-files,
but it has mostly caused confusion when a repository is of "medium"
size. That is, not enormous like the Windows OS repository, but also not
so small that this incremental repack isn't valuable.
The solution presented here is to collect pack-files for repack if their
expected size is smaller than the batch-size parameter until either the
total expected size exceeds the batch-size or all pack-files are
considered. If there are at least two pack-files, then these are
combined to a new pack-file whose size should not be too much larger
than the batch-size.
This new strategy should succeed in keeping the number of pack-files
small in these "medium" size repositories. The concern about churn is
likely not interesting, as the real control over that is the frequency
in which the repack command is run.
Signed-off-by: Derrick Stolee <redacted>
Reviewed-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
René Scharfe [Tue, 11 Aug 2020 17:15:03 +0000 (19:15 +0200)]
upload-pack: remove superfluous sigchain_pop() call
2997178ee6 (upload-pack: split check_unreachable() in two, prep for
get_reachable_list(), 2016-06-12) moved most code of has_unreachable()
into the new function do_reachable_revlist(). The latter takes care to
ignore SIGPIPE during its operations, and restores the original signal
handler before returning.
However, a sigchain_pop(SIGPIPE) call remained in the error handling
code of has_unreachable(), which does nothing because the stack is
empty after do_reachable_revlist() cleaned up after itself. Remove it.
Signed-off-by: René Scharfe <redacted>
Reviewed-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Mon, 10 Aug 2020 22:29:19 +0000 (22:29 +0000)]
t6425: be more flexible with rename/delete conflict messages
t6425 was very picky about the exact output message produced by a
rename/delete conflict, in a way that just scratches the surface of the
mess that was built into merge-recursive. The idea was that it would
try to find the possible combinations of different conflict types, and
when more than one was present for one path, it would try to provide a
combined message that covered all the cases.
There's a lot to unravel here...
First, there's a basic conflict type known as modify/delete, which is a
content conflict. It occurs when one side deletes a file, but the other
modifies it.
There is also a path conflict known as a rename/delete. This occurs
when one side deletes a path, and the other renames it. This is not a
content conflict, it is a path conflict. It will often occur in
combination with a content conflict, though, namely a modify/delete. As
such, these two were often combined.
Another type of conflict that can exist is a directory/file conflict.
For example, one side adds a new file at some path, and the other side
of history adds a directory at the same path. The path that was "added"
could have been put there by a rename, though. Thus, we have the
possibility of a single path being affected by a modify/delete, a
rename/delete, and a directory/file conflict.
In part, this was a natural by-product of merge-recursive's design.
Since it was doing a four way merge with the contents of the working
tree being the fourth factor it had to consider, it had working tree
handling spread all over the code. It also had directory/file conflict
handling spread everywhere through all the other types of conflicts.
And our testsuite has a huge number of directory/file conflict tests
because trying to get them right required modifying so many different
codepaths. A natural outgrowth of this kind of structure is conflict
messages that combine all the different types that the current codepath
is considering.
However, if we want to make the different conflict types orthogonal and
avoid repeating ourselves and getting very brittle code, then we need to
split the messages from these different conflict types apart. Besides,
trying to determine all possible permutations is a _royal_ mess. The
code to handle the rename/delete/directory/file conflict output is
already somewhat hard to parse, and is somewhat brittle. But if we
really wanted to go that route, then we'd have to have special handling
for the following types of combinations:
* rename/add/delete:
on side of history that didn't rename the given file, remove the file
instead and place an unrelated file in the way of the rename
* rename/rename(2to1)/mode conflict/delete/delete:
two different files, one executable and the other not, are renamed
to the same location, each side deletes the source file that the
other side renames
* rename/rename(1to2)/add/add:
file renamed differently on each side of history, with each side
placing an unrelated file in the way of the other
* rename/rename(1to2)/content conflict/file location/(D/F)/(D/F)/:
both sides modify a file in conflicting way, both rename that file
but to different paths, one side renames the directory which the
other side had renamed that file into causing it to possibly need a
transitive rename, and each side puts a directory in the way of the
other's path.
Let's back away from this path of insanity, and allow the different
types of conflicts to be handled by separate pieces of non-repeated code
by allowing the conflict messages to be split into their separate types.
(If multiple conflict types affect a single path, the conflict messages
can be printed sequentially.) Start this path with a simple change:
modify this test to be more flexible and accept the output either merge
backend (recursive or the new ort) will produce.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Mon, 10 Aug 2020 22:29:18 +0000 (22:29 +0000)]
t642[23]: be more flexible for add/add conflicts involving pair renames
Much like the last commit accepted 'add/add' and 'rename/add'
interchangably, we also want to do the same for 'add/add' and
'rename/rename'. This also allows us to avoid the ambiguity in meaning
with 'rename/rename' (is it two separate files renamed to the same
location, or one file renamed on both sides but differently)?
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Mon, 10 Aug 2020 22:29:17 +0000 (22:29 +0000)]
t6422, t6426: be more flexible for add/add conflicts involving renames
merge-recursive treats an add/add conflict where one of the adds came
from a rename as a separate 'rename/add' type of conflict. However, if
there is not content conflict after the content merge(s), then the file
is not considered to be conflicted. That suggests the conflict type is
really just add/add. Other merge engines might choose to print messages
to the console that just refer to these as add/add conflicts; accept
both types of output.
Note: it could help to notify users if the three-way content merge of
the rename had content conflicts, because when we then go to two-way
merge THAT with the conflicting add we can get nested conflict markers.
merge-recursive, unfortunately, doesn't do that, but other merge engines
could.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Mon, 10 Aug 2020 22:29:16 +0000 (22:29 +0000)]
t6423: add an explanation about why one of the tests does not pass
I had long since forgotten the idea behind this test and why it failed,
and took a little while to figure it out. To prevent others from having
to spend a similar time on it, add an explanation in the comments.
However, the reasoning in the explanation makes me question why I
considered it a failure at all. I'm not sure if I had a better reason
when I originally wrote it, but for now just add commentary about the
possible expectations and why it behaves the way it does right now.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Mon, 10 Aug 2020 22:29:15 +0000 (22:29 +0000)]
t6416, t6423: clarify some comments and fix some typos
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>
Elijah Newren [Mon, 10 Aug 2020 22:29:14 +0000 (22:29 +0000)]
t6422: fix multiple errors with the mod6 test expectations
This test had multiple issues causing it to fail for the wrong
reason(s):
* rename/rename(1to2) conflicts have always left the original source
path present in the working directory and index (at stage 1). Thus,
the triple rename/rename(1to2) should result in 9 unstaged files,
not 6.
* It messed up the three-way content merge for checking the results of
merging for one of the renames, accidentally turning it into a
two-way merge.
* It got the contents of the base files it was using to compare
against wrong, due to an off-by-one error, and overwrite-redirection
('>') instead of append-redirection ('>>').
* It used slightly too-long conflict markers
* It didn't include filenames in the conflict marker hunks (granted,
that was a shortcoming of the merge-recursive backend for rename/add
and rename/rename(2to1) conflicts, but since it's
test_expect_failure anyway we might as well make it expect our
preferred behavior rather than some compromise that we can't yet
reach anyway).
Fix these issues so that a merge backend which correctly handles these
kinds of nested conflicts will pass the test.
Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>