aboutsummaryrefslogtreecommitdiffstats
path: root/builtin/commit-graph.c (unfollow)
AgeCommit message (Collapse)AuthorFilesLines
2020-09-29Eighteenth batchJunio C Hamano1-0/+24
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25Seventeenth batchJunio C Hamano1-0/+19
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25t1506: rev-parse A..B and A...BJunio C Hamano1-0/+18
Because these constructs can be used to parse user input to be passed to rev-list --objects, e.g. range=$(git rev-parse v1.0..v2.0) && git rev-list --objects $range | git pack-objects --stdin the endpoints (v1.0 and v2.0 in the example) are shown without peeling them to underlying commits, even when they are annotated tags. Make sure it stays that way. While at it, ensure "rev-parse A...B" also keeps the endpoints A and B unpeeled, even though the negative side (i.e. the merge-base between A and B) has to become a commit. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25protocol: re-enable v2 protocol by defaultJeff King3-11/+2
Protocol v2 became the default in v2.26.0 via 684ceae32d (fetch: default to protocol version 2, 2019-12-23). More widespread use turned up a regression in negotiation. That was fixed in v2.27.0 via 4fa3f00abb (fetch-pack: in protocol v2, in_vain only after ACK, 2020-04-27), but we also reverted the default to v0 as a precuation in 11c7f2a30b (Revert "fetch: default to protocol version 2", 2020-04-22). In v2.28.0, we re-enabled it for experimental users with 3697caf4b9 (config: let feature.experimental imply protocol.version=2, 2020-05-20) and haven't heard any complaints. v2.28 has only been out for 2 months, but I'd generally expect people turning on feature.experimental to also stay pretty up-to-date. So we're not likely to collect much more data by waiting. In addition, we have no further reports from people running v2.26.0, and of course some people have been setting protocol.version manually for ages. Let's move forward with v2 as the default again. It's possible there are still lurking bugs, but we won't know until it gets more widespread use. And we can find and squash them just like any other bug at this point. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-24pull: don't warn if pull.ff has been setAlex Henrie2-3/+21
A user who understands enough to set pull.ff does not need additional instructions. Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-24diff: fix modified lines stats with --stat and --numstatThomas Guyot-Sionnest2-13/+11
Only skip diffstats when both oids are valid and identical. This check was causing both false-positives (files included in diffstats with no actual changes (0 lines modified) and false-negatives (showing 0 lines modified in stats when files had actually changed). Also replaced same_contents with may_differ to avoid confusion. Signed-off-by: Thomas Guyot-Sionnest <tguyot@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-23hooks--update.sample: use hash-agnostic zero OIDDenton Liu1-1/+1
The update sample hook has the zero OID hardcoded as 40 zeros. However, with the introduction of SHA-256 support, this assumption no longer holds true. Replace the hardcoded $z40 with a call to git hash-object --stdin </dev/null | tr '[0-9a-f]' '0' so the sample hook becomes hash-agnostic. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-23hooks--pre-push.sample: use hash-agnostic zero OIDDenton Liu1-3/+3
The pre-push sample hook has the zero OID hardcoded as 40 zeros. However, with the introduction of SHA-256 support, this assumption no longer holds true. Replace the hardcoded $z40 with a call to git hash-object --stdin </dev/null | tr '[0-9a-f]' '0' so the sample hook becomes hash-agnostic. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-23hooks--pre-push.sample: modernize scriptDenton Liu1-8/+8
The preferred form for a command substitution is $() over ``. Use this form for the command substitution in the sample hook. The preferred form for conditional tests is to use `test` over []. Replace [] with `test`. Finally, replace all instances of "sha" with "oid". Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-22Sixteenth batchJunio C Hamano1-0/+31
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-22builtin/clone: avoid failure with GIT_DEFAULT_HASHbrian m. carlson4-4/+20
If a user is cloning a SHA-1 repository with GIT_DEFAULT_HASH set to "sha256", then we can end up with a repository where the repository format version is 0 but the extensions.objectformat key is set to "sha256". This is both wrong (the user has a SHA-1 repository) and nonfunctional (because the extension cannot be used in a v0 repository). This happens because in a clone, we initially set up the repository, and then change its algorithm based on what the remote side tells us it's using. We've initially set up the repository as SHA-256 in this case, and then later on reset the repository version without clearing the extension. We could just always set the extension in this case, but that would mean that our SHA-1 repositories weren't compatible with older Git versions, even though there's no reason why they shouldn't be. And we also don't want to initialize the repository as SHA-1 initially, since that means if we're cloning an empty repository, we'll have failed to honor the GIT_DEFAULT_HASH variable and will end up with a SHA-1 repository, not a SHA-256 repository. Neither of those are appealing, so let's tell the repository initialization code if we're doing a reinit like this, and if so, to clear the extension if we're using SHA-1. This makes sure we produce a valid and functional repository and doesn't break any of our other use cases. Reported-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-21diff-highlight: correctly match blank lines for flushJeff King1-1/+1
We try to flush the output from diff-highlight whenever we see a blank line. That lets you see the output for each commit as soon as it is generated, even if Git is still chugging away at a diff, or traversing to find the next commit. However, our "blank line" match checks length($_). That won't ever be true, because we haven't chomped the line ending. As a result, we never flush. Instead, let's use a simple regex which handles line endings in with the end-of-line marker. This has been broken since the initial version in 927a13fe87 (contrib: add diff highlight script, 2011-10-18). Probably nobody noticed because: - most output is big enough, or comes fast enough, that it flushes anyway. And it can be difficult to notice the difference between "show a commit, then pause" and "pause, then show two commits". I only noticed because I was viewing "git log" output on a repo with a very slow textconv filter. - if stdout is going to the terminal (and not another pager like less), then the flush isn't necessary. So any manual testing would show it appearing to work. You can easily see the difference with something like: echo '* diff=slow' >>.gitattributes git -c diff.slow.textconv='sleep 1; cat' \ -c pager.log='diff-highlight | less' \ log -p That should generate one commit every second or so (more if it touches multiple files), but without this patch it waits for many seconds before generating several pages of output. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-20docs: explain how to deal with files that are always modifiedbrian m. carlson1-0/+33
Users frequently have problems where two filenames differ only in case, causing one of those files to show up consistently as being modified. Let's add a FAQ entry that explains how to deal with that. In addition, let's explain another common case where files are consistently modified, which is when files using a smudge or clean filter have not been run through that filter. Explain the way to fix this as well. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-20docs: explain why reverts are not always applied on mergebrian m. carlson1-0/+21
A common scenario is for a user to apply a change to one branch and cherry-pick it into another, then later revert it in the first branch. This results in the change being present when the two branches are merged, which is confusing to many users. We already have documentation for how this works in `git merge`, but it is clear from the frequency with which this is asked that it's hard to grasp. We also don't explain to users that they are better off doing a rebase in this case, which will do what they intended. Let's add an entry to the FAQ telling users what's happening and advising them to use rebase here. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-20docs: explain why squash merges are broken with long-running branchesbrian m. carlson1-0/+32
In many projects, squash merges are commonly used, primarily to keep a tidy history in the face of developers who do not use logically independent, bisectable commits. As common as this is, this tends to cause significant problems when squash merges are used to merge long-running branches due to the lack of any new merge bases. Even very experienced developers may make this mistake, so let's add a FAQ entry explaining why this is problematic and explaining that regular merge commits should be used to merge two long-running branches. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-19send-pack: run GPG after atomic push checkingHan Xin2-28/+53
The refs update commands can be sent to the server side in two different ways: GPG-signed or unsigned. We should run these two operations in the same "Finally, tell the other end!" code block, but they are seperated by the "Clear the status for each ref" code block. This will result in a slight performance loss, because the failed atomic push will still perform unnecessary preparations for shallow advertise and GPG-signed commands buffers, and user may have to be bothered by the (possible) GPG passphrase input when there is nothing to sign. Add a new test case to t5534 to ensure GPG will not be called when the GPG-signed atomic push fails. Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-19git-p4: use HEAD~$n to find parent commit for unshelveLuke Diamand2-2/+2
Found-by: Liu Xuhui (Jackson) <Xuhui.Liu@amd.com> Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-19git-p4 unshelve: adding a commit breaks git-p4 unshelveLuke Diamand1-2/+5
git-p4 unshelve uses HEAD^$n to find the parent commit, which fails if there is an additional commit. Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-19pack-write: use hashwrite_be32() in write_idx_file()René Scharfe1-8/+4
Call hashwrite_be32() instead of open-coding it. This shortens the code a bit and makes it easier to read. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-18Fifteenth batchJunio C Hamano1-1/+28
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-18Doc: prefer more specific file nameAaron Lipman4-3/+3
Change filters.txt to ref-reachability-filters.txt in order to avoid squatting on a file name that might be useful for another purpose. Signed-off-by: Aaron Lipman <alipman88@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-18ref-filter: make internal reachable-filter API more preciseAaron Lipman2-18/+14
The internal reachable-filter API is a bit loose and imprecise; it also bleeds unnecessarily into the public header. Tighten the API by: * renaming do_merge_filter() to reach_filter() * separating parameters to explicitly identify what data is used by the function instead of passing an entire ref_filter_cbdata struct * renaming and moving internal constants from header to source file Signed-off-by: Aaron Lipman <alipman88@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-18commit-graph: introduce 'commitGraph.maxNewFilters'Taylor Blau4-2/+43
Introduce a configuration variable to specify a default value for the recently-introduce '--max-new-filters' option of 'git commit-graph write'. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-18builtin/commit-graph.c: introduce '--max-new-filters=<n>'Taylor Blau6-8/+112
Introduce a command-line flag to specify the maximum number of new Bloom filters that a 'git commit-graph write' is willing to compute from scratch. Prior to this patch, a commit-graph write with '--changed-paths' would compute Bloom filters for all selected commits which haven't already been computed (i.e., by a previous commit-graph write with '--split' such that a roll-up or replacement is performed). This behavior can cause prohibitively-long commit-graph writes for a variety of reasons: * There may be lots of filters whose diffs take a long time to generate (for example, they have close to the maximum number of changes, diffing itself takes a long time, etc). * Old-style commit-graphs (which encode filters with too many entries as not having been computed at all) cause us to waste time recomputing filters that appear to have not been computed only to discover that they are too-large. This can make the upper-bound of the time it takes for 'git commit-graph write --changed-paths' to be rather unpredictable. To make this command behave more predictably, introduce '--max-new-filters=<n>' to allow computing at most '<n>' Bloom filters from scratch. This lets "computing" already-known filters proceed quickly, while bounding the number of slow tasks that Git is willing to do. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17commit-graph: rename 'split_commit_graph_opts'Taylor Blau3-34/+34
In the subsequent commit, additional options will be added to the commit-graph API which have nothing to do with splitting. Rename the 'split_commit_graph_opts' structure to the more-generic 'commit_graph_opts' to encompass both. Likewise, rename the 'flags' member to instead be 'split_flags' to clarify that it only has to do with the behavior implied by '--split'. Suggested-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17bloom: encode out-of-bounds filters as non-emptyTaylor Blau6-9/+54
When a changed-path Bloom filter has either zero, or more than a certain number (commonly 512) of entries, the commit-graph machinery encodes it as "missing". More specifically, it sets the indices adjacent in the BIDX chunk as equal to each other to indicate a "length 0" filter; that is, that the filter occupies zero bytes on disk. This has heretofore been fine, since the commit-graph machinery has no need to care about these filters with too few or too many changed paths. Both cases act like no filter has been generated at all, and so there is no need to store them. In a subsequent commit, however, the commit-graph machinery will learn to only compute Bloom filters for some commits in the current commit-graph layer. This is a change from the current implementation which computes Bloom filters for all commits that are in the layer being written. Critically for this patch, only computing some of the Bloom filters means adding a third state for length 0 Bloom filters: zero entries, too many entries, or "hasn't been computed". It will be important for that future patch to distinguish between "not representable" (i.e., zero or too-many changed paths), and "hasn't been computed". In particular, we don't want to waste time recomputing filters that have already been computed. To that end, change how we store Bloom filters in the "computed but not representable" category: - Bloom filters with no entries are stored as a single byte with all bits low (i.e., all queries to that Bloom filter will return "definitely not") - Bloom filters with too many entries are stored as a single byte with all bits set high (i.e., all queries to that Bloom filter will return "maybe"). These rules are sufficient to not incur a behavior change by changing the on-disk representation of these two classes. Likewise, no specification changes are necessary for the commit-graph format, either: - Filters that were previously empty will be recomputed and stored according to the new rules, and - old clients reading filters generated by new clients will interpret the filters correctly and be none the wiser to how they were generated. Clients will invoke the Bloom machinery in more cases than before, but this can be addressed by returning a NULL filter when all bits are set high. This can be addressed in a future patch. Note that this does increase the size of on-disk commit-graphs, but far less than other proposals. In particular, this is generally more efficient than storing a bitmap for which commits haven't computed their Bloom filters. Storing a bitmap incurs a penalty of one bit per commit, whereas storing explicit filters as above incurs a penalty of one byte per too-large or empty commit. In practice, these boundary commits likely occupy a small proportion of the overall number of commits, and so the size penalty is likely smaller than storing a bitmap for all commits. See, for example, these relative proportions of such boundary commits (collected by SZEDER Gábor): | Percentage of | commit-graph | | | commits modifying | file size | | ├────────┬──────────────┼───────────────────┤ pct. | | 0 path | >= 512 paths | before | after | change | ┌────────────────┼────────┼──────────────┼─────────┼─────────┼───────────┤ | android-base | 13.20% | 0.13% | 37.468M | 37.534M | +0.1741 % | | cmssw | 0.15% | 0.23% | 17.118M | 17.119M | +0.0091 % | | cpython | 3.07% | 0.01% | 7.967M | 7.971M | +0.0423 % | | elasticsearch | 0.70% | 1.00% | 8.833M | 8.835M | +0.0128 % | | gcc | 0.00% | 0.08% | 16.073M | 16.074M | +0.0030 % | | gecko-dev | 0.14% | 0.64% | 59.868M | 59.874M | +0.0105 % | | git | 0.11% | 0.02% | 3.895M | 3.895M | +0.0020 % | | glibc | 0.02% | 0.10% | 3.555M | 3.555M | +0.0021 % | | go | 0.00% | 0.07% | 3.186M | 3.186M | +0.0018 % | | homebrew-cask | 0.40% | 0.02% | 7.035M | 7.035M | +0.0065 % | | homebrew-core | 0.01% | 0.01% | 11.611M | 11.611M | +0.0002 % | | jdk | 0.26% | 5.64% | 5.537M | 5.540M | +0.0590 % | | linux | 0.01% | 0.51% | 63.735M | 63.740M | +0.0073 % | | llvm-project | 0.12% | 0.03% | 25.515M | 25.516M | +0.0050 % | | rails | 0.10% | 0.10% | 6.252M | 6.252M | +0.0027 % | | rust | 0.07% | 0.17% | 9.364M | 9.364M | +0.0033 % | | tensorflow | 0.09% | 1.02% | 7.009M | 7.010M | +0.0158 % | | webkit | 0.05% | 0.31% | 17.405M | 17.406M | +0.0047 % | (where the above increase is determined by computing a non-split commit-graph before and after this patch). Given that these projects are all "large" by commit count, the storage cost by writing these filters explicitly is negligible. In the most extreme example, android-base (which has 494,848 commits at the time of writing) would have its commit-graph increase by a modest 68.4 KB. Finally, a test to exercise filters which contain too many changed path entries will be introduced in a subsequent patch. Suggested-by: SZEDER Gábor <szeder.dev@gmail.com> Suggested-by: Jakub Narębski <jnareb@gmail.com> Helped-by: Derrick Stolee <dstolee@microsoft.com> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17contrib/completion: complete options that take refs for format-patchDenton Liu1-0/+4
The completion for format-patch currently suggests --base=, --interdiff= and --range-diff= as options. However, with these `=` forms of the options, there is no space and we'd enter the `--*` case which means we don't call the __git_complete_revlist() at the end. Teach _git_format_patch() to complete refs in the case of --base=, --interdiff= and --range-diff=. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17remote: don't show success message when set-head failsChristian Schlack1-1/+1
Suppress the message 'origin/HEAD set to master' in case of an error. $ git remote set-head origin -a error: Not a valid ref: refs/remotes/origin/master origin/HEAD set to master Signed-off-by: Christian Schlack <christian@backhub.co> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17packfile: actually set approximate_object_count_validJeff King2-0/+5
The approximate_object_count() function tries to compute the count only once per process. But ever since it was introduced in 8e3f52d778 (find_unique_abbrev: move logic out of get_short_sha1(), 2016-10-03), we failed to actually set the "valid" flag, meaning we'd compute it fresh on every call. This turns out not to be _too_ bad, because we're only iterating through the packed_git list, and not making any system calls. But since it may get called for every abbreviated hash we output, even this can add up if you have many packs. Here are before-and-after timings for a new perf test which just asks rev-list to abbreviate each commit hash (the test repo is linux.git, with commit-graphs): Test origin HEAD ---------------------------------------------------------------------------- 5303.3: rev-list (1) 28.91(28.46+0.44) 29.03(28.65+0.38) +0.4% 5303.4: abbrev-commit (1) 1.18(1.06+0.11) 1.17(1.02+0.14) -0.8% 5303.7: rev-list (50) 28.95(28.56+0.38) 29.50(29.17+0.32) +1.9% 5303.8: abbrev-commit (50) 3.67(3.56+0.10) 3.57(3.42+0.15) -2.7% 5303.11: rev-list (1000) 30.34(29.89+0.43) 30.82(30.35+0.46) +1.6% 5303.12: abbrev-commit (1000) 86.82(86.52+0.29) 77.82(77.59+0.22) -10.4% 5303.15: load 10,000 packs 0.08(0.02+0.05) 0.08(0.02+0.06) +0.0% It doesn't help at all when we have 1 pack (5303.4), but we get a 10% speedup when there are 1000 packs (5303.12). That's a modest speedup for a case that's already slow and we'd hope to avoid in general (note how slow it is even after, because we have to look in each of those packs for abbreviations). But it's a one-line change that clearly matches the original intent, so it seems worth doing. The included perf test may also be useful for keeping an eye on any regressions in the overall abbreviation code. Reported-by: Rasmus Villemoes <rv@rasmusvillemoes.dk> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: add trace2 regions for task executionDerrick Stolee1-0/+2
Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: add auto condition for commit-graph taskDerrick Stolee3-0/+93
Instead of writing a new commit-graph in every 'git maintenance run --auto' process (when maintenance.commit-graph.enalbed is configured to be true), only write when there are "enough" commits not in a commit-graph file. This count is controlled by the maintenance.commit-graph.auto config option. To compute the count, use a depth-first search starting at each ref, and leaving markers using the SEEN flag. If this count reaches the limit, then terminate early and start the task. Otherwise, this operation will peel every ref and parse the commit it points to. If these are all in the commit-graph, then this is typically a very fast operation. Users with many refs might feel a slow-down, and hence could consider updating their limit to be very small. A negative value will force the step to run every time. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: use pointers to check --autoDerrick Stolee3-2/+18
The 'git maintenance run' command has an '--auto' option. This is used by other Git commands such as 'git commit' or 'git fetch' to check if maintenance should be run after adding data to the repository. Previously, this --auto option was only used to add the argument to the 'git gc' command as part of the 'gc' task. We will be expanding the other tasks to perform a check to see if they should do work as part of the --auto flag, when they are enabled by config. First, update the 'gc' task to perform the auto check inside the maintenance process. This prevents running an extra 'git gc --auto' command when not needed. It also shows a model for other tasks. Second, use the 'auto_condition' function pointer as a signal for whether we enable the maintenance task under '--auto'. For instance, we do not want to enable the 'fetch' task in '--auto' mode, so that function pointer will remain NULL. Now that we are not automatically calling 'git gc', a test in t5514-fetch-multiple.sh must be changed to watch for 'git maintenance' instead. We continue to pass the '--auto' option to the 'git gc' command when necessary, because of the gc.autoDetach config option changes behavior. Likely, we will want to absorb the daemonizing behavior implied by gc.autoDetach as a maintenance.autoDetach config option. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: create maintenance.<task>.enabled configDerrick Stolee5-5/+44
Currently, a normal run of "git maintenance run" will only run the 'gc' task, as it is the only one enabled. This is mostly for backwards- compatible reasons since "git maintenance run --auto" commands replaced previous "git gc --auto" commands after some Git processes. Users could manually run specific maintenance tasks by calling "git maintenance run --task=<task>" directly. Allow users to customize which steps are run automatically using config. The 'maintenance.<task>.enabled' option then can turn on these other tasks (or turn off the 'gc' task). Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: take a lock on the objects directoryDerrick Stolee1-0/+20
Performing maintenance on a Git repository involves writing data to the .git directory, which is not safe to do with multiple writers attempting the same operation. Ensure that only one 'git maintenance' process is running at a time by holding a file-based lock. Simply the presence of the .git/maintenance.lock file will prevent future maintenance. This lock is never committed, since it does not represent meaningful data. Instead, it is only a placeholder. If the lock file already exists, then no maintenance tasks are attempted. This will become very important later when we implement the 'prefetch' task, as this is our stop-gap from creating a recursive process loop between 'git fetch' and 'git maintenance run --auto'. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: add --task optionDerrick Stolee3-4/+98
A user may want to only run certain maintenance tasks in a certain order. Add the --task=<task> option, which allows a user to specify an ordered list of tasks to run. These cannot be run multiple times, however. Here is where our array of maintenance_task pointers becomes critical. We can sort the array of pointers based on the task order, but we do not want to move the struct data itself in order to preserve the hashmap references. We use the hashmap to match the --task=<task> arguments into the task struct data. Keep in mind that the 'enabled' member of the maintenance_task struct is a placeholder for a future 'maintenance.<task>.enabled' config option. Thus, we use the 'enabled' member to specify which tasks are run when the user does not specify any --task=<task> arguments. The 'enabled' member should be ignored if --task=<task> appears. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: add commit-graph taskDerrick Stolee5-4/+45
The first new task in the 'git maintenance' builtin is the 'commit-graph' task. This updates the commit-graph file incrementally with the command git commit-graph write --reachable --split By writing an incremental commit-graph file using the "--split" option we minimize the disruption from this operation. The default behavior is to merge layers until the new "top" layer is less than half the size of the layer below. This provides quick writes most of the time, with the longer writes following a power law distribution. Most importantly, concurrent Git processes only look at the commit-graph-chain file for a very short amount of time, so they will verly likely not be holding a handle to the file when we try to replace it. (This only matters on Windows.) If a concurrent process reads the old commit-graph-chain file, but our job expires some of the .graph files before they can be read, then those processes will see a warning message (but not fail). This could be avoided by a future update to use the --expire-time argument when writing the commit-graph. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: initialize task arrayDerrick Stolee1-1/+42
In anticipation of implementing multiple maintenance tasks inside the 'maintenance' builtin, use a list of structs to describe the work to be done. The struct maintenance_task stores the name of the task (as given by a future command-line argument) along with a function pointer to its implementation and a boolean for whether the step is enabled. A list these structs are initialized with the full list of implemented tasks along with a default order. For now, this list only contains the "gc" task. This task is also the only task enabled by default. The run subcommand will return a nonzero exit code if any task fails. However, it will attempt all tasks in its loop before returning with the failure. Also each failed task will print an error message. Helped-by: Taylor Blau <me@ttaylorr.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: replace run_auto_gc()Derrick Stolee10-23/+25
The run_auto_gc() method is used in several places to trigger a check for repo maintenance after some Git commands, such as 'git commit' or 'git fetch'. To allow for extra customization of this maintenance activity, replace the 'git gc --auto [--quiet]' call with one to 'git maintenance run --auto [--quiet]'. As we extend the maintenance builtin with other steps, users will be able to select different maintenance activities. Rename run_auto_gc() to run_auto_maintenance() to be clearer what is happening on this call, and to expose all callers in the current diff. Rewrite the method to use a struct child_process to simplify the calls slightly. Since 'git fetch' already allows disabling the 'git gc --auto' subprocess, add an equivalent option with a different name to be more descriptive of the new behavior: '--[no-]maintenance'. Update the documentation to include these options at the same time. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: add --quiet optionDerrick Stolee3-6/+23
Maintenance activities are commonly used as steps in larger scripts. Providing a '--quiet' option allows those scripts to be less noisy when run on a terminal window. Turn this mode on by default when stderr is not a terminal. Pipe the option to the 'git gc' child process. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17maintenance: create basic maintenance runnerDerrick Stolee8-0/+175
The 'gc' builtin is our current entrypoint for automatically maintaining a repository. This one tool does many operations, such as repacking the repository, packing refs, and rewriting the commit-graph file. The name implies it performs "garbage collection" which means several different things, and some users may not want to use this operation that rewrites the entire object database. Create a new 'maintenance' builtin that will become a more general- purpose command. To start, it will only support the 'run' subcommand, but will later expand to add subcommands for scheduling maintenance in the background. For now, the 'maintenance' builtin is a thin shim over the 'gc' builtin. In fact, the only option is the '--auto' toggle, which is handed directly to the 'gc' builtin. The current change is isolated to this simple operation to prevent more interesting logic from being lost in all of the boilerplate of adding a new builtin. Use existing builtin/gc.c file because we want to share code between the two builtins. It is possible that we will have 'maintenance' replace the 'gc' builtin entirely at some point, leaving 'git gc' as an alias for some specific arguments to 'git maintenance run'. Create a new test_subcommand helper that allows us to test if a certain subcommand was run. It requires storing the GIT_TRACE2_EVENT logs in a file. A negation mode is available that will be used in later tests. Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17bloom/diff: properly short-circuit on max_changesDerrick Stolee4-16/+100
Commit e3696980 (diff: halt tree-diff early after max_changes, 2020-03-30) intended to create a mechanism to short-circuit a diff calculation after a certain number of paths were modified. By incrementing a "num_changes" counter throughout the recursive ll_diff_tree_paths(), this was supposed to match the number of changes that would be written into the changed-path Bloom filters. Unfortunately, this was not implemented correctly and instead misses simple cases like file modifications. This then does not stop very large changed-path filters from being written (unless they add or remove many files). To start, change the implementation in ll_diff_tree_paths() to instead use the global diff_queue_diff struct's 'nr' member as the count. This is a way to simplify the logic instead of making more mistakes in the complicated diff code. This has a drawback: the diff_queue_diff struct only lists the paths corresponding to blob changes, not their leading directories. Thus, get_or_compute_bloom_filter() needs an additional check to see if the hashmap with the leading directories becomes too large. One reason why this was not caught by test cases was that the test in t4216-log-bloom.sh that was supposed to check this "too many changes" condition only checked this on the initial commit of a repository. The old logic counted these values correctly. Update this test in a few ways: 1. Use GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS to reduce the limit, allowing smaller commits to engage with this logic. 2. Create several interesting cases of edits, adds, removes, and mode changes (in the second commit). By testing both sides of the inequality with the *_MAX_CHANGED_PATHS variable, we can see that the count is exactly correct, so none of these changes are missed or over-counted. 3. Use the trace2 data value filter_found_large to verify that these commits are on the correct side of the limit. Another way to verify the behavior is correct is through performance tests. By testing on my local copies of the Git repository and the Linux kernel repository, I could measure the effect of these short-circuits when computing a fresh commit-graph file with changed-path Bloom filters using the command GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS=N time \ git commit-graph write --reachable --changed-paths and reporting the wall time and resulting commit-graph size. For Git, the results are | | N=1 | N=10 | N=512 | |--------|----------------|----------------|----------------| | HEAD~1 | 10.90s 9.18MB | 11.11s 9.34MB | 11.31s 9.35MB | | HEAD | 9.21s 8.62MB | 11.11s 9.29MB | 11.29s 9.34MB | For Linux, the results are | | N=1 | N=20 | N=512 | |--------|----------------|---------------|---------------| | HEAD~1 | 61.28s 64.3MB | 76.9s 72.6MB | 77.6s 72.6MB | | HEAD | 49.44s 56.3MB | 68.7s 65.9MB | 69.2s 65.9MB | Naturally, the improvement becomes much less as the limit grows, as fewer commits satisfy the short-circuit. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17bloom: use provided 'struct bloom_filter_settings'Taylor Blau4-19/+19
When 'get_or_compute_bloom_filter()' needs to compute a Bloom filter from scratch, it looks to the default 'struct bloom_filter_settings' in order to determine the maximum number of changed paths, number of bits per entry, and so on. All of these values have so far been constant, and so there was no need to pass in a pointer from the caller (eg., the one that is stored in the 'struct write_commit_graph_context'). Start passing in a 'struct bloom_filter_settings *' instead of using the default values to respect graph-specific settings (eg., in the case of setting 'GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS'). In order to have an initialized value for these settings, move its initialization to earlier in the commit-graph write. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17bloom: split 'get_bloom_filter()' in twoTaylor Blau7-13/+62
'get_bloom_filter' takes a flag to control whether it will compute a Bloom filter if the requested one is missing. In the next patch, we'll add yet another parameter to this method, which would force all but one caller to specify an extra 'NULL' parameter at the end. Instead of doing this, split 'get_bloom_filter' into two functions: 'get_bloom_filter' and 'get_or_compute_bloom_filter'. The former only looks up a Bloom filter (and does not compute one if it's missing, thus dropping the 'compute_if_not_present' flag). The latter does compute missing Bloom filters, with an additional parameter to store whether or not it needed to do so. This simplifies many call-sites, since the majority of existing callers to 'get_bloom_filter' do not want missing Bloom filters to be computed (so they can drop the parameter entirely and use the simpler version of the function). While we're at it, instrument the new 'get_or_compute_bloom_filter()' with counters in the 'write_commit_graph_context' struct which store the number of filters that we did and didn't compute, as well as filters that were truncated. It would be nice to drop the 'compute_if_not_present' flag entirely, since all remaining callers of 'get_or_compute_bloom_filter' pass it as '1', but this will change in a future patch and hence cannot be removed. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-17commit-graph.c: store maximum changed pathsTaylor Blau3-3/+16
For now, we assume that there is a fixed constant describing the maximum number of changed paths we are willing to store in a Bloom filter. Prepare for that to (at least partially) not be the case by making it a member of the 'struct bloom_filter_settings'. This will be helpful in the subsequent patches by reducing the size of test cases that exercise storing too many changed paths, as well as preparing for an eventual future in which this value might change. This patch alone does not cause newly generated Bloom filters to use a custom upper-bound on the maximum number of changed paths a single Bloom filter can hold, that will occur in a later patch. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-16mergetool: allow auto-merge for meld to follow the vim-diff behaviorLin Sun3-16/+96
Make the mergetool used with "meld" backend behave similarly to "vimdiff" by telling it to auto-merge non-conflicting parts and highlight the conflicting parts when `mergetool.meld.useAutoMerge` is configured with `true`, or `auto` for detecting the `--auto-merge` option automatically. Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Helped-by: David Aguilar <davvid@gmail.com> Signed-off-by: Lin Sun <lin.sun@zoom.us> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-16ref-filter: allow merged and no-merged filtersAaron Lipman13-65/+92
Enable ref-filter to process multiple merged and no-merged filters, and extend functionality to git branch, git tag and git for-each-ref. This provides an easy way to check for branches that are "graduation candidates:" $ git branch --no-merged master --merged next If passed more than one merged (or more than one no-merged) filter, refs must be reachable from any one of the merged commits, and reachable from none of the no-merged commits. Signed-off-by: Aaron Lipman <alipman88@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-16Doc: cover multiple contains/no-contains filtersAaron Lipman4-0/+15
Update documentation for "git branch", "git for-each-ref" and "git tag" with notes explaining what happens when passed multiple --contains or --no-contains filters. This behavior is useful to document prior to enabling multiple merged/no-merged filters, in order to demonstrate consistent behavior between merged/no-merged and contains/no-contains filters. Signed-off-by: Aaron Lipman <alipman88@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-16t3201: test multiple branch filter combinationsAaron Lipman1-11/+36
Add tests covering the behavior of passing multiple contains/no-contains filters to git branch, e.g.: $ git branch --contains feature_a --contains feature_b $ git branch --no-contains feature_a --no-contains feature_b When passed more than one contains (or no-contains) filter, the tips of the branches returned must be reachable from any of the contains commits and from none of the the no-contains commits. This logic is useful to describe prior to enabling multiple merged/no-merged filters, so that future tests will demonstrate consistent behavior between merged/no-merged and contains/no-contains filters. Signed-off-by: Aaron Lipman <alipman88@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-15commit-graph-format.txt: fix no-parent valueConor Davis1-1/+1
The correct value from commit-graph.c: #define GRAPH_PARENT_NONE 0x70000000 Signed-off-by: Conor Davis <git@conor.fastmail.fm> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-15fetch: pass --ipv4 and --ipv6 options to sub-fetchesAlex Riesen1-0/+4
The options indicate user intent for the whole fetch operation, and ignoring them in sub-fetches (i.e. "--all" and recursive fetching of submodules) is quite unexpected when, for instance, it is intended to limit all of the communication to a specific transport protocol for some reason. Signed-off-by: Alex Riesen <alexander.riesen@cetitec.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>