summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'ew/format-patch-mboxrd' into jchjchTaylor Blau2022-11-185-5/+18
|\ | | | | | | | | | | | | Teach `format-patch` a convenient alias for `--pretty=mboxrd`. * ew/format-patch-mboxrd: format-patch: add --mboxrd alias for --pretty=mboxrd
| * format-patch: add --mboxrd alias for --pretty=mboxrdEric Wong2022-11-145-5/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | mboxrd is a superior output format when used with --stdout and needs more exposure. Including pretty-formats.txt would be excessive, since documenting --pretty= for `git format-patch' would likely be confusing to users. Instead of documenting --pretty, add an --mboxrd alias to save keystrokes and improve documentation. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | Merge branch 'rs/multi-filter-args' into jchTaylor Blau2022-11-184-41/+27
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a bug where `pack-objects` would not respect multiple `--filter` arguments when invoked directly. * rs/multi-filter-args: list-objects-filter: remove OPT_PARSE_LIST_OBJECTS_FILTER_INIT() pack-object: simplify --filter handling pack-objects: fix handling of multiple --filter options
| * | list-objects-filter: remove OPT_PARSE_LIST_OBJECTS_FILTER_INIT()René Scharfe2022-11-122-20/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | OPT_PARSE_LIST_OBJECTS_FILTER_INIT() with a non-NULL second argument passes a function pointer via an object pointer, which is undefined. It may work fine on platforms that implement C99 extension J.5.7 (Function pointer casts). Remove the unused macro and avoid the dependency on that extension. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | pack-object: simplify --filter handlingRené Scharfe2022-11-121-22/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pack-objects uses OPT_PARSE_LIST_OBJECTS_FILTER_INIT() to initialize the a rev_info struct lazily before populating its filter member using the --filter option values. It tracks whether the initialization is needed using the .have_revs member of the callback data. There is a better way: Use a stand-alone list_objects_filter_options struct and build a rev_info struct with its .filter member after option parsing. This allows using the simpler OPT_PARSE_LIST_OBJECTS_FILTER() and getting rid of the extra callback mechanism. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | pack-objects: fix handling of multiple --filter optionsRené Scharfe2022-11-122-1/+21
| |/ | | | | | | | | | | | | | | | | | | | | Since 5cb28270a1 (pack-objects: lazily set up "struct rev_info", don't leak, 2022-03-28) --filter options given to git pack-objects overrule earlier ones, letting only the leftmost win and leaking the memory allocated for earlier ones. Fix that by only initializing the rev_info struct once. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | Merge branch 'js/drop-mingw-test-cmp' into jchTaylor Blau2022-11-183-69/+3
|\ \ | | | | | | | | | | | | | | | | | | | | | Use `git diff --no-index` as a test_cmp on Windows. * js/drop-mingw-test-cmp: tests(mingw): avoid very slow `mingw_test_cmp` t0021: use Windows-friendly `pwd`
| * | tests(mingw): avoid very slow `mingw_test_cmp`Johannes Schindelin2022-11-142-67/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is more performant to run `git diff --no-index` than running the `mingw_test_cmp` code with MSYS2's Bash, i.e. the Bash that Git for Windows uses. And a lot more readable. The original reason why Git's test suite needs the `mingw_test_cmp` function at all (and why `cmp` is not good enough) is that Git's test suite is not actually trying to compare binary files when it calls `test_cmp`, but it compares text files. And those text files can contain CR/LF line endings depending on the circumstances. Note: The original fix in the Git for Windows project implemented a test helper that avoids the overhead of the diff machinery, in favor of implementing a behavior that is more in line with what `mingw_test_cmp` does now. This was done to minimize the risk in using something as complex as the diff machinery to perform something as simple as determining whether text output is identical to the expected output or not. This approach has served Git for Windows well for years, but the attempt to upstream this saw a lot of backlash and distractions during the review, was disliked by the Git maintainer and was therefore abandoned. For full details, see the thread at https://lore.kernel.org/git/pull.1309.git.1659106382128.gitgitgadget@gmail.com/t Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | t0021: use Windows-friendly `pwd`Johannes Schindelin2022-11-141-2/+2
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In Git for Windows, when passing paths from shell scripts to regular Win32 executables, thanks to the MSYS2 runtime a somewhat magic path conversion happens that lets the shell script think that there is a file at `/git/Makefile` and the Win32 process it spawned thinks that the shell script said `C:/git-sdk-64/git/Makefile` instead. This conversion is documented in detail over here: https://www.msys2.org/docs/filesystem-paths/#automatic-unix-windows-path-conversion As all automatic conversions, there are gaps. For example, to avoid mistaking command-line options like `/LOG=log.txt` (which are quite common in the Windows world) from being mistaken for a Unix-style absolute path, the MSYS2 runtime specifically exempts arguments containing a `=` character from that conversion. We are about to change `test_cmp` to use `git diff --no-index`, which involves spawning precisely such a Win32 process. In combination, this would cause a failure in `t0021-conversion.sh` where we pass an absolute path containing an equal character to the `test_cmp` function. Seeing as the Unix tools like `cp` and `diff` that are used by Git's test suite in the Git for Windows SDK (thanks to the MSYS2 project) understand both Unix-style as well as Windows-style paths, we can stave off this problem by simply switching to Windows-style paths and side-stepping the need for any automatic path conversion. Note: The `PATH` variable is obviously special, as it is colon-separated in the MSYS2 Bash used by Git for Windows, and therefore _cannot_ contain absolute Windows-style paths, lest the colon after the drive letter is mistaken for a path separator. Therefore, we need to be careful to keep the Unix-style when modifying the `PATH` variable. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | Merge branch 'pw/config-int-parse-fixes' into jchTaylor Blau2022-11-184-5/+43
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | Assorted fixes of parsing end-user input as integers. * pw/config-int-parse-fixes: git_parse_signed(): avoid integer overflow config: require at least one digit when parsing numbers git_parse_unsigned: reject negative values
| * | git_parse_signed(): avoid integer overflowPhillip Wood2022-11-091-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git_parse_signed() checks that the absolute value of the parsed string is less than or equal to a caller supplied maximum value. When calculating the absolute value there is a integer overflow if `val == INTMAX_MIN`. To fix this avoid negating `val` when it is negative by having separate overflow checks for positive and negative values. An alternative would be to special case INTMAX_MIN before negating `val` as it is always out of range. That would enable us to keep the existing code but I'm not sure that the current two-stage check is any clearer than the new version. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | config: require at least one digit when parsing numbersPhillip Wood2022-11-093-0/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the input to strtoimax() or strtoumax() does not contain any digits then they return zero and set `end` to point to the start of the input string. git_parse_[un]signed() do not check `end` and so fail to return an error and instead return a value of zero if the input string is a valid units factor without any digits (e.g "k"). Tests are added to check that 'git config --int' and OPT_MAGNITUDE() reject a units specifier without a leading digit. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | git_parse_unsigned: reject negative valuesPhillip Wood2022-11-093-0/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git_parse_unsigned() relies on strtoumax() which unfortunately parses negative values as large positive integers. Fix this by rejecting any string that contains '-' as we do in strtoul_ui(). I've chosen to treat negative numbers as invalid input and set errno to EINVAL rather than ERANGE one the basis that they are never acceptable if we're looking for a unsigned integer. This is also consistent with the existing behavior of rejecting "1–2" with EINVAL. As we do not have unit tests for this function it is tested indirectly by checking that negative values of reject for core.bigFileThreshold are rejected. As this function is also used by OPT_MAGNITUDE() a test is added to check that rejects negative values too. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | | Merge branch 'gc/submodule-clone-update-with-branches' into jchTaylor Blau2022-11-1820-46/+357
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git clone --recurse-submodules" and "git submodule update" learns to honor the "propagete branches" option. * gc/submodule-clone-update-with-branches: clone, submodule update: create and check out branches submodule--helper: remove update_data.suboid submodule update: refactor update targets submodule: return target of submodule symref t5617: drop references to remote-tracking branches submodule--helper clone: create named branch repo-settings: add submodule_propagate_branches clone: teach --detach option
| * | | clone, submodule update: create and check out branchesGlen Choo2022-10-303-4/+227
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach "git submodule update" to: - create the branch with the same name as the current superproject branch when cloning a submodule - check out that branch (instead of the commit OID) when updating the submodule worktree when submodule branching is enabled (submodule.propagateBranches = true) on the superproject and a branch is checked out. "git clone --recurse-submodules" also learns this trick because it is implemented with "git submodule update --recursive". This approach of checking out the branch will not result in a dirty worktree for freshly cloned submodules because we can ensure that the submodule branch points to the superproject gitlink. In other cases, it does not work as well, but we can handle them incrementally: - "git pull --recurse-submodules" merges the superproject tree, (changing the gitlink without updating the submodule branches), and runs "git submodule update" to update the worktrees, so it is almost guaranteed to result in a dirty worktree. The implementation of "git pull --recurse-submodules" is likely to change drastically as submodule.propagateBranches work progresses (e.g. "git merge" learns to recurse in to submodules), and we may be able to replace the "git submodule update" invocation, or teach it new tricks that make the update behave well. - The user might make changes to the submodule branch without committing them back to superproject. This is primarily affects "git checkout --recurse-submodules", since that is the primary way of switching away from a branch and leaving behind WIP (as opposed to "git submodule update", which is run post-checkout). In a future series, "git checkout --recurse-submodules" will learn to consider submodule branches. We can introduce appropriate guardrails then, e.g. requiring that the superproject working tree is not dirty before switching away. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | submodule--helper: remove update_data.suboidGlen Choo2022-10-301-8/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | update_data.suboid's value is only used in update_submodule()'s call chain, where it represents the OID of the submodule's HEAD. If the submodule is newly cloned, it is set to null_oid(). Instead of checking for the null OID, just check if the submodule is newly cloned. This makes update_submodule() the only function where update_data.suboid is used, so replace it with a local variable. As a result, the submodule_up_to_date check is more explicit, which makes the next commit slightly easier to reason about. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | submodule update: refactor update targetsGlen Choo2022-10-301-11/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor two "git submodule update" code locations so that they no longer refer to oids directly. This shrinks a subsequent commit's diff, where this code will need to handle branches. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | submodule: return target of submodule symrefGlen Choo2022-10-3010-17/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | resolve_gitlink_ref() can tell us which oid the submodule ref is pointing to, but in a future commit, we would also like to know the symbolic ref target if we are checking a symbolic ref. Teach resolve_gitlink_ref() to "return" the symbolic ref's target via an "out" parameter. This changes resolve_gitlink_ref()'s signature so that new callers trying to use the old signature will be stopped by the compiler. If we returned the target instead (just like refs_resolve_ref_unsafe()), we would be more consistent with refs_resolve_ref_unsafe(), but callers expecting the old signature will get the opposite return value from what they expect (since exit code 0 means success, but NULL pointer means failure). We should do this refactor once we think that nobody will try to use the old signature. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | t5617: drop references to remote-tracking branchesGlen Choo2022-10-301-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It has included submodule cloning tests without remote-tracking branches tests since f05da2b48b (clone, submodule: pass partial clone filters to submodules, 2022-02-04) at least. Rename it accordingly so that we can put future submodule cloning tests there. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | submodule--helper clone: create named branchGlen Choo2022-10-301-0/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When submodule branching is enabled (i.e. "submodule.propagateBranches = true"), submodules are expected to have the same set of branches as the superproject. To support this behavior in newly cloned submodules, teach "git submodule--helper clone" to: - clone with the "--detach" flag (so that the submodule doesn't create a branch corresponding to the remote's HEAD) - create a branch when using the --branch and --branch-oid flags The --branch and --branch-oid flags are only allowed when submodule branching is enabled, otherwise the named branch might conflict with the branch from the submodule remote's HEAD. This functionality will be tested in a later commit where "git submodule update" uses it to create and check out the correct branch when submodule branching is enabled. "git submodule add" (which also invokes "git submodule--helper clone") should also do something similar when submodule branching is enabled, but this is left for a later series. Arguably, "--detach" should also be the default for "submodule.propagateBranches=false" since it doesn't make sense to create a submodule branch when the submodule is always expected to be in detached HEAD. But, to be conservative, this commit does not change the behavior of "submodule.propagateBranches=false". Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | repo-settings: add submodule_propagate_branchesGlen Choo2022-10-305-8/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When processes recurse into submodules, the child processes have to use the same value of "submodule.propagateBranches" as the parent process regardless of whether the process is spawned in the superproject or submodule, otherwise the behavior may be inconsistent if the repositories don't agree on the config. We haven't needed a way to propagate the config because the only command that reads "submodule.propagateBranches" is "git branch", which only has one mode of operation with "--recurse-submodules". However, a future commit will teach "submodule.propagateBranches" to "git submodule update", making this necessary. Propagate "submodule.propagateBranches" to child processes by adding a corresponding GIT_INTERNAL_* environment variable and repository setting, and setting the environment variable inside prepare_submodule_repo_env(). Then, refactor builtin/branch.c to read the repository setting. Using an internal environment variable is a potentially leaky abstraction because environment variables can come from sources besides the parent process. A more robust solution would be to teach Git that the repository is a submodule and to only read "submodule.propagateBranches" from the superproject config. There is WIP for this on the ML [1]. Another alternative would be to pass "-c submodule.propagateBranches" to all child processes. This is error-prone because many different processes are invoked directly or indirectly by "git submodule update" (e.g. "git submodule--helper clone", "git clone", "git checkout"). With an environment variable, we can avoid this work because prepare_submodule_repo_env() is already called for submodule child processes. [1] https://lore.kernel.org/git/20220310004423.2627181-1-emilyshaffer@google.com/ Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | clone: teach --detach optionGlen Choo2022-10-303-4/+38
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach "git clone" the "--detach" option, which leaves the cloned repo in detached HEAD (like "git checkout --detach"). In addition, if the clone is not bare, do not create the local branch pointed to by the remote's HEAD symref (bare clones always copy all remote branches directly to local branches, so the branch is still created in the bare case). This is especially useful in the "submodule.propagateBranches" workflow, where local submodule branches are named after the superproject's branches, so it makes no sense to create a local branch named after the submodule's remote's branch. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | | | Merge branch 'cw/submodule-status-in-parallel' into jchTaylor Blau2022-11-1810-47/+441
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Allow the internal "diff-files" engine to run "how has this submodule changed?" in parallel to speed up "git status". * cw/submodule-status-in-parallel: diff-lib: parallelize run_diff_files for submodules diff-lib: refactor match_stat_with_submodule submodule: move status parsing into function submodule: strbuf variable rename run-command: add duplicate_output_fn to run_processes_parallel_opts
| * | | | diff-lib: parallelize run_diff_files for submodulesCalvin Wan2022-11-086-6/+287
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | During the iteration of the index entries in run_diff_files, whenever a submodule is found and needs its status checked, a subprocess is spawned for it. Instead of spawning the subprocess immediately and waiting for its completion to continue, hold onto all submodules and relevant information in a list. Then use that list to create tasks for run_processes_parallel. Subprocess output is duplicated and passed to status_pipe_output which parses it. Add config option submodule.diffJobs to set the maximum number of parallel jobs. The option defaults to 1 if unset. If set to 0, the number of jobs is set to online_cpus(). Since run_diff_files is called from many different commands, I chose to grab the config option in the function rather than adding variables to every git command and then figuring out how to pass them all in. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | diff-lib: refactor match_stat_with_submoduleCalvin Wan2022-11-081-11/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Flatten out the if statements in match_stat_with_submodule so the logic is more readable and easier for future patches to add to. orig_flags didn't need to be set if the cache entry wasn't a GITLINK so defer setting it. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | submodule: move status parsing into functionCalvin Wan2022-11-081-32/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A future patch requires the ability to parse the output of git status --porcelain=2. Move parsing code from is_submodule_modified to parse_status_porcelain. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | submodule: strbuf variable renameCalvin Wan2022-11-081-10/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A prepatory change for a future patch that moves the status parsing logic to a separate function. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | run-command: add duplicate_output_fn to run_processes_parallel_optsCalvin Wan2022-11-084-2/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add duplicate_output_fn as an optionally set function in run_process_parallel_opts. If set, output from each child process is copied and passed to the callback function whenever output from the child process is buffered to allow for separate parsing. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | Merge branch 'ab/run-hook-api-cleanup' into cw/submodule-status-in-parallelJunio C Hamano2022-10-209-208/+268
| |\ \ \ \ | | |/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * ab/run-hook-api-cleanup: run-command.c: remove "max_processes", add "const" to signal() handler run-command.c: pass "opts" further down, and use "opts->processes" run-command.c: use "opts->processes", not "pp->max_processes" run-command.c: don't copy "data" to "struct parallel_processes" run-command.c: don't copy "ungroup" to "struct parallel_processes" run-command.c: don't copy *_fn to "struct parallel_processes" run-command.c: make "struct parallel_processes" const if possible run-command API: move *_tr2() users to "run_processes_parallel()" run-command API: have run_process_parallel() take an "opts" struct run-command.c: use designated init for pp_init(), add "const" run-command API: don't fall back on online_cpus() run-command API: make "n" parameter a "size_t" run-command tests: use "return", not "exit" run-command API: have "run_processes_parallel{,_tr2}()" return void run-command test helper: use "else if" pattern
* | | | | Merge branch 'tl/notes--blankline' into jchTaylor Blau2022-11-183-14/+34
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 'git notes append' was taught '--[no-]blank-line' to conditionally add a LF between a new and existing note. * tl/notes--blankline: notes.c: introduce "--no-blank-line" option notes.c: provide tips when target and append note are both empty notes.c: drop unreachable code in 'append_edit()' notes.c: cleanup for "designated init" and "char ptr init" notes.c: cleanup 'strbuf_grow' call in 'append_edit'
| * | | | | notes.c: introduce "--no-blank-line" optionTeng Long2022-11-093-3/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When appending to a given object which has note and if the appended note is not empty too, we will insert a blank line at first. The blank line serves as a split line, but sometimes we just want to omit it then append on the heels of the target note. So, we add a new "OPT_BOOL()" option to determain whether a new blank line is insert at first. Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | notes.c: provide tips when target and append note are both emptyTeng Long2022-11-092-3/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When "git notes append <object>" is executed, if there is no note in the given object and the appended note is empty too, we could print the exact tips to end-user. Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | notes.c: drop unreachable code in 'append_edit()'Teng Long2022-11-091-6/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Situation of note "removing" shouldn't happen in 'append_edit()', unless it's a bug. So, let's drop the unreachable "else" code in "append_edit()". The notes operation "append" is different with "add", the latter supports to overwrite the existing note then let the "removing" happen (e.g. execute `git notes add -f -F /dev/null` on an existing note), but the former will not because it only does "appends" but not doing "overwrites". Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | notes.c: cleanup for "designated init" and "char ptr init"Teng Long2022-11-091-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Let's do some cleanup for the following two places in "append_edit()". The first place is "char *logmsg;" need to be initialized with NULL. The second place is "struct note_data d = { 0, 0, NULL, STRBUF_INIT };" could be replaced with designated init format. Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | notes.c: cleanup 'strbuf_grow' call in 'append_edit'Teng Long2022-11-091-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Let's cleanup the unnecessary 'strbuf_grow' call in 'append_edit'. This "strbuf_grow(&d.buf, size + 1);" is prepared for insert a blank line if needed, but actually when inserting, "strbuf_insertstr(&d.buf, 0, "\n");" will do the "grow" for us. Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
* | | | | | Merge branch 'ds/packed-refs-v2' into jchTaylor Blau2022-11-1837-695/+2157
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * ds/packed-refs-v2: (30 commits) refs: skip hashing when writing packed-refs v2 p1401: create performance test for ref operations ci: run GIT_TEST_PACKED_REFS_VERSION=2 in some builds t*: skip packed-refs v2 over http tests t3210: require packed-refs v1 for some tests t5502: add PACKED_REFS_V1 prerequisite t5312: allow packed-refs v2 format t1409: test with packed-refs v2 packed-backend: create GIT_TEST_PACKED_REFS_VERSION packed-refs: write prefix chunks packed-refs: read optional prefix chunks packed-refs: read file format v2 packed-refs: write file format version 2 packed-backend: create shell of v2 writes config: add config values for packed-refs v2 packed-backend: create abstraction for writing refs packed-backend: extract iterator/updates merge packed-backend: extract add_write_error() refs: extract packfile format to new file chunk-format: parse trailing table of contents ...
| * | | | | | refs: skip hashing when writing packed-refs v2Derrick Stolee2022-11-072-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 'skip_hash' option in 'struct hashfile' indicates that we want to use the hashfile API as a buffered writer, and not use the hash function to create a trailing hash. We still write a trailing null hash to indicate that we do not have a checksum at the end. This feature is enabled for index writes using the 'index.computeHash' config key. Create a similar (currently hidden) option for the packed-refs v2 file format: refs.hashPackedRefs. This defaults to false because performance is compared to the packed-refs v1 file format which does have a checksum anywhere. This change results in improvements to p1401 when using a repository with a 42 MB packed-refs file (600,000+ refs). Test HEAD~1 HEAD -------------------------------------------------------------------- 1401.1: git pack-refs (v1) 0.38(0.31+0.52) 0.37(0.28+0.52) -2.6% 1401.5: git pack-refs (v2) 0.39(0.33+0.52) 0.30(0.28+0.46) -23.1% Note that these tests update a ref and then repack the packed-refs file. The following benchmarks are from a hyperfine experiment that only ran the 'git pack-refs --all' command for the two formats, but also compared the effect when refs.hashPackedRefs=true. Benchmark 1: v1 Time (mean ± σ): 163.5 ms ± 18.1 ms [User: 117.8 ms, System: 38.1 ms] Range (min … max): 131.3 ms … 190.4 ms 50 runs Benchmark 2: v2-no-hash Time (mean ± σ): 95.8 ms ± 15.1 ms [User: 72.5 ms, System: 23.0 ms] Range (min … max): 82.9 ms … 131.2 ms 50 runs Benchmark 3: v2-hashing Time (mean ± σ): 100.8 ms ± 16.4 ms [User: 77.2 ms, System: 23.1 ms] Range (min … max): 83.0 ms … 131.1 ms 50 runs Summary 'v2-no-hash' ran 1.05 ± 0.24 times faster than 'v2-hashing' 1.71 ± 0.33 times faster than 'v1' In this case of repeatedly rewriting the same refs seems to demonstrate a smaller improvement than the p1401 test. However, the overall reduction from v1 matches the expected reduction in file size. In my tests, the 42 MB packed-refs (v1) file was compacted to 28 MB in the v2 format. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | p1401: create performance test for ref operationsDerrick Stolee2022-11-071-0/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | TBD Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | ci: run GIT_TEST_PACKED_REFS_VERSION=2 in some buildsDerrick Stolee2022-11-071-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The linux-TEST-vars CI build helps us check that certain opt-in features are still exercised in at least one environment. The new GIT_TEST_PACKED_REFS_VERSION environment variable now passes the test suite when set to "2", so add this to that list of variables. This provides nearly the same coverage of the v2 format as we had in the v1 format. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | t*: skip packed-refs v2 over http testsDerrick Stolee2022-11-075-0/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The GIT_TEST_PACKED_REFS_VERSION=2 environment variable helps us test the packed-refs file format in its v2 version. This variable makes the Git process act as if the extensions.refFormat config key has "packed-v2" in its list. This means that if the environment variable is removed, the repository is in a bad state. This is sufficient for most test cases. However, tests that fetch over HTTP appear to lose this environment variable when executed through the HTTP server. Since the repositories are created via Git commands in the tests, the packed-refs files end up in the v2 format, but the server processes do not understand this and start serving empty payloads since they do not recognize any refs. The preferred long-term solution would be to ensure that the GIT_TEST_* environment variable persists into the HTTP server. However, these tests are not exercising any particularly tricky parts of the packed-refs file format. It may not be worth the effort to pass the environment variable and instead we can unset the environment variable (with a comment explaining why) in these tests. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | t3210: require packed-refs v1 for some testsDerrick Stolee2022-11-071-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Three tests in t3210-pack-refs.sh corrupt a packed-refs file to test that Git properly discovers and handles those failures. These tests assume that the file is in the v1 format, so add the PACKED_REFS_V1 prereq to skip these tests when GIT_TEST_PACKED_REFS_VERSION=2. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | t5502: add PACKED_REFS_V1 prerequisiteDerrick Stolee2022-11-072-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The last test in t5502-quickfetch.sh exploits the packed-refs v1 file format by appending 1000 lines to the packed-refs file. If the packed-refs file is in the v2 format, this corrupts the file as unreadable. Instead of making the test slower, let's ignore it when GIT_TEST_PACKED_REFS_VERSION=2. The test is really about 'git fetch', not the packed-refs format. Create a prerequisite in case we want to use this technique again in the future. An alternative would be to write those 1000 refs using a different mechanism, but let's opt for the simpler case for now. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | t5312: allow packed-refs v2 formatDerrick Stolee2022-11-071-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | One test in t5312 uses 'grep' to detect that a ref is written in the packed-refs file instead of a loose object. This does not work when the packed-refs file is in v2 format, such as when GIT_TEST_PACKED_REFS_VERSION=2. Since the test already checks that the loose ref is missing, it suffices to check that 'git rev-parse' succeeds. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | t1409: test with packed-refs v2Derrick Stolee2022-11-071-3/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | t1409-avoid-packing-refs.sh seeks to test that the packed-refs file is not modified unnecessarily. One way it does this is by creating a packed-refs file, then munging its contents and verifying that the munged data remains after other commands. For packed-refs v1, it suffices to add a line that is similar to a comment. For packed-refs v2, we cannot even add to the file without messing up the trailing table of contents of its chunked format. However, we can manipulate the last bytes that are within the trailing hash and use 'tail -c 4' to read them. This makes t1409 pass with GIT_TEST_PACKED_REFS_VERSION=2. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | packed-backend: create GIT_TEST_PACKED_REFS_VERSIONDerrick Stolee2022-11-073-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When set, this will create a default value for the packed-refs file version on writes. When set to "2", it will automatically add the "packed-v2" value to extensions.refFormat. Not all tests pass with GIT_TEST_PACKED_REFS_VERSION=2 because they care specifically about the content of the packed-refs file. These tests will be updated in following changes. To start, though, disable the GIT_TEST_PACKED_REFS_VERSION environment variable in t3212-ref-formats.sh, since that script already tests both versions, including upgrade scenarios. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | packed-refs: write prefix chunksDerrick Stolee2022-11-071-0/+103
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Tests already cover that we will start reading these prefixes. TODO: discuss time and space savings over typical approach. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | packed-refs: read optional prefix chunksDerrick Stolee2022-11-073-0/+170
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | packed-refs: read file format v2Derrick Stolee2022-11-074-55/+372
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | packed-refs: write file format version 2Derrick Stolee2022-11-073-2/+115
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | TODO: add writing tests. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
| * | | | | | packed-backend: create shell of v2 writesDerrick Stolee2022-11-074-11/+110
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>