summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* refs: pass repo when retrieving submodule ref storePatrick Steinhardt2024-05-178-21/+33
| | | | | | | | | | | | | | | | | | | | | | Looking up submodule ref stores has two deficiencies: - The initialized subrepo will be attributed to `the_repository`. - The submodule ref store will be tracked in a global map. This makes it impossible to have submodule ref stores for a repository other than `the_repository`. Modify the function to accept the parent repository as parameter and move the global map into `struct repository`. Like this it becomes possible to look up submodule ref stores for arbitrary repositories. Note that this also adds a new reference to `the_repository` in `resolve_gitlink_ref()`, which is part of the refs interfaces. This will get adjusted in the next patch. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: track ref stores via strmapPatrick Steinhardt2024-05-171-57/+14
| | | | | | | | | | | | | | | | | | The refs code has two global maps that track the submodule and worktree ref stores. Even though both of these maps track values by strings, we still use a `struct hashmap` instead of a `struct strmap`. This has the benefit of saving us an allocation because we can combine key and value in a single struct. But it does introduce significant complexity that is completely unneeded. Refactor the code to use `struct strmap`s instead to reduce complexity. It's unlikely that this will have any real-world impact on performance given that most repositories likely won't have all that many ref stores. Furthermore, this refactoring allows us to de-globalize those maps and move them into `struct repository` in a subsequent commit more easily. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: implement releasing ref storagesPatrick Steinhardt2024-05-177-0/+68
| | | | | | | | | | | | | | | | | | Ref storages are typically only initialized once for `the_repository` and then never released. Until now we got away with that without causing memory leaks because `the_repository` stays reachable, and because the ref backend is reachable via `the_repository` its memory basically never leaks. This is about to change though because of the upcoming migration logic, which will create a secondary ref storage. In that case, we will either have to release the old or new ref storage to avoid leaks. Implement a new `release` callback and expose it via a new `ref_storage_release()` function. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: rename `init_db` callback to avoid confusionPatrick Steinhardt2024-05-179-29/+29
| | | | | | | | | | | | | Reference backends have two callbacks `init` and `init_db`. The similarity of these two callbacks has repeatedly confused me whenever I was looking at them, where I always had to look up which of them does what. Rename the `init_db` callback to `create_on_disk`, which should hopefully be clearer. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: adjust names for `init` and `init_db` callbacksPatrick Steinhardt2024-05-173-16/+16
| | | | | | | | | | The names of the functions that implement the `init` and `init_db` callbacks in the "files" and "packed" backends do not match the names of the callbacks, which is inconsistent. Rename them so that they match, which makes it easier to discover their respective implementations. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ps/refs-without-the-repository' into ↵Junio C Hamano2024-05-1677-680/+1052
|\ | | | | | | | | | | | | | | | | | | | | ps/refs-without-the-repository-updates * ps/refs-without-the-repository: refs: remove functions without ref store cocci: apply rules to rewrite callers of "refs" interfaces cocci: introduce rules to transform "refs" to pass ref store refs: add `exclude_patterns` parameter to `for_each_fullref_in()` refs: introduce missing functions that accept a `struct ref_store`
| * refs: remove functions without ref storePatrick Steinhardt2024-05-072-268/+209
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The preceding commit has rewritten all callers of ref-related functions to use the equivalents that accept a `struct ref_store`. Consequently, the respective variants without the ref store are now unused. Remove them. There are likely patch series in-flight that use the now-removed functions. To help the authors, the old implementations have been added to "refs.c" in an ifdef'd section as a reference for how to migrate each of the respective callers. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt2024-05-0775-436/+711
| | | | | | | | | | | | | | | | | | Apply the rules that rewrite callers of "refs" interfaces to explicitly pass `struct ref_store`. The resulting patch has been applied with the `--whitespace=fix` option. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * cocci: introduce rules to transform "refs" to pass ref storePatrick Steinhardt2024-05-071-0/+103
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most of the functions in "refs.h" have two flavors: one that accepts a `struct ref_store`, and one that figures it out via `the_repository`. As part of the libification efforts we want to get rid of the latter variant and stop relying on `the_repository` altogether. Introduce a set of Coccinelle rules that transform callers of the "refs" interfaces to pass a `struct ref_store`. These rules are not yet applied by this patch so that it can be reviewed standalone more easily. This will be done in the next patch. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * refs: add `exclude_patterns` parameter to `for_each_fullref_in()`Patrick Steinhardt2024-05-075-13/+16
| | | | | | | | | | | | | | | | | | | | | | The `for_each_fullref_in()` function is supposedly the ref-store-less equivalent of `refs_for_each_fullref_in()`, but the latter has gained a new parameter `exclude_patterns` over time. Bring these two functions back in sync again by adding the parameter to the former function, as well. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * refs: introduce missing functions that accept a `struct ref_store`Patrick Steinhardt2024-05-072-14/+64
| | | | | | | | | | | | | | | | | | | | | | | | While most of the functions in "refs.h" have a variant that accepts a `struct ref_store`, some don't. Callers of these functions are thus forced to implicitly rely on `the_repository` to figure out the ref store that is to be used. Introduce those missing functions to address this shortcoming. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | The fourth batchJunio C Hamano2024-05-151-1/+19
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ds/scalar-reconfigure-all-fix'Junio C Hamano2024-05-152-3/+45
|\ \ | | | | | | | | | | | | | | | | | | Scalar fix. * ds/scalar-reconfigure-all-fix: scalar: avoid segfault in reconfigure --all
| * | scalar: avoid segfault in reconfigure --allDerrick Stolee2024-05-082-3/+45
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | During the latest v2.45.0 update, 'scalar reconfigure --all' started to segfault on my machine. Breaking it down via the debugger, it was faulting on a NULL reference to the_hash_algo, which is a macro pointing to the_repository->hash_algo. In my case, this is due to one of my repositories having a detached HEAD, which requires get_oid_hex() to parse that the HEAD reference is valid. Another way to cause a failure is to use the "includeIf.onbranch" config key, which will lead to a BUG() statement. My first inclination was to try to refactor cmd_reconfigure() to execute 'git for-each-repo' instead of this loop. In addition to the difficulty of executing 'scalar reconfigure' within 'git for-each-repo', it would be difficult to perform the clean-up logic for non-existent repos if we relied on that child process. Instead, I chose to move the temporary repo to be within the loop and reinstate the_repository to its old value after we are done performing logic on the current array item. Add tests to t9210-scalar.sh to test 'scalar reconfigure --all' with multiple registered repos. There are two different ways that the old use of the_repository could trigger bugs. These issues are being solved independently to be more careful about the_repository being uninitialized, but the change in this patch around the use of the_repository is still a good safety precaution. Co-authored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'vd/doc-merge-tree-x-option'Junio C Hamano2024-05-151-0/+5
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Doc update. * vd/doc-merge-tree-x-option: Documentation/git-merge-tree.txt: document -X
| * | | Documentation/git-merge-tree.txt: document -XVictoria Dye2024-05-081-0/+5
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add an entry in the 'merge-tree' builtin documentation for -X/--strategy-option (added in 6a4c9e7b32 (merge-tree: add -X strategy option, 2023-09-24)). The same option is documented for 'merge', 'rebase', 'revert', etc. in their respective Documentation/ files, so let's do the same for 'merge-tree'. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'rs/external-diff-with-exit-code'Junio C Hamano2024-05-153-3/+47
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "--exit-code" option of "git diff" command learned to work with the "--ext-diff" option. * rs/external-diff-with-exit-code: diff: fix --exit-code with external diff diff: report unmerged paths as changes in run_diff_cmd()
| * | | diff: fix --exit-code with external diffRené Scharfe2024-05-062-3/+38
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | You can ask the diff machinery to let the exit code indicate whether there are changes, e.g. with --exit-code. It as two ways to calculate that bit: The quick one assumes blobs with different hashes have different content, and the more elaborate way actually compares the contents, possibly applying transformations like ignoring whitespace. Always use the slower path by setting the flag diff_from_contents, because any of the files could have an external diff driver set via an attribute, which might consider binary differences irrelevant, like e.g. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | diff: report unmerged paths as changes in run_diff_cmd()René Scharfe2024-05-062-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | You can ask the diff machinery to let the exit code indicate whether there are changes, e.g. with --quiet. It as two ways to calculate that bit: The quick one assumes blobs with different hashes have different content, and the more elaborate way actually compares the contents, possibly applying transformations like ignoring whitespace. The quick way considers an unmerged file to be a change and reports exit code 1, which makes sense. The slower path uses the struct diff_options member found_changes to indicate whether the blobs differ even with the transformations applied. It's not set for unmerged files, though, resulting in exit code 0. Set found_changes in run_diff_cmd() for unmerged files, for a consistent exit code of 1 if there's an unmerged file, regardless of whether whitespace is ignored. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jt/port-ci-whitespace-check-to-gitlab'Junio C Hamano2024-05-154-64/+109
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "whitespace check" task that was enabled for GitHub Actions CI has been ported to GitLab CI. * jt/port-ci-whitespace-check-to-gitlab: gitlab-ci: add whitespace error check ci: make the whitespace report optional ci: separate whitespace check script github-ci: fix link to whitespace error ci: pre-collapse GitLab CI sections
| * | | | gitlab-ci: add whitespace error checkJustin Tobler2024-05-031-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GitLab CI does not have a job to check for whitespace errors introduced by a set of changes. Reuse the existing generic `whitespace-check.sh` to create the job for GitLab pipelines. Note that the `$CI_MERGE_REQUEST_TARGET_BRANCH_SHA` variable is only available in GitLab merge request pipelines and therefore the CI job is configured to only run as part of those pipelines. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ci: make the whitespace report optionalJustin Tobler2024-05-031-12/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `check-whitespace` CI job generates a formatted output file containing whitespace error information. As not all CI providers support rendering a formatted summary, make its generation optional. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ci: separate whitespace check scriptJustin Tobler2024-05-032-64/+78
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `check-whitespace` CI job is only available as a GitHub action. To help enable this job with other CI providers, first separate the logic performing the whitespace check into its own script. In subsequent commits, this script is further generalized allowing its reuse. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | github-ci: fix link to whitespace errorJustin Tobler2024-05-031-11/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the `check-whitespace` CI job detects whitespace errors, a formatted summary of the issue is generated. This summary contains links to the commits and blobs responsible for the whitespace errors. The generated links for blobs do not work and result in a 404. Instead of using the reference name in the link, use the commit ID directly. This fixes the broken link and also helps enable future generalization of the script for other CI providers by removing one of the GitHub specific CI variables used. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ci: pre-collapse GitLab CI sectionsJustin Tobler2024-05-031-1/+1
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Sections of CI output defined by `begin_group()` and `end_group()` are expanded in GitLab pipelines by default. This can make CI job output rather noisy and harder to navigate. Update the behavior for GitLab pipelines to now collapse sections by default. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ow/refspec-glossary-update'Junio C Hamano2024-05-151-1/+2
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Doc update. * ow/refspec-glossary-update: Documentation: Mention that refspecs are explained elsewhere
| * | | | Documentation: Mention that refspecs are explained elsewhereØystein Walle2024-05-061-1/+2
| | |_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The syntax for refspecs are explained in more detail in documention for git-fetch and git-push. Give a hint to the user too look there more fore information Signed-off-by: Øystein Walle <oystwa@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jp/tag-trailer'Junio C Hamano2024-05-156-28/+181
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git tag" learned the "--trailer" option to futz with the trailers in the same way as "git commit" does. * jp/tag-trailer: builtin/tag: add --trailer option builtin/commit: refactor --trailer logic builtin/commit: use ARGV macro to collect trailers
| * | | | builtin/tag: add --trailer optionJohn Passaro2024-05-073-11/+157
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-tag supports interpreting trailers from an annotated tag message, using --list --format="%(trailers)". However, the available methods to add a trailer to a tag message (namely -F or --editor) are not as ergonomic. In a previous patch, we moved git-commit's implementation of its --trailer option to the trailer.h API. Let's use that new function to teach git-tag the same --trailer option, emulating as much of git-commit's behavior as much as possible. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: John Passaro <john.a.passaro@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/commit: refactor --trailer logicJohn Passaro2024-05-073-8/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-commit adds user trailers to the commit message by passing its `--trailer` arguments to a child process running `git-interpret-trailers --in-place`. This logic is broadly useful, not just for git-commit but for other commands constructing message bodies (e.g. git-tag). Let's move this logic from git-commit to a new function in the trailer API, so that it can be re-used in other commands. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: John Passaro <john.a.passaro@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/commit: use ARGV macro to collect trailersJohn Passaro2024-05-071-9/+1
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace git-commit's callback for --trailer with the standard OPT_PASSTHRU_ARGV macro. The callback only adds its values to a strvec and sanity-checks that `unset` is always false; both of these are already implemented in the parse-option API. Signed-off-by: John Passaro <john.a.passaro@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'ps/config-subcommands'Junio C Hamano2024-05-156-370/+812
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The operation mode options (like "--get") the "git config" command uses have been deprecated and replaced with subcommands (like "git config get"). * ps/config-subcommands: builtin/config: display subcommand help builtin/config: introduce "edit" subcommand builtin/config: introduce "remove-section" subcommand builtin/config: introduce "rename-section" subcommand builtin/config: introduce "unset" subcommand builtin/config: introduce "set" subcommand builtin/config: introduce "get" subcommand builtin/config: introduce "list" subcommand builtin/config: pull out function to handle `--null` builtin/config: pull out function to handle config location builtin/config: use `OPT_CMDMODE()` to specify modes builtin/config: move "fixed-value" option to correct group builtin/config: move option array around config: clarify memory ownership when preparing comment strings
| * | | | builtin/config: display subcommand helpPatrick Steinhardt2024-05-062-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, `git config -h` would have printed help for the old-style syntax. Now that all modes have proper subcommands though it is preferable to instead display the subcommand help. Drop the `NO_INTERNAL_HELP` flag to do so. While at it, drop the help mismatch in t0450 and add the `--get-colorbool` option to the usage such that git-config(1)'s synopsis and `git config -h` match. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "edit" subcommandPatrick Steinhardt2024-05-063-36/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new "edit" subcommand to git-config(1). Please refer to preceding commits regarding the motivation behind this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "remove-section" subcommandPatrick Steinhardt2024-05-063-6/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new "remove-section" subcommand to git-config(1). Please refer to preceding commits regarding the motivation behind this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "rename-section" subcommandPatrick Steinhardt2024-05-063-15/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new "rename-section" subcommand to git-config(1). Please refer to preceding commits regarding the motivation behind this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "unset" subcommandPatrick Steinhardt2024-05-063-26/+84
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new "unset" subcommand to git-config(1). Please refer to preceding commits regarding the motivation behind this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "set" subcommandPatrick Steinhardt2024-05-063-63/+140
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new "set" subcommand to git-config(1). Please refer to preceding commits regarding the motivation behind this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "get" subcommandPatrick Steinhardt2024-05-063-99/+194
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new "get" subcommand to git-config(1). Please refer to preceding commits regarding the motivation behind this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: introduce "list" subcommandPatrick Steinhardt2024-05-063-64/+162
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While git-config(1) has several modes, those modes are not exposed with subcommands but instead by specifying action flags like `--unset` or `--list`. This user interface is not really in line with how our more modern commands work, where it is a lot more customary to say e.g. `git remote list`. Furthermore, to add to the confusion, git-config(1) also allows the user to request modes implicitly by just specifying the correct number of arguments. Thus, `git config foo.bar` will retrieve the value of "foo.bar" while `git config foo.bar baz` will set it to "baz". Overall, this makes for a confusing interface that could really use a makeover. It hurts discoverability of what you can do with git-config(1) and is comparatively easy to get wrong. Converting the command to have subcommands instead would go a long way to help address these issues. One concern in this context is backwards compatibility. Luckily, we can introduce subcommands without breaking backwards compatibility at all. This is because all the implicit modes of git-config(1) require that the first argument is a properly formatted config key. And as config keys _must_ have a dot in their name, any value without a dot would have been discarded by git-config(1) previous to this change. Thus, given that none of the subcommands do have a dot, they are unambiguous. Introduce the first such new subcommand, which is "git config list". To retain backwards compatibility we only conditionally use subcommands and will fall back to the old syntax in case no subcommand was detected. This should help to transition to the new-style syntax until we eventually deprecate and remove the old-style syntax. Note that the way we handle this we're duplicating some functionality across old and new syntax. While this isn't pretty, it helps us to ensure that there really is no change in behaviour for the old syntax. Amend tests such that we run them both with old and new style syntax. As tests are now run twice, state from the first run may be still be around in the second run and thus cause tests to fail. Add cleanup logic as required to fix such tests. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: pull out function to handle `--null`Patrick Steinhardt2024-05-061-6/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pull out function to handle the `--null` option, which we are about to reuse in subsequent commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: pull out function to handle config locationPatrick Steinhardt2024-05-061-65/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There's quite a bunch of options to git-config(1) that allow the user to specify which config location to use when reading or writing config options. The logic to handle this is thus by necessity also quite involved. Pull it out into a separate function so that we can reuse it in subsequent commits which introduce proper subcommands. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: use `OPT_CMDMODE()` to specify modesPatrick Steinhardt2024-05-062-18/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The git-config(1) command has various different modes which are accessible via e.g. `--get-urlmatch` or `--unset-all`. These modes are declared with `OPT_BIT()`, which causes two minor issues: - The respective modes also have a negated form `--no-get-urlmatch`, which is unintended. - We have to manually handle exclusiveness of the modes. Switch these options to instead use `OPT_CMDMODE()`, which is made exactly for this usecase. Remove the now-unneeded check that only a single mode is given, which is now handled by the parse-options interface. While at it, format optional placeholders for arguments to conform to our style guidelines by using `[<placeholder>]`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: move "fixed-value" option to correct groupPatrick Steinhardt2024-05-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `--fixed-value` option can be used to alter how the value-pattern parameter is interpreted for the various actions of git-config(1). But while it is an option, it is currently listed as part of the actions group, which is wrong. Move the option to the "Other" group, which hosts the various options known to git-config(1). Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | builtin/config: move option array aroundPatrick Steinhardt2024-05-061-48/+48
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move around the option array. This will help us with a follow-up commit that introduces subcommands to git-config(1). Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | config: clarify memory ownership when preparing comment stringsPatrick Steinhardt2024-05-063-16/+13
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The ownership of memory returned when preparing a comment string is quite intricate: when the returned value is different than the passed value, then the caller is responsible to free the memory. This is quite subtle, and it's even easier to miss because the returned value is in fact a `const char *`. Adapt the function to always return either `NULL` or a newly allocated string. The function is called at most once per git-config(1), so it's not like this micro-optimization really matters. Thus, callers are now always responsible for freeing the value. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'js/unit-test-suite-runner'Junio C Hamano2024-05-1511-30/+74
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "test-tool" has been taught to run testsuite tests in parallel, bypassing the need to use the "prove" tool. * js/unit-test-suite-runner: cmake: let `test-tool` run the unit tests, too ci: use test-tool as unit test runner on Windows t/Makefile: run unit tests alongside shell tests unit tests: add rule for running with test-tool test-tool run-command testsuite: support unit tests test-tool run-command testsuite: remove hardcoded filter test-tool run-command testsuite: get shell from env t0080: turn t-basic unit test into a helper
| * | | | cmake: let `test-tool` run the unit tests, tooJohannes Schindelin2024-05-061-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `test-tool` recently learned to run the unit tests. To this end, it needs to link with `test-lib.c`, which was done in the `Makefile`, and this patch does it in the CMake definition, too. This is a companion of 44400f58407e (t0080: turn t-basic unit test into a helper, 2024-02-02). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | ci: use test-tool as unit test runner on WindowsJosh Steadmon2024-05-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Although the previous commit changed t/Makefile to run unit tests alongside shell tests, the Windows CI still needs a separate unit-tests step due to how the test sharding works. We want to avoid using `prove` as a test running on Windows due to performance issues [1], so use the new test-tool runner instead. [1] https://lore.kernel.org/git/850ea42c-f103-68d5-896b-9120e2628686@gmx.de/ Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t/Makefile: run unit tests alongside shell testsJeff King2024-05-063-3/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a wrapper script to allow `prove` to run both shell tests and unit tests from a single invocation. This avoids issues around running prove twice in CI, as discussed in [1]. Additionally, this moves the unit tests into the main dev workflow, so that errors can be spotted more quickly. Accordingly, we remove the separate unit tests step for Linux CI. (We leave the Windows CI unit-test step as-is, because the sharding scheme there involves selecting specific test files rather than running `make test`.) [1] https://lore.kernel.org/git/pull.1613.git.1699894837844.gitgitgadget@gmail.com/ Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>