summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2024-05-07refs: use transaction in `refs_create_symref()`Karthik Nayak3-8/+41
The `refs_create_symref()` function updates a symref to a given new target. To do this, it uses a ref-backend specific function `create_symref()`. In the previous commits, we introduced symref support in transactions. This means we can now use transactions to perform symref updates and don't have to resort to `create_symref()`. Doing this allows us to remove and cleanup `create_symref()`, which we will do in the following commit. Modify the expected error message for a test in 't/t0610-reftable-basics.sh', since the error is now thrown from 'refs.c'. This is because in transactional updates, F/D conflicts are caught before we're in the reference backend. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07refs: add support for transactional symref updatesKarthik Nayak4-39/+196
The reference backends currently support transactional reference updates. While this is exposed to users via 'git-update-ref' and its '--stdin' mode, it is also used internally within various commands. However, we do not support transactional updates of symrefs. This commit adds support for symrefs in both the 'files' and the 'reftable' backend. Here, we add and use `ref_update_has_null_new_value()`, a helper function which is used to check if there is a new_value in a reference update. The new value could either be a symref target `new_target` or a OID `new_oid`. We also add another common function `ref_update_check_old_target` which will be used to check if the update's old_target corresponds to a reference's current target. Now transactional updates (verify, create, delete, update) can be used for: - regular refs - symbolic refs - conversion of regular to symbolic refs and vice versa This also allows us to expose this to users via new commands in 'git-update-ref' in the future. Note that a dangling symref update does not record a new reflog entry, which is unchanged before and after this commit. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07refs: move `original_update_refname` to 'refs.c'Karthik Nayak4-33/+25
The files backend and the reftable backend implement `original_update_refname` to obtain the original refname of the update. Move it out to 'refs.c' and only expose it internally to the refs library. This will be used in an upcoming commit to also introduce another common functionality for the two backends. We also rename the function to `ref_update_original_update_refname` to keep it consistent with the upcoming other 'ref_update_*' functions that'll be introduced. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07refs: support symrefs in 'reference-transaction' hookKarthik Nayak2-9/+25
The 'reference-transaction' hook runs whenever a reference update is made to the system. In a previous commit, we added the `old_target` and `new_target` fields to the `reference_transaction_update()`. In following commits we'll also add the code to handle symref's in the reference backends. Support symrefs also in the 'reference-transaction' hook, by modifying the current format: <old-oid> SP <new-oid> SP <ref-name> LF to be be: <old-value> SP <new-value> SP <ref-name> LF where for regular refs the output would not change and remain the same. But when either 'old-value' or 'new-value' is a symref, we print the ref as 'ref:<ref-target>'. This does break backward compatibility, but the 'reference-transaction' hook's documentation always stated that support for symbolic references may be added in the future. We do not add any tests in this commit since there is no git command which activates this flow, in an upcoming commit, we'll start using transaction based symref updates as the default, we'll add tests there for the hook too. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07files-backend: extract out `create_symref_lock()`Karthik Nayak1-14/+37
The function `create_symref_locked()` creates a symref by creating a '<symref>.lock' file and then committing the symref lock, which creates the final symref. Extract the early half of `create_symref_locked()` into a new helper function `create_symref_lock()`. Because the name of the new function is too similar to the original, rename the original to `create_and_commit_symref()` to avoid confusion. The new function `create_symref_locked()` can be used to create the symref lock in a separate step from that of committing it. This allows to add transactional support for symrefs, where the lock would be created in the preparation step and the lock would be committed in the finish step. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07refs: accept symref values in `ref_transaction_update()`Karthik Nayak14-24/+71
The function `ref_transaction_update()` obtains ref information and flags to create a `ref_update` and add them to the transaction at hand. To extend symref support in transactions, we need to also accept the old and new ref targets and process it. This commit adds the required parameters to the function and modifies all call sites. The two parameters added are `new_target` and `old_target`. The `new_target` is used to denote what the reference should point to when the transaction is applied. Some functions allow this parameter to be NULL, meaning that the reference is not changed. The `old_target` denotes the value the reference must have before the update. Some functions allow this parameter to be NULL, meaning that the old value of the reference is not checked. We also update the internal function `ref_transaction_add_update()` similarly to take the two new parameters. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06repository: stop setting SHA1 as the default object hashPatrick Steinhardt1-20/+0
During the startup of Git, we call `initialize_the_repository()` to set up `the_repository` as well as `the_index`. Part of this setup is also to set the default object hash of the repository to SHA1. This has the effect that `the_hash_algo` is getting initialized to SHA1, as well. This default hash algorithm eventually gets overridden by most Git commands via `setup_git_directory()`, which also detects the actual hash algorithm used by the repository. There are some commands though that don't access a repository at all, or at a later point only, and thus retain the default hash function for some amount of time. As some of the the preceding commits demonstrate, this can lead to subtle issues when we access `the_hash_algo` when no repository has been set up. Address this issue by dropping the set up of the default hash algorithm completely. The effect of this is that `the_hash_algo` will map to a `NULL` pointer and thus cause Git to crash when something tries to access the hash algorithm without it being properly initialized. It thus forces all Git commands to explicitly set up the hash algorithm in case there is no repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06oss-fuzz/commit-graph: set up hash algorithmPatrick Steinhardt1-0/+1
Our fuzzing setups don't work in a proper repository, but only use the in-memory configured `the_repository`. Consequently, we never go through the full repository setup procedures and thus do not set up the hash algo used by the repository. The commit-graph fuzzer does rely on a properly initialized hash algo though. Initialize it explicitly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06builtin/shortlog: don't set up revisions without repoPatrick Steinhardt1-1/+1
It is possible to run git-shortlog(1) outside of a repository by passing it output from git-log(1) via standard input. Obviously, as there is no repository in that context, it is thus unsupported to pass any revisions as arguments. Regardless of that we still end up calling `setup_revisions()`. While that works alright, it is somewhat strange. Furthermore, this is about to cause problems when we unset the default object hash. Refactor the code to only call `setup_revisions()` when we have a repository. This is safe to do as we already verify that there are no arguments when running outside of a repository anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06builtin/diff: explicitly set hash algo when there is no repoPatrick Steinhardt1-0/+9
The git-diff(1) command can be used outside repositories to diff two files with each other. But even if there is no repository we will end up hashing the files that we are diffing so that we can print the "index" line: ``` diff --git a/a b/b index 7898192..6178079 100644 --- a/a +++ b/b @@ -1 +1 @@ -a +b ``` We implicitly use SHA1 to calculate the hash here, which is because `the_repository` gets initialized with SHA1 during the startup routine. We are about to stop doing this though such that `the_repository` only ever has a hash function when it was properly initialized via a repo's configuration. To give full control to our users, we would ideally add a new switch to git-diff(1) that allows them to specify the hash function when executed outside of a repository. But for now, we only convert the code to make this explicit such that we can stop setting the default hash algorithm for `the_repository`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06builtin/bundle: abort "verify" early when there is no repositoryPatrick Steinhardt1-0/+5
Verifying a bundle requires us to have a repository. This is encoded in `verify_bundle()`, which will return an error if there is no repository. We call `open_bundle()` before we call `verify_bundle()` though, which already performs some verifications even though we may ultimately abort due to a missing repository. This is problematic because `open_bundle()` already reads the bundle header and verifies that it contains a properly formatted hash. When there is no repository we have no clue what hash function to expect though, so we always end up assuming SHA1 here, which may or may not be correct. Furthermore, we are about to stop initializing `the_hash_algo` when there is no repository, which will lead to segfaults. Check early on whether we have a repository to fix this issue. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06builtin/blame: don't access potentially unitialized `the_hash_algo`Patrick Steinhardt1-3/+2
We access `the_hash_algo` in git-blame(1) before we have executed `parse_options_start()`, which may not be properly set up in case we have no repository. This is fine for most of the part because all the call paths that lead to it (git-blame(1), git-annotate(1) as well as git-pick-axe(1)) specify `RUN_SETUP` and thus require a repository. There is one exception though, namely when passing `-h` to print the help. Here we will access `the_hash_algo` even if there is no repo. This works fine right now because `the_hash_algo` gets sets up to point to the SHA1 algorithm via `initialize_repository()`. But we're about to stop doing this, and thus the code would lead to a `NULL` pointer exception. Prepare the code for this and only access `the_hash_algo` after we are sure that there is a proper repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06builtin/rev-parse: allow shortening to more than 40 hex charactersPatrick Steinhardt2-3/+8
The `--short=` option for git-rev-parse(1) allows the user to specify to how many characters object IDs should be shortened to. The option is broken though for SHA256 repositories because we set the maximum allowed hash size to `the_hash_algo->hexsz` before we have even set up the repo. Consequently, `the_hash_algo` will always be SHA1 and thus we truncate every hash after at most 40 characters. Fix this by accessing `the_hash_algo` only after we have set up the repo. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06remote-curl: fix parsing of detached SHA256 headsPatrick Steinhardt2-1/+33
The dumb HTTP transport tries to read the remote HEAD reference by downloading the "HEAD" file and then parsing it via `http_fetch_ref()`. This function will either parse the file as an object ID in case it is exactly `the_hash_algo->hexsz` long, or otherwise it will check whether the reference starts with "ref :" and parse it as a symbolic ref. This is broken when parsing detached HEADs of a remote SHA256 repository because we never update `the_hash_algo` to the discovered remote object hash. Consequently, `the_hash_algo` will always be the fallback SHA1 hash algorithm, which will cause us to fail parsing HEAD altogteher when it contains a SHA256 object ID. Fix this issue by setting up `the_hash_algo` via `repo_set_hash_algo()`. While at it, let's make the expected SHA1 fallback explicit in our code, which also addresses an upcoming issue where we are going to remove the SHA1 fallback for `the_hash_algo`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06attr: fix BUG() when parsing attrs outside of repoPatrick Steinhardt2-0/+21
If either the `--attr-source` option or the `GIT_ATTR_SOURCE` envvar are set, then `compute_default_attr_source()` will try to look up the value as a treeish. It is possible to hit that function while outside of a Git repository though, for example when using `git grep --no-index`. In that case, Git will hit a bug because we try to look up the main ref store outside of a repository. Handle the case gracefully and detect when we try to look up an attr source without a repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06attr: don't recompute default attribute sourcePatrick Steinhardt1-9/+16
The `default_attr_source()` function lazily computes the attr source supposedly once, only. This is done via a static variable `attr_source` that contains the resolved object ID of the attr source's tree. If the variable is the null object ID then we try to look up the attr source, otherwise we skip over it. This approach is flawed though: the variable will never be set to anything else but the null object ID in case there is no attr source. Consequently, we re-compute the information on every call. And in the worst case, when we silently ignore bad trees, this will cause us to try and look up the treeish every single time. Improve this by introducing a separate variable `has_attr_source` to track whether we already computed the attr source and, if so, whether we have an attr source or not. This also allows us to convert the `ignore_bad_attr_tree` to not be static anymore as the code will only be executed once anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06parse-options-cb: only abbreviate hashes when hash algo is knownPatrick Steinhardt2-1/+19
The `OPT__ABBREV()` option can be used to add an option that abbreviates object IDs. When given a length longer than `the_hash_algo->hexsz`, then it will instead set the length to that maximum length. It may not always be guaranteed that we have `the_hash_algo` initialized properly as the hash algorithm can only be set up after we have set up `the_repository`. In that case, the hash would always be truncated to the hex length of SHA1, which may not be what the user desires. In practice it's not a problem as all commands that use `OPT__ABBREV()` also have `RUN_SETUP` set and thus cannot work without a repository. Consequently, both `the_repository` and `the_hash_algo` would be properly set up. Regardless of that, harden the code to not truncate the length when we didn't set up a repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06path: move `validate_headref()` to its only userPatrick Steinhardt3-54/+53
While `validate_headref()` is only called from `is_git_directory()` in "setup.c", it is currently implemented in "path.c". Move it over such that it becomes clear that it is only really used during setup in order to discover repositories. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06path: harden validation of HEAD with non-standard hashesPatrick Steinhardt1-1/+1
The `validate_headref()` function takes a path to a supposed "HEAD" file and checks whether its format is something that we understand. It is used as part of our repository discovery to check whether a specific directory is a Git directory or not. Part of the validation is a check for a detached HEAD that contains a plain object ID. To do this validation we use `get_oid_hex()`, which relies on `the_hash_algo`. At this point in time the hash algo cannot yet be initialized though because we didn't yet read the Git config. Consequently, it will always be the SHA1 hash algorithm. In practice this works alright because `get_oid_hex()` only ends up checking whether the prefix of the buffer is a valid object ID. And because SHA1 is shorter than SHA256, the function will successfully parse SHA256 object IDs, as well. It is somewhat fragile though and not really the intent to only check for SHA1. With this in mind, harden the code to use `get_oid_hex_any()` to check whether the "HEAD" file parses as any known hash. One might be hard pressed to tighten the check even further and fully validate the file contents, not only the prefix. In practice though that wouldn't make a lot of sense as it could be that the repository uses a hash function that produces longer hashes than SHA256, but which the current version of Git doesn't understand yet. We'd still want to detect the repository as proper Git repository in that case, and we will fail eventually with a proper error message that the hash isn't understood when trying to set up the repository format. It follows that we could just leave the current code intact, as in practice the code change doesn't have any user visible impact. But it also prepares us for `the_hash_algo` being unset when there is no repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06Merge branch 'ps/the-index-is-no-more' into ps/undecided-is-not-necessarily-sha1Junio C Hamano41-455/+435
* ps/the-index-is-no-more: repository: drop `initialize_the_repository()` repository: drop `the_index` variable builtin/clone: stop using `the_index` repository: initialize index in `repo_init()` builtin: stop using `the_index` t/helper: stop using `the_index`
2024-05-06Merge branch 'jc/no-default-attr-tree-in-bare' into ↵Junio C Hamano3-10/+10
ps/undecided-is-not-necessarily-sha1 * jc/no-default-attr-tree-in-bare: stop using HEAD for attributes in bare repository by default
2024-05-06cmake: let `test-tool` run the unit tests, tooJohannes Schindelin1-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>
2024-05-06ci: use test-tool as unit test runner on WindowsJosh Steadmon1-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>
2024-05-06t/Makefile: run unit tests alongside shell testsJeff King3-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>
2024-05-06unit tests: add rule for running with test-toolJosh Steadmon2-2/+10
In the previous commit, we added support in test-tool for running collections of unit tests. Now, add rules in t/Makefile for running in this way. This new rule can be executed from the top-level Makefile via `make DEFAULT_UNIT_TEST_TARGET=unit-tests-test-tool unit-tests`, or by setting DEFAULT_UNIT_TEST_TARGET in config.mak. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06test-tool run-command testsuite: support unit testsJosh Steadmon1-3/+14
Teach the testsuite runner in `test-tool run-command testsuite` how to run unit tests: if TEST_SHELL_PATH is not set, run the programs directly from CWD, rather than defaulting to "sh" as an interpreter. With this change, you can now use test-tool to run the unit tests: $ make $ cd t/unit-tests/bin $ ../../helper/test-tool run-command testsuite This should be helpful on Windows to allow running tests without requiring Perl (for `prove`), as discussed in [1] and [2]. This again breaks backwards compatibility, as it is now required to set TEST_SHELL_PATH properly for executing shell scripts, but again, as noted in [2], there are no longer any such invocations in our codebase. [1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.2109091323150.59@tvgsbejvaqbjf.bet/ [2] 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>
2024-05-06test-tool run-command testsuite: remove hardcoded filterJosh Steadmon1-3/+1
`test-tool run-command testsuite` currently assumes that it will only be running the shell test suite, and therefore filters out anything that does not match a hardcoded pattern of "t[0-9][0-9][0-9][0-9]-*.sh". Later in this series, we'll adapt `test-tool run-command testsuite` to also support unit tests, which do not follow the same naming conventions as the shell tests, so this hardcoded pattern is inconvenient. Since `testsuite` also allows specifying patterns on the command-line, let's just remove this pattern. As noted in [1], there are no longer any uses of `testsuite` in our codebase, it should be OK to break backwards compatibility in this case. We also add a new filter to avoid trying to execute "." and "..", so that users who wish to execute every test in a directory can do so without specifying a pattern. [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>
2024-05-06test-tool run-command testsuite: get shell from envJosh Steadmon1-1/+8
When running tests through `test-tool run-command testsuite`, we currently hardcode `sh` as the command interpreter. As discussed in [1], this is incorrect, and we should be using the shell set in TEST_SHELL_PATH instead. Add a shell_path field in struct testsuite so that we can pass this to the task runner callback. If this is non-null, we'll use it as the argv[0] of the subprocess. Otherwise, we'll just execute the test program directly. We will use this feature in a later commit to enable running binary executable unit tests. However, for now when setting up the struct testsuite in testsuite(), use the value of TEST_SHELL_PATH if it's set, otherwise keep the original behavior by defaulting to `sh`. [1] https://lore.kernel.org/git/20240123005913.GB835964@coredump.intra.peff.net/ Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06t0080: turn t-basic unit test into a helperJosh Steadmon6-17/+20
While t/unit-tests/t-basic.c uses the unit-test framework added in e137fe3b29 (unit tests: add TAP unit test framework, 2023-11-09), it is not a true unit test in that it intentionally fails in order to exercise various codepaths in the unit-test framework. Thus, we intentionally exclude it when running unit tests through the various t/Makefile targets. Instead, it is executed by t0080-unit-test-output.sh, which verifies its output follows the TAP format expected for the various pass, skip, or fail cases. As such, it makes more sense for t-basic to be a helper item for t0080-unit-test-output.sh, so let's move it to t/helper/test-example-tap.c and adjust Makefiles as necessary. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06ci: fix Python dependency on Ubuntu 24.04Patrick Steinhardt1-2/+6
Newer versions of Ubuntu have dropped Python 2 starting with Ubuntu 23.04. By default though, our CI setups will try to use that Python version on all Ubuntu-based jobs except for the "linux-gcc" one. We didn't notice this issue due to two reasons: - The "ubuntu:latest" tag always points to the latest LTS release. Until a few weeks ago this was Ubuntu 22.04, which still had Python 2. - Our Docker-based CI jobs had their own script to install dependencies until 9cdeb34b96 (ci: merge scripts which install dependencies, 2024-04-12), where we didn't even try to install Python at all for many of them. Since the CI refactorings have originally been implemented, Ubuntu 24.04 was released, and it being an LTS versions means that the "latest" tag now points to that Python-2-less version. Consequently, those jobs that use "ubuntu:latest" broke. Address this by using Python 2 on Ubuntu 20.04, only, whereas we use Python 3 on all other Ubuntu jobs. Eventually, we should think about dropping support for Python 2 completely. Reported-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06Documentation: Mention that refspecs are explained elsewhereØystein Walle1-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>
2024-05-06format-patch: run range-diff with larger creation-factorJunio C Hamano3-1/+12
We see too often that a range-diff added to format-patch output shows too many "unmatched" patches. This is because the default value for creation-factor is set to a relatively low value. It may be justified for other uses (like you have a yet-to-be-sent new iteration of your series, and compare it against the 'seen' branch that has an older iteration, probably with the '--left-only' option, to pick out only your patches while ignoring the others) of "range-diff" command, but when the command is run as part of the format-patch, the user _knows_ and expects that the patches in the old and the new iterations roughly correspond to each other, so we can and should use a much higher default. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06gitlab-ci: add smoke test for fuzzersPatrick Steinhardt1-0/+9
Our GitLab CI setup has a test gap where the fuzzers aren't exercised at all. Add a smoke test, similar to the one we have in GitHub Workflows. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06builtin/config: display subcommand helpPatrick Steinhardt2-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>
2024-05-06builtin/config: introduce "edit" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: introduce "remove-section" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: introduce "rename-section" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: introduce "unset" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: introduce "set" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: introduce "get" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: introduce "list" subcommandPatrick Steinhardt3-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>
2024-05-06builtin/config: pull out function to handle `--null`Patrick Steinhardt1-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>
2024-05-06builtin/config: pull out function to handle config locationPatrick Steinhardt1-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>
2024-05-06builtin/config: use `OPT_CMDMODE()` to specify modesPatrick Steinhardt2-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>
2024-05-06builtin/config: move "fixed-value" option to correct groupPatrick Steinhardt1-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>
2024-05-06builtin/config: move option array aroundPatrick Steinhardt1-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>
2024-05-06config: clarify memory ownership when preparing comment stringsPatrick Steinhardt3-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>
2024-05-06diff: fix --exit-code with external diffRené Scharfe2-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>
2024-05-06diff: report unmerged paths as changes in run_diff_cmd()René Scharfe2-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>
2024-05-06refs: return conflict error when checking packed refsIvan Tse2-1/+19
The TRANSACTION_NAME_CONFLICT error code refers to a failure to create a ref due to a name conflict with another ref. An example of this is a directory/file conflict such as ref names A/B and A. "git fetch" uses this error code to more accurately describe the error by recommending to the user that they try running "git remote prune" to remove any old refs that are deleted by the remote which would clear up any directory/file conflicts. This helpful error message is not displayed when the conflicted ref is stored in packed refs. This change fixes this by ensuring error return code consistency in `lock_raw_ref`. Signed-off-by: Ivan Tse <ivan.tse1@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>