summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2020-09-29Merge branch 'bc/faq-misc'Junio C Hamano1-0/+86
More FAQ entries. * bc/faq-misc: docs: explain how to deal with files that are always modified docs: explain why reverts are not always applied on merge docs: explain why squash merges are broken with long-running branches
2020-09-29clone: add tests for --template and some disallowed option pairsSean Barag1-1/+45
Some combinations of command-line options to `git clone` are invalid, but there were previously no tests ensuring those combinations reported errors. Similarly, `git clone --template` didn't appear to have any tests. Helped-by: Jeff King <peff@peff.net> Helped-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Sean Barag <sean@barag.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-29diff: get rid of redundant 'dense' argumentSergey Organov6-23/+18
Get rid of 'dense' argument that is redundant for every function that has 'struct rev_info *rev' argument as well, as the value of 'dense' passed is always taken from 'rev->dense_combined_merges' field. The only place where this was not the case is in 'submodule.c' where 'diff_tree_combined_merge()' was called with '1' for 'dense' argument. However, at that call the 'revs' instance used is local to the function, and we now just set 'revs->dense_combined_merges' to 1 in this local instance. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28packfile: fix memory leak in add_delta_base_cache()Matheus Tavares1-2/+5
When add_delta_base_cache() is called with a base that is already in the cache, no operation is performed. But the check is done after allocating space for a new entry, so we end up leaking memory on the early return. In addition, the caller never free()'s the base as it expects the function to take ownership of it. But the base is not released when we skip insertion, so it also gets leaked. To fix these problems, move the allocation of a new entry further down in add_delta_base_cache(), and free() the base on early return. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28packfile: fix race condition on unpack_entry()Matheus Tavares1-17/+24
The third phase of unpack_entry() performs the following sequence in a loop, until all the deltas enumerated in phase one are applied and the entry is fully reconstructed: 1. Add the current base entry to the delta base cache 2. Unpack the next delta 3. Patch the unpacked delta on top of the base When the optional object reading lock is enabled, the above steps will be performed while holding the lock. However, step 2. momentarily releases it so that inflation can be performed in parallel for increased performance. Because the `base` buffer inserted in the cache at 1. is not duplicated, another thread can potentially free() it while the lock is released at 2. (e.g. when there is no space left in the cache to insert another entry). In this case, the later attempt to dereference `base` at 3. will cause a segmentation fault. This problem was observed during a multithreaded git-grep execution on a repository with large objects. To fix the race condition (and later segmentation fault), let's reorder the aforementioned steps so that `base` is only added to the cache at the end. This will prevent the buffer from being released by another thread while it is still in use. An alternative solution which would not require the reordering would be to duplicate `base` before inserting it in the cache. However, as Phil Hord mentioned, memcpy()'ing large bases can negatively affect performance: in his experiments, this alternative approach slowed git-grep down by 10% to 20%. Reported-by: Phil Hord <phil.hord@gmail.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28fetch: do not override partial clone filterJonathan Tan3-3/+12
When a fetch with the --filter argument is made, the configured default filter is set even if one already exists. This change was made in 5e46139376 ("builtin/fetch: remove unique promisor remote limitation", 2019-06-25) - in particular, changing from: * If this is the FIRST partial-fetch request, we enable partial * on this repo and remember the given filter-spec as the default * for subsequent fetches to this remote. to: * If this is a partial-fetch request, we enable partial on * this repo if not already enabled and remember the given * filter-spec as the default for subsequent fetches to this * remote. (The given filter-spec is "remembered" even if there is already an existing one.) This is problematic whenever a lazy fetch is made, because lazy fetches are made using "git fetch --filter=blob:none", but this will also happen if the user invokes "git fetch --filter=<filter>" manually. Therefore, restore the behavior prior to 5e46139376, which writes a filter-spec only if the current fetch request is the first partial-fetch one (for that remote). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28Doc: show example scissors lineEvan Gates1-4/+3
The text tries to say the code accepts many variations that look remotely like scissors and perforation marks, but gives too little detail for users to decide what is and what is not taken as a scissors line for themselves. Instead of describing the heuristics more, just spell out what will always be accepted, namely "-- >8 --", as it would not help users to give them more choices and flexibility and be "creative" in their scissors line. Signed-off-by: Evan Gates <evan.gates@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28cmake: ensure that the `vcpkg` packages are found on WindowsJohannes Schindelin1-0/+7
On Windows, we use the `vcpkg` project to manage the dependencies, via `compat/vcbuild/`. Let's make sure that these dependencies are found by default. This is needed because we are about to recommend loading the Git worktree as a folder into Visual Studio, relying on the automatic CMake support (which would make it relatively cumbersome to adjust the search path used by CMake manually). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-28cmake: do find Git for Windows' shell interpreterJohannes Schindelin1-1/+1
By default, Git for Windows does not install its `sh.exe` into the `PATH`. However, our current `CMakeLists.txt` expects to find a shell interpreter in the `PATH`. So let's fall back to looking in the default location where Git for Windows _does_ install a relatively convenient `sh.exe`: `C:\Program Files\Git\bin\sh.exe` Helped-by: Øystein Walle <oystwa@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27config/uploadpack.txt: fix typo in `--filter=tree:<n>`Martin Ågren1-1/+1
That should be a ":", not a second "=". While at it, refer to the placeholder "<n>" as "<n>", not "n" (see, e.g., the entry just before this one). Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27config/fmt-merge-msg.txt: drop space in quoteMartin Ågren1-1/+1
We document how `merge.suppressDest` can be used to omit " into <branch name>" from the title of the merge message. It is true that we omit the space character before "into", but that lone double quote character risks ending up on the wrong side of a line break, looking a bit out of place. This currently happens with, e.g., 80-character terminals. Drop that leading quoted space. The result should be just as clear about how this option affects the formatted message. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27worktree: use skip_prefix to parse targetMartin Ågren1-7/+7
Instead of checking for "refs/heads/" using `starts_with()`, then skipping past "refs/heads/" using `strlen()`, just use `skip_prefix()`. In `is_worktree_being_rebased()`, we can adjust the indentation while we're here and lose a pair of parentheses which isn't needed and which might even make the reader wonder what they're missing and why that grouping is there. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27worktree: rename copy-pasted variableMartin Ågren1-6/+6
As the commit message of 04a3dfb8b5 ("worktree.c: check whether branch is bisected in another worktree", 2016-04-22) indicates, the function `is_worktree_being_bisected()` is based on the older function `is_worktree_being_rebased()`. This heritage can also be seen in the name of the variable where we store our return value: It was never adapted while copy-editing and remains as `found_rebase`. Rename the variable to make clear that we're looking for a bisect(ion), nothing else. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27worktree: update renamed variable in commentMartin Ågren1-1/+1
The comment above `add_head_info()` mentions "head_sha1", but it was renamed to "head_oid" in 0f05154c70 ("worktree: convert struct worktree to object_id", 2017-10-15). Update the comment. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27worktree: inline `worktree_ref()` into its only callerMartin Ågren2-18/+6
We have `strbuf_worktree_ref()`, which works on a strbuf, and a wrapper for it, `worktree_ref()` which returns a string. We even make this wrapper available through worktree.h. But it only has a single caller, sitting right next to it in worktree.c. Just inline the wrapper into its only caller. This means the caller can quite naturally reuse a single strbuf. We currently achieve something similar by having a static strbuf in the wrapper. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27wt-status: introduce wt_status_state_free_buffers()Martin Ågren4-9/+18
When we have a `struct wt_status_state`, we manually free its `branch`, `onto` and `detached_from`, or sometimes just one or two of them. Provide a function `wt_status_state_free_buffers()` which does the freeing. The callers are still aware of these fields, e.g., they check whether `branch` was populated or not. But this way, they don't need to know about *all* of them, and if `struct wt_status_state` gets more fields, they will not need to learn to free them. Users of `struct wt_status` (which contains a `wt_status_state`) already have `wt_status_collect_free_buffers()` (corresponding to `wt_status_collect()`) which we can also teach to use this new helper. Finally, note that we're currently leaving dangling pointers behind. Some callers work on a stack-allocated struct, where this is obviously ok. But for the users of `run_status()` in builtin/commit.c, there are ample opportunities for someone to mistakenly use those dangling pointers. We seem to be ok for now, but it's a use-after-free waiting to happen. Let's leave NULL-pointers behind instead. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27wt-status: print to s->fp, not stdoutMartin Ågren1-25/+32
We pass around a `FILE *` in the `struct wt_status` and almost always print to it. But in a few places, we write to `stdout` instead, either explicitly through `fprintf(stdout, ...)` or implicitly with `printf(...)` (and a few `putchar(...)`). Always be explicit about writing to `s->fp`. To the best of my understanding, this never mattered in practice because these spots are involved in various forms of `git status` which always end up at standard output anyway. When we do write to another file, it's because we're creating a commit message template, and these code paths aren't involved. But let's be consistent to help future readers and avoid future bugs. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27wt-status: replace sha1 mentions with oidMartin Ågren1-4/+4
`abbrev_sha1_in_line()` uses a `struct object_id oid` and should be fully prepared to handle non-SHA1 object ids. Rename it to `abbrev_oid_in_line()`. A few comments in `wt_status_get_detached_from()` mention "sha1". The variable they refer to was renamed in e86ab2c1cd ("wt-status: convert to struct object_id", 2017-02-21). Update the comments to reference "oid" instead. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: allow multiple groups to be specifiedJeff King5-29/+127
Now that shortlog supports reading from trailers, it can be useful to combine counts from multiple trailers, or between trailers and authors. This can be done manually by post-processing the output from multiple runs, but it's non-trivial to make sure that each name/commit pair is counted only once. This patch teaches shortlog to accept multiple --group options on the command line, and pull data from all of them. That makes it possible to run: git shortlog -ns --group=author --group=trailer:co-authored-by to get a shortlog that counts authors and co-authors equally. The implementation is mostly straightforward. The "group" enum becomes a bitfield, and the trailer key becomes a list. I didn't bother implementing the multi-group semantics for reading from stdin. It would be possible to do, but the existing matching code makes it awkward, and I doubt anybody cares. The duplicate suppression we used for trailers now covers authors and committers as well (though in non-trailer single-group mode we can skip the hash insertion and lookup, since we only see one value per commit). There is one subtlety: we now care about the case when no group bit is set (in which case we default to showing the author). The caller in builtin/log.c needs to be adapted to ask explicitly for authors, rather than relying on shortlog_init(). It would be possible with some gymnastics to make this keep working as-is, but it's not worth it for a single caller. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: parse trailer identsJeff King3-3/+30
Trailers don't necessarily contain name/email identity values, so shortlog has so far treated them as opaque strings. However, since many trailers do contain identities, it's useful to treat them as such when they can be parsed. That lets "-e" work as usual, as well as mailmap. When they can't be parsed, we'll continue with the old behavior of treating them as a single string (there's no new test for that here, since the existing tests cover a trailer like this). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: rename parse_stdin_ident()Jeff King1-3/+3
This function is actually useful for parsing any identity, whether from stdin or not. We'll need it for handling trailers. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: de-duplicate trailer valuesJeff King3-1/+88
The current documentation is vague about what happens with --group=trailer:signed-off-by when we see a commit with: Signed-off-by: One Signed-off-by: Two Signed-off-by: One We clearly should credit both "One" and "Two", but should "One" get credited twice? The current code does so, but mostly because that was the easiest thing to do. It's probably more useful to count each commit at most once. This will become especially important when we allow values from multiple sources in a future patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: match commit trailers with --groupJeff King4-1/+72
If a project uses commit trailers, this patch lets you use shortlog to see who is performing each action. For example, running: git shortlog -ns --group=trailer:reviewed-by in git.git shows who has reviewed. You can even use a custom format to see things like who has helped whom: git shortlog --format="...helped %an (%ad)" \ --group=trailer:helped-by Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27trailer: add interface for iterating over commit trailersJeff King2-2/+79
The trailer code knows how to parse out the trailers and re-format them, but there's no easy way to iterate over the trailers (you can use trailer_info, but you have to then do a bunch of extra parsing). Let's add an iteration interface that makes this easy to do. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-27shortlog: add grouping optionJeff King4-12/+65
In preparation for adding more grouping types, let's refactor the committer/author grouping code and add a user-facing option that binds them together. In particular: - the main option is now "--group", to make it clear that the various group types are mutually exclusive. The "--committer" option is an alias for "--group=committer". - we keep an enum rather than a binary flag, to prepare for more values - we prefer switch statements to ternary assignment, since other group types will need more custom code Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-26t9902: avoid using the branch name `master`Johannes Schindelin1-5/+5
The completion tests used that name unnecessarily, and it is a non-inclusive term, so let's avoid using it here. Since three of the touched test cases make use of the fact that two of the branch names (`master` and `maint`) start with the same letter (or even with the same two letters), we choose to replace the use of `master` by a name that also has that property: `main`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-26tests: avoid variations of the `master` branch nameJohannes Schindelin7-58/+58
The term `master` has a loaded history that serves as a constant reminder of racial injustice. The Git project has no desire to perpetuate this and already started avoiding it. The test suite uses variations of this name for branches other than the default one. Apart from t3200, where we just addressed this in the previous commit, those instances can be renamed in an automated manner because they do not require any changes outside of the test script, so let's do that. Seeing as the touched branches have very little (if anything) to do with the default branch, we choose to use a completely separate naming scheme: `topic_<number>` (it cannot be `topic-<number>` because t5515 uses the `test_oid` machinery with the term, and that machinery uses shell variables internally, whose names cannot contain dashes). This trick was performed by this (GNU) sed invocation: $ sed -i 's/master\([a-z0-9]\)/topic_\1/g' t/t*.sh Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-26ref-filter: plug memory leak in reach_filter()René Scharfe1-1/+3
21bf933928 (ref-filter: allow merged and no-merged filters, 2020-09-15) added an early return to reach_filter(). Avoid leaking the memory of a then unused array by postponing its allocation until we know we need it. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-26completion: complete refs after 'git restore -s'Ákos Uzonyi1-0/+7
Currently only the long version (--source=) supports completion. Add completion support to the short (-s) option too. Signed-off-by: Ákos Uzonyi <uzonyi.akos@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-26completion: use "prev" variable instead of introducing "prevword"Ákos Uzonyi1-34/+32
In both _git_checkout and _git_switch a new "prevword" variable were introduced, however the "prev" variable already contains the last word. The "prevword" variable is replaced with "prev", and the case is moved to the beginning of the function, like it's done in many other places (e.g. _git_commit). Also the indentaion of the case is fixed. Signed-off-by: Ákos Uzonyi <uzonyi.akos@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25Seventeenth batchJunio C Hamano1-0/+19
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25Merge branch 'jk/diff-highlight-blank-match-fix'Junio C Hamano1-1/+1
"diff-highlight" (in contrib/) had a logic to flush its output upon seeing a blank line but the way it detected a blank line was broken. * jk/diff-highlight-blank-match-fix: diff-highlight: correctly match blank lines for flush
2020-09-25Merge branch 'hx/push-atomic-with-cert'Junio C Hamano2-26/+51
"git push" that wants to be atomic and wants to send push certificate learned not to prepare and sign the push certificate when it fails the local check (hence due to atomicity it is known that no certificate is needed). * hx/push-atomic-with-cert: send-pack: run GPG after atomic push checking
2020-09-25Merge branch 'rs/misc-cleanups'Junio C Hamano1-8/+4
Code cleanup. * rs/misc-cleanups: pack-write: use hashwrite_be32() in write_idx_file()
2020-09-25Merge branch 'ld/p4-unshelve-fix'Junio C Hamano2-2/+5
The "unshelve" subcommand of "git p4" used incorrectly used commit^N where it meant to say commit~N to name the Nth generation ancestor, which has been corrected. * ld/p4-unshelve-fix: git-p4: use HEAD~$n to find parent commit for unshelve git-p4 unshelve: adding a commit breaks git-p4 unshelve
2020-09-25Merge branch 'jx/proc-receive-hook'Junio C Hamano48-100/+4325
"git receive-pack" that accepts requests by "git push" learned to outsource most of the ref updates to the new "proc-receive" hook. * jx/proc-receive-hook: doc: add documentation for the proc-receive hook transport: parse report options for tracking refs t5411: test updates of remote-tracking branches receive-pack: new config receive.procReceiveRefs doc: add document for capability report-status-v2 New capability "report-status-v2" for git-push receive-pack: feed report options to post-receive receive-pack: add new proc-receive hook t5411: add basic test cases for proc-receive hook transport: not report a non-head push as a branch
2020-09-25Merge branch 'ds/maintenance-part-1'Junio C Hamano24-28/+568
A "git gc"'s big brother has been introduced to take care of more repository maintenance tasks, not limited to the object database cleaning. * ds/maintenance-part-1: maintenance: add trace2 regions for task execution maintenance: add auto condition for commit-graph task maintenance: use pointers to check --auto maintenance: create maintenance.<task>.enabled config maintenance: take a lock on the objects directory maintenance: add --task option maintenance: add commit-graph task maintenance: initialize task array maintenance: replace run_auto_gc() maintenance: add --quiet option maintenance: create basic maintenance runner
2020-09-25sequencer: stop abbreviating stopped-sha fileJunio C Hamano1-5/+6
The object name written to this file is not exposed to end-users and the only reader of this file immediately expands it back to a full object name. Stop abbreviating while writing, and expect a full object name while reading, which simplifies the code a bit. 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-25maintenance: add start/stop subcommandsDerrick Stolee8-0/+207
Add new subcommands to 'git maintenance' that start or stop background maintenance using 'cron', when available. This integration is as simple as I could make it, barring some implementation complications. The schedule is laid out as follows: 0 1-23 * * * $cmd maintenance run --schedule=hourly 0 0 * * 1-6 $cmd maintenance run --schedule=daily 0 0 * * 0 $cmd maintenance run --schedule=weekly where $cmd is a properly-qualified 'git for-each-repo' execution: $cmd=$path/git --exec-path=$path for-each-repo --config=maintenance.repo where $path points to the location of the Git executable running 'git maintenance start'. This is critical for systems with multiple versions of Git. Specifically, macOS has a system version at '/usr/bin/git' while the version that users can install resides at '/usr/local/bin/git' (symlinked to '/usr/local/libexec/git-core/git'). This will also use your locally-built version if you build and run this in your development environment without installing first. This conditional schedule avoids having cron launch multiple 'git for-each-repo' commands in parallel. Such parallel commands would likely lead to the 'hourly' and 'daily' tasks competing over the object database lock. This could lead to to some tasks never being run! Since the --schedule=<frequency> argument will run all tasks with _at least_ the given frequency, the daily runs will also run the hourly tasks. Similarly, the weekly runs will also run the daily and hourly tasks. The GIT_TEST_CRONTAB environment variable is not intended for users to edit, but instead as a way to mock the 'crontab [-l]' command. This variable is set in test-lib.sh to avoid a future test from accidentally running anything with the cron integration from modifying the user's schedule. We use GIT_TEST_CRONTAB='test-tool crontab <file>' in our tests to check how the schedule is modified in 'git maintenance (start|stop)' commands. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25maintenance: add [un]register subcommandsDerrick Stolee3-2/+84
In preparation for launching background maintenance from the 'git maintenance' builtin, create register/unregister subcommands. These commands update the new 'maintenance.repos' config option in the global config so the background maintenance job knows which repositories to maintain. These commands allow users to add a repository to the background maintenance list without disrupting the actual maintenance mechanism. For example, a user can run 'git maintenance register' when no background maintenance is running and it will not start the background maintenance. A later update to start running background maintenance will then pick up this repository automatically. The opposite example is that a user can run 'git maintenance unregister' to remove the current repository from background maintenance without halting maintenance for other repositories. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25for-each-repo: run subcommands on configured reposDerrick Stolee8-0/+152
It can be helpful to store a list of repositories in global or system config and then iterate Git commands on that list. Create a new builtin that makes this process simple for experts. We will use this builtin to run scheduled maintenance on all configured repositories in a future change. The test is very simple, but does highlight that the "--" argument is optional. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25maintenance: add --schedule option and configDerrick Stolee4-4/+118
Maintenance currently triggers when certain data-size thresholds are met, such as number of pack-files or loose objects. Users may want to run certain maintenance tasks based on frequency instead. For example, a user may want to perform a 'prefetch' task every hour, or 'gc' task every day. To help these users, update the 'git maintenance run' command to include a '--schedule=<frequency>' option. The allowed frequencies are 'hourly', 'daily', and 'weekly'. These values are also allowed in a new config value 'maintenance.<task>.schedule'. The 'git maintenance run --schedule=<frequency>' checks the '*.schedule' config value for each enabled task to see if the configured frequency is at least as frequent as the frequency from the '--schedule' argument. We use the following order, for full clarity: 'hourly' > 'daily' > 'weekly' Use new 'enum schedule_priority' to track these values numerically. The following cron table would run the scheduled tasks with the correct frequencies: 0 1-23 * * * git -C <repo> maintenance run --schedule=hourly 0 0 * * 1-6 git -C <repo> maintenance run --schedule=daily 0 0 * * 0 git -C <repo> maintenance run --schedule=weekly This cron schedule will run --schedule=hourly every hour except at midnight. This avoids a concurrent run with the --schedule=daily that runs at midnight every day except the first day of the week. This avoids a concurrent run with the --schedule=weekly that runs at midnight on the first day of the week. Since --schedule=daily also runs the 'hourly' tasks and --schedule=weekly runs the 'hourly' and 'daily' tasks, we will still see all tasks run with the proper frequencies. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25maintenance: optionally skip --auto processDerrick Stolee3-0/+24
Some commands run 'git maintenance run --auto --[no-]quiet' after doing their normal work, as a way to keep repositories clean as they are used. Currently, users who do not want this maintenance to occur would set the 'gc.auto' config option to 0 to avoid the 'gc' task from running. However, this does not stop the extra process invocation. On Windows, this extra process invocation can be more expensive than necessary. Allow users to drop this extra process by setting 'maintenance.auto' to 'false'. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25maintenance: add incremental-repack auto conditionDerrick Stolee3-0/+68
The incremental-repack task updates the multi-pack-index by deleting pack- files that have been replaced with new packs, then repacking a batch of small pack-files into a larger pack-file. This incremental repack is faster than rewriting all object data, but is slower than some other maintenance activities. The 'maintenance.incremental-repack.auto' config option specifies how many pack-files should exist outside of the multi-pack-index before running the step. These pack-files could be created by 'git fetch' commands or by the loose-objects task. The default value is 10. Setting the option to zero disables the task with the '--auto' option, and a negative value makes the task run every time. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25maintenance: auto-size incremental-repack batchDerrick Stolee2-3/+76
When repacking during the 'incremental-repack' task, we use the --batch-size option in 'git multi-pack-index repack'. The initial setting used --batch-size=0 to repack everything into a single pack-file. This is not sustainable for a large repository. The amount of work required is also likely to use too many system resources for a background job. Update the 'incremental-repack' task by dynamically computing a --batch-size option based on the current pack-file structure. The dynamic default size is computed with this idea in mind for a client repository that was cloned from a very large remote: there is likely one "big" pack-file that was created at clone time. Thus, do not try repacking it as it is likely packed efficiently by the server. Instead, we select the second-largest pack-file, and create a batch size that is one larger than that pack-file. If there are three or more pack-files, then this guarantees that at least two will be combined into a new pack-file. Of course, this means that the second-largest pack-file size is likely to grow over time and may eventually surpass the initially-cloned pack-file. Recall that the pack-file batch is selected in a greedy manner: the packs are considered from oldest to newest and are selected if they have size smaller than the batch size until the total selected size is larger than the batch size. Thus, that oldest "clone" pack will be first to repack after the new data creates a pack larger than that. We also want to place some limits on how large these pack-files become, in order to bound the amount of time spent repacking. A maximum batch-size of two gigabytes means that large repositories will never be packed into a single pack-file using this job, but also that repack is rather expensive. This is a trade-off that is valuable to have if the maintenance is being run automatically or in the background. Users who truly want to optimize for space and performance (and are willing to pay the upfront cost of a full repack) can use the 'gc' task to do so. Create a test for this two gigabyte limit by creating an EXPENSIVE test that generates two pack-files of roughly 2.5 gigabytes in size, then performs an incremental repack. Check that the --batch-size argument in the subcommand uses the hard-coded maximum. Helped-by: Chris Torek <chris.torek@gmail.com> Reported-by: Son Luong Ngoc <sluongng@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25maintenance: add incremental-repack taskDerrick Stolee4-0/+133
The previous change cleaned up loose objects using the 'loose-objects' that can be run safely in the background. Add a similar job that performs similar cleanups for pack-files. One issue with running 'git repack' is that it is designed to repack all pack-files into a single pack-file. While this is the most space-efficient way to store object data, it is not time or memory efficient. This becomes extremely important if the repo is so large that a user struggles to store two copies of the pack on their disk. Instead, perform an "incremental" repack by collecting a few small pack-files into a new pack-file. The multi-pack-index facilitates this process ever since 'git multi-pack-index expire' was added in 19575c7 (multi-pack-index: implement 'expire' subcommand, 2019-06-10) and 'git multi-pack-index repack' was added in ce1e4a1 (midx: implement midx_repack(), 2019-06-10). The 'incremental-repack' task runs the following steps: 1. 'git multi-pack-index write' creates a multi-pack-index file if one did not exist, and otherwise will update the multi-pack-index with any new pack-files that appeared since the last write. This is particularly relevant with the background fetch job. When the multi-pack-index sees two copies of the same object, it stores the offset data into the newer pack-file. This means that some old pack-files could become "unreferenced" which I will use to mean "a pack-file that is in the pack-file list of the multi-pack-index but none of the objects in the multi-pack-index reference a location inside that pack-file." 2. 'git multi-pack-index expire' deletes any unreferenced pack-files and updaes the multi-pack-index to drop those pack-files from the list. This is safe to do as concurrent Git processes will see the multi-pack-index and not open those packs when looking for object contents. (Similar to the 'loose-objects' job, there are some Git commands that open pack-files regardless of the multi-pack-index, but they are rarely used. Further, a user that self-selects to use background operations would likely refrain from using those commands.) 3. 'git multi-pack-index repack --bacth-size=<size>' collects a set of pack-files that are listed in the multi-pack-index and creates a new pack-file containing the objects whose offsets are listed by the multi-pack-index to be in those objects. The set of pack- files is selected greedily by sorting the pack-files by modified time and adding a pack-file to the set if its "expected size" is smaller than the batch size until the total expected size of the selected pack-files is at least the batch size. The "expected size" is calculated by taking the size of the pack-file divided by the number of objects in the pack-file and multiplied by the number of objects from the multi-pack-index with offset in that pack-file. The expected size approximates how much data from that pack-file will contribute to the resulting pack-file size. The intention is that the resulting pack-file will be close in size to the provided batch size. The next run of the incremental-repack task will delete these repacked pack-files during the 'expire' step. In this version, the batch size is set to "0" which ignores the size restrictions when selecting the pack-files. It instead selects all pack-files and repacks all packed objects into a single pack-file. This will be updated in the next change, but it requires doing some calculations that are better isolated to a separate change. These steps are based on a similar background maintenance step in Scalar (and VFS for Git) [1]. This was incredibly effective for users of the Windows OS repository. After using the same VFS for Git repository for over a year, some users had _thousands_ of pack-files that combined to up to 250 GB of data. We noticed a few users were running into the open file descriptor limits (due in part to a bug in the multi-pack-index fixed by af96fe3 (midx: add packs to packed_git linked list, 2019-04-29). These pack-files were mostly small since they contained the commits and trees that were pushed to the origin in a given hour. The GVFS protocol includes a "prefetch" step that asks for pre-computed pack- files containing commits and trees by timestamp. These pack-files were grouped into "daily" pack-files once a day for up to 30 days. If a user did not request prefetch packs for over 30 days, then they would get the entire history of commits and trees in a new, large pack-file. This led to a large number of pack-files that had poor delta compression. By running this pack-file maintenance step once per day, these repos with thousands of packs spanning 200+ GB dropped to dozens of pack- files spanning 30-50 GB. This was done all without removing objects from the system and using a constant batch size of two gigabytes. Once the work was done to reduce the pack-files to small sizes, the batch size of two gigabytes means that not every run triggers a repack operation, so the following run will not expire a pack-file. This has kept these repos in a "clean" state. [1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/PackfileMaintenanceStep.cs Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25midx: use start_delayed_progress()Derrick Stolee2-12/+12
Now that the multi-pack-index may be written as part of auto maintenance at the end of a command, reduce the progress output when the operations are quick. Use start_delayed_progress() instead of start_progress(). Update t5319-multi-pack-index.sh to use GIT_PROGRESS_DELAY=0 now that the progress indicators are conditional. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25midx: enable core.multiPackIndex by defaultDerrick Stolee4-10/+13
The core.multiPackIndex setting has been around since c4d25228ebb (config: create core.multiPackIndex setting, 2018-07-12), but has been disabled by default. If a user wishes to use the multi-pack-index feature, then they must enable this config and run 'git multi-pack-index write'. The multi-pack-index feature is relatively stable now, so make the config option true by default. For users that do not use a multi-pack-index, the only extra cost will be a file lookup to see if a multi-pack-index file exists (once per process, per object directory). Also, this config option will be referenced by an upcoming "incremental-repack" task in the maintenance builtin, so move the config option into the repository settings struct. Note that if GIT_TEST_MULTI_PACK_INDEX=1, then we want to ignore the config option and treat core.multiPackIndex as enabled. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>