t5503: simplify setup of test which exercises failure of backfill
In the testcase to exercise backfilling of tags for fetches we evoke a
failure of the backfilling mechanism by creating a reference that later
on causes a D/F conflict. Because the assumption was that git-fetch(1)
would notice the D/F conflict early on this conflicting reference was
created via the reference-transaction hook just when we were about to
write the backfilled tag. As it turns out though this is not the case,
and the fetch fails in the same way when we create the conflicting ref
up front.
Simplify the test setup creating the reference up front, which allows us
to get rid of the hook script.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When fetching with the `--prune` flag we will delete any local
references matching the fetch refspec which have disappeared on the
remote. This step is not currently covered by the `--atomic` flag: we
delete branches even though updating of local references has failed,
which means that the fetch is not an all-or-nothing operation.
Fix this bug by passing in the global transaction into `prune_refs()`:
if one is given, then we'll only queue up deletions and not commit them
right away.
This change also improves performance when pruning many branches in a
repository with a big packed-refs file: every references is pruned in
its own transaction, which means that we potentially have to rewrite
the packed-refs files for every single reference we're about to prune.
The following benchmark demonstrates this: it performs a pruning fetch
from a repository with a single reference into a repository with 100k
references, which causes us to prune all but one reference. This is of
course a very artificial setup, but serves to demonstrate the impact of
only having to write the packed-refs file once:
Benchmark 1: git fetch --prune --atomic +refs/*:refs/* (HEAD~)
Time (mean ± σ): 2.366 s ± 0.021 s [User: 0.858 s, System: 1.508 s]
Range (min … max): 2.328 s … 2.407 s 10 runs
Benchmark 2: git fetch --prune --atomic +refs/*:refs/* (HEAD)
Time (mean ± σ): 1.369 s ± 0.017 s [User: 0.715 s, System: 0.641 s]
Range (min … max): 1.346 s … 1.400 s 10 runs
Summary
'git fetch --prune --atomic +refs/*:refs/* (HEAD)' ran
1.73 ± 0.03 times faster than 'git fetch --prune --atomic +refs/*:refs/* (HEAD~)'
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch: make `--atomic` flag cover backfilling of tags
When fetching references from a remote we by default also fetch all tags
which point into the history we have fetched. This is a separate step
performed after updating local references because it requires us to walk
over the history on the client-side to determine whether the remote has
announced any tags which point to one of the fetched commits.
This backfilling of tags isn't covered by the `--atomic` flag: right
now, it only applies to the step where we update our local references.
This is an oversight at the time the flag was introduced: its purpose is
to either update all references or none, but right now we happily update
local references even in the case where backfilling failed.
Fix this by pulling up creation of the reference transaction such that
we can pass the same transaction to both the code which updates local
references and to the code which backfills tags. This allows us to only
commit the transaction in case both actions succeed.
Note that we also have to start passing the transaction into
`find_non_local_tags()`: this function is responsible for finding all
tags which we need to backfill. Right now, it will happily return tags
which have already been updated with our local references. But when we
use a single transaction for both local references and backfilling then
it may happen that we try to queue the same reference update twice to
the transaction, which consequently triggers a bug. We thus have to skip
over any tags which have already been queued.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs: add interface to iterate over queued transactional updates
There is no way for a caller to see whether a reference update has
already been queued up for a given reference transaction. There are
multiple alternatives to provide this functionality:
- We may add a function that simply tells us whether a specific
reference has already been queued. If implemented naively then
this would potentially be quadratic in runtime behaviour if this
question is asked repeatedly because we have to iterate over all
references every time. The alternative would be to add a hashmap
of all queued reference updates to speed up the lookup, but this
adds overhead to all callers.
- We may add a flag to `ref_transaction_add_update()` that causes it
to skip duplicates, but this has the same runtime concerns as the
first alternative.
- We may add an interface which lets callers collect all updates
which have already been queued such that he can avoid re-adding
them. This is the most flexible approach and puts the burden on
the caller, but also allows us to not impact any of the existing
callsites which don't need this information.
This commit implements the last approach: it allows us to compute the
map of already-queued updates once up front such that we can then skip
all subsequent references which are already part of this map.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the backfilling of tags fails we do not report this error to the
caller, but only report it implicitly at a later point when reporting
updated references. This leaves callers unable to act upon the
information of whether the backfilling succeeded or not.
Refactor the function to return an error code and pass it up the
callstack. This causes us to correctly propagate the error back to the
user of git-fetch(1).
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fetch: control lifecycle of FETCH_HEAD in a single place
There are two different locations where we're appending to FETCH_HEAD:
first when storing updated references, and second when backfilling tags.
Both times we open the file, append to it and then commit it into place,
which is essentially duplicate work.
Improve the lifecycle of updating FETCH_HEAD by opening and committing
it once in `do_fetch()`, where we pass the structure down to the code
which wants to append to it.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The fetch code flow is a bit hard to understand right now:
1. We optionally prune all references which have vanished on the
remote side.
2. We fetch and update all other references locally.
3. We update the upstream branch in the gitconfig.
4. We backfill tags pointing into the history we have just fetched.
It is quite confusing that we fetch objects and update references in
both (2) and (4), which is further stressed by the point that we use a
`skip` goto label to jump from (3) to (4) in case we fail to update the
gitconfig as expected.
Reorder the code to first update all local references, and only after we
have done so update the upstream branch information. This improves the
code flow and furthermore makes it easier to refactor the way we update
references together.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When using git-fetch(1) with the `--atomic` flag the expectation is that
either all of the references are updated, or alternatively none are in
case the fetch fails. While we already have tests for this, we do not
have any tests which exercise atomicity either when pruning deleted refs
or when backfilling tags. This gap in test coverage hides that we indeed
don't handle atomicity correctly for both of these cases.
Add test cases which cover these testing gaps to demonstrate the broken
behaviour. Note that tests are not marked as `test_expect_failure`: this
is done to explicitly demonstrate the current known-wrong behaviour, and
they will be fixed up as soon as we fix the underlying bugs.
While at it this commit also adds another test case which demonstrates
that backfilling of tags does not return an error code in case the
backfill fails. This bug will also be fixed by a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 16 Feb 2022 23:14:30 +0000 (15:14 -0800)]
Merge branch 'js/no-more-legacy-stash'
Removal of unused code and doc.
* js/no-more-legacy-stash:
stash: stop warning about the obsolete `stash.useBuiltin` config setting
stash: remove documentation for `stash.useBuiltin`
add: remove support for `git-legacy-stash`
git-sh-setup: remove remnant bits referring to `git-legacy-stash`
Junio C Hamano [Wed, 16 Feb 2022 23:14:30 +0000 (15:14 -0800)]
Merge branch 'js/diff-filter-negation-fix'
"git diff --diff-filter=aR" is now parsed correctly.
* js/diff-filter-negation-fix:
diff-filter: be more careful when looking for negative bits
diff.c: move the diff filter bits definitions up a bit
docs(diff): lose incorrect claim about `diff-files --diff-filter=A`
Junio C Hamano [Wed, 16 Feb 2022 23:14:30 +0000 (15:14 -0800)]
Merge branch 'en/fetch-negotiation-default-fix'
Interaction between fetch.negotiationAlgorithm and
feature.experimental configuration variables has been corrected.
* en/fetch-negotiation-default-fix:
repo-settings: rename the traditional default fetch.negotiationAlgorithm
repo-settings: fix error handling for unknown values
repo-settings: fix checking for fetch.negotiationAlgorithm=default
Junio C Hamano [Wed, 16 Feb 2022 23:14:29 +0000 (15:14 -0800)]
Merge branch 'en/remerge-diff'
"git log --remerge-diff" shows the difference from mechanical merge
result and the result that is actually recorded in a merge commit.
* en/remerge-diff:
diff-merges: avoid history simplifications when diffing merges
merge-ort: mark conflict/warning messages from inner merges as omittable
show, log: include conflict/warning messages in --remerge-diff headers
diff: add ability to insert additional headers for paths
merge-ort: format messages slightly different for use in headers
merge-ort: mark a few more conflict messages as omittable
merge-ort: capture and print ll-merge warnings in our preferred fashion
ll-merge: make callers responsible for showing warnings
log: clean unneeded objects during `log --remerge-diff`
show, log: provide a --remerge-diff capability
Junio C Hamano [Wed, 16 Feb 2022 23:14:27 +0000 (15:14 -0800)]
Merge branch 'hn/reftable-coverity-fixes'
Problems identified by Coverity in the reftable code have been
corrected.
* hn/reftable-coverity-fixes:
reftable: add print functions to the record types
reftable: make reftable_record a tagged union
reftable: remove outdated file reftable.c
reftable: implement record equality generically
reftable: make reftable-record.h function signatures const correct
reftable: handle null refnames in reftable_ref_record_equal
reftable: drop stray printf in readwrite_test
reftable: order unittests by complexity
reftable: all xxx_free() functions accept NULL arguments
reftable: fix resource warning
reftable: ignore remove() return value in stack_test.c
reftable: check reftable_stack_auto_compact() return value
reftable: fix resource leak blocksource.c
reftable: fix resource leak in block.c error path
reftable: fix OOB stack write in print functions
Junio C Hamano [Sat, 12 Feb 2022 00:56:01 +0000 (16:56 -0800)]
Merge branch 'tg/fetch-prune-exit-code-fix'
When "git fetch --prune" failed to prune the refs it wanted to
prune, the command issued error messages but exited with exit
status 0, which has been corrected.
* tg/fetch-prune-exit-code-fix:
fetch --prune: exit with error if pruning fails
Junio C Hamano [Sat, 12 Feb 2022 00:55:58 +0000 (16:55 -0800)]
Merge branch 'jc/doc-log-messages'
Update the contributor-facing documents on proposed log messages.
* jc/doc-log-messages:
SubmittingPatches: explain why we care about log messages
CodingGuidelines: hint why we value clearly written log messages
SubmittingPatches: write problem statement in the log in the present tense
* ab/no-errno-from-resolve-ref-unsafe:
refs API: remove "failure_errno" from refs_resolve_ref_unsafe()
sequencer: don't use die_errno() on refs_resolve_ref_unsafe() failure
Junio C Hamano [Wed, 9 Feb 2022 22:21:01 +0000 (14:21 -0800)]
Merge branch 'js/sparse-vs-split-index'
Mark in various places in the code that the sparse index and the
split index features are mutually incompatible.
* js/sparse-vs-split-index:
split-index: it really is incompatible with the sparse index
t1091: disable split index
sparse-index: sparse index is disallowed when split index is active
Junio C Hamano [Wed, 9 Feb 2022 22:21:01 +0000 (14:21 -0800)]
Merge branch 'jt/clone-not-quite-empty'
Cloning from a repository that does not yet have any branches or
tags but has other refs resulted in a "remote transport reported
error", which has been corrected.
* jt/clone-not-quite-empty:
clone: support unusual remote ref configurations
Junio C Hamano [Wed, 9 Feb 2022 22:21:00 +0000 (14:21 -0800)]
Merge branch 'jt/sparse-checkout-leading-dir-fix'
"git sparse-checkout init" failed to write into $GIT_DIR/info
directory when the repository was created without one, which has
been corrected to auto-create it.
* jt/sparse-checkout-leading-dir-fix:
sparse-checkout: create leading directory
Junio C Hamano [Wed, 9 Feb 2022 22:21:00 +0000 (14:21 -0800)]
Merge branch 'ab/config-based-hooks-2'
More "config-based hooks".
* ab/config-based-hooks-2:
run-command: remove old run_hook_{le,ve}() hook API
receive-pack: convert push-to-checkout hook to hook.h
read-cache: convert post-index-change to use hook.h
commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
git-p4: use 'git hook' to run hooks
send-email: use 'git hook run' for 'sendemail-validate'
git hook run: add an --ignore-missing flag
hooks: convert worktree 'post-checkout' hook to hook library
hooks: convert non-worktree 'post-checkout' hook to hook library
merge: convert post-merge to use hook.h
am: convert applypatch-msg to use hook.h
rebase: convert pre-rebase to use hook.h
hook API: add a run_hooks_l() wrapper
am: convert {pre,post}-applypatch to use hook.h
gc: use hook library for pre-auto-gc hook
hook API: add a run_hooks() wrapper
hook: add 'run' subcommand
"git fetch --negotiate-only" is an internal command used by "git
push" to figure out which part of our history is missing from the
other side. It should never recurse into submodules even when
fetch.recursesubmodules configuration variable is set, nor it
should trigger "gc". The code has been tightened up to ensure it
only does common ancestry discovery and nothing else.
* gc/fetch-negotiate-only-early-return:
fetch: help translators by reusing the same message template
fetch --negotiate-only: do not update submodules
fetch: skip tasks related to fetching objects
fetch: use goto cleanup in cmd_fetch()
Junio C Hamano [Wed, 9 Feb 2022 22:20:59 +0000 (14:20 -0800)]
Merge branch 'tl/doc-cli-options-first'
We explain that revs come first before the pathspec among command
line arguments, but did not spell out that dashed options come
before other args, which has been corrected.
* tl/doc-cli-options-first:
git-cli.txt: clarify "options first and then args"
The conditional inclusion mechanism of configuration files using
"[includeIf <condition>]" learns to base its decision on the
URL of the remote repository the repository interacts with.
* jt/conditional-config-on-remote-url:
config: include file if remote URL matches a glob
config: make git_config_include() static
completion: handle unusual characters for sparse-checkout
Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
Note there are two tests for this new functionality - one for spaces and
accents and one for backslashes and tabs. The backslashes and tabs test
uses FUNNYNAMES to avoid running on Windows. This is because:
1. Backslashes are explicitly not allowed in Windows file paths.
2. Although tabs appear to be allowed when creating a file in a Windows
bash shell, they actually are not renderable (and appear as empty boxes
in the shell).
Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de> Co-authored-by: Lessley Dennington <lessleydennington@gmail.com> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Lessley Dennington <lessleydennington@gmail.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use new __gitcomp_directories method to complete directory names in cone
mode sparse-checkouts. This method addresses the caveat of poor
performance in monorepos from the previous commit (by completing only one
level of directories).
The unusual character caveat from the previous commit will be fixed by the
final commit in this series.
Correct multiple issues with tab completion of the git sparse-checkout
command. These issues were:
1. git sparse-checkout <TAB> previously resulted in an incomplete list of
subcommands (it was missing reapply and add).
2. Subcommand options were not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, cone mode sparse checkouts should complete
only directory names.
Note that while the new strategy of just using git ls-tree to complete on
directory names is simple and a step in the right direction, it does have
some caveats. These are:
1. Likelihood of poor performance in large monorepos (as a result of
recursively completing directory names).
2. Inability to handle paths containing unusual characters.
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington <lessleydennington@gmail.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Sat, 5 Feb 2022 17:42:32 +0000 (09:42 -0800)]
Merge branch 'ms/update-index-racy'
"git update-index --refresh" has been taught to deal better with
racy timestamps (just like "git status" already does).
* ms/update-index-racy:
update-index: refresh should rewrite index in case of racy timestamps
t7508: add tests capturing racy timestamp handling
t7508: fix bogus mtime verification
test-lib: introduce API for verifying file mtime
Junio C Hamano [Sat, 5 Feb 2022 17:42:31 +0000 (09:42 -0800)]
Merge branch 'ab/cat-file'
Assorted updates to "git cat-file", especially "-h".
* ab/cat-file:
cat-file: s/_/-/ in typo'd usage_msg_optf() message
cat-file: don't whitespace-pad "(...)" in SYNOPSIS and usage output
cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters)
object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY
cat-file: correct and improve usage information
cat-file: fix remaining usage bugs
cat-file: make --batch-all-objects a CMDMODE
cat-file: move "usage" variable to cmd_cat_file()
cat-file docs: fix SYNOPSIS and "-h" output
parse-options API: add a usage_msg_optf()
cat-file tests: test messaging on bad objects/paths
cat-file tests: test bad usage
Junio C Hamano [Sat, 5 Feb 2022 17:42:31 +0000 (09:42 -0800)]
Merge branch 'jc/qsort-s-alignment-fix'
Fix a hand-rolled alloca() imitation that may have violated
alignment requirement of data being sorted in compatibility
implementation of qsort_s() and stable qsort().
* jc/qsort-s-alignment-fix:
stable-qsort: avoid using potentially unaligned access
compat/qsort_s.c: avoid using potentially unaligned access
Junio C Hamano [Sat, 5 Feb 2022 17:42:30 +0000 (09:42 -0800)]
Merge branch 'rs/apply-symlinks-use-strset'
"git apply" (ab)used the util pointer of the string-list to keep
track of how each symbolic link needs to be handled, which has been
simplified by using strset.
* rs/apply-symlinks-use-strset:
apply: use strsets to track symlinks
Junio C Hamano [Sat, 5 Feb 2022 17:42:30 +0000 (09:42 -0800)]
Merge branch 'rs/grep-expr-cleanup'
Code clean-up.
* rs/grep-expr-cleanup:
grep: use grep_and_expr() in compile_pattern_and()
grep: extract grep_binexp() from grep_or_expr()
grep: use grep_not_expr() in compile_pattern_not()
grep: use grep_or_expr() in compile_pattern_or()
* jh/p4-spawning-external-commands-cleanup:
git-p4: don't print shell commands as python lists
git-p4: pass command arguments as lists instead of using shell
git-p4: don't select shell mode using the type of the command argument
Junio C Hamano [Sat, 5 Feb 2022 17:42:28 +0000 (09:42 -0800)]
Merge branch 'pb/pull-rebase-autostash-fix'
"git pull --rebase" ignored the rebase.autostash configuration
variable when the remote history is a descendant of our history,
which has been corrected.
* pb/pull-rebase-autostash-fix:
pull --rebase: honor rebase.autostash when fast-forwarding
Shaoxuan Yuan [Wed, 2 Feb 2022 07:28:44 +0000 (15:28 +0800)]
builtin/diff.c: fix "git-diff" usage string typo
Remove mistaken right square brackets from "git-diff"
usage string. Make the usage string conform to "git-diff"
documentation (Documentation/git-diff.txt).
Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:37 +0000 (02:37 +0000)]
diff-merges: avoid history simplifications when diffing merges
Doing diffs for merges are special; they should typically avoid history
simplification. For example, with
git log --diff-merges=first-parent -- path
the default history simplification would remove merge commits from
consideration if the file "path" matched the second parent. That is
counter to what the user wants when looking for first-parent diffs.
Similar comments can be made for --diff-merges=separate (which diffs
against both parents) and --diff-merges=remerge (which diffs against a
remerge of the merge commit).
However, history simplification still makes sense if not doing diffing
merges, and it also makes sense for the combined and dense-combined
forms of diffing merges (because both of those are defined to only show
a diff when the merge result at the relevant paths differs from *both*
parents).
So, for separate, first-parent, and remerge styles of diff-merges, turn
off history simplification.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:36 +0000 (02:37 +0000)]
merge-ort: mark conflict/warning messages from inner merges as omittable
A recursive merge involves merging the merge bases of the two branches
being merged. Such an inner merge can itself generate conflict notices.
While such notices may be useful when initially trying to create a
merge, they seem to just be noise when investigating merges later with
--remerge-diff. (Especially when both sides of the outer merge resolved
the conflict the same way leading to no overall conflict.) Remove them.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:35 +0000 (02:37 +0000)]
show, log: include conflict/warning messages in --remerge-diff headers
Conflicts such as modify/delete, rename/rename, or file/directory are
not representable via content conflict markers, and the normal output
messages notifying users about these were dropped with --remerge-diff.
While we don't want these messages randomly shown before the commit
and diff headers, we do want them to still be shown; include them as
part of the diff headers instead.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:34 +0000 (02:37 +0000)]
diff: add ability to insert additional headers for paths
When additional headers are provided, we need to
* add diff_filepairs to diff_queued_diff for each paths in the
additional headers map which, unless that path is part of
another diff_filepair already found in diff_queued_diff
* format the headers (colorization, line_prefix for --graph)
* make sure the various codepaths that attempt to return early
if there are "no changes" take into account the headers that
need to be shown.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:33 +0000 (02:37 +0000)]
merge-ort: format messages slightly different for use in headers
When users run
git show --remerge-diff $MERGE_COMMIT
or
git log -p --remerge-diff ...
stdout is not an appropriate location to dump conflict messages, but we
do want to provide them to users. We will include them in the diff
headers instead...but for that to work, we need for any multiline
messages to replace newlines with both a newline and a space. Add a new
flag to signal when we want these messages modified in such a fashion,
and use it in path_msg() to modify these messages this way. Also, allow
a special prefix to be specified for these headers.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:32 +0000 (02:37 +0000)]
merge-ort: mark a few more conflict messages as omittable
path_msg() has the ability to mark messages as omittable, designed for
remerge-diff where we'll instead be showing conflict messages as diff
headers for a subsequent diff. While all these messages are very useful
when trying to create a merge initially, early use with the
--remerge-diff feature (the only user of this omittable conflict message
capability), suggests that the particular messages marked in this commit
are just noise when trying to see what changes users made to create a
merge commit. Mark them as omittable.
Note that there were already a few messages marked as omittable in
merge-ort when doing a remerge-diff, because the development of
--remerge-diff preceded the upstreaming of merge-ort and I was trying to
ensure merge-ort could handle all the necessary requirements. See
commit c5a6f65527 ("merge-ort: add modify/delete handling and delayed
output processing", 2020-12-03) for the initial details. For some
examples of already-marked-as-omittable messages, see either
"Auto-merging <path>" or some of the submodule update hints. This
commit just adds two more messages that should also be omittable.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:31 +0000 (02:37 +0000)]
merge-ort: capture and print ll-merge warnings in our preferred fashion
Instead of immediately printing ll-merge warnings to stderr, we save
them in our output strbuf. Besides allowing us to move these warnings
to a special file for --remerge-diff, this has two other benefits for
regular merges done by merge-ort:
* The deferral of messages ensures we can print all messages about
any given path together (merge-recursive was known to sometimes
intersperse messages about other paths, particularly when renames
were involved).
* The deferral of messages means we can avoid printing spurious
conflict messages when we just end up aborting due to local user
modifications in the way. (In contrast to merge-recursive.c which
prematurely checks for local modifications in the way via
unpack_trees() and gets the check wrong both in terms of false
positives and false negatives relative to renames, merge-ort does
not perform the local modifications in the way check until the
checkout() step after the full merge has been computed.)
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:30 +0000 (02:37 +0000)]
ll-merge: make callers responsible for showing warnings
Since some callers may want to send warning messages to somewhere other
than stdout/stderr, stop printing "warning: Cannot merge binary files"
from ll-merge and instead modify the return status of ll_merge() to
indicate when a merge of binary files has occurred. Message printing
probably does not belong in a "low-level merge" anyway.
This commit continues printing the message as-is, just from the callers
instead of within ll_merge(). Future changes will start handling the
message differently in the merge-ort codepath.
There was one special case here: the callers in rerere.c do NOT check
for and print such a message; since those code paths explicitly skip
over binary files, there is no reason to check for a return status of
LL_MERGE_BINARY_CONFLICT or print the related message.
Note that my methodology included first modifying ll_merge() to return
a struct, so that the compiler would catch all the callers for me and
ensure I had modified all of them. After modifying all of them, I then
changed the struct to an enum.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:29 +0000 (02:37 +0000)]
log: clean unneeded objects during `log --remerge-diff`
The --remerge-diff option will need to create new blobs and trees
representing the "automatic merge" state. If one is traversing a
long project history, one can easily get hundreds of thousands of
loose objects generated during `log --remerge-diff`. However, none of
those loose objects are needed after we have completed our diff
operation; they can be summarily deleted.
Add a new helper function to tmp_objdir to discard all the contained
objects, and call it after each merge is handled.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 02:37:28 +0000 (02:37 +0000)]
show, log: provide a --remerge-diff capability
When this option is specified, we remerge all (two parent) merge commits
and diff the actual merge commit to the automatically created version,
in order to show how users removed conflict markers, resolved the
different conflict versions, and potentially added new changes outside
of conflict regions in order to resolve semantic merge problems (or,
possibly, just to hide other random changes).
This capability works by creating a temporary object directory and
marking it as the primary object store. This makes it so that any blobs
or trees created during the automatic merge are easily removable
afterwards by just deleting all objects from the temporary object
directory.
There are a few ways that this implementation is suboptimal:
* `log --remerge-diff` becomes slow, because the temporary object
directory can fill with many loose objects while running
* the log output can be muddied with misplaced "warning: cannot merge
binary files" messages, since ll-merge.c unconditionally writes those
messages to stderr while running instead of allowing callers to
manage them.
* important conflict and warning messages are simply dropped; thus for
conflicts like modify/delete or rename/rename or file/directory which
are not representable with content conflict markers, there may be no
way for a user of --remerge-diff to know that there had been a
conflict which was resolved (and which possibly motivated other
changes in the merge commit).
* when fixing the previous issue, note that some unimportant conflict
and warning messages might start being included. We should instead
make sure these remain dropped.
Subsequent commits will address these issues.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 03:42:40 +0000 (03:42 +0000)]
repo-settings: rename the traditional default fetch.negotiationAlgorithm
Give the traditional default fetch.negotiationAlgorithm the name
'consecutive'. Also allow a choice of 'default' to have Git decide
between the choices (currently, picking 'skipping' if
feature.experimental is true and 'consecutive' otherwise). Update the
documentation accordingly.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 03:42:39 +0000 (03:42 +0000)]
repo-settings: fix error handling for unknown values
In commit af3a67de01 ("negotiator: unknown fetch.negotiationAlgorithm
should error out", 2018-08-01), error handling for an unknown
fetch.negotiationAlgorithm was added with the code die()ing. This was
also added to the documentation for the fetch.negotiationAlgorithm
option, to make it explicit that the code would die on unknown values.
This behavior was lost with commit aaf633c2ad ("repo-settings: create
feature.experimental setting", 2019-08-13). Restore it so that the
behavior again matches the documentation.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Wed, 2 Feb 2022 03:42:38 +0000 (03:42 +0000)]
repo-settings: fix checking for fetch.negotiationAlgorithm=default
In commit 3050b6dfc75d (repo-settings.c: simplify the setup,
2021-09-21), the branch for handling fetch.negotiationAlgorithm=default
was deleted. Since this value is documented in
Documentation/config/fetch.txt, restore the check for this value.
Note that this change caused an observable bug: if someone sets
feature.experimental=true in config, and then passes "-c
fetch.negotiationAlgorithm=default" on the command line in an attempt to
override the config, then the override is ignored. Fix the bug by not
ignoring the value of "default".
Technically, before commit 3050b6dfc75d, repo-settings would treat any
fetch.negotiationAlgorithm value other than "skipping" or "noop" as a
request for "default", but I think it probably makes more sense to
ignore such broken requests and leave fetch.negotiationAlgorithm with
the default value rather than the value of "default". (If that sounds
confusing, note that "default" is usually the default value, but when
feature.experimental=true, "skipping" is the default value.)
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
perl Git.pm: don't ignore signalled failure in _cmd_close()
Fix misbehavior in Git.pm that dates back to the very first version of
the library in git.git added in b1edc53d062 (Introduce Git.pm (v4),
2006-06-24). When we fail to execute a command we shouldn't ignore all
signals, those can happen e.g. if abort() is called, or if the command
segfaults.
Because of this we'd consider e.g. a command that died due to LSAN
exiting with abort() successful, as is the case with the tests listed
as running successfully with SANITIZE=leak in 9081a421a6d (checkout:
fix "branch info" memory leaks, 2021-11-16). We did run them
successfully, but only because we ignored these errors.
This was then made worse by the use of "abort_on_error=1" for LSAN
added in 85b81b35ff9 (test-lib: set LSAN_OPTIONS to abort by default,
2017-09-05). Doing that makes sense, but without providing that option
we'd have a "$? >> 8" of "23" on failure, with abort_on_error=1 we'll
get "0".
All of our tests pass even without the SIGPIPE exception being added
here, but as the code appears to have been trying to ignore it let's
keep ignoring it for now.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thomas Gummerer [Mon, 31 Jan 2022 13:30:47 +0000 (13:30 +0000)]
fetch --prune: exit with error if pruning fails
When pruning refs fails, we print an error to stderr, but still
exit 0 from 'git fetch'. Since this is a genuine error, fetch
should be exiting with some non-zero exit code. Make it so.
The --prune option was introduced in f360d844de ("builtin-fetch: add
--prune option", 2009-11-10). Unfortunately it's unclear from that
commit whether ignoring the exit code was an oversight or
intentional, but it feels like an oversight.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Robert Coup [Fri, 28 Jan 2022 14:36:02 +0000 (14:36 +0000)]
fetch: fix negotiate-only error message
The error message when invoking a negotiate-only fetch without providing
any tips incorrectly refers to a --negotiate-tip=* argument. Fix this to
use the actual argument, --negotiation-tip=*.
Signed-off-by: Robert Coup <robert@coup.net.nz> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 28 Jan 2022 01:58:18 +0000 (01:58 +0000)]
sparse-checkout: fix a couple minor memory leaks
These were introduced in commit 55dfcf9591 ("sparse-checkout: clear
tracked sparse dirs", 2021-09-08) and missed in my review at the time.
Plug the leaks.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff-filter: be more careful when looking for negative bits
The `--diff-filter=<bits>` option allows to filter the diff by certain
criteria, for example `R` to only show renamed files. It also supports
negating a filter via a down-cased letter, i.e. `r` to show _everything
but_ renamed files.
However, the code is a bit overzealous when trying to figure out whether
`git diff` should start with all diff-filters turned on because the user
provided a lower-case letter: if the `--diff-filter` argument starts
with an upper-case letter, we must not start with all bits turned on.
Even worse, it is possible to specify the diff filters in multiple,
separate options, e.g. `--diff-filter=AM [...] --diff-filter=m`.
Let's accumulate the include/exclude filters independently, and only
special-case the "only exclude filters were specified" case after
parsing the options altogether.
Note: The code replaced by this commit took pains to avoid setting any
unused bits of `options->filter`. That was unnecessary, though, as all
accesses happen via the `filter_bit_tst()` function using specific bits,
and setting the unused bits has no effect. Therefore, we can simplify
the code by using `~0` (or in this instance, `~<unwanted-bit>`).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
docs(diff): lose incorrect claim about `diff-files --diff-filter=A`
Originally, before we had `--intent-to-add`, there was no way that `git
diff-files` could see added files: if a file did not exist in the index,
`git diff-files` would not show it because it looks only at worktree
files when there is an index entry at the same path.
We used this example in the documentation of the diff options to explain
that not every `--diff-filter=<option>` has an effect in all scenarios.
Even when we added `--intent-to-add`, the comment was still correct,
because initially we showed such files as modified instead of added.
However, when that bug was fixed in feea6946a5b (diff-files: treat
"i-t-a" files as "not-in-index", 2020-06-20), the comment in the
documentation became incorrect.
Let's just remove it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
stash: remove documentation for `stash.useBuiltin`
In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03),
we removed the setting, and for a couple of major versions, we still
documented the setting, telling users that it is gone.
We can now safely remove even the documentation.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 90a6bb98d11 (legacy stash -p: respect the add.interactive.usebuiltin
setting, 2019-12-21), we added support to use the built-in `add -p` from
the scripted `stash -p`.
In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03),
we retired the scripted `stash` (including the scripted `stash -p`).
Therefore this support is no longer necessary.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-sh-setup: remove remnant bits referring to `git-legacy-stash`
In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03),
we removed `git-legacy-stash.sh`. But `git-sh-setup.sh` somehow still
thinks about it. Let's just not.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 27 Jan 2022 19:02:57 +0000 (11:02 -0800)]
SubmittingPatches: write problem statement in the log in the present tense
We give a guidance for proposed log message to write problem
statement first, followed by the reasoning behind, and recipe for,
the solution. Clarify that we describe the situation _before_ the
proposed patch is applied in the present tense (not in the past
tense e.g. "we used to do X, but thanks to this commit we now do Y")
for consistency.
Taylor Blau [Tue, 25 Jan 2022 22:41:20 +0000 (17:41 -0500)]
pack-bitmap.c: gracefully fallback after opening pack/MIDX
When opening a MIDX/pack-bitmap, we call open_midx_bitmap_1() or
open_pack_bitmap_1() respectively in a loop over the set of MIDXs/packs.
By design, these functions are supposed to be called over every pack and
MIDX, since only one of them should have a valid bitmap.
Ordinarily we return '0' from these two functions in order to indicate
that we successfully loaded a bitmap To signal that we couldn't load a
bitmap corresponding to the MIDX/pack (either because one doesn't exist,
or because there was an error with loading it), we can return '-1'. In
either case, the callers each enumerate all MIDXs/packs to ensure that
at most one bitmap per-kind is present.
But when we fail to load a bitmap that does exist (for example, loading
a MIDX bitmap without finding a corresponding reverse index), we'll
return -1 but leave the 'midx' field non-NULL. So when we fallback to
loading a pack bitmap, we'll complain that the bitmap we're trying to
populate already is "opened", even though it isn't.
Rectify this by setting the '->pack' and '->midx' field back to NULL as
appropriate. Two tests are added: one to ensure that the MIDX-to-pack
bitmap fallback works, and another to ensure we still complain when
there are multiple pack bitmaps in a repository.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Jan 2022 22:41:17 +0000 (17:41 -0500)]
midx: read `RIDX` chunk when present
When a MIDX contains the new `RIDX` chunk, ensure that the reverse index
is read from it instead of the on-disk .rev file. Since we need to
encode the object order in the MIDX itself for correctness reasons,
there is no point in storing the same data again outside of the MIDX.
So, this patch stops writing separate .rev files, and reads it out of
the MIDX itself. This is possible to do with relatively little new code,
since the format of the RIDX chunk is identical to the data in the .rev
file. In other words, we can implement this by pointing the
`revindex_data` field at the reverse index chunk of the MIDX instead of
the .rev file without any other changes.
Note that we have two knobs that are adjusted for the new tests:
GIT_TEST_MIDX_WRITE_REV and GIT_TEST_MIDX_READ_RIDX. The former controls
whether the MIDX .rev is written at all, and the latter controls whether
we read the MIDX's RIDX chunk.
Both are necessary to ensure that the test added at the beginning of
this series continues to work. This is because we always need to write
the RIDX chunk in the MIDX in order to change its checksum, but we want
to make sure reading the existing .rev file still works (since the RIDX
chunk takes precedence by default).
Arguably this isn't a very interesting mode to test, because the
precedence rules mean that we'll always read the RIDX chunk over the
.rev file. But it makes it impossible for a user to induce corruption in
their repository by adjusting the test knobs (since if we had an
either/or knob they could stop writing the RIDX chunk, allowing them to
tweak the MIDX's object order without changing its checksum).
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Jan 2022 22:41:15 +0000 (17:41 -0500)]
t/lib-bitmap.sh: parameterize tests over reverse index source
To prepare for reading the reverse index data out of the MIDX itself,
teach the `test_rev_exists` function to take an expected "source" for
the reverse index data.
When given "rev", it asserts that the MIDX's `.rev` file exists, and is
loaded when verifying the integrity of its bitmaps. Otherwise, it
ensures that trace2 reports the source of the reverse index data as the
same string which was given to test_rev_exists().
The following patch will implement reading the reverse index data from
the MIDX itself.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Jan 2022 22:41:13 +0000 (17:41 -0500)]
t5326: move tests to t/lib-bitmap.sh
In t5326, we have a handful of tests that we would like to run twice:
once using the MIDX's new `RIDX` chunk as the source of the
reverse-index cache, and once using the separate `.rev` file.
But because these tests mutate the state of the underlying repository,
and then make assumptions about those mutations occurring in a certain
sequence, simply running the tests twice in the same repository is
awkward.
Instead, extract the core of interesting tests into t/lib-bitmap.sh to
prepare for them to be run twice, each in a separate test script. This
means that they can each operate on a separate repository, removing any
concerns about mutating state.
For now, this patch is a strict cut-and-paste of some tests from t5326.
The tests which did not move are not interesting with respect to the
source of their reverse index data.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Jan 2022 22:41:10 +0000 (17:41 -0500)]
t5326: extract `test_rev_exists`
To determine which source of data is used for the MIDX's reverse index
cache, introduce a helper which forces loading the reverse index, and
then looks for the special trace2 event introduced in a previous commit.
For now, this helper just looks for when the legacy MIDX .rev file was
loaded, but in a subsequent commit will become parameterized over the
the reverse index's source.
This function replaces checking for the existence of the .rev file. We
could write a similar helper to ensure that the .rev file is cleaned up
after repacking, but it will make subsequent tests more difficult to
write, and provides marginal value since we already check that the MIDX
.bitmap file is removed.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Jan 2022 22:41:08 +0000 (17:41 -0500)]
t5326: drop unnecessary setup
The core.multiPackIndex config became true by default back in 18e449f86b
(midx: enable core.multiPackIndex by default, 2020-09-25), so it is no
longer necessary to enable it explicitly.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Jan 2022 22:41:05 +0000 (17:41 -0500)]
pack-revindex.c: instrument loading on-disk reverse index
In a subsequent commit, we'll use the MIDX's new 'RIDX' chunk as a
source for the reverse index's data. But it will be useful for tests to
be able to determine whether the reverse index was loaded from the
separate .rev file, or from a chunk within the MIDX.
To instrument this, add a trace2 event which the tests can look for in
order to determine the reverse index's source.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
