summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* midx: allow marking a pack as preferredTaylor Blau2021-04-017-18/+148
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When multiple packs in the multi-pack index contain the same object, the MIDX machinery must make a choice about which pack it associates with that object. Prior to this patch, the lowest-ordered[1] pack was always selected. Pack selection for duplicate objects is relatively unimportant today, but it will become important for multi-pack bitmaps. This is because we can only invoke the pack-reuse mechanism when all of the bits for reused objects come from the reuse pack (in order to ensure that all reused deltas can find their base objects in the same pack). To encourage the pack selection process to prefer one pack over another (the pack to be preferred is the one a caller would like to later use as a reuse pack), introduce the concept of a "preferred pack". When provided, the MIDX code will always prefer an object found in a preferred pack over any other. No format changes are required to store the preferred pack, since it will be able to be inferred with a corresponding MIDX bitmap, by looking up the pack associated with the object in the first bit position (this ordering is described in detail in a subsequent commit). [1]: the ordering is specified by MIDX internals; for our purposes we can consider the "lowest ordered" pack to be "the one with the most-recent mtime. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t/helper/test-read-midx.c: add '--show-objects'Taylor Blau2021-03-301-4/+20
| | | | | | | | | | | | | | | | The 'read-midx' helper is used in places like t5319 to display basic information about a multi-pack-index. In the next patch, the MIDX writing machinery will learn a new way to choose from which pack an object is selected when multiple copies of that object exist. To disambiguate which pack introduces an object so that this feature can be tested, add a '--show-objects' option which displays additional information about each object in the MIDX. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/multi-pack-index.c: display usage on unrecognized commandTaylor Blau2021-03-301-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When given a sub-command that it doesn't understand, 'git multi-pack-index' dies with the following message: $ git multi-pack-index bogus fatal: unrecognized subcommand: bogus Instead of 'die()'-ing, we can display the usage text, which is much more helpful: $ git.compile multi-pack-index bogus error: unrecognized subcommand: bogus usage: git multi-pack-index [<options>] write or: git multi-pack-index [<options>] verify or: git multi-pack-index [<options>] expire or: git multi-pack-index [<options>] repack [--batch-size=<size>] --object-dir <file> object directory containing set of packfile and pack-index pairs --progress force progress reporting While we're at it, clean up some duplication between the "no sub-command" and "unrecognized sub-command" conditionals. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/multi-pack-index.c: don't enter bogus cmd_modeTaylor Blau2021-03-301-2/+8
| | | | | | | | | | | | Even before the recent refactoring, 'git multi-pack-index' calls 'trace2_cmd_mode()' before verifying that the sub-command is recognized. Push this call down into the individual sub-commands so that we don't enter a bogus command mode. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/multi-pack-index.c: split sub-commandsTaylor Blau2021-03-301-25/+105
| | | | | | | | | | | | | | | | | | | | | | | | Handle sub-commands of the 'git multi-pack-index' builtin (e.g., "write", "repack", etc.) separately from one another. This allows sub-commands with unique options, without forcing cmd_multi_pack_index() to reject invalid combinations itself. This comes at the cost of some duplication and boilerplate. Luckily, the duplication is reduced to a minimum, since common options are shared among sub-commands due to a suggestion by Ævar. (Sub-commands do have to retain the common options, too, since this builtin accepts common options on either side of the sub-command). Roughly speaking, cmd_multi_pack_index() parses options (including common ones), and stops at the first non-option, which is the sub-command. It then dispatches to the appropriate sub-command, which parses the remaining options (also including common options). Unknown options are kept by the sub-commands in order to detect their presence (and complain that too many arguments were given). Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/multi-pack-index.c: define common usage with a macroTaylor Blau2021-03-301-1/+16
| | | | | | | | | | | | Factor out the usage message into pieces corresponding to each mode. This avoids options specific to one sub-command from being shared with another in the usage. A subsequent commit will use these #define macros to have usage variables for each sub-command without duplicating their contents. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/multi-pack-index.c: don't handle 'progress' separatelyTaylor Blau2021-03-301-5/+3
| | | | | | | | | | | | | Now that there is a shared 'flags' member in the options structure, there is no need to keep track of whether to force progress or not, since ultimately the decision of whether or not to show a progress meter is controlled by a bit in the flags member. Manipulate that bit directly, and drop the now-unnecessary 'progress' field while we're at it. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* builtin/multi-pack-index.c: inline 'flags' with optionsTaylor Blau2021-03-301-7/+6
| | | | | | | | | | | | | | | | | | Subcommands of the 'git multi-pack-index' command (e.g., 'write', 'verify', etc.) will want to optionally change a set of shared flags that are eventually passed to the MIDX libraries. Right now, options and flags are handled separately. That's fine, since the options structure is never passed around. But a future patch will make it so that common options shared by all sub-commands are defined in a common location. That means that "flags" would have to become a global variable. Group it with the options structure so that we reduce the number of global variables we have overall. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ds/chunked-file-api' into tb/reverse-midxJunio C Hamano2021-02-2510-468/+655
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * ds/chunked-file-api: commit-graph.c: display correct number of chunks when writing chunk-format: add technical docs chunk-format: restore duplicate chunk checks midx: use 64-bit multiplication for chunk sizes midx: use chunk-format read API commit-graph: use chunk-format read API chunk-format: create read chunk API midx: use chunk-format API in write_midx_internal() midx: drop chunk progress during write midx: return success/failure in chunk write methods midx: add num_large_offsets to write_midx_context midx: add pack_perm to write_midx_context midx: add entries to write_midx_context midx: use context in write_midx_pack_names() midx: rename pack_info to write_midx_context commit-graph: use chunk-format write API chunk-format: create chunk format write API commit-graph: anonymize data in chunk_write_fn
| * commit-graph.c: display correct number of chunks when writingTaylor Blau2021-02-241-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When writing a commit-graph, a progress meter is shown which indicates the number of pieces of data to write (one per commit in each chunk). In 47410aa837 (commit-graph: use chunk-format write API, 2021-02-18), the number of chunks became tracked by the new chunk-format API. But a stray local variable was left behind from when write_commit_graph_file() used to keep track of the same. Since this was no longer updated after 47410aa837, the progress meter appeared broken: $ git commit-graph write --reachable Expanding reachable commits in commit graph: 837569, done. Writing out commit graph in 3 passes: 166% (4187845/2512707), done. Drop the local variable and rely instead on the chunk-format API to tell us the correct number of chunks. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * chunk-format: add technical docsDerrick Stolee2021-02-183-0/+122
| | | | | | | | | | | | | | | | | | | | | | The chunk-based file format is now an API in the code, but we should also take time to document it as a file format. Specifically, it matches the CHUNK LOOKUP sections of the commit-graph and multi-pack-index files, but there are some commonalities that should be grouped in this document. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * chunk-format: restore duplicate chunk checksDerrick Stolee2021-02-181-0/+9
| | | | | | | | | | | | | | | | | | | | | | Before refactoring into the chunk-format API, the commit-graph parsing logic included checks for duplicate chunks. It is unlikely that we would desire a chunk-based file format that allows duplicate chunk IDs in the table of contents, so add duplicate checks into read_table_of_contents(). Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: use 64-bit multiplication for chunk sizesDerrick Stolee2021-02-181-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | When calculating the sizes of certain chunks, we should use 64-bit multiplication always. This allows us to properly predict the chunk sizes without risk of overflow. Other possible overflows were discovered by evaluating each multiplication in midx.c and ensuring that at least one side of the operator was of type size_t or off_t. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: use chunk-format read APIDerrick Stolee2021-02-182-50/+29
| | | | | | | | | | | | | | | | | | | | Instead of parsing the table of contents directly, use the chunk-format API methods read_table_of_contents() and pair_chunk(). In particular, we can use the return value of pair_chunk() to generate an error when a required chunk is missing. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * commit-graph: use chunk-format read APIDerrick Stolee2021-02-182-106/+55
| | | | | | | | | | | | | | | | | | | | Instead of parsing the table of contents directly, use the chunk-format API methods read_table_of_contents() and pair_chunk(). While the current implementation loses the duplicate-chunk detection, that will be added in a future change. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * chunk-format: create read chunk APIDerrick Stolee2021-02-182-0/+127
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add the capability to read the table of contents, then pair the chunks with necessary logic using read_chunk_fn pointers. Callers will be added in future changes, but the typical outline will be: 1. initialize a 'struct chunkfile' with init_chunkfile(NULL). 2. call read_table_of_contents(). 3. for each chunk to parse, a. call pair_chunk() to assign a pointer with the chunk position, or b. call read_chunk() to run a callback on the chunk start and size. 4. call free_chunkfile() to clear the 'struct chunkfile' data. We are re-using the anonymous 'struct chunkfile' data, as it is internal to the chunk-format API. This gives it essentially two modes: write and read. If the same struct instance was used for both reads and writes, then there would be failures. 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>
| * midx: use chunk-format API in write_midx_internal()Derrick Stolee2021-02-181-86/+20
| | | | | | | | | | | | | | | | | | | | The chunk-format API allows writing the table of contents and all chunks using the anonymous 'struct chunkfile' type. We only need to convert our local chunk logic to this API for the multi-pack-index writes to share that logic with the commit-graph file writes. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: drop chunk progress during writeDerrick Stolee2021-02-181-7/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Most expensive operations in write_midx_internal() use the context struct's progress member, and these indicate the process of the expensive operations within the chunk writing methods. However, there is a competing progress struct that counts the progress over all chunks. This is not very helpful compared to the others, so drop it. This also reduces our barriers to combining the chunk writing code with chunk-format.c. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: return success/failure in chunk write methodsDerrick Stolee2021-02-181-36/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, the chunk-writing methods in midx.c have returned the amount of data written so the writer method could compare this with the table of contents. This presents with some interesting issues: 1. If a chunk writing method has a bug that miscalculates the written bytes, then we can satisfy the table of contents without actually writing the right amount of data to the hashfile. The commit-graph writing code checks the hashfile struct directly for a more robust verification. 2. There is no way for a chunk writing method to gracefully fail. Returning an int presents an opportunity to fail without a die(). 3. The current pattern doesn't match chunk_write_fn type exactly, so we cannot share code with commit-graph.c For these reasons, convert the midx chunk writer methods to return an 'int'. Since none of them fail at the moment, they all return 0. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: add num_large_offsets to write_midx_contextDerrick Stolee2021-02-181-7/+10
| | | | | | | | | | | | | | | | | | | | | | In an effort to align write_midx_internal() with the chunk-format API, continue to group necessary data into "struct write_midx_context". This change collects the "uint32_t num_large_offsets" into the context. With this new data, write_midx_large_offsets() now matches the chunk_write_fn type. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: add pack_perm to write_midx_contextDerrick Stolee2021-02-181-19/+21
| | | | | | | | | | | | | | | | | | | | | | | | In an effort to align write_midx_internal() with the chunk-format API, continue to group necessary data into "struct write_midx_context". This change collects the "uint32_t *pack_perm" and large_offsets_needed bit into the context. Update write_midx_object_offsets() to match chunk_write_fn. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: add entries to write_midx_contextDerrick Stolee2021-02-181-23/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In an effort to align write_midx_internal() with the chunk-format API, continue to group necessary data into "struct write_midx_context". This change collects the "struct pack_midx_entry *entries" list and its count into the context. Update write_midx_oid_fanout() and write_midx_oid_lookup() to take the context directly, as these are easy conversions with this new data. Only the callers of write_midx_object_offsets() and write_midx_large_offsets() are updated here, since additional data in the context before those methods can match chunk_write_fn. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: use context in write_midx_pack_names()Derrick Stolee2021-02-181-11/+10
| | | | | | | | | | | | | | | | | | | | | | In an effort to align the write_midx_internal() to use the chunk-format API, start converting chunk writing methods to match chunk_write_fn. The first case is to convert write_midx_pack_names() to take "void *data". We already have the necessary data in "struct write_midx_context", so this conversion is rather mechanical. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * midx: rename pack_info to write_midx_contextDerrick Stolee2021-02-181-65/+65
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In an effort to streamline our chunk-based file formats, align some of the code structure in write_midx_internal() to be similar to the patterns in write_commit_graph_file(). Specifically, let's create a "struct write_midx_context" that can be used as a data parameter to abstract function types. This change only renames "struct pack_info" to "struct write_midx_context" and the names of instances from "packs" to "ctx". In future changes, we will expand the data inside "struct write_midx_context" and align our chunk-writing method with the chunk-format API. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * commit-graph: use chunk-format write APIDerrick Stolee2021-02-181-82/+37
| | | | | | | | | | | | | | | | | | | | The commit-graph write logic is ready to make use of the chunk-format write API. Each chunk write method is already in the correct prototype. We only need to use the 'struct chunkfile' pointer and the correct API calls. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * chunk-format: create chunk format write APIDerrick Stolee2021-02-183-0/+112
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In anticipation of combining the logic from the commit-graph and multi-pack-index file formats, create a new chunk-format API. Use a 'struct chunkfile' pointer to keep track of data that has been registered for writes. This struct is anonymous outside of chunk-format.c to ensure no user attempts to interfere with the data. The next change will use this API in commit-graph.c, but the general approach is: 1. initialize the chunkfile with init_chunkfile(f). 2. add chunks in the intended writing order with add_chunk(). 3. write any header information to the hashfile f. 4. write the chunkfile data using write_chunkfile(). 5. free the chunkfile struct using free_chunkfile(). 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>
| * commit-graph: anonymize data in chunk_write_fnDerrick Stolee2021-02-061-10/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | In preparation for creating an API around file formats using chunks and tables of contents, prepare the commit-graph write code to use prototypes that will match this new API. Specifically, convert chunk_write_fn to take a "void *data" parameter instead of the commit-graph-specific "struct write_commit_graph_context" pointer. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | The tenth batchJunio C Hamano2021-02-231-0/+7
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/test-lib'Junio C Hamano2021-02-2341-124/+107
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Test framework clean-up. * ab/test-lib: test-lib-functions: assert correct parameter count test-lib-functions: remove bug-inducing "diagnostics" helper param test libs: rename "diff-lib" to "lib-diff" t/.gitattributes: sort lines test-lib-functions: move function to lib-bitmap.sh test libs: rename gitweb-lib.sh to lib-gitweb.sh test libs: rename bundle helper to "lib-bundle.sh" test-lib-functions: remove generate_zero_bytes() wrapper test-lib-functions: move test_set_index_version() to its user test lib: change "error" to "BUG" as appropriate test-lib: remove check_var_migration
| * | test-lib-functions: assert correct parameter countÆvar Arnfjörð Bjarmason2021-02-121-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add assertions of the correct parameter count of various functions, in particularly the wrappers for the shell "test" built-in. In an earlier commit we fixed a bug with an incorrect number of arguments being passed to "test_path_is_{file,missing}". Let's also guard other similar functions from the same sort of misuse. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test-lib-functions: remove bug-inducing "diagnostics" helper paramÆvar Arnfjörð Bjarmason2021-02-125-16/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the optional "diagnostics" parameter of the test_path_is_{file,dir,missing} functions. We have a lot of uses of these functions, but the only legitimate use of the diagnostics parameter is from when the functions themselves were introduced in 2caf20c52b7 (test-lib: user-friendly alternatives to test [-d|-f|-e], 2010-08-10). But as the the rest of this diff demonstrates its presence did more to silently introduce bugs in our tests. Fix such bugs in the tests added in ae4e89e549b (gc: add --keep-largest-pack option, 2018-04-15), and c04ba51739a (t6046: testcases checking whether updates can be skipped in a merge, 2018-04-19). Let's also assert that those functions are called with exactly one parameter, a follow-up commit will add similar asserts to other functions in test-lib-functions.sh that we didn't have existing misuse of. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test libs: rename "diff-lib" to "lib-diff"Ævar Arnfjörð Bjarmason2021-02-1223-30/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | Rename the "diff-lib" to "lib-diff". With this rename and preceding commits there is no remaining t/*lib* which doesn't follow the convention of being called t/lib-*. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | t/.gitattributes: sort linesÆvar Arnfjörð Bjarmason2021-02-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | Sort the lines starting with "/", the only out-of-place line was added along with most of the file in 614f4f0f350 (Fix the remaining tests that failed with core.autocrlf=true, 2017-05-09). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test-lib-functions: move function to lib-bitmap.shÆvar Arnfjörð Bjarmason2021-02-104-27/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move a function added to test-lib-functions.sh in ea047a8eb4f (t5310: factor out bitmap traversal comparison, 2020-02-14) into a new lib-bitmap.sh. The test-lib-functions.sh file should be for functions that are widely used across the test suite, if something's only used by a few tests it makes more sense to have it in a lib-*.sh file. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test libs: rename gitweb-lib.sh to lib-gitweb.shÆvar Arnfjörð Bjarmason2021-02-104-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename gitweb-lib.sh to lib-gitweb.sh for consistency with other test library files. When it was introduced in 05526071cb5 (gitweb: split test suite into library and tests, 2009-08-27) this naming pattern was more common. Since then all but one other such library which didn't start with "lib-*.sh" such as t6000lib.sh has been been renamed, see e.g. 9d488eb40e2 (Move t6000lib.sh to lib-*, 2010-05-07). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test libs: rename bundle helper to "lib-bundle.sh"Ævar Arnfjörð Bjarmason2021-02-103-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename the recently introduced test-bundle-functions.sh to be consistent with other lib-*.sh files, which is the convention for these sorts of shared test library functions. The new test-bundle-functions.sh was introduced in 9901164d81d (test: add helper functions for git-bundle, 2021-01-11). It was the only test-*.sh of this nature. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test-lib-functions: remove generate_zero_bytes() wrapperÆvar Arnfjörð Bjarmason2021-02-102-8/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since d5cfd142ec1 (tests: teach the test-tool to generate NUL bytes and use it, 2019-02-14) the generate_zero_bytes() functions has been a thin wrapper for "test-tool genzeros". Let's have its only user call that directly instead. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test-lib-functions: move test_set_index_version() to its userÆvar Arnfjörð Bjarmason2021-02-102-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the test_set_index_version() function to its only user. This function has only been used in one place since its addition in 5d9fc888b48 (test-lib: allow setting the index format version, 2014-02-23). Let's have that test script define it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test lib: change "error" to "BUG" as appropriateÆvar Arnfjörð Bjarmason2021-02-101-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change two uses of "error" in test-lib-functions.sh to "BUG". In the first instance in "test_cmp_rev" the author of the "BUG" function added in [1] had another in-flight patch adding this in [2], and the two were never consolidated. In the second case in "test_atexit" added in [3] that we could have instead used "BUG" appears to have been missed. 1. 165293af3ce (tests: send "bug in the test script" errors to the script's stderr, 2018-11-19) 2. 30d0b6dccbc (test-lib-functions: make 'test_cmp_rev' more informative on failure, 2018-11-19) 3. 900721e15c4 (test-lib: introduce 'test_atexit', 2019-03-13) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | test-lib: remove check_var_migrationÆvar Arnfjörð Bjarmason2021-02-101-30/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the check_var_migration() migration helper. This was added back in [1], [2] and [3] to warn users to migrate from e.g. the "GIT_FSMONITOR_TEST" name to "GIT_TEST_FSMONITOR". I daresay that having been warning about this since late 2018 (or v2.20.0) was sufficient time to give everyone interested a heads-up about moving to the new names. I don't see the need for going through the "do this later" codepath anticipated in [1], let's just remove this instead. 1. 4cb54d0aa8e (fsmonitor: update GIT_TEST_FSMONITOR support, 2018-09-18) 2. 1f357b045b5 (read-cache: update TEST_GIT_INDEX_VERSION support, 2018-09-18) 3. 5765d97b71d (preload-index: update GIT_FORCE_PRELOAD_TEST support, 2018-09-18) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'ab/diff-deferred-free'Junio C Hamano2021-02-234-20/+60
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | A small memleak in "diff -I<regexp>" has been corrected. * ab/diff-deferred-free: diff: plug memory leak from regcomp() on {log,diff} -I diff: add an API for deferred freeing
| * | | diff: plug memory leak from regcomp() on {log,diff} -IÆvar Arnfjörð Bjarmason2021-02-111-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a memory leak in 296d4a94e7 (diff: add -I<regex> that ignores matching changes, 2020-10-20) by freeing the memory it allocates in the newly introduced diff_free(). See the previous commit for details on that. This memory leak was intentionally introduced in 296d4a94e7, see the discussion on a previous iteration of it in https://lore.kernel.org/git/xmqqeelycajx.fsf@gitster.c.googlers.com/ At that time freeing the memory was somewhat tedious, but since it isn't anymore with the newly introduced diff_free() let's use it. Let's retain the pattern for diff_free_file() and add a diff_free_ignore_regex(), even though (unlike "diff_free_file") we don't need to call it elsewhere. I think this'll make for more readable code than gradually accumulating a giant diff_free() function, sharing "int i" across unrelated code etc. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | diff: add an API for deferred freeingÆvar Arnfjörð Bjarmason2021-02-114-20/+48
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a diff_free() function to free anything we may have allocated in the "diff_options" struct, and the ability to make calling it a noop by setting "no_free" in "diff_options". This is required because when e.g. "git diff" is run we'll allocate things in that struct, use the diff machinery once, and then exit. But if we run e.g. "git log -p" we're going to re-use what we allocated across multiple diff_flush() calls, and only want to free things at the end. We've thus ended up with features like the recently added "diff -I"[1] where we'll leak memory. As it turns out it could have simply used the pattern established in 6ea57703f6 (log: prepare log/log-tree to reuse the diffopt.close_file attribute, 2016-06-22). Manually adding more such flags to things log_tree_commit() every time we need to allocate something would be tedious. Let's instead move that fclose() code it to a new diff_free(), in anticipation of freeing more things in that function in follow-up commits. Some functions such as log_tree_commit() need an idiom of optionally retaining a previous "no_free", as they may either free the memory themselves, or their caller may do so. I'm keeping that idiom in log_show_early() for good measure, even though I don't think it's currently called in this manner. It also gets passed an existing "struct rev_info", so future callers may want to set the "no_free" flag. This change is a bit hard to read because while the freeing pattern we're introducing isn't unusual, the "file" member is a special snowflake. We usually don't want to fclose() it. This is because "file" is usually stdout, in which case we don't want to fclose() it. We only want to opt-in to closing it when we e.g. open a file on the filesystem. Thus the opt-in "close_file" flag. So the API in general just needs a "no_free" flag to defer freeing, but the "file" member still needs its "close_file" flag. This is made more confusing because while refactoring this code we could replace some "close_file=0" with "no_free=1", whereas others need to set both flags. This is because there were some cases where an existing "close_file=0" meant "let's defer deallocation", and others where it meant "we don't want to close this file handle at all". 1. 296d4a94e7 (diff: add -I<regex> that ignores matching changes, 2020-10-20) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ab/pager-exit-log'Junio C Hamano2021-02-233-13/+142
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a pager spawned by us exited, the trace log did not record its exit status correctly, which has been corrected. * ab/pager-exit-log: pager: properly log pager exit code when signalled run-command: add braces for "if" block in wait_or_whine() pager: test for exit code with and without SIGPIPE pager: refactor wait_for_pager() function
| * | | | pager: properly log pager exit code when signalledÆvar Arnfjörð Bjarmason2021-02-022-7/+57
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When git invokes a pager that exits with non-zero the common case is that we'll already return the correct SIGPIPE failure from git itself, but the exit code logged in trace2 has always been incorrectly reported[1]. Fix that and log the correct exit code in the logs. Since this gives us something to test outside of our recently-added tests needing a !MINGW prerequisite, let's refactor the test to run on MINGW and actually check for SIGPIPE outside of MINGW. The wait_or_whine() is only called with a true "in_signal" from from finish_command_in_signal(), which in turn is only used in pager.c. The "in_signal && !WIFEXITED(status)" case is not covered by tests. Let's log the default -1 in that case for good measure. 1. The incorrect logging of the exit code in was seemingly copy/pasted into finish_command_in_signal() in ee4512ed481 (trace2: create new combined trace facility, 2019-02-22) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | run-command: add braces for "if" block in wait_or_whine()Ævar Arnfjörð Bjarmason2021-02-021-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add braces to an "if" block in the wait_or_whine() function. This isn't needed now, but will make a subsequent commit easier to read. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | pager: test for exit code with and without SIGPIPEÆvar Arnfjörð Bjarmason2021-02-021-0/+82
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add tests for how git behaves when the pager itself exits with non-zero, as well as for us exiting with 141 when we're killed with SIGPIPE due to the pager not consuming its output. There is some recent discussion[1] about these semantics, but aside from what we want to do in the future, we should have a test for the current behavior. This test construct is stolen from 7559a1be8a0 (unblock and unignore SIGPIPE, 2014-09-18). The reason not to make the test itself depend on the MINGW prerequisite is to make a subsequent commit easier to read. 1. https://lore.kernel.org/git/87o8h4omqa.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | pager: refactor wait_for_pager() functionÆvar Arnfjörð Bjarmason2021-02-021-11/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor the wait_for_pager() function. Since 507d7804c0b (pager: don't use unsafe functions in signal handlers, 2015-09-04) the wait_for_pager() and wait_for_pager_atexit() callers diverged on more than they shared. Let's extract the common code into a new close_pager_fds() helper, and move the parts unique to the only to callers to those functions. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'ta/hash-function-transition-doc'Junio C Hamano2021-02-232-147/+150
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Update formatting and grammar of the hash transition plan documentation, plus some updates. * ta/hash-function-transition-doc: doc: use https links doc hash-function-transition: move rationale upwards doc hash-function-transition: fix incomplete sentence doc hash-function-transition: use upper case consistently doc hash-function-transition: use SHA-1 and SHA-256 consistently doc hash-function-transition: fix asciidoc output
| * | | | | doc: use https linksThomas Ackermann2021-02-052-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use only https links for lore.kernel.org. Signed-off-by: Thomas Ackermann <th.acker@arcor.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>