"git rebase" has learned to use the merge backend (i.e. the
machinery that drives "rebase -i") by default, while allowing
"--apply" option to use the "apply" backend (e.g. the moral
equivalent of "format-patch piped to am"). The rebase.backend
configuration variable can be set to customize.
* en/rebase-backend:
rebase: rename the two primary rebase backends
rebase: change the default backend from "am" to "merge"
rebase: make the backend configurable via config setting
rebase tests: repeat some tests using the merge backend instead of am
rebase tests: mark tests specific to the am-backend with --am
rebase: drop '-i' from the reflog for interactive-based rebases
git-prompt: change the prompt for interactive-based rebases
rebase: add an --am option
rebase: move incompatibility checks between backend options a bit earlier
git-rebase.txt: add more details about behavioral differences of backends
rebase: allow more types of rebases to fast-forward
t3432: make these tests work with either am or merge backends
rebase: fix handling of restrict_revision
rebase: make sure to pass along the quiet flag to the sequencer
rebase, sequencer: remove the broken GIT_QUIET handling
t3406: simplify an already simple test
rebase (interactive-backend): fix handling of commits that become empty
rebase (interactive-backend): make --keep-empty the default
t3404: directly test the behavior of interest
git-rebase.txt: update description of --allow-empty-message
"git check-ignore" did not work when the given path is explicitly
marked as not ignored with a negative entry in the .gitignore file.
* en/check-ignore:
check-ignore: fix documentation and implementation to match
The object reachability bitmap machinery and the partial cloning
machinery were not prepared to work well together, because some
object-filtering criteria that partial clones use inherently rely
on object traversal, but the bitmap machinery is an optimization
to bypass that object traversal. There however are some cases
where they can work together, and they were taught about them.
* jk/object-filter-with-bitmap:
rev-list --count: comment on the use of count_right++
pack-objects: support filters with bitmaps
pack-bitmap: implement BLOB_LIMIT filtering
pack-bitmap: implement BLOB_NONE filtering
bitmap: add bitmap_unset() function
rev-list: use bitmap filters for traversal
pack-bitmap: basic noop bitmap filter infrastructure
rev-list: allow commit-only bitmap traversals
t5310: factor out bitmap traversal comparison
rev-list: allow bitmaps when counting objects
rev-list: make --count work with --objects
rev-list: factor out bitmap-optimized routines
pack-bitmap: refuse to do a bitmap traversal with pathspecs
rev-list: fallback to non-bitmap traversal when filtering
pack-bitmap: fix leak of haves/wants object lists
pack-bitmap: factor out type iterator initialization
"git remote rename X Y" needs to adjust configuration variables
(e.g. branch.<name>.remote) whose value used to be X to Y.
branch.<name>.pushRemote is now also updated.
* bw/remote-rename-update-config:
remote rename/remove: gently handle remote.pushDefault config
config: provide access to the current line number
remote rename/remove: handle branch.<name>.pushRemote config values
remote: clean-up config callback
remote: clean-up by returning early to avoid one indentation
pull --rebase/remote rename: document and honor single-letter abbreviations rebase types
check-ignore has two different modes, and neither of these modes has an
implementation that matches the documentation. These modes differ in
whether they just print paths or whether they also print the final
pattern matched by the path. The fix is different for both modes, so
I'll discuss both separately.
=== First (default) mode ===
The first mode is documented as:
For each pathname given via the command-line or from a file via
--stdin, check whether the file is excluded by .gitignore (or other
input files to the exclude mechanism) and output the path if it is
excluded.
However, it fails to do this because it did not account for negated
patterns. Commands other than check-ignore verify exclusion rules via
calling
... -> treat_one_path() -> is_excluded() -> last_matching_pattern()
while check-ignore has a call path of the form:
... -> check_ignore() -> last_matching_pattern()
The fact that the latter does not include the call to is_excluded()
means that it is susceptible to to messing up negated patterns (since
that is the only significant thing is_excluded() adds over
last_matching_pattern()). Unfortunately, we can't make it just call
is_excluded(), because the same codepath is used by the verbose mode
which needs to know the matched pattern in question. This brings us
to...
=== Second (verbose) mode ===
The second mode, known as verbose mode, references the first in the
documentation and says:
Also output details about the matching pattern (if any) for each
given pathname. For precedence rules within and between exclude
sources, see gitignore(5).
The "Also" means it will print patterns that match the exclude rules as
noted for the first mode, and also print which pattern matches. Unless
more information is printed than just pathname and pattern (which is not
done), this definition is somewhat ill-defined and perhaps even
self-contradictory for negated patterns: A path which matches a negated
exclude pattern is NOT excluded and thus shouldn't be printed by the
former logic, while it certainly does match one of the explicit patterns
and thus should be printed by the latter logic.
=== Resolution ==
Since the second mode exists to find out which pattern matches given
paths, and showing the user a pattern that begins with a '!' is
sufficient for them to figure out whether the pattern is excluded, the
existing behavior is desirable -- we just need to update the
documentation to match the implementation (i.e. it is about printing
which pattern is matched by paths, not about showing which paths are
excluded).
For the first or default mode, users just want to know whether a pattern
is excluded. As such, the existing documentation is desirable; change
the implementation to match the documented behavior.
Finally, also adjust a few tests in t0008 that were caught up by this
discrepancy in how negated paths were handled.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up.
* jk/mailinfo-cleanup:
mailinfo: factor out some repeated header handling
mailinfo: be more liberal with header whitespace
mailinfo: simplify parsing of header values
mailinfo: treat header values as C strings
"git config" learned to show in which "scope", in addition to in
which file, each config setting comes from.
* mr/show-config-scope:
config: add '--show-scope' to print the scope of a config value
submodule-config: add subomdule config scope
config: teach git_config_source to remember its scope
config: preserve scope in do_git_config_sequence
config: clarify meaning of command line scoping
config: split repo scope to local and worktree
config: make scope_name non-static and rename it
t1300: create custom config file without special characters
t1300: fix over-indented HERE-DOCs
config: fix typo in variable name
Preparation for SHA-256 migration continues.
* bc/hash-independent-tests-part-8: (21 commits)
t6024: update for SHA-256
t6006: make hash size independent
t6000: abstract away SHA-1-specific constants
t5703: make test work with SHA-256
t5607: make hash size independent
t5318: update for SHA-256
t5515: make test hash independent
t5321: make test hash independent
t5313: make test hash independent
t5309: make test hash independent
t5302: make hash size independent
t4060: make test work with SHA-256
t4211: add test cases for SHA-256
t4211: move SHA-1-specific test cases into a directory
t4013: make test hash independent
t3311: make test work with SHA-256
t3310: make test work with SHA-256
t3309: make test work with SHA-256
t3308: make test work with SHA-256
t3206: make hash size independent
...
Two related changes, with separate rationale for each:
Rename the 'interactive' backend to 'merge' because:
* 'interactive' as a name caused confusion; this backend has been used
for many kinds of non-interactive rebases, and will probably be used
in the future for more non-interactive rebases than interactive ones
given that we are making it the default.
* 'interactive' is not the underlying strategy; merging is.
* the directory where state is stored is not called
.git/rebase-interactive but .git/rebase-merge.
Rename the 'am' backend to 'apply' because:
* Few users are familiar with git-am as a reference point.
* Related to the above, the name 'am' makes sentences in the
documentation harder for users to read and comprehend (they may read
it as the verb from "I am"); avoiding this difficult places a large
burden on anyone writing documentation about this backend to be very
careful with quoting and sentence structure and often forces
annoying redundancy to try to avoid such problems.
* Users stumble over pronunciation ("am" as in "I am a person not a
backend" or "am" as in "the first and thirteenth letters in the
alphabet in order are "A-M"); this may drive confusion when one user
tries to explain to another what they are doing.
* While "am" is the tool driving this backend, the tool driving git-am
is git-apply, and since we are driving towards lower-level tools
for the naming of the merge backend we may as well do so here too.
* The directory where state is stored has never been called
.git/rebase-am, it was always called .git/rebase-apply.
For all the reasons listed above:
* Modify the documentation to refer to the backends with the new names
* Provide a brief note in the documentation connecting the new names
to the old names in case users run across the old names anywhere
(e.g. in old release notes or older versions of the documentation)
* Change the (new) --am command line flag to --apply
* Rename some enums, variables, and functions to reinforce the new
backend names for us as well.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order to ensure the merge/interactive backend gets similar coverage
to the am one, add some tests for cases where previously only the am
backend was tested.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have many rebase tests in the testsuite, and often the same test is
repeated multiple times just testing different backends. For those
tests that were specifically trying to test the am backend, add the --am
flag.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A large variety of rebase types are supported by the interactive
machinery, not just the explicitly interactive ones. These all share
the same code and write the same reflog messages, but the "-i" moniker
in those messages doesn't really have much meaning. It also becomes
somewhat distracting once we switch the default from the am-backend to
the interactive one. Just remove the "-i" from these messages.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the past, we had different prompts for different types of rebases:
REBASE: for am-based rebases
REBASE-m: for merge-based rebases
REBASE-i: for interactive-based rebases
It's not clear why this distinction was necessary or helpful; when the
prompt was added in commit e75201963f ("Improve bash prompt to detect
various states like an unfinished merge", 2007-09-30), it simply added
these three different types. Perhaps there was a useful purpose back
then, but there have been some changes:
* The merge backend was deleted after being implemented on top of the
interactive backend, causing the prompt for merge-based rebases to
change from REBASE-m to REBASE-i.
* The interactive backend is used for multiple different types of
non-interactive rebases, so the "-i" part of the prompt doesn't
really mean what it used to.
* Rebase backends have gained more abilities and have a great deal of
overlap, sometimes making it hard to distinguish them.
* Behavioral differences between the backends have also been ironed
out.
* We want to change the default backend from am to interactive, which
means people would get "REBASE-i" by default if we didn't change
the prompt, and only if they specified --am or --whitespace or -C
would they get the "REBASE" prompt.
* In the future, we plan to have "--whitespace", "-C", and even "--am"
run the interactive backend once it can handle everything the
am-backend can.
For all these reasons, make the prompt for any type of rebase just be
"REBASE".
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the past, we dis-allowed rebases using the interactive backend from
performing a fast-forward to short-circuit the rebase operation. This
made sense for explicitly interactive rebases and some implicitly
interactive rebases, but certainly became overly stringent when the
merge backend was re-implemented via the interactive backend.
Just as the am-based rebase has always had to disable the fast-forward
based on a variety of conditions or flags (e.g. --signoff, --whitespace,
etc.), we need to do the same but now with a few more options. However,
continuing to use REBASE_FORCE for tracking this is problematic because
the interactive backend used it for a different purpose. (When
REBASE_FORCE wasn't set, the interactive backend would not fast-forward
the whole series but would fast-forward individual "pick" commits at the
beginning of the todo list, and then a squash or something would cause
it to start generating new commits.) So, introduce a new
allow_preemptive_ff flag contained within cmd_rebase() and use it to
track whether we are going to allow a pre-emptive fast-forward that
short-circuits the whole rebase.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t3432 had several stress tests for can_fast_forward(), whose intent was
to ensure we were using the optimization of just fast forwarding when
possible. However, these tests verified that fast forwards had happened
based on the output that rebase printed to the terminal. We can instead
test more directly that we actually fast-forwarded by checking the
reflog, which also has the side effect of making the tests applicable
for the merge/interactive backend.
This change does lose the distinction between "noop" and "noop-force",
but as stated in commit c9efc21683 ("t3432: test for --no-ff's
interaction with fast-forward", 2019-08-27) which introduced that
distinction: "These tests aren't supposed to endorse the status quo,
just test for what we're currently doing.".
This change does not actually run these tests with the merge/interactive
backend; instead this is just a preparatory commit. A subsequent commit
which fixes can_fast_forward() to work with that backend will then also
change t3432 to add tests of that backend as well.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
restrict_revision in the original shell script was an excluded revision
range. It is also treated that way by the am-backend. In the
conversion from shell to C (see commit 6ab54d17be ("rebase -i:
implement the logic to initialize $revisions in C", 2018-08-28)), the
interactive-backend accidentally treated it as a positive revision
rather than a negated one.
This was missed as there were no tests in the testsuite that tested an
interactive rebase with fork-point behavior.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the merge backend was re-implemented on top of the interactive
backend, the output of rebase --merge changed a little. This change
allowed this test to be simplified, though it wasn't noticed until now.
Simplify the testcase a little.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As established in the previous commit and commit b00bf1c9a8
(git-rebase: make --allow-empty-message the default, 2018-06-27), the
behavior for rebase with different backends in various edge or corner
cases is often more happenstance than design. This commit addresses
another such corner case: commits which "become empty".
A careful reader may note that there are two types of commits which would
become empty due to a rebase:
* [clean cherry-pick] Commits which are clean cherry-picks of upstream
commits, as determined by `git log --cherry-mark ...`. Re-applying
these commits would result in an empty set of changes and a
duplicative commit message; i.e. these are commits that have
"already been applied" upstream.
* [become empty] Commits which are not empty to start, are not clean
cherry-picks of upstream commits, but which still become empty after
being rebased. This happens e.g. when a commit has changes which
are a strict subset of the changes in an upstream commit, or when
the changes of a commit can be found spread across or among several
upstream commits.
Clearly, in both cases the changes in the commit in question are found
upstream already, but the commit message may not be in the latter case.
When cherry-mark can determine a commit is already upstream, then
because of how cherry-mark works this means the upstream commit message
was about the *exact* same set of changes. Thus, the commit messages
can be assumed to be fully interchangeable (and are in fact likely to be
completely identical). As such, the clean cherry-pick case represents a
case when there is no information to be gained by keeping the extra
commit around. All rebase types have always dropped these commits, and
no one to my knowledge has ever requested that we do otherwise.
For many of the become empty cases (and likely even most), we will also
be able to drop the commit without loss of information -- but this isn't
quite always the case. Since these commits represent cases that were
not clean cherry-picks, there is no upstream commit message explaining
the same set of changes. Projects with good commit message hygiene will
likely have the explanation from our commit message contained within or
spread among the relevant upstream commits, but not all projects run
that way. As such, the commit message of the commit being rebased may
have reasoning that suggests additional changes that should be made to
adapt to the new base, or it may have information that someone wants to
add as a note to another commit, or perhaps someone even wants to create
an empty commit with the commit message as-is.
Junio commented on the "become-empty" types of commits as follows[1]:
WRT a change that ends up being empty (as opposed to a change that
is empty from the beginning), I'd think that the current behaviour
is desireable one. "am" based rebase is solely to transplant an
existing history and want to stop much less than "interactive" one
whose purpose is to polish a series before making it publishable,
and asking for confirmation ("this has become empty--do you want to
drop it?") is more appropriate from the workflow point of view.
[1] https://lore.kernel.org/git/xmqqfu1fswdh.fsf@gitster-ct.c.googlers.com/
I would simply add that his arguments for "am"-based rebases actually
apply to all non-explicitly-interactive rebases. Also, since we are
stating that different cases should have different defaults, it may be
worth providing a flag to allow users to select which behavior they want
for these commits.
Introduce a new command line flag for selecting the desired behavior:
--empty={drop,keep,ask}
with the definitions:
drop: drop commits which become empty
keep: keep commits which become empty
ask: provide the user a chance to interact and pick what to do with
commits which become empty on a case-by-case basis
In line with Junio's suggestion, if the --empty flag is not specified,
pick defaults as follows:
explicitly interactive: ask
otherwise: drop
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Different rebase backends have different treatment for commits which
start empty (i.e. have no changes relative to their parent), and the
--keep-empty option was added at some point to allow adjusting behavior.
The handling of commits which start empty is actually quite similar to
commit b00bf1c9a8 (git-rebase: make --allow-empty-message the default,
2018-06-27), which pointed out that the behavior for various backends is
often more happenstance than design. The specific change made in that
commit is actually quite relevant as well and much of the logic there
directly applies here.
It makes a lot of sense in 'git commit' to error out on the creation of
empty commits, unless an override flag is provided. However, once
someone determines that there is a rare case that merits using the
manual override to create such a commit, it is somewhere between
annoying and harmful to have to take extra steps to keep such
intentional commits around. Granted, empty commits are quite rare,
which is why handling of them doesn't get considered much and folks tend
to defer to existing (accidental) behavior and assume there was a reason
for it, leading them to just add flags (--keep-empty in this case) that
allow them to override the bad defaults. Fix the interactive backend so
that --keep-empty is the default, much like we did with
--allow-empty-message. The am backend should also be fixed to have
--keep-empty semantics for commits that start empty, but that is not
included in this patch other than a testcase documenting the failure.
Note that there was one test in t3421 which appears to have been written
expecting --keep-empty to not be the default as correct behavior. This
test was introduced in commit 00b8be5a4d ("add tests for rebasing of
empty commits", 2013-06-06), which was part of a series focusing on
rebase topology and which had an interesting original cover letter at
https://lore.kernel.org/git/1347949878-12578-1-git-send-email-martinvonz@gmail.com/
which noted
Your input especially appreciated on whether you agree with the
intent of the test cases.
and then went into a long example about how one of the many tests added
had several questions about whether it was correct. As such, I believe
most the tests in that series were about testing rebase topology with as
many different flags as possible and were not trying to state in general
how those flags should behave otherwise.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code to compute the commit-graph has been taught to use a more
robust way to tell if two object directories refer to the same
thing.
* tb/commit-graph-object-dir:
commit-graph.h: use odb in 'load_commit_graph_one_fd_st'
commit-graph.c: remove path normalization, comparison
commit-graph.h: store object directory in 'struct commit_graph'
commit-graph.h: store an odb in 'struct write_commit_graph_context'
t5318: don't pass non-object directory to '--object-dir'
The index-pack code now diagnoses a bad input packstream that
records the same object twice when it is used as delta base; the
code used to declare a software bug when encountering such an
input, but it is an input error.
* jk/index-pack-dupfix:
index-pack: downgrade twice-resolved REF_DELTA to die()
The code to automatically shrink the fan-out in the notes tree had
an off-by-one bug, which has been killed.
* jh/notes-fanout-fix:
notes.c: fix off-by-one error when decreasing notes fanout
t3305: check notes fanout more carefully and robustly
The way "git submodule status" reports an initialized but not yet
populated submodule has not been reimplemented correctly when a
part of the "git submodule" command was rewritten in C, which has
been corrected.
* pk/status-of-uncloned-submodule:
t7400: testcase for submodule status on unregistered inner git repos
submodule: fix status of initialized but not cloned submodules
t7400: add a testcase for submodule status on empty dirs
The diff-* plumbing family of subcommands now pay attention to the
diff.wsErrorHighlight configuration, which has been ignored before;
this allows "git add -p" to also show the whitespace problems to
the end user.
* jk/diff-honor-wserrhighlight-in-plumbing:
diff: move diff.wsErrorHighlight to "basic" config
Some rough edges in the sparse-checkout feature, especially around
the cone mode, have been cleaned up.
* ds/sparse-checkout-harden:
sparse-checkout: fix cone mode behavior mismatch
sparse-checkout: improve docs around 'set' in cone mode
sparse-checkout: escape all glob characters on write
sparse-checkout: use C-style quotes in 'list' subcommand
sparse-checkout: unquote C-style strings over --stdin
sparse-checkout: write escaped patterns in cone mode
sparse-checkout: properly match escaped characters
sparse-checkout: warn on globs in cone patterns
sparse-checkout: detect short patterns
sparse-checkout: cone mode does not recognize "**"
sparse-checkout: fix documentation typo for core.sparseCheckoutCone
clone: fix --sparse option with URLs
sparse-checkout: create leading directories
t1091: improve here-docs
t1091: use check_files to reduce boilerplate
A low-level API function get_oid(), that accepts various ways to
name an object, used to issue end-user facing error messages
without l10n, which has been updated to be translatable.
* jk/get-oid-error-message-i18n:
sha1-name: mark get_oid() error messages for translation
t1506: drop space after redirection operator
t1400: avoid "test" string comparisons
Allow the rebase.missingCommitsCheck configuration to kick in when
"rebase --edit-todo" and "rebase --continue" restarts the procedure.
* ag/edit-todo-drop-check:
rebase-interactive: warn if commit is dropped with `rebase --edit-todo'
sequencer: move check_todo_list_from_file() to rebase-interactive.c
Test updates.
* dl/test-must-fail-fixes-2:
t4124: only mark git command with test_must_fail
t3507: use test_path_is_missing()
t3507: fix indentation
t3504: do check for conflict marker after failed cherry-pick
t3419: stop losing return code of git command
t3415: increase granularity of test_auto_{fixup,squash}()
t3415: stop losing return codes of git commands
t3310: extract common notes_merge_files_gone()
t3030: use test_path_is_missing()
t2018: replace "sha" with "oid"
t2018: don't lose return code of git commands
t2018: teach do_checkout() to accept `!` arg
t2018: be more discerning when checking for expected exit codes
t2018: improve style of if-statement
t2018: add space between function name and ()
t2018: remove trailing space from test description
"git rebase -i" identifies existing commits in its todo file with
their abbreviated object name, which could become ambigous as it
goes to create new commits, and has a mechanism to avoid ambiguity
in the main part of its execution. A few other cases however were
not covered by the protection against ambiguity, which has been
corrected.
* js/rebase-i-with-colliding-hash:
rebase -i: also avoid SHA-1 collisions with missingCommitsCheck
rebase -i: re-fix short SHA-1 collision
parse_insn_line(): improve error message when parsing failed
A new version of fsmonitor-watchman hook has been introduced, to
avoid races.
* kw/fsmonitor-watchman-racefix:
fsmonitor: update documentation for hook version and watchman hooks
fsmonitor: add fsmonitor hook scripts for version 2
fsmonitor: handle version 2 of the hooks that will use opaque token
fsmonitor: change last update timestamp on the index_state to opaque token
The transport protocol version 2 becomes the default one.
* jn/promote-proto2-to-default:
fetch: default to protocol version 2
protocol test: let protocol.version override GIT_TEST_PROTOCOL_VERSION
test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate
config doc: protocol.version is not experimental
fetch test: use more robust test for filtered objects
Two help messages given when "git add" notices the user gave it
nothing to add have been updated to use advise() API.
* hw/advice-add-nothing:
add: change advice config variables used by the add API
add: use advise function to display hints
Just as rev-list recently learned to combine filters and bitmaps, let's
do the same for pack-objects. The infrastructure is all there; we just
need to pass along our filter options, and the pack-bitmap code will
decide to use bitmaps or not.
This unsurprisingly makes things faster for partial clones of large
repositories (here we're cloning linux.git):
Test HEAD^ HEAD
------------------------------------------------------------------------------
5310.11: simulated partial clone 38.94(37.28+5.87) 11.06(11.27+4.07) -71.6%
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Just as the previous commit implemented BLOB_NONE, we can support
BLOB_LIMIT filters by looking at the sizes of any blobs in the result
and unsetting their bits as appropriate. This is slightly more expensive
than BLOB_NONE, but still produces a noticeable speedup (these results
are on git.git):
Test HEAD~2 HEAD
------------------------------------------------------------------------------------
5310.9: rev-list count with blob:none 1.80(1.77+0.02) 0.22(0.20+0.02) -87.8%
5310.10: rev-list count with blob:limit=1k 1.99(1.96+0.03) 0.29(0.25+0.03) -85.4%
The implementation is similar to the BLOB_NONE one, with the exception
that we have to go object-by-object while walking the blob-type bitmap
(since we can't mask out the matches, but must look up the size
individually for each blob). The trick with using ctz64() is taken from
show_objects_for_type(), which likewise needs to find individual bits
(but wants to quickly skip over big chunks without blobs).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We can easily support BLOB_NONE filters with bitmaps. Since we know the
types of all of the objects, we just need to clear the result bits of
any blobs.
Note two subtleties in the implementation (which I also called out in
comments):
- we have to include any blobs that were specifically asked for (and
not reached through graph traversal) to match the non-bitmap version
- we have to handle in-pack and "ext_index" objects separately.
Arguably prepare_bitmap_walk() could be adding these ext_index
objects to the type bitmaps. But it doesn't for now, so let's match
the rest of the bitmap code here (it probably wouldn't be an
efficiency improvement to do so since the cost of extending those
bitmaps is about the same as our loop here, but it might make the
code a bit simpler).
Here are perf results for the new test on git.git:
Test HEAD^ HEAD
--------------------------------------------------------------------------------
5310.9: rev-list count with blob:none 1.67(1.62+0.05) 0.22(0.21+0.02) -86.8%
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ever since we added reachability bitmap support, we've been able to use
it with rev-list to get the full list of objects, like:
git rev-list --objects --use-bitmap-index --all
But you can't do so without --objects, since we weren't ready to just
show the commits. However, the internals of the bitmap code are mostly
ready for this: they avoid opening up trees when walking to fill in the
bitmaps. We just need to actually pass in the rev_info to
traverse_bitmap_commit_list() so it knows which types to bother
triggering our callback for.
For completeness, the perf test now covers both the existing --objects
case, as well as the new commits-only behavior (the objects one got way
faster when we introduced bitmaps, but obviously isn't improved now).
Here are numbers for linux.git:
Test HEAD^ HEAD
------------------------------------------------------------------------
5310.7: rev-list (commits) 8.29(8.10+0.19) 1.76(1.72+0.04) -78.8%
5310.8: rev-list (objects) 8.06(7.94+0.12) 8.14(7.94+0.13) +1.0%
That run was cheating a little, as I didn't have any commit-graph in the
repository, and we'd built it by default these days when running git-gc.
Here are numbers with a commit-graph:
Test HEAD^ HEAD
------------------------------------------------------------------------
5310.7: rev-list (commits) 0.70(0.58+0.12) 0.51(0.46+0.04) -27.1%
5310.8: rev-list (objects) 6.20(6.09+0.10) 6.27(6.16+0.11) +1.1%
Still an improvement, but a lot less impressive.
We could have the perf script remove any commit-graph to show the
out-sized effect, but it probably makes sense to leave it in what would
be a more typical setup.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We check the results of "rev-list --use-bitmap-index" by comparing it to
the same traversal without the bitmap option. However, this is a little
tricky to do because of some output differences (see the included
comment for details). Let's pull this out into a helper function, since
we'll be adding some similar tests.
While we're at it, let's also try to confirm that the bitmap output did
indeed use bitmaps. Since the code internally falls back to the
non-bitmap path in some cases, the tests are at risk of becoming trivial
noops.
This is a bit fragile, as not all outputs will differ (e.g., looking at
only the commits from a fully-bitmapped pack will end up exactly the
same as the normal traversal order, since it also matches the pack
order). So we'll provide an escape hatch by which tests can disable this
check (which should only be used after manually confirming that bitmaps
kicked in).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The prior commit taught "--count --objects" to work without bitmaps. We
should be able to get the same answer much more quickly with bitmaps.
Note that we punt on the max_count case here. This perhaps _could_ be
made to work if we find all of the boundary commits and treat them as
UNINTERESTING, subtracting them (and their reachable objects) from the
set we return. That implies an actual commit traversal, but we'd still
be faster due to avoiding opening up any trees. Given the complexity and
the fact that anyone is unlikely to want this, it makes sense to just
fall back to the non-bitmap case for now.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current behavior from "rev-list --count --objects" is nonsensical:
we enumerate all of the objects except commits, but then give a count of
commits. This wasn't planned, and is just what the code happens to do.
Instead, let's give the answer the user almost certainly wanted: the
full count of objects.
Note that there are more complicated cases around cherry-marking, etc.
We'll punt on those for now, but let the user know that we can't produce
an answer (rather than giving them something useless).
We'll test both the new feature as well as a vanilla --count of commits,
since that surprisingly doesn't seem to be covered in the existing
tests.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
On Windows, it is quite common to work with network drives. The format
of the paths to network drives (or "network shares", or UNC paths) is:
\\<server>\<share>\...
We already have a couple regression tests revolving around those types
of paths, but we missed cloning and fetching from UNC paths without
leading `file://` (and with backslashes instead of forward slashes).
This lil' patch closes that gap.
It gets a bit silly to add the commands to the name of the test script,
so let's just rename it while we're testing more UNC stuff.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When grepping through the output of a command in the test suite, there
is always a chance that something goes wrong, in which case there would
not be anything useful to debug.
Let's redirect the output into a file instead, and grep that file, so
that the log can be inspected easily if the grep fails.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "--use-bitmap-index" option is usually aspirational: if we have
bitmaps and the request can be fulfilled more quickly using them we'll
do so, but otherwise fall back to a non-bitmap traversal.
The exception is object filtering, which explicitly dies if the two
options are combined. Let's convert this to the usual fallback behavior.
This is a minor convenience for now (since the caller can easily know
that --filter and --use-bitmap-index don't combine), but will become
much more useful as we start to support _some_ filters with bitmaps, but
not others.
The test infrastructure here is bigger than necessary for checking this
one small feature. But it will serve as the basis for more filtering
bitmap tests in future patches.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git grep --no-index" should not get affected by the contents of
the .gitmodules file but when "--recurse-submodules" is given or
the "submodule.recurse" variable is set, it did. Now these
settings are ignored in the "--no-index" mode.
* pb/do-not-recurse-grep-no-index:
grep: ignore --recurse-submodules if --no-index is given
