summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2021-04-19parallel-checkout: add configuration optionsLibravatar Matheus Tavares4-10/+54
Make parallel checkout configurable by introducing two new settings: checkout.workers and checkout.thresholdForParallelism. The first defines the number of workers (where one means sequential checkout), and the second defines the minimum number of entries to attempt parallel checkout. To decide the default value for checkout.workers, the parallel version was benchmarked during three operations in the linux repo, with cold cache: cloning v5.8, checking out v5.8 from v2.6.15 (checkout I) and checking out v5.8 from v5.7 (checkout II). The four tables below show the mean run times and standard deviations for 5 runs in: a local file system on SSD, a local file system on HDD, a Linux NFS server, and Amazon EFS (all on Linux). Each parallel checkout test was executed with the number of workers that brings the best overall results in that environment. Local SSD: Sequential 10 workers Speedup Clone 8.805 s ± 0.043 s 3.564 s ± 0.041 s 2.47 ± 0.03 Checkout I 9.678 s ± 0.057 s 4.486 s ± 0.050 s 2.16 ± 0.03 Checkout II 5.034 s ± 0.072 s 3.021 s ± 0.038 s 1.67 ± 0.03 Local HDD: Sequential 10 workers Speedup Clone 32.288 s ± 0.580 s 30.724 s ± 0.522 s 1.05 ± 0.03 Checkout I 54.172 s ± 7.119 s 54.429 s ± 6.738 s 1.00 ± 0.18 Checkout II 40.465 s ± 2.402 s 38.682 s ± 1.365 s 1.05 ± 0.07 Linux NFS server (v4.1, on EBS, single availability zone): Sequential 32 workers Speedup Clone 240.368 s ± 6.347 s 57.349 s ± 0.870 s 4.19 ± 0.13 Checkout I 242.862 s ± 2.215 s 58.700 s ± 0.904 s 4.14 ± 0.07 Checkout II 65.751 s ± 1.577 s 23.820 s ± 0.407 s 2.76 ± 0.08 EFS (v4.1, replicated over multiple availability zones): Sequential 32 workers Speedup Clone 922.321 s ± 2.274 s 210.453 s ± 3.412 s 4.38 ± 0.07 Checkout I 1011.300 s ± 7.346 s 297.828 s ± 0.964 s 3.40 ± 0.03 Checkout II 294.104 s ± 1.836 s 126.017 s ± 1.190 s 2.33 ± 0.03 The above benchmarks show that parallel checkout is most effective on repositories located on an SSD or over a distributed file system. For local file systems on spinning disks, and/or older machines, the parallelism does not always bring a good performance. For this reason, the default value for checkout.workers is one, a.k.a. sequential checkout. To decide the default value for checkout.thresholdForParallelism, another benchmark was executed in the "Local SSD" setup, where parallel checkout showed to be beneficial. This time, we compared the runtime of a `git checkout -f`, with and without parallelism, after randomly removing an increasing number of files from the Linux working tree. The "sequential fallback" column below corresponds to the executions where checkout.workers was 10 but checkout.thresholdForParallelism was equal to the number of to-be-updated files plus one (so that we end up writing sequentially). Each test case was sampled 15 times, and each sample had a randomly different set of files removed. Here are the results: sequential fallback 10 workers speedup 10 files 772.3 ms ± 12.6 ms 769.0 ms ± 13.6 ms 1.00 ± 0.02 20 files 780.5 ms ± 15.8 ms 775.2 ms ± 9.2 ms 1.01 ± 0.02 50 files 806.2 ms ± 13.8 ms 767.4 ms ± 8.5 ms 1.05 ± 0.02 100 files 833.7 ms ± 21.4 ms 750.5 ms ± 16.8 ms 1.11 ± 0.04 200 files 897.6 ms ± 30.9 ms 730.5 ms ± 14.7 ms 1.23 ± 0.05 500 files 1035.4 ms ± 48.0 ms 677.1 ms ± 22.3 ms 1.53 ± 0.09 1000 files 1244.6 ms ± 35.6 ms 654.0 ms ± 38.3 ms 1.90 ± 0.12 2000 files 1488.8 ms ± 53.4 ms 658.8 ms ± 23.8 ms 2.26 ± 0.12 From the above numbers, 100 files seems to be a reasonable default value for the threshold setting. Note: Up to 1000 files, we observe a drop in the execution time of the parallel code with an increase in the number of files. This is a rather odd behavior, but it was observed in multiple repetitions. Above 1000 files, the execution time increases according to the number of files, as one would expect. About the test environments: Local SSD tests were executed on an i7-7700HQ (4 cores with hyper-threading) running Manjaro Linux. Local HDD tests were executed on an Intel(R) Xeon(R) E3-1230 (also 4 cores with hyper-threading), HDD Seagate Barracuda 7200.14 SATA 3.1, running Debian. NFS and EFS tests were executed on an Amazon EC2 c5n.xlarge instance, with 4 vCPUs. The Linux NFS server was running on a m6g.large instance with 2 vCPUSs and a 1 TB EBS GP2 volume. Before each timing, the linux repository was removed (or checked out back to its previous state), and `sync && sysctl vm.drop_caches=3` was executed. Co-authored-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-19parallel-checkout: make it truly parallelLibravatar Matheus Tavares7-27/+496
Use multiple worker processes to distribute the queued entries and call write_pc_item() in parallel for them. The items are distributed uniformly in contiguous chunks. This minimizes the chances of two workers writing to the same directory simultaneously, which could affect performance due to lock contention in the kernel. Work stealing (or any other format of re-distribution) is not implemented yet. The protocol between the main process and the workers is quite simple. They exchange binary messages packed in pkt-line format, and use PKT-FLUSH to mark the end of input (from both sides). The main process starts the communication by sending N pkt-lines, each corresponding to an item that needs to be written. These packets contain all the necessary information to load, smudge, and write the blob associated with each item. Then it waits for the worker to send back N pkt-lines containing the results for each item. The resulting packet must contain: the identification number of the item that it refers to, the status of the operation, and the lstat() data gathered after writing the file (iff the operation was successful). For now, checkout always uses a hardcoded value of 2 workers, only to demonstrate that the parallel checkout framework correctly divides and writes the queued entries. The next patch will add user configurations and define a more reasonable default, based on tests with the said settings. Co-authored-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Co-authored-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-19unpack-trees: add basic support for parallel checkoutLibravatar Matheus Tavares5-3/+418
This new interface allows us to enqueue some of the entries being checked out to later uncompress them, apply in-process filters, and write out the files in parallel. For now, the parallel checkout machinery is enabled by default and there is no user configuration, but run_parallel_checkout() just writes the queued entries in sequence (without spawning additional workers). The next patch will actually implement the parallelism and, later, we will make it configurable. Note that, to avoid potential data races, not all entries are eligible for parallel checkout. Also, paths that collide on disk (e.g. case-sensitive paths in case-insensitive file systems), are detected by the parallel checkout code and skipped, so that they can be safely sequentially handled later. The collision detection works like the following: - If the collision was at basename (e.g. 'a/b' and 'a/B'), the framework detects it by looking for EEXIST and EISDIR errors after an open(O_CREAT | O_EXCL) failure. - If the collision was at dirname (e.g. 'a/b' and 'A'), it is detected at the has_dirs_only_path() check, which is done for the leading path of each item in the parallel checkout queue. Both verifications rely on the fact that, before enqueueing an entry for parallel checkout, checkout_entry() makes sure that there is no file at the entry's path and that its leading components are all real directories. So, any later change in these conditions indicates that there was a collision (either between two parallel-eligible entries or between an eligible and an ineligible one). After all parallel-eligible entries have been processed, the collided (and thus, skipped) entries are sequentially fed to checkout_entry() again. This is similar to the way the current code deals with collisions, overwriting the previously checked out entries with the subsequent ones. The only difference is that, since we no longer create the files in the same order that they appear on index, we are not able to determine which of the colliding entries will survive on disk (for the classic code, it is always the last entry). Co-authored-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Co-authored-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-07The seventh batchLibravatar Junio C Hamano1-0/+29
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-07Merge branch 'ab/fsck-api-cleanup'Libravatar Junio C Hamano7-216/+210
Fsck API clean-up. * ab/fsck-api-cleanup: fetch-pack: use new fsck API to printing dangling submodules fetch-pack: use file-scope static struct for fsck_options fetch-pack: don't needlessly copy fsck_options fsck.c: move gitmodules_{found,done} into fsck_options fsck.c: add an fsck_set_msg_type() API that takes enums fsck.c: pass along the fsck_msg_id in the fsck_error callback fsck.[ch]: move FOREACH_FSCK_MSG_ID & fsck_msg_id from *.c to *.h fsck.c: give "FOREACH_MSG_ID" a more specific name fsck.c: undefine temporary STR macro after use fsck.c: call parse_msg_type() early in fsck_set_msg_type() fsck.h: re-order and re-assign "enum fsck_msg_type" fsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enum fsck.c: refactor fsck_msg_type() to limit scope of "int msg_type" fsck.c: rename remaining fsck_msg_id "id" to "msg_id" fsck.c: remove (mostly) redundant append_msg_id() function fsck.c: rename variables in fsck_set_msg_type() for less confusion fsck.h: use "enum object_type" instead of "int" fsck.h: use designed initializers for FSCK_OPTIONS_{DEFAULT,STRICT} fsck.c: refactor and rename common config callback
2021-04-07Merge branch 'cc/downcase-opt-help'Libravatar Junio C Hamano2-5/+5
A few option description strings started with capital letters, which were corrected. * cc/downcase-opt-help: column, range-diff: downcase option description
2021-04-07Merge branch 'js/security-md'Libravatar Junio C Hamano3-0/+183
SECURITY.md that is facing individual contributors and end users has been introduced. Also a procedure to follow when preparing embargoed releases has been spelled out. * js/security-md: Document how we do embargoed releases SECURITY: describe how to report vulnerabilities
2021-04-07Merge branch 'ps/pack-bitmap-optim'Libravatar Junio C Hamano2-0/+15
Optimize "rev-list --use-bitmap-index --objects" corner case that uses negative tags as the stopping points. * ps/pack-bitmap-optim: pack-bitmap: avoid traversal of objects referenced by uninteresting tag
2021-04-07Merge branch 'zh/commit-trailer'Libravatar Junio C Hamano3-1/+347
"git commit" learned "--trailer <key>[=<value>]" option; together with the interpret-trailers command, this will make it easier to support custom trailers. * zh/commit-trailer: commit: add --trailer option
2021-04-07Merge branch 'js/cmake-vsbuild'Libravatar Junio C Hamano2-7/+24
CMake update for vsbuild. * js/cmake-vsbuild: cmake(install): include vcpkg dlls cmake: add a preparatory work-around to accommodate `vcpkg` cmake(install): fix double .exe suffixes cmake: support SKIP_DASHED_BUILT_INS
2021-04-07Merge branch 'ds/clarify-hashwrite'Libravatar Junio C Hamano1-15/+18
The hashwrite() API uses a buffering mechanism to avoid calling write(2) too frequently. This logic has been refactored to be easier to understand. * ds/clarify-hashwrite: csum-file: make hashwrite() more readable
2021-04-07Merge branch 'ah/plugleaks'Libravatar Junio C Hamano10-56/+74
Plug or annotate remaining leaks that trigger while running the very basic set of tests. * ah/plugleaks: transport: also free remote_refs in transport_disconnect() parse-options: don't leak alias help messages parse-options: convert bitfield values to use binary shift init-db: silence template_dir leak when converting to absolute path init: remove git_init_db_config() while fixing leaks worktree: fix leak in dwim_branch() clone: free or UNLEAK further pointers when finished reset: free instead of leaking unneeded ref symbolic-ref: don't leak shortened refname in check_symref()
2021-04-02The sixth batchLibravatar Junio C Hamano1-0/+7
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-02Merge branch 'zh/format-patch-fractional-reroll-count'Libravatar Junio C Hamano6-12/+88
"git format-patch -v<n>" learned to allow a reroll count that is not an integer. * zh/format-patch-fractional-reroll-count: format-patch: allow a non-integral version numbers
2021-04-02Merge branch 'jh/simple-ipc'Libravatar Junio C Hamano21-52/+3315
A simple IPC interface gets introduced to build services like fsmonitor on top. * jh/simple-ipc: t0052: add simple-ipc tests and t/helper/test-simple-ipc tool simple-ipc: add Unix domain socket implementation unix-stream-server: create unix domain socket under lock unix-socket: disallow chdir() when creating unix domain sockets unix-socket: add backlog size option to unix_stream_listen() unix-socket: eliminate static unix_stream_socket() helper function simple-ipc: add win32 implementation simple-ipc: design documentation for new IPC mechanism pkt-line: add options argument to read_packetized_to_strbuf() pkt-line: add PACKET_READ_GENTLE_ON_READ_ERROR option pkt-line: do not issue flush packets in write_packetized_*() pkt-line: eliminate the need for static buffer in packet_write_gently()
2021-04-02Merge branch 'mt/parallel-checkout-part-1'Libravatar Junio C Hamano11-137/+277
Preparatory API changes for parallel checkout. * mt/parallel-checkout-part-1: entry: add checkout_entry_ca() taking preloaded conv_attrs entry: move conv_attrs lookup up to checkout_entry() entry: extract update_ce_after_write() from write_entry() entry: make fstat_output() and read_blob_entry() public entry: extract a header file for entry.c functions convert: add classification for conv_attrs struct convert: add get_stream_filter_ca() variant convert: add [async_]convert_to_working_tree_ca() variants convert: make convert_attrs() and convert structs public
2021-03-30The fifth batchLibravatar Junio C Hamano1-12/+17
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-30Merge branch 'jc/doc-format-patch-clarify'Libravatar Junio C Hamano1-2/+19
Explain pieces of the format-patch output upfront before the rest of the documentation starts referring to them. * jc/doc-format-patch-clarify: format-patch: give an overview of what a "patch" message is
2021-03-30Merge branch 'ab/detox-gettext-tests'Libravatar Junio C Hamano1-1/+1
Testfix. * ab/detox-gettext-tests: mktag tests: fix broken "&&" chain
2021-03-30Merge branch 'hx/pack-objects-chunk-comment'Libravatar Junio C Hamano1-2/+2
Comment update. * hx/pack-objects-chunk-comment: pack-objects: fix comment of reused_chunk.difference
2021-03-30Merge branch 'rf/send-email-hookspath'Libravatar Junio C Hamano3-1/+46
"git send-email" learned to honor the core.hooksPath configuration. * rf/send-email-hookspath: git-send-email: Respect core.hooksPath setting
2021-03-30Merge branch 'ab/remove-rebase-usebuiltin'Libravatar Junio C Hamano3-34/+0
Remove the final hint that we used to have a scripted "git rebase". * ab/remove-rebase-usebuiltin: rebase: remove transitory rebase.useBuiltin setting & env
2021-03-30Merge branch 'cs/http-use-basic-after-failed-negotiate'Libravatar Junio C Hamano1-7/+8
When accessing a server with a URL like https://user:pass@site/, we did not to fall back to the basic authentication with the credential material embedded in the URL after the "Negotiate" authentication failed. Now we do. * cs/http-use-basic-after-failed-negotiate: remote-curl: fall back to basic auth if Negotiate fails
2021-03-30Merge branch 'ab/diff-no-index-tests'Libravatar Junio C Hamano1-0/+60
More test coverage over "diff --no-index". * ab/diff-no-index-tests: diff --no-index tests: test mode normalization diff --no-index tests: add test for --exit-code
2021-03-30Merge branch 'ab/read-tree'Libravatar Junio C Hamano11-147/+205
Code simplification by removing support for a caller that is long gone. * ab/read-tree: tree.h API: simplify read_tree_recursive() signature tree.h API: expose read_tree_1() as read_tree_at() archive: stop passing "stage" through read_tree_recursive() ls-files: refactor away read_tree() ls-files: don't needlessly pass around stage variable tree.c API: move read_tree() into builtin/ls-files.c ls-files tests: add meaningful --with-tree tests show tests: add test for "git show <tree>"
2021-03-30Merge branch 'bs/asciidoctor-installation-hints'Libravatar Junio C Hamano1-1/+3
Doc update. * bs/asciidoctor-installation-hints: INSTALL: note on using Asciidoctor to build doc
2021-03-30Merge branch 'mt/checkout-remove-nofollow'Libravatar Junio C Hamano5-33/+39
When "git checkout" removes a path that does not exist in the commit it is checking out, it wasn't careful enough not to follow symbolic links, which has been corrected. * mt/checkout-remove-nofollow: checkout: don't follow symlinks when removing entries symlinks: update comment on threaded_check_leading_path()
2021-03-29column, range-diff: downcase option descriptionLibravatar Chinmoy Chakraborty2-5/+5
It is customary not to begin the help text for each option given to the parse-options API with a capital letter. Various (sub)commands' option arrays don't follow the guideline provided by the parse_options Documentation regarding the descriptions. Downcase the first word of some option descriptions for "column" and "range-diff". Signed-off-by: Chinmoy Chakraborty <chinmoy12c@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-29cmake(install): include vcpkg dllsLibravatar Dennis Ameling2-0/+9
Our CMake configuration generates not only build definitions, but also install definitions: After building Git using `msbuild git.sln`, the built artifacts can be installed via `msbuild INSTALL.vcxproj`. To specify _where_ the files should be installed, the `-DCMAKE_INSTALL_PREFIX=<path>` option can be used when running CMake. However, this process would really only install the files that were just built. On Windows, we need more than that: We also need the `.dll` files of the dependencies (such as libcurl). The `vcpkg` ecosystem, which we use to obtain those dependencies, can be asked to install said `.dll` files really easily, so let's do that. This requires more than just the built `vcpkg` artifacts in the CI build definition; We now clone the `vcpkg` repository so that the relevant CMake scripts are available, in particular the ones related to defining the toolchain. Signed-off-by: Dennis Ameling <dennis@dennisameling.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-29cmake: add a preparatory work-around to accommodate `vcpkg`Libravatar Johannes Schindelin1-5/+9
We are about to add support for installing the `.dll` files of Git's dependencies (such as libcurl) in the CMake configuration. The `vcpkg` ecosystem from which we get said dependencies makes that relatively easy: simply turn on `X_VCPKG_APPLOCAL_DEPS_INSTALL`. However, current `vcpkg` introduces a limitation if one does that: While it is totally cool with CMake to specify multiple targets within one invocation of `install(TARGETS ...) (at least according to https://cmake.org/cmake/help/latest/command/install.html#command:install), `vcpkg`'s parser insists on a single target per `install(TARGETS ...)` invocation. Well, that's easily accomplished: Let's feed the targets individually to the `install(TARGETS ...)` function in a `foreach()` look. This also has the advantage that we do not have to manually cull off the two entries from the `${PROGRAMS_BUILT}` array before scheduling the remainder to be installed into `libexec/git-core`. Instead, we iterate through the array and decide for each entry where it wants to go. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fetch-pack: use new fsck API to printing dangling submodulesLibravatar Ævar Arnfjörð Bjarmason4-51/+39
Refactor the check added in 5476e1efde (fetch-pack: print and use dangling .gitmodules, 2021-02-22) to make use of us now passing the "msg_id" to the user defined "error_func". We can now compare against the FSCK_MSG_GITMODULES_MISSING instead of parsing the generated message. Let's also replace register_found_gitmodules() with directly manipulating the "gitmodules_found" member. A recent commit moved it into "fsck_options" so we could do this here. I'm sticking this callback in fsck.c. Perhaps in the future we'd like to accumulate such callbacks into another file (maybe fsck-cb.c, similar to parse-options-cb.c?), but while we've got just the one let's just put it into fsck.c. A better alternative in this case would be some library some more obvious library shared by fetch-pack.c ad builtin/index-pack.c, but there isn't such a thing. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fetch-pack: use file-scope static struct for fsck_optionsLibravatar Ævar Arnfjörð Bjarmason1-3/+3
Change code added in 5476e1efde (fetch-pack: print and use dangling .gitmodules, 2021-02-22) so that we use a file-scoped "static struct fsck_options" instead of defining one in the "fsck_gitmodules_oids()" function. We use this pattern in all of builtin/{fsck,index-pack,mktag,unpack-objects}.c. It's odd to see fetch-pack be the odd one out. One might think that we're using other fsck_options structs in fetch-pack, or doing on fsck twice there, but we're not. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fetch-pack: don't needlessly copy fsck_optionsLibravatar Ævar Arnfjörð Bjarmason1-7/+3
Change the behavior of the .gitmodules validation added in 5476e1efde (fetch-pack: print and use dangling .gitmodules, 2021-02-22) so we're using one "fsck_options". I found that code confusing to read. One might think that not setting up the error_func earlier means that we're relying on the "error_func" not being set in some code in between the two hunks being modified here. But we're not, all we're doing in the rest of "cmd_index_pack()" is further setup by calling fsck_set_msg_types(), and assigning to do_fsck_object. So there was no reason in 5476e1efde to make a shallow copy of the fsck_options struct before setting error_func. Let's just do this setup at the top of the function, along with the "walk" assignment. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: move gitmodules_{found,done} into fsck_optionsLibravatar Ævar Arnfjörð Bjarmason3-15/+19
Move the gitmodules_{found,done} static variables added in 159e7b080bf (fsck: detect gitmodules files, 2018-05-02) into the fsck_options struct. It makes sense to keep all the context in the same place. This requires changing the recently added register_found_gitmodules() function added in 5476e1efde (fetch-pack: print and use dangling .gitmodules, 2021-02-22) to take fsck_options. That function will be removed in a subsequent commit, but as it'll require the new gitmodules_found attribute of "fsck_options" we need this intermediate step first. An earlier version of this patch removed the small amount of duplication we now have between FSCK_OPTIONS_{DEFAULT,STRICT} with a FSCK_OPTIONS_COMMON macro. I don't think such de-duplication is worth it for this amount of copy/pasting. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: add an fsck_set_msg_type() API that takes enumsLibravatar Ævar Arnfjörð Bjarmason3-11/+22
Change code I added in acf9de4c94e (mktag: use fsck instead of custom verify_tag(), 2021-01-05) to make use of a new API function that takes the fsck_msg_{id,type} types, instead of arbitrary strings that we'll (hopefully) parse into those types. At the time that the fsck_set_msg_type() API was introduced in 0282f4dced0 (fsck: offer a function to demote fsck errors to warnings, 2015-06-22) it was only intended to be used to parse user-supplied data. For things that are purely internal to the C code it makes sense to have the compiler check these arguments, and to skip the sanity checking of the data in fsck_set_msg_type() which is redundant to checks we get from the compiler. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: pass along the fsck_msg_id in the fsck_error callbackLibravatar Ævar Arnfjörð Bjarmason5-6/+14
Change the fsck_error callback to also pass along the fsck_msg_id. Before this change the only way to get the message id was to parse it back out of the "message". Let's pass it down explicitly for the benefit of callers that might want to use it, as discussed in [1]. Passing the msg_type is now redundant, as you can always get it back from the msg_id, but I'm not changing that convention. It's really common to need the msg_type, and the report() function itself (which calls "fsck_error") needs to call fsck_msg_type() to discover it. Let's not needlessly re-do that work in the user callback. 1. https://lore.kernel.org/git/87blcja2ha.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.[ch]: move FOREACH_FSCK_MSG_ID & fsck_msg_id from *.c to *.hLibravatar Ævar Arnfjörð Bjarmason2-66/+66
Move the FOREACH_FSCK_MSG_ID macro and the fsck_msg_id enum it helps define from fsck.c to fsck.h. This is in preparation for having non-static functions take the fsck_msg_id as an argument. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: give "FOREACH_MSG_ID" a more specific nameLibravatar Ævar Arnfjörð Bjarmason1-3/+3
Rename the FOREACH_MSG_ID macro to FOREACH_FSCK_MSG_ID in preparation for moving it over to fsck.h. It's good convention to name macros in *.h files in such a way as to clearly not clash with any other names in other files. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: undefine temporary STR macro after useLibravatar Ævar Arnfjörð Bjarmason1-0/+1
In f417eed8cde (fsck: provide a function to parse fsck message IDs, 2015-06-22) the "STR" macro was introduced, but that short macro name was not undefined after use as was done earlier in the same series for the MSG_ID macro in c99ba492f1c (fsck: introduce identifiers for fsck messages, 2015-06-22). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: call parse_msg_type() early in fsck_set_msg_type()Libravatar Ævar Arnfjörð Bjarmason1-2/+1
There's no reason to defer the calling of parse_msg_type() until after we've checked if the "id < 0". This is not a hot codepath, and parse_msg_type() itself may die on invalid input. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.h: re-order and re-assign "enum fsck_msg_type"Libravatar Ævar Arnfjörð Bjarmason1-4/+6
Change the values in the "enum fsck_msg_type" from being manually assigned to using default C enum values. This means we end up with a FSCK_IGNORE=0, which was previously defined as "2". I'm confident that nothing relies on these values, we always compare them for equality. Let's not omit "0" so it won't be assumed that we're using these as a boolean somewhere. This also allows us to re-structure the fields to mark which are "private" v.s. "public". See the preceding commit for a rationale for not simply splitting these into two enums, namely that this is used for both the private and public fsck API. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enumLibravatar Ævar Arnfjörð Bjarmason5-20/+25
Move the FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} defines into a new fsck_msg_type enum. These defines were originally introduced in: - ba002f3b28a (builtin-fsck: move common object checking code to fsck.c, 2008-02-25) - f50c4407305 (fsck: disallow demoting grave fsck errors to warnings, 2015-06-22) - efaba7cc77f (fsck: optionally ignore specific fsck issues completely, 2015-06-22) - f27d05b1704 (fsck: allow upgrading fsck warnings to errors, 2015-06-22) The reason these were defined in two different places is because we use FSCK_{IGNORE,INFO,FATAL} only in fsck.c, but FSCK_{ERROR,WARN} are used by external callbacks. Untangling that would take some more work, since we expose the new "enum fsck_msg_type" to both. Similar to "enum object_type" it's not worth structuring the API in such a way that only those who need FSCK_{ERROR,WARN} pass around a different type. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: refactor fsck_msg_type() to limit scope of "int msg_type"Libravatar Ævar Arnfjörð Bjarmason1-7/+5
Refactor "if options->msg_type" and other code added in 0282f4dced0 (fsck: offer a function to demote fsck errors to warnings, 2015-06-22) to reduce the scope of the "int msg_type" variable. This is in preparation for changing its type in a subsequent commit, only using it in the "!options->msg_type" scope makes that change This also brings the code in line with the fsck_set_msg_type() function (also added in 0282f4dced0), which does a similar check for "!options->msg_type". Another minor benefit is getting rid of the style violation of not having braces for the body of the "if". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: rename remaining fsck_msg_id "id" to "msg_id"Libravatar Ævar Arnfjörð Bjarmason1-3/+3
Rename the remaining variables of type fsck_msg_id from "id" to "msg_id". This change is relatively small, and is worth the churn for a later change where we have different id's in the "report" function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: remove (mostly) redundant append_msg_id() functionLibravatar Ævar Arnfjörð Bjarmason1-19/+2
Remove the append_msg_id() function in favor of calling prepare_msg_ids(). We already have code to compute the camel-cased msg_id strings in msg_id_info, let's use it. When the append_msg_id() function was added in 71ab8fa840f (fsck: report the ID of the error/warning, 2015-06-22) the prepare_msg_ids() function didn't exist. When prepare_msg_ids() was added in a46baac61eb (fsck: factor out msg_id_info[] lazy initialization code, 2018-05-26) this code wasn't moved over to lazy initialization. This changes the behavior of the code to initialize all the messages instead of just camel-casing the one we need on the fly. Since the common case is that we're printing just one message this is mostly redundant work. But that's OK in this case, reporting this fsck issue to the user isn't performance-sensitive. If we were somehow doing so in a tight loop (in a hopelessly broken repository?) this would help, since we'd save ourselves from re-doing this work for identical messages, we could just grab the prepared string from msg_id_info after the first invocation. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.c: rename variables in fsck_set_msg_type() for less confusionLibravatar Ævar Arnfjörð Bjarmason2-13/+13
Rename variables in a function added in 0282f4dced0 (fsck: offer a function to demote fsck errors to warnings, 2015-06-22). It was needlessly confusing that it took a "msg_type" argument, but then later declared another "msg_type" of a different type. Let's rename that to "severity", and rename "id" to "msg_id" and "msg_id" to "msg_id_str" etc. This will make a follow-up change smaller. While I'm at it properly indent the fsck_set_msg_type() argument list. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.h: use "enum object_type" instead of "int"Libravatar Ævar Arnfjörð Bjarmason4-4/+8
Change the fsck_walk_func to use an "enum object_type" instead of an "int" type. The types are compatible, and ever since this was added in 355885d5315 (add generic, type aware object chain walker, 2008-02-25) we've used entries from object_type (OBJ_BLOB etc.). So this doesn't really change anything as far as the generated code is concerned, it just gives the compiler more information and makes this easier to read. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-28fsck.h: use designed initializers for FSCK_OPTIONS_{DEFAULT,STRICT}Libravatar Ævar Arnfjörð Bjarmason1-2/+8
Refactor the definitions of FSCK_OPTIONS_{DEFAULT,STRICT} to use designated initializers. This allows us to omit those fields that are initialized to 0 or NULL. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-27cmake(install): fix double .exe suffixesLibravatar Dennis Ameling1-2/+2
By mistake, the `.exe` extension is appended _twice_ when installing the dashed executables into `libexec/git-core/` on Windows (the extension is already appended when adding items to the `git_links` list in the `#Creating hardlinks` section). Signed-off-by: Dennis Ameling <dennis@dennisameling.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-27cmake: support SKIP_DASHED_BUILT_INSLibravatar Johannes Schindelin1-0/+4
Just like the Makefile-based build learned to skip hard-linking the dashed built-ins in 179227d6e21 (Optionally skip linking/copying the built-ins, 2020-09-21), this patch teaches the CMake-based build the same trick. Note: In contrast to the Makefile-based process, the built-ins would only be linked during installation, not already when Git is built. Therefore, the CMake-based build that we use in our CI builds _already_ does not link those built-ins (because the files are not installed anywhere, they are used to run the test suite in-place). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>