aboutsummaryrefslogtreecommitdiffstats
path: root/builtin
AgeCommit message (Collapse)AuthorFilesLines
15 hoursMerge branch 'la/hide-trailer-info'Junio C Hamano1-6/+6
The trailer API has been reshuffled a bit. * la/hide-trailer-info: trailer unit tests: inspect iterator contents trailer: document parse_trailers() usage trailer: retire trailer_info_get() from API trailer: make trailer_info struct private trailer: make parse_trailers() return trailer_info pointer interpret-trailers: access trailer_info with new helpers sequencer: use the trailer iterator trailer: teach iterator about non-trailer lines trailer: add unit tests for trailer iterator Makefile: sort UNIT_TEST_PROGRAMS
4 daysMerge branch 'kn/ref-transaction-symref'Junio C Hamano13-16/+20
Updates to symbolic refs can now be made as a part of ref transaction. * kn/ref-transaction-symref: refs: remove `create_symref` and associated dead code refs: rename `refs_create_symref()` to `refs_update_symref()` refs: use transaction in `refs_create_symref()` refs: add support for transactional symref updates refs: move `original_update_refname` to 'refs.c' refs: support symrefs in 'reference-transaction' hook files-backend: extract out `create_symref_lock()` refs: accept symref values in `ref_transaction_update()`
8 daysMerge branch 'ps/refs-without-the-repository'Junio C Hamano34-239/+384
The refs API lost functions that implicitly assumes to work on the primary ref_store by forcing the callers to pass a ref_store as an argument. * 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`
9 daysMerge branch 'jp/tag-trailer'Junio C Hamano2-26/+32
"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
9 daysMerge branch 'ps/config-subcommands'Junio C Hamano1-98/+414
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
10 daysSync with Git 2.45.1Junio C Hamano3-8/+131
* tag 'v2.45.1': (42 commits) Git 2.45.1 Git 2.44.1 Git 2.43.4 Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks ...
2024-05-08Merge branch 'ps/the-index-is-no-more'Junio C Hamano29-375/+359
The singleton index_state instance "the_index" has been eliminated by always instantiating "the_repository" and replacing references to "the_index" with references to its .index member. * 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-08Merge branch 'bc/credential-scheme-enhancement'Junio C Hamano4-8/+41
The credential helper protocol, together with the HTTP layer, have been enhanced to support authentication schemes different from username & password pair, like Bearer and NTLM. * bc/credential-scheme-enhancement: credential: add method for querying capabilities credential-cache: implement authtype capability t: add credential tests for authtype credential: add support for multistage credential rounds t5563: refactor for multi-stage authentication docs: set a limit on credential line length credential: enable state capability credential: add an argument to keep state http: add support for authtype and credential docs: indicate new credential protocol fields credential: add a field called "ephemeral" credential: gate new fields on capability credential: add a field for pre-encoded credentials http: use new headers for each object request remote-curl: reset headers on new request credential: add an authtype field
2024-05-07cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt34-239/+384
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>
2024-05-07refs: add `exclude_patterns` parameter to `for_each_fullref_in()`Patrick Steinhardt2-4/+4
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>
2024-05-07builtin/tag: add --trailer optionJohn Passaro1-9/+29
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>
2024-05-07builtin/commit: refactor --trailer logicJohn Passaro1-8/+2
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>
2024-05-07builtin/commit: use ARGV macro to collect trailersJohn Passaro1-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>
2024-05-07refs: rename `refs_create_symref()` to `refs_update_symref()`Karthik Nayak2-2/+2
The `refs_create_symref()` function is used to update/create a symref. But it doesn't check the old target of the symref, if existing. It force updates the symref. In this regard, the name `refs_create_symref()` is a bit misleading. So let's rename it to `refs_update_symref()`. This is akin to how 'git-update-ref(1)' also allows us to create apart from update. While we're here, rename the arguments in the function to clarify what they actually signify and reduce confusion. 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 Nayak6-4/+8
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-06builtin/config: display subcommand helpPatrick Steinhardt1-2/+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 Steinhardt1-26/+55
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 Steinhardt1-0/+32
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 Steinhardt1-0/+32
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 Steinhardt1-0/+39
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 Steinhardt1-0/+63
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 Steinhardt1-9/+60
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 Steinhardt1-12/+78
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 Steinhardt1-18/+14
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 Steinhardt1-5/+6
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-02trailer: make parse_trailers() return trailer_info pointerLinus Arver1-2/+2
This is the second and final preparatory commit for making the trailer_info struct private to the trailer implementation. Make trailer_info_get() do the actual work of allocating a new trailer_info struct, and return a pointer to it. Because parse_trailers() wraps around trailer_info_get(), it too can return this pointer to the caller. From the trailer API user's perspective, the call to trailer_info_new() can be replaced with parse_trailers(); do so in interpret-trailers. Because trailer_info_new() is no longer called by interpret-trailers, remove this function from the trailer API. With this change, we no longer allocate trailer_info on the stack --- all uses of it are via a pointer where the actual data is always allocated at runtime through trailer_info_new(). Make trailer_info_release() free this dynamically allocated memory. Finally, due to the way the function signatures of parse_trailers() and trailer_info_get() have changed, update the callsites in format_trailers_from_commit() and trailer_iterator_init() accordingly. Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-02interpret-trailers: access trailer_info with new helpersLinus Arver1-6/+6
Instead of directly accessing trailer_info members, access them indirectly through new helper functions exposed by the trailer API. This is the first of two preparatory commits which will allow us to use the so-called "pimpl" (pointer to implementation) idiom for the trailer API, by making the trailer_info struct private to the trailer implementation (and thus hidden from the API). Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-30Merge branch 'js/for-each-repo-keep-going'Junio C Hamano2-5/+15
A scheduled "git maintenance" job is expected to work on all repositories it knows about, but it stopped at the first one that errored out. Now it keeps going. * js/for-each-repo-keep-going: maintenance: running maintenance should not stop on errors for-each-repo: optionally keep going on an error
2024-04-30Merge branch 'aj/stash-staged-fix'Junio C Hamano1-2/+2
"git stash -S" did not handle binary files correctly, which has been corrected. * aj/stash-staged-fix: stash: fix "--staged" with binary files
2024-04-30Merge branch 'jc/format-patch-rfc-more'Junio C Hamano1-4/+22
The "--rfc" option of "git format-patch" learned to take an optional string value to be used in place of "RFC" to tweak the "[PATCH]" on the subject header. * jc/format-patch-rfc-more: format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)] format-patch: allow --rfc to optionally take a value, like --rfc=WIP
2024-04-30Merge branch 'ds/format-patch-rfc-and-k'Junio C Hamano1-1/+3
The "-k" and "--rfc" options of "format-patch" will now error out when used together, as one tells us not to add anything to the title of the commit, and the other one tells us to add "RFC" in addition to "PATCH". * ds/format-patch-rfc-and-k: format-patch: ensure that --rfc and -k are mutually exclusive
2024-04-30Merge branch 'xx/disable-replace-when-building-midx'Junio C Hamano1-0/+3
The procedure to build multi-pack-index got confused by the replace-refs mechanism, which has been corrected by disabling the latter. * xx/disable-replace-when-building-midx: midx: disable replace objects
2024-04-29Sync with 2.44.1Johannes Schindelin3-8/+131
* maint-2.44: (41 commits) Git 2.44.1 Git 2.43.4 Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel ...
2024-04-25Merge branch 'rj/add-i-leak-fix'Junio C Hamano1-3/+6
Leakfix. * rj/add-i-leak-fix: add: plug a leak on interactive_add add-patch: plug a leak handling the '/' command add-interactive: plug a leak in get_untracked_files apply: plug a leak in apply_data
2024-04-24maintenance: running maintenance should not stop on errorsJohannes Schindelin1-3/+4
In https://github.com/microsoft/git/issues/623, it was reported that maintenance stops on a missing repository, omitting the remaining repositories that were scheduled for maintenance. This is undesirable, as it should be a best effort type of operation. It should still fail due to the missing repository, of course, but not leave the non-missing repositories in unmaintained shapes. Let's use `for-each-repo`'s shiny new `--keep-going` option that we just introduced for that very purpose. This change will be picked up when running `git maintenance start`, which is run implicitly by `scalar reconfigure`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-24for-each-repo: optionally keep going on an errorJohannes Schindelin1-2/+11
In https://github.com/microsoft/git/issues/623, it was reported that the regularly scheduled maintenance stops if one repo in the middle of the list was found to be missing. This is undesirable, and points out a gap in the design of `git for-each-repo`: We need a mode where that command does not stop on an error, but continues to try running the specified command with the other repositories. Imitating the `--keep-going` option of GNU make, this commit teaches `for-each-repo` the same trick: to continue with the operation on all the remaining repositories in case there was a problem with one repository, still setting the exit code to indicate an error occurred. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-23Merge branch 'ps/run-auto-maintenance-in-receive-pack'Junio C Hamano1-11/+10
The "receive-pack" program (which responds to "git push") was not converted to run "git maintenance --auto" when other codepaths that used to run "git gc --auto" were updated, which has been corrected. * ps/run-auto-maintenance-in-receive-pack: builtin/receive-pack: convert to use git-maintenance(1) run-command: introduce function to prepare auto-maintenance process
2024-04-23Merge branch 'ta/fast-import-parse-path-fix'Junio C Hamano1-78/+84
The way "git fast-import" handles paths described in its input has been tightened up and more clearly documented. * ta/fast-import-parse-path-fix: fast-import: make comments more precise fast-import: forbid escaped NUL in paths fast-import: document C-style escapes for paths fast-import: improve documentation for path quoting fast-import: remove dead strbuf fast-import: allow unquoted empty path for root fast-import: directly use strbufs for paths fast-import: tighten path unquoting
2024-04-23format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]Junio C Hamano1-2/+6
In the previous step, the "--rfc" option of "format-patch" learned to take an optional string value to prepend to the subject prefix, so that --rfc=WIP can give "[WIP PATCH]". There may be cases in which the extra string wants to come after the subject prefix. Extend the mechanism to allow "--rfc=-(WIP)" [*] to signal that the extra string is to be appended instead of getting prepended, resulting in "[PATCH (WIP)]". In the documentation, discourage (ab)using "--rfc=-RFC" to say "[PATCH RFC]" just to be different, when "[RFC PATCH]" is the norm. [Footnote] * The syntax takes inspiration from Perl's open syntax that opens pipes "open fh, '|-', 'cmd'", where the dash signals "the other stuff comes here". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-23format-patch: allow --rfc to optionally take a value, like --rfc=WIPJunio C Hamano1-4/+19
With the "--rfc" option, we can tweak the "[PATCH]" (or whatever string specified with the "--subject-prefix" option, instead of "PATCH") that we prefix the title of the commit with into "[RFC PATCH]", but some projects may want "[rfc PATCH]". Adding a new option, e.g., "--rfc-lowercase", to support such need every time somebody wants to use different strings would lead to insanity of accumulating unbounded number of such options. Allow an optional value specified for the option, so that users can use "--rfc=rfc" (think of "--rfc" without value as a short-hand for "--rfc=RFC") if they wanted to. This can of course be (ab)used to make the prefix "[WIP PATCH]" by passing "--rfc=WIP". Passing an empty string, i.e., "--rfc=", is the same as "--no-rfc" to override an option given earlier on the same command line. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-22add: plug a leak on interactive_addRubén Justo1-3/+6
Plug a leak we have since 5a76aff1a6 (add: convert to use parse_pathspec, 2013-07-14). This leak can be triggered with: $ git add -p anything Fixing this leak allows us to mark as leak-free the following tests: + t3701-add-interactive.sh + t7514-commit-patch.sh Mark them with "TEST_PASSES_SANITIZE_LEAK=true" to notice and fix promply any new leak that may be introduced and triggered by them in the future. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-22stash: fix "--staged" with binary filesAdam Johnson1-2/+2
"git stash --staged" errors out when given binary files, after saving the stash. This behaviour dates back to the addition of the feature in 41a28eb6c1 (stash: implement '--staged' option for 'push' and 'save', 2021-10-18). Adding the "--binary" option of "diff-tree" fixes this. The "diff-tree" call in stash_patch() also omits "--binary", but that is fine since binary files cannot be selected interactively. Helped-By: Jeff King <peff@peff.net> Helped-By: Randall S. Becker <randall.becker@nexbridge.ca> Signed-off-by: Adam Johnson <me@adamj.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-19format-patch: ensure that --rfc and -k are mutually exclusiveDragan Simic1-1/+3
Fix a bug that allows the "--rfc" and "-k" options to be specified together when "git format-patch" is executed, which was introduced in the commit e0d7db7423a9 ("format-patch: --rfc honors what --subject-prefix sets"). Add a couple of additional tests to t4014, to cover additional cases of the mutual exclusivity between different "git format-patch" options. Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-19Sync with 2.43.4Johannes Schindelin3-8/+131
* maint-2.43: (40 commits) Git 2.43.4 Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories ...
2024-04-19Sync with 2.42.2Johannes Schindelin3-8/+131
* maint-2.42: (39 commits) Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' ...
2024-04-19Sync with 2.41.1Johannes Schindelin3-8/+131
* maint-2.41: (38 commits) Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' docs: document security issues around untrusted .git dirs ...
2024-04-19Sync with 2.40.2Johannes Schindelin4-26/+135
* maint-2.40: (39 commits) Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' docs: document security issues around untrusted .git dirs upload-pack: disable lazy-fetching by default ...
2024-04-19Sync with 2.39.4Johannes Schindelin4-26/+135
* maint-2.39: (38 commits) Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' docs: document security issues around untrusted .git dirs upload-pack: disable lazy-fetching by default fetch/clone: detect dubious ownership of local repositories ...
2024-04-19Merge branch 'ownership-checks-in-local-clones'Johannes Schindelin1-5/+34
This topic addresses two CVEs: - CVE-2024-32020: Local clones may end up hardlinking files into the target repository's object database when source and target repository reside on the same disk. If the source repository is owned by a different user, then those hardlinked files may be rewritten at any point in time by the untrusted user. - CVE-2024-32021: When cloning a local source repository that contains symlinks via the filesystem, Git may create hardlinks to arbitrary user-readable files on the same filesystem as the target repository in the objects/ directory. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-19clone: prevent hooks from running during a cloneJohannes Schindelin1-1/+11
Critical security issues typically combine relatively common vulnerabilities such as case confusion in file paths with other weaknesses in order to raise the severity of the attack. One such weakness that has haunted the Git project in many a submodule-related CVE is that any hooks that are found are executed during a clone operation. Examples are the `post-checkout` and `fsmonitor` hooks. However, Git's design calls for hooks to be disabled by default, as only disabled example hooks are copied over from the templates in `<prefix>/share/git-core/templates/`. As a defense-in-depth measure, let's prevent those hooks from running. Obviously, administrators can choose to drop enabled hooks into the template directory, though, _and_ it is also possible to override `core.hooksPath`, in which case the new check needs to be disabled. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-18builtin/clone: stop using `the_index`Patrick Steinhardt1-4/+3
Convert git-clone(1) to use `the_repository->index` instead of `the_index`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-18builtin: stop using `the_index`Patrick Steinhardt28-371/+356
Convert builtins to use `the_repository->index` instead of `the_index`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-17init: refactor the template directory discovery into its own functionJohannes Schindelin1-18/+4
We will need to call this function from `hook.c` to be able to prevent hooks from running that were written as part of a `clone` but did not originate from the template directory. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17submodule: require the submodule path to contain directories onlyJohannes Schindelin1-1/+31
Submodules are stored in subdirectories of their superproject. When these subdirectories have been replaced with symlinks by a malicious actor, all kinds of mayhem can be caused. This _should_ not be possible, but many CVEs in the past showed that _when_ possible, it allows attackers to slip in code that gets executed during, say, a `git clone --recursive` operation. Let's add some defense-in-depth to disallow submodule paths to have anything except directories in them. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17clone_submodule: avoid using `access()` on directoriesJohannes Schindelin1-1/+1
In 0060fd1511b (clone --recurse-submodules: prevent name squatting on Windows, 2019-09-12), I introduced code to verify that a git dir either does not exist, or is at least empty, to fend off attacks where an inadvertently (and likely maliciously) pre-populated git dir would be used while cloning submodules recursively. The logic used `access(<path>, X_OK)` to verify that a directory exists before calling `is_empty_dir()` on it. That is a curious way to check for a directory's existence and might well fail for unwanted reasons. Even the original author (it was I ;-) ) struggles to explain why this function was used rather than `stat()`. This code was _almost_ copypastad in the previous commit, but that `access()` call was caught during review. Let's use `stat()` instead also in the code that was almost copied verbatim. Let's not use `lstat()` because in the unlikely event that somebody snuck a symbolic link in, pointing to a crafted directory, we want to verify that that directory is empty. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17submodules: submodule paths must not contain symlinksJohannes Schindelin1-0/+35
When creating a submodule path, we must be careful not to follow symbolic links. Otherwise we may follow a symbolic link pointing to a gitdir (which are valid symbolic links!) e.g. while cloning. On case-insensitive filesystems, however, we blindly replace a directory that has been created as part of the `clone` operation with a symlink when the path to the latter differs only in case from the former's path. Let's simply avoid this situation by expecting not ever having to overwrite any existing file/directory/symlink upon cloning. That way, we won't even replace a directory that we just created. This addresses CVE-2024-32002. Reported-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17clone: prevent clashing git dirs when cloning submodule in parallelFilip Hejsek1-0/+17
While it is expected to have several git dirs within the `.git/modules/` tree, it is important that they do not interfere with each other. For example, if one submodule was called "captain" and another submodule "captain/hooks", their respective git dirs would clash, as they would be located in `.git/modules/captain/` and `.git/modules/captain/hooks/`, respectively, i.e. the latter's files could clash with the actual Git hooks of the former. To prevent these clashes, and in particular to prevent hooks from being written and then executed as part of a recursive clone, we introduced checks as part of the fix for CVE-2019-1387 in a8dee3ca61 (Disallow dubiously-nested submodule git directories, 2019-10-01). It is currently possible to bypass the check for clashing submodule git dirs in two ways: 1. parallel cloning 2. checkout --recurse-submodules Let's check not only before, but also after parallel cloning (and before checking out the submodule), that the git dir is not clashing with another one, otherwise fail. This addresses the parallel cloning issue. As to the parallel checkout issue: It requires quite a few manual steps to create clashing git dirs because Git itself would refuse to initialize the inner one, as demonstrated by the test case. Nevertheless, let's teach the recursive checkout (namely, the `submodule_move_head()` function that is used by the recursive checkout) to be careful to verify that it does not use a clashing git dir, and if it does, disable it (by deleting the `HEAD` file so that subsequent Git calls won't recognize it as a git dir anymore). Note: The parallel cloning test case contains a `cat err` that proved to be highly useful when analyzing the racy nature of the operation (the operation can fail with three different error messages, depending on timing), and was left on purpose to ease future debugging should the need arise. Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17upload-pack: disable lazy-fetching by defaultJeff King1-0/+2
The upload-pack command tries to avoid trusting the repository in which it's run (e.g., by not running any hooks and not using any config that contains arbitrary commands). But if the server side of a fetch or a clone is a partial clone, then either upload-pack or its child pack-objects may run a lazy "git fetch" under the hood. And it is very easy to convince fetch to run arbitrary commands. The "server" side can be a local repository owned by someone else, who would be able to configure commands that are run during a clone with the current user's permissions. This issue has been designated CVE-2024-32004. The fix in this commit's parent helps in this scenario, as well as in related scenarios using SSH to clone, where the untrusted .git directory is owned by a different user id. But if you received one as a zip file, on a USB stick, etc, it may be owned by your user but still untrusted. This has been designated CVE-2024-32465. To mitigate the issue more completely, let's disable lazy fetching entirely during `upload-pack`. While fetching from a partial repository should be relatively rare, it is certainly not an unreasonable workflow. And thus we need to provide an escape hatch. This commit works by respecting a GIT_NO_LAZY_FETCH environment variable (to skip the lazy-fetch), and setting it in upload-pack, but only when the user has not already done so (which gives us the escape hatch). The name of the variable is specifically chosen to match what has already been added in 'master' via e6d5479e7a (git: extend --no-lazy-fetch to work across subprocesses, 2024-02-27). Since we're building this fix as a backport for older versions, we could cherry-pick that patch and its earlier steps. However, we don't really need the niceties (like a "--no-lazy-fetch" option) that it offers. By using the same name, everything should just work when the two are eventually merged, but here are a few notes: - the blocking of the fetch in e6d5479e7a is incomplete! It sets fetch_if_missing to 0 when we setup the repository variable, but that isn't enough. pack-objects in particular will call prefetch_to_pack() even if that variable is 0. This patch by contrast checks the environment variable at the lowest level before we call the lazy fetch, where we can be sure to catch all code paths. Possibly the setting of fetch_if_missing from e6d5479e7a can be reverted, but it may be useful to have. For example, some code may want to use that flag to change behavior before it gets to the point of trying to start the fetch. At any rate, that's all outside the scope of this patch. - there's documentation for GIT_NO_LAZY_FETCH in e6d5479e7a. We can live without that here, because for the most part the user shouldn't need to set it themselves. The exception is if they do want to override upload-pack's default, and that requires a separate documentation section (which is added here) - it would be nice to use the NO_LAZY_FETCH_ENVIRONMENT macro added by e6d5479e7a, but those definitions have moved from cache.h to environment.h between 2.39.3 and master. I just used the raw string literals, and we can replace them with the macro once this topic is merged to master. At least with respect to CVE-2024-32004, this does render this commit's parent commit somewhat redundant. However, it is worth retaining that commit as defense in depth, and because it may help other issues (e.g., symlink/hardlink TOCTOU races, where zip files are not really an interesting attack vector). The tests in t0411 still pass, but now we have _two_ mechanisms ensuring that the evil command is not run. Let's beef up the existing ones to check that they failed for the expected reason, that we refused to run upload-pack at all with an alternate user id. And add two new ones for the same-user case that both the restriction and its escape hatch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17midx: disable replace objectsXing Xin1-0/+3
We observed a series of clone failures arose in a specific set of repositories after we fully enabled the MIDX bitmap feature within our Codebase service. These failures were accompanied with error messages such as: Cloning into bare repository 'clone.git'... remote: Enumerating objects: 8, done. remote: Total 8 (delta 0), reused 0 (delta 0), pack-reused 8 (from 1) Receiving objects: 100% (8/8), done. fatal: did not receive expected object ... fatal: fetch-pack: invalid index-pack output Temporarily disabling the MIDX feature eliminated the reported issues. After some investigation we found that all repositories experiencing failures contain replace references, which seem to be improperly acknowledged by the MIDX bitmap generation logic. A more thorough explanation about the root cause from Taylor Blau says: Indeed, the pack-bitmap-write machinery does not itself call disable_replace_refs(). So when it generates a reachability bitmap, it is doing so with the replace refs in mind. You can see that this is indeed the cause of the problem by looking at the output of an instrumented version of Git that indicates what bits are being set during the bitmap generation phase. With replace refs (incorrectly) enabled, we get: [2, 4, 6, 8, 13, 3, 6, 7, 3, 4, 6, 8] and doing the same after calling disable_replace_refs(), we instead get: [2, 5, 6, 13, 3, 6, 7, 3, 4, 6, 8] Single pack bitmaps are unaffected by this issue because we generate them from within pack-objects, which does call disable_replace_refs(). This patch updates the MIDX logic to disable replace objects within the multi-pack-index builtin, and a test showing a clone (which would fail with MIDX bitmap) is added to demonstrate the bug. Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-17builtin/receive-pack: convert to use git-maintenance(1)Patrick Steinhardt1-11/+10
In 850b6edefa (auto-gc: extract a reusable helper from "git fetch", 2020-05-06), we have introduced a helper function `run_auto_gc()` that kicks off `git gc --auto`. The intent of this function was to pass down the "--quiet" flag to git-gc(1) as required without duplicating this at all callsites. In 7c3e9e8cfb (auto-gc: pass --quiet down from am, commit, merge and rebase, 2020-05-06) we then converted callsites that need to pass down this flag to use the new helper function. This has the notable omission of git-receive-pack(1), which is the only remaining user of `git gc --auto` that sets up the proccess manually. This is probably because it unconditionally passes down the `--quiet` flag and thus didn't benefit much from the new helper function. In a95ce12430 (maintenance: replace run_auto_gc(), 2020-09-17) we then replaced `run_auto_gc()` with `run_auto_maintenance()` which invokes git-maintenance(1) instead of git-gc(1). This command is the modern replacement for git-gc(1) and is both more thorough and also more flexible because administrators can configure which tasks exactly to run during maintenance. But due to git-receive-pack(1) not using `run_auto_gc()` in the first place it did not get converted to use git-maintenance(1) like we do everywhere else now. Address this oversight and start to use the newly introduced function `prepare_auto_maintenance()`. This will also make it easier for us to adapt this code together with all the other callsites that invoke auto-maintenance in the future. This removes the last internal user of `git gc --auto`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-16credential: add method for querying capabilitiesbrian m. carlson2-0/+16
Right now, there's no specific way to determine whether a credential helper or git credential itself supports a given set of capabilities. It would be helpful to have such a way, so let's let credential helpers and git credential take an argument, "capability", which has it list the capabilities and a version number on standard output. Specifically choose a format that is slightly different from regular credential output and assume that no capabilities are supported if a non-zero exit status occurs or the data deviates from the format. It is common for users to write small shell scripts as the argument to credential.helper, which will almost never be designed to emit capabilities. We want callers to gracefully handle this case by assuming that they are not capable of extended support because that is almost certainly the case, and specifying the error behavior up front does this and preserves backwards compatibility in a graceful way. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-16credential-cache: implement authtype capabilitybrian m. carlson1-3/+17
Now that we have full support in Git for the authtype capability, let's add support to the cache credential helper. When parsing data, we always set the initial capabilities because we're the helper, and we need both the initial and helper capabilities to be set in order to have the helper capabilities take effect. When emitting data, always emit the supported capability and make sure we emit items only if we have them and they're supported by the caller. Since we may no longer have a username or password, be sure to emit those conditionally as well so we don't segfault on a NULL pointer. Similarly, when comparing credentials, consider both the password and credential fields when we're matching passwords. Adjust the partial credential detection code so that we can store credentials missing a username or password as long as they have an authtype and credential. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-16credential: add support for multistage credential roundsbrian m. carlson1-0/+1
Over HTTP, NTLM and Kerberos require two rounds of authentication on the client side. It's possible that there are custom authentication schemes that also implement this same approach. Since these are tricky schemes to implement and the HTTP library in use may not always handle them gracefully on all systems, it would be helpful to allow the credential helper to implement them instead for increased portability and robustness. To allow this to happen, add a boolean flag, continue, that indicates that instead of failing when we get a 401, we should retry another round of authentication. However, this necessitates some changes in our current credential code so that we can make this work. Keep the state[] headers between iterations, but only use them to send to the helper and only consider the new ones we read from the credential helper to be valid on subsequent iterations. That avoids us passing stale data when we finally approve or reject the credential. Similarly, clear the multistage and wwwauth[] values appropriately so that we don't pass stale data or think we're trying a multiround response when we're not. Remove the credential values so that we can actually fill a second time with new responses. Limit the number of iterations of reauthentication we do to 3. This means that if there's a problem, we'll terminate with an error message instead of retrying indefinitely and not informing the user (and possibly conducting a DoS on the server). In our tests, handle creating multiple response output files from our helper so we can verify that each of the messages sent is correct. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-16credential: gate new fields on capabilitybrian m. carlson3-5/+7
We support the new credential and authtype fields, but we lack a way to indicate to a credential helper that we'd like them to be used. Without some sort of indication, the credential helper doesn't know if it should try to provide us a username and password, or a pre-encoded credential. For example, the helper might prefer a more restricted Bearer token if pre-encoded credentials are possible, but might have to fall back to more general username and password if not. Let's provide a simple way to indicate whether Git (or, for that matter, the helper) is capable of understanding the authtype and credential fields. We send this capability when we generate a request, and the other side may reply to indicate to us that it does, too. For now, don't enable sending capabilities for the HTTP code. In a future commit, we'll introduce appropriate handling for that code, which requires more in-depth work. The logic for determining whether a capability is supported may seem complex, but it is not. At each stage, we emit the capability to the following stage if all preceding stages have declared it. Thus, if the caller to git credential fill didn't declare it, then we won't send it to the helper, and if fill's caller did send but the helper doesn't understand it, then we won't send it on in the response. If we're an internal user, then we know about all capabilities and will request them. For "git credential approve" and "git credential reject", we set the helper capability before calling the helper, since we assume that the input we're getting from the external program comes from a previous call to "git credential fill", and thus we'll invoke send a capability to the helper if and only if we got one from the standard input, which is the correct behavior. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-17builtin/clone: refuse local clones of unsafe repositoriesPatrick Steinhardt1-0/+14
When performing a local clone of a repository we end up either copying or hardlinking the source repository into the target repository. This is significantly more performant than if we were to use git-upload-pack(1) and git-fetch-pack(1) to create the new repository and preserves both disk space and compute time. Unfortunately though, performing such a local clone of a repository that is not owned by the current user is inherently unsafe: - It is possible that source files get swapped out underneath us while we are copying or hardlinking them. While we do perform some checks here to assert that we hardlinked the expected file, they cannot reliably thwart time-of-check-time-of-use (TOCTOU) style races. It is thus possible for an adversary to make us copy or hardlink unexpected files into the target directory. Ideally, we would address this by starting to use openat(3P), fstatat(3P) and friends. Due to platform compatibility with Windows we cannot easily do that though. Furthermore, the scope of these fixes would likely be quite broad and thus not fit for an embargoed security release. - Even if we handled TOCTOU-style races perfectly, hardlinking files owned by a different user into the target repository is not a good idea in general. It is possible for an adversary to rewrite those files to contain whatever data they want even after the clone has completed. Address these issues by completely refusing local clones of a repository that is not owned by the current user. This reuses our existing infra we have in place via `ensure_valid_ownership()` and thus allows a user to override the safety guard by adding the source repository path to the "safe.directory" configuration. This addresses CVE-2024-32020. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17builtin/clone: abort when hardlinked source and target file differPatrick Steinhardt1-1/+20
When performing local clones with hardlinks we refuse to copy source files which are symlinks as a mitigation for CVE-2022-39253. This check can be raced by an adversary though by changing the file to a symlink after we have checked it. Fix the issue by checking whether the hardlinked destination file matches the source file and abort in case it doesn't. This addresses CVE-2024-32021. Reported-by: Apple Product Security <product-security@apple.com> Suggested-by: Linus Torvalds <torvalds@linuxfoundation.org> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-17builtin/clone: stop resolving symlinks when copying filesPatrick Steinhardt1-5/+1
When a user performs a local clone without `--no-local`, then we end up copying the source repository into the target repository directly. To optimize this even further, we try to hardlink files into place instead of copying data over, which helps both disk usage and speed. There is an important edge case in this context though, namely when we try to hardlink symlinks from the source repository into the target repository. Depending on both platform and filesystem the resulting behaviour here can be different: - On macOS and NetBSD, calling link(3P) with a symlink target creates a hardlink to the file pointed to by the symlink. - On Linux, calling link(3P) instead creates a hardlink to the symlink itself. To unify this behaviour, 36596fd2df (clone: better handle symlinked files at .git/objects/, 2019-07-10) introduced logic to resolve symlinks before we try to link(3P) files. Consequently, the new behaviour was to always create a hard link to the target of the symlink on all platforms. Eventually though, we figured out that following symlinks like this can cause havoc when performing a local clone of a malicious repository, which resulted in CVE-2022-39253. This issue was fixed via 6f054f9fb3 (builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28), by refusing symlinks in the source repository. But even though we now shouldn't ever link symlinks anymore, the code that resolves symlinks still exists. In the best case the code does not end up doing anything because there are no symlinks anymore. In the worst case though this can be abused by an adversary that rewrites the source file after it has been checked not to be a symlink such that it actually is a symlink when we call link(3P). Thus, it is still possible to recreate CVE-2022-39253 due to this time-of-check-time-of-use bug. Remove the call to `realpath()`. This doesn't yet address the actual vulnerability, which will be handled in a subsequent commit. Reported-by: Apple Product Security <product-security@apple.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-16Merge branch 'rs/date-mode-pass-by-value'Junio C Hamano1-2/+2
The codepaths that reach date_mode_from_type() have been updated to pass "struct date_mode" by value to make them thread safe. * rs/date-mode-pass-by-value: date: make DATE_MODE thread-safe
2024-04-15Merge branch 'gt/add-u-commit-i-pathspec-check'Junio C Hamano3-4/+17
"git add -u <pathspec>" and "git commit [-i] <pathspec>" did not diagnose a pathspec element that did not match any files in certain situations, unlike "git add <pathspec>" did. * gt/add-u-commit-i-pathspec-check: builtin/add: error out when passing untracked path with -u builtin/commit: error out when passing untracked path with -i revision: optionally record matches with pathspec elements
2024-04-15Merge branch 'ds/fetch-config-parse-microfix'Junio C Hamano1-0/+1
A config parser callback function fell through instead of returning after recognising and processing a variable, wasting cycles, which has been corrected. * ds/fetch-config-parse-microfix: fetch: return when parsing submodule.recurse
2024-04-15Merge branch 'ma/win32-unix-domain-socket'Junio C Hamano2-0/+5
Windows binary used to decide the use of unix-domain socket at build time, but it learned to make the decision at runtime instead. * ma/win32-unix-domain-socket: Win32: detect unix socket support at runtime
2024-04-15fast-import: make comments more preciseThalia Archibald1-3/+3
The former is somewhat imprecise. The latter became out of sync with the behavior in e814c39c2f (fast-import: refactor parsing of spaces, 2014-06-18). Signed-off-by: Thalia Archibald <thalia@archibald.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15fast-import: forbid escaped NUL in pathsThalia Archibald1-0/+2
NUL cannot appear in paths. Even disregarding filesystem path limitations, the tree object format delimits with NUL, so such a path cannot be encoded by Git. When a quoted path is unquoted, it could possibly contain NUL from "\000". Forbid it so it isn't truncated. fast-import still has other issues with NUL, but those will be addressed later. Signed-off-by: Thalia Archibald <thalia@archibald.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15fast-import: remove dead strbufThalia Archibald1-5/+0
The strbuf in `note_change_n` is to copy the remainder of `p` before potentially invalidating it when reading the next line. However, `p` is not used after that point. It has been unused since the function was created in a8dd2e7d2b (fast-import: Add support for importing commit notes, 2009-10-09) and looks to be a fossil from adapting `file_change_m`. Remove it. Signed-off-by: Thalia Archibald <thalia@archibald.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15fast-import: allow unquoted empty path for rootThalia Archibald1-3/+0
Ever since filerename was added in f39a946a1f (Support wholesale directory renames in fast-import, 2007-07-09) and filecopy in b6f3481bb4 (Teach fast-import to recursively copy files/directories, 2007-07-15), both have produced an error when the destination path is empty. Later, when support for targeting the root directory with an empty string was added in 2794ad5244 (fast-import: Allow filemodify to set the root, 2010-10-10), this had the effect of allowing the quoted empty string (`""`), but forbidding its unquoted variant (``). This seems to have been intended as simple data validation for parsing two paths, rather than a syntax restriction, because it was not extended to the other operations. All other occurrences of paths (in filemodify, filedelete, the source of filecopy and filerename, and ls) allow both. For most of this feature's lifetime, the documentation has not prescribed the use of quoted empty strings. In e5959106d6 (Documentation/fast-import: put explanation of M 040000 <dataref> "" in context, 2011-01-15), its documentation was changed from “`<path>` may also be an empty string (`""`) to specify the root of the tree” to “The root of the tree can be represented by an empty string as `<path>`”. Thus, we should assume that some front-ends have depended on this behavior. Remove this restriction for the destination paths of filecopy and filerename and change tests targeting the root to test `""` and ``. Signed-off-by: Thalia Archibald <thalia@archibald.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15fast-import: directly use strbufs for pathsThalia Archibald1-37/+27
Previously, one case would not write the path to the strbuf: when the path is unquoted and at the end of the string. It was essentially copy-on-write. However, with the logic simplification of the previous commit, this case was eliminated and the strbuf is always populated. Directly use the strbufs now instead of an alias. Since this already changes all the lines that use the strbufs, rename them from `uq` to be more descriptive. That they are unquoted is not their most important property, so name them after what they carry. Additionally, `file_change_m` no longer needs to copy the path before reading inline data. Signed-off-by: Thalia Archibald <thalia@archibald.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-15fast-import: tighten path unquotingThalia Archibald1-43/+65
Path parsing in fast-import is inconsistent and many unquoting errors are suppressed or not checked. <path> appears in the grammar in these places: filemodify ::= 'M' SP <mode> (<dataref> | 'inline') SP <path> LF filedelete ::= 'D' SP <path> LF filecopy ::= 'C' SP <path> SP <path> LF filerename ::= 'R' SP <path> SP <path> LF ls ::= 'ls' SP <dataref> SP <path> LF ls-commit ::= 'ls' SP <path> LF and fast-import.c parses them in five different ways: 1. For filemodify and filedelete: Try to unquote <path>. If it unquotes without errors, use the unquoted version; otherwise, treat it as literal bytes to the end of the line (including any number of SP). 2. For filecopy (source) and filerename (source): Try to unquote <path>. If it unquotes without errors, use the unquoted version; otherwise, treat it as literal bytes up to, but not including, the next SP. 3. For filecopy (dest) and filerename (dest): Like 1., but an unquoted empty string is forbidden. 4. For ls: If <path> starts with `"`, unquote it and report parse errors; otherwise, treat it as literal bytes to the end of the line (including any number of SP). 5. For ls-commit: Unquote <path> and report parse errors. (It must start with `"` to disambiguate from ls.) In the first three, any errors from trying to unquote a string are suppressed, so a quoted string that contains invalid escapes would be interpreted as literal bytes. For example, `"\xff"` would fail to unquote (because hex escapes are not supported), and it would instead be interpreted as the byte sequence '"', '\\', 'x', 'f', 'f', '"', which is certainly not intended. Some front-ends erroneously use their language's standard quoting routine instead of matching Git's, which could silently introduce escapes that would be incorrectly parsed due to this and lead to data corruption. The documentation states “To use a source path that contains SP the path must be quoted.”, so it is expected that some implementations depend on spaces being allowed in paths in the final position. Thus we have two documented ways to parse paths, so simplify the implementation to that. Now we have: 1. `parse_path_eol` for filemodify, filedelete, filecopy (dest), filerename (dest), ls, and ls-commit: If <path> starts with `"`, unquote it and report parse errors; otherwise, treat it as literal bytes to the end of the line (including any number of SP). 2. `parse_path_space` for filecopy (source) and filerename (source): If <path> starts with `"`, unquote it and report parse errors; otherwise, treat it as literal bytes up to, but not including, the next SP. It must be followed by SP. There remain two special cases: The dest <path> in filecopy and rename cannot be an unquoted empty string (this will be addressed subsequently) and <path> in ls-commit must be quoted to disambiguate it from ls. Signed-off-by: Thalia Archibald <thalia@archibald.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-12Merge branch 'jc/checkout-detach-wo-tracking-report'Junio C Hamano1-1/+2
"git checkout/switch --detach foo", after switching to the detached HEAD state, gave the tracking information for the 'foo' branch, which was pointless. Tested-by: M Hickford <mirth.hickford@gmail.com> cf. <CAGJzqsmE9FDEBn=u3ge4LA3ha4fDbm4OWiuUbMaztwjELBd7ug@mail.gmail.com> * jc/checkout-detach-wo-tracking-report: checkout: omit "tracking" information on a detached HEAD
2024-04-12Merge branch 'js/merge-tree-3-trees'Junio C Hamano1-1/+1
Match the option argument type in the help text to the correct type updated by a recent series. * js/merge-tree-3-trees: merge-tree: fix argument type of the `--merge-base` option
2024-04-12merge-tree: fix argument type of the `--merge-base` optionJohannes Schindelin1-1/+1
In 5f43cf5b2e4 (merge-tree: accept 3 trees as arguments, 2024-01-28), I taught `git merge-tree` to perform three-way merges on trees. This commit even changed the manual page to state that the `--merge-base` option takes a tree-ish rather than requiring a commit. But I forgot to adjust the in-program help text. This patch fixes that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-10Merge branch 'kn/clarify-update-ref-doc'Junio C Hamano1-13/+13
Doc update, as a preparation to enhance "git update-ref --stdin". * kn/clarify-update-ref-doc: githooks: use {old,new}-oid instead of {old,new}-value update-ref: use {old,new}-oid instead of {old,new}value
2024-04-09Merge branch 'rj/use-adv-if-enabled'Junio C Hamano1-11/+7
Use advice_if_enabled() API to rewrite a simple pattern to call advise() after checking advice_enabled(). * rj/use-adv-if-enabled: add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE
2024-04-09Merge branch 'ps/pack-refs-auto'Junio C Hamano2-48/+69
"git pack-refs" learned the "--auto" option, which is a useful addition to be triggered from "git gc --auto". Acked-by: Karthik Nayak <karthik.188@gmail.com> cf. <CAOLa=ZRAEA7rSUoYL0h-2qfEELdbPHbeGpgBJRqesyhHi9Q6WQ@mail.gmail.com> * ps/pack-refs-auto: builtin/gc: pack refs when using `git maintenance run --auto` builtin/gc: forward git-gc(1)'s `--auto` flag when packing refs t6500: extract objects with "17" prefix builtin/gc: move `struct maintenance_run_opts` builtin/pack-refs: introduce new "--auto" flag builtin/pack-refs: release allocated memory refs/reftable: expose auto compaction via new flag refs: remove `PACK_REFS_ALL` flag refs: move `struct pack_refs_opts` to where it's used t/helper: drop pack-refs wrapper refs/reftable: print errors on compaction failure reftable/stack: gracefully handle failed auto-compaction due to locks reftable/stack: use error codes when locking fails during compaction reftable/error: discern locked/outdated errors reftable/stack: fix error handling in `reftable_stack_init_addition()`
2024-04-05date: make DATE_MODE thread-safeRené Scharfe1-2/+2
date_mode_from_type() modifies a static variable and returns a pointer to it. This is not thread-safe. Most callers of date_mode_from_type() use it via the macro DATE_MODE and pass its result on to functions like show_date(), which take a const pointer and don't modify the struct. Avoid the static storage by putting the variable on the stack and returning the whole struct date_mode. Change functions that take a constant pointer to expect the whole struct instead. Reduce the cost of passing struct date_mode around on 64-bit systems by reordering its members to close the hole between the 32-bit wide .type and the 64-bit aligned .strftime_fmt as well as the alignment hole at the end. sizeof reports 24 before and 16 with this change on x64. Keep .type at the top to still allow initialization without designator -- though that's only done in a single location, in builtin/blame.c. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05Merge branch 'jk/core-comment-string'Junio C Hamano9-38/+39
core.commentChar used to be limited to a single byte, but has been updated to allow an arbitrary multi-byte sequence. * jk/core-comment-string: config: add core.commentString config: allow multi-byte core.commentChar environment: drop comment_line_char compatibility macro wt-status: drop custom comment-char stringification sequencer: handle multi-byte comment characters when writing todo list find multi-byte comment chars in unterminated buffers find multi-byte comment chars in NUL-terminated strings prefer comment_line_str to comment_line_char for printing strbuf: accept a comment string for strbuf_add_commented_lines() strbuf: accept a comment string for strbuf_commented_addf() strbuf: accept a comment string for strbuf_stripspace() environment: store comment_line_char as a string strbuf: avoid shadowing global comment_line_char name commit: refactor base-case of adjust_comment_line_char() strbuf: avoid static variables in strbuf_add_commented_lines() strbuf: simplify comment-handling in add_lines() helper config: forbid newline as core.commentChar
2024-04-05Merge branch 'rs/config-comment'Junio C Hamano4-12/+22
"git config" learned "--comment=<message>" option to leave a comment immediately after the "variable = value" on the same line in the configuration file. * rs/config-comment: config: allow tweaking whitespace between value and comment config: fix --comment formatting config: add --comment option to add a comment
2024-04-05fetch: return when parsing submodule.recurseDerrick Stolee1-0/+1
When parsing config keys, the normal pattern is to return 0 after completing the logic for a specific config key, since no other key will match. One instance, for "submodule.recurse", was missing this case in builtin/fetch.c. This is a very minor change, and will have minimal impact to performance. This particular block was edited recently in 56e8bb4fb4 (fetch: use `fetch_config` to store "fetch.recurseSubmodules" value, 2023-05-17), which led to some hesitation that perhaps this omission was on purpose. However, no later cases within git_fetch_config() will match the key if equal to "submodule.recurse" and neither will any key matches within the catch-all git_default_config(). Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03builtin/add: error out when passing untracked path with -uGhanshyam Thakkar1-1/+8
When passing untracked path with -u option, it silently succeeds. There is no error message and the exit code is zero. This is inconsistent with other instances of git commands where the expected argument is a known path. In those other instances, we error out when the path is not known. Fix this by passing a character array to add_files_to_cache() to collect the pathspec matching information and report the error if a pathspec does not match any cache entry. Also add a testcase to cover this scenario. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03builtin/commit: error out when passing untracked path with -iGhanshyam Thakkar1-1/+6
When we provide a pathspec which does not match any tracked path alongside --include, we do not error like without --include. If there is something staged, it will commit the staged changes and ignore the pathspec which does not match any tracked path. And if nothing is staged, it will print the status. Exit code is 0 in both cases (unlike without --include). This is also described in the TODO comment before the relevant testcase. Fix this by passing a character array to add_files_to_cache() to collect the pathspec matching information and error out if the given path is untracked. Also, amend the testcase to check for the error message and remove the TODO comment. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03revision: optionally record matches with pathspec elementsJunio C Hamano3-4/+5
Unlike "git add" and other end-user facing commands, where it is diagnosed as an error to give a pathspec with an element that does not match any path, the diff machinery does not care if some elements of the pathspec do not match. Given that the diff machinery is heavily used in pathspec-limited "git log" machinery, and it is common for a path to come and go while traversing the project history, this is usually a good thing. However, in some cases we would want to know if all the pathspec elements matched. For example, "git add -u <pathspec>" internally uses the machinery used by "git diff-files" to decide contents from what paths to add to the index, and as an end-user facing command, "git add -u" would want to report an unmatched pathspec element. Add a new .ps_matched member next to the .prune_data member in "struct rev_info" so that we can optionally keep track of the use of .prune_data pathspec elements that can be inspected by the caller. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03Win32: detect unix socket support at runtimeMatthias Aßhauer2-0/+5
Windows 10 build 17063 introduced support for unix sockets to Windows. bb390b1 (git-compat-util: include declaration for unix sockets in windows, 2021-09-14) introduced a way to build git with unix socket support on Windows, but you still had to decide at build time which Windows version the compiled executable was supposed to run on. We can detect at runtime wether the operating system supports unix sockets and act accordingly for all supported Windows versions. This fixes https://github.com/git-for-windows/git/issues/3892 Signed-off-by: Matthias Aßhauer <mha1993@live.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-03Merge branch 'bl/cherry-pick-empty'Junio C Hamano2-7/+47
Allow git-cherry-pick(1) to automatically drop redundant commits via a new `--empty` option, similar to the `--empty` options for git-rebase(1) and git-am(1). Includes a soft deprecation of `--keep-redundant-commits` as well as some related docs changes and sequencer code cleanup. * bl/cherry-pick-empty: cherry-pick: add `--empty` for more robust redundant commit handling cherry-pick: enforce `--keep-redundant-commits` incompatibility sequencer: do not require `allow_empty` for redundant commit options sequencer: handle unborn branch with `--allow-empty` rebase: update `--empty=ask` to `--empty=stop` docs: clean up `--empty` formatting in git-rebase(1) and git-am(1) docs: address inaccurate `--empty` default with `--exec`
2024-04-03Merge branch 'rs/strbuf-expand-bad-format'Junio C Hamano3-26/+10
Code clean-up. * rs/strbuf-expand-bad-format: cat-file: use strbuf_expand_bad_format() factor out strbuf_expand_bad_format()
2024-04-02update-ref: use {old,new}-oid instead of {old,new}valueKarthik Nayak1-13/+13
The `git-update-ref` command is used to modify references. The usage of {old,new}value in the documentation refers to the OIDs. This is fine since the command only works with regular references which hold OIDs. But if the command is updated to support symrefs, we'd also be dealing with {old,new}-refs. To improve clarity around what exactly {old,new}value mean, let's rename it to {old,new}-oid. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-01Merge branch 'jk/rebase-apply-leakfix'Junio C Hamano1-3/+3
Leakfix. * jk/rebase-apply-leakfix: rebase: use child_process_clear() to clean
2024-04-01Merge branch 'pb/advice-merge-conflict'Junio C Hamano1-5/+9
Hints that suggest what to do after resolving conflicts can now be squelched by disabling advice.mergeConflict. Acked-by: Phillip Wood <phillip.wood123@gmail.com> cf. <e040c631-42d9-4501-a7b8-046f8dac6309@gmail.com> * pb/advice-merge-conflict: builtin/am: allow disabling conflict advice sequencer: allow disabling conflict advice
2024-04-01Merge branch 'jk/pretty-subject-cleanup'Junio C Hamano3-3/+3
Code clean-up in the "git log" machinery that implements custom log message formatting. * jk/pretty-subject-cleanup: format-patch: fix leak of empty header string format-patch: simplify after-subject MIME header handling format-patch: return an allocated string from log_write_email_headers() log: do not set up extra_headers for non-email formats pretty: drop print_email_subject flag pretty: split oneline and email subject printing shortlog: stop setting pp.print_email_subject
2024-04-01Merge branch 'pw/checkout-conflict-errorfix'Junio C Hamano1-22/+38
"git checkout --conflict=bad" reported a bad conflictStyle as if it were given to a configuration variable; it has been corrected to report that the command line option is bad. * pw/checkout-conflict-errorfix: checkout: fix interaction between --conflict and --merge checkout: cleanup --conflict=<style> parsing merge options: add a conflict style member merge-ll: introduce LL_MERGE_OPTIONS_INIT xdiff-interface: refactor parsing of merge.conflictstyle
2024-03-30add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPORubén Justo1-3/+3
By following a similar reasoning as in previous commits, there are no reason why we should not use the advise_if_enabled() API to display the ADVICE_ADD_EMBEDDED_REPO advice. This advice was introduced in 532139940c (add: warn when adding an embedded repository, 2017-06-14). Some tests were included in the commit, but none is testing this advice. Which, note, we only want to display once per run. So, use the advise_if_enabled() machinery to show the ADVICE_ADD_EMBEDDED_REPO advice and include a test to notice any possible breakage. Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-30add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPECRubén Justo1-4/+2
Since 93b0d86aaf (git-add: error out when given no arguments., 2006-12-20) we display a message when no arguments are given to "git add". Part of that message was converted to advice in bf66db37f1 (add: use advise function to display hints, 2020-01-07). Following the same line of reasoning as in the previous commit, it is sensible to use advise_if_enabled() here. Therefore, use advise_if_enabled() in builtin/add.c to show the ADVICE_ADD_EMPTY_PATHSPEC advice, and don't bother checking there the visibility of the advice or displaying the instruction on how to disable it. Also add a test for these messages, in order to detect a possible change in them. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-30add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILERubén Justo1-4/+2
Since b3b18d1621 (advice: revamp advise API, 2020-03-02), we can use advise_if_enabled() to display an advice. This API encapsulates three actions: 1.- checking the visibility of the advice 2.- displaying the advice when appropriate 3.- displaying instructions on how to disable the advice, when appropriate The code we have in builtin/add.c to display the ADVICE_ADD_IGNORED_FILE advice, is doing these three things. However, the instructions displayed on how to disable the hint are not shown in the normalized way that advise_if_enabled() introduced. This may cause distraction. There is no reason not to use the new API here. On the contrary, by using it we gain simplicity in the code and avoid possible distractions. For these reasons, use the newer advise_if_enabled() machinery to show the ADVICE_ADD_IGNORED_FILE advice, and don't bother checking the visibility or displaying the instruction on how to disable the advice. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-30checkout: omit "tracking" information on a detached HEADJunio C Hamano1-1/+2
By definition, a detached HEAD state is tentative and there is no configured "upstream" that it always wants to integrate with. But if you detach from a branch that is behind its upstream, e.g., $ git checkout -t -b main origin/main $ git checkout main $ git reset --hard HEAD^ $ git checkout --detach main you'd see "you are behind your upstream origin/main". This does not happen when you replace the last step in the above with any of these $ git checkout HEAD^0 $ git checkout --detach HEAD $ git checkout --detach origin/main Before 32669671 (checkout: introduce --detach synonym for "git checkout foo^{commit}", 2011-02-08) introduced the "--detach" option, the rule to decide if we show the tracking information used to be: If --quiet is not given, and if the given branch name is a real local branch (i.e. the one we can compute the file path under .git/, like 'refs/heads/master' or "HEAD" which stand for the name of the current branch", then give the tracking information. to exclude things like "git checkout master^0" (which was the official way to detach HEAD at the commit before that commit) and "git checkout origin/master^0" from showing tracking information, but still do show the tracking information for the current branch for "git checkout HEAD". The introduction of an explicit option "--detach" broke this subtley. The new rule should have been If --quiet is given, do not bother with tracking info. If --detach is given, do not bother with tracking info. Otherwise, if we know that the branch name given is a real local branch, or if we were given "HEAD" and "HEAD" is not detached, then attempt to show the tracking info. but it allowed "git checkout --detach master" to also show the tracking info by mistake. Let's tighten the rule to fix this. Reported-by: mirth hickford <mirth.hickford@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-28Merge branch 'jc/show-untracked-false'Junio C Hamano1-19/+38
The status.showUntrackedFiles configuration variable had a name that tempts users to set a Boolean value expressed in our usual "false", "off", and "0", but it only took "no". This has been corrected so "true" and its synonyms are taken as "normal", while "false" and its synonyms are taken as "no". * jc/show-untracked-false: status: allow --untracked=false and friends status: unify parsing of --untracked= and status.showUntrackedFiles
2024-03-28Merge branch 'eb/hash-transition'Junio C Hamano14-43/+104
Work to support a repository that work with both SHA-1 and SHA-256 hash algorithms has started. * eb/hash-transition: (30 commits) t1016-compatObjectFormat: add tests to verify the conversion between objects t1006: test oid compatibility with cat-file t1006: rename sha1 to oid test-lib: compute the compatibility hash so tests may use it builtin/ls-tree: let the oid determine the output algorithm object-file: handle compat objects in check_object_signature tree-walk: init_tree_desc take an oid to get the hash algorithm builtin/cat-file: let the oid determine the output algorithm rev-parse: add an --output-object-format parameter repository: implement extensions.compatObjectFormat object-file: update object_info_extended to reencode objects object-file-convert: convert commits that embed signed tags object-file-convert: convert commit objects when writing object-file-convert: don't leak when converting tag objects object-file-convert: convert tag objects when writing object-file-convert: add a function to convert trees between algorithms object: factor out parse_mode out of fast-import and tree-walk into in object.h cache: add a function to read an OID of a specific algorithm tag: sign both hashes commit: export add_header_signature to support handling signatures on tags ...
2024-03-25cherry-pick: add `--empty` for more robust redundant commit handlingBrian Lyles1-1/+36
As with git-rebase(1) and git-am(1), git-cherry-pick(1) can result in a commit being made redundant if the content from the picked commit is already present in the target history. However, git-cherry-pick(1) does not have the same options available that git-rebase(1) and git-am(1) have. There are three things that can be done with these redundant commits: drop them, keep them, or have the cherry-pick stop and wait for the user to take an action. git-rebase(1) has the `--empty` option added in commit e98c4269c8 (rebase (interactive-backend): fix handling of commits that become empty, 2020-02-15), which handles all three of these scenarios. Similarly, git-am(1) got its own `--empty` in 7c096b8d61 (am: support --empty=<option> to handle empty patches, 2021-12-09). git-cherry-pick(1), on the other hand, only supports two of the three possiblities: Keep the redundant commits via `--keep-redundant-commits`, or have the cherry-pick fail by not specifying that option. There is no way to automatically drop redundant commits. In order to bring git-cherry-pick(1) more in-line with git-rebase(1) and git-am(1), this commit adds an `--empty` option to git-cherry-pick(1). It has the same three options (keep, drop, and stop), and largely behaves the same. The notable difference is that for git-cherry-pick(1), the default will be `stop`, which maintains the current behavior when the option is not specified. Like the existing `--keep-redundant-commits`, `--empty=keep` will imply `--allow-empty`. The `--keep-redundant-commits` option will be documented as a deprecated synonym of `--empty=keep`, and will be supported for backwards compatibility for the time being. Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25cherry-pick: enforce `--keep-redundant-commits` incompatibilityBrian Lyles1-0/+1
When `--keep-redundant-commits` was added in b27cfb0d8d (git-cherry-pick: Add keep-redundant-commits option, 2012-04-20), it was not marked as incompatible with the various operations needed to continue or exit a cherry-pick (`--continue`, `--skip`, `--abort`, and `--quit`). Enforce this incompatibility via `verify_opt_compatible` like we do for the other various options. Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25rebase: update `--empty=ask` to `--empty=stop`Brian Lyles1-6/+10
When git-am(1) got its own `--empty` option in 7c096b8d61 (am: support --empty=<option> to handle empty patches, 2021-12-09), `stop` was used instead of `ask`. `stop` is a more accurate term for describing what really happens, and consistency is good. Update git-rebase(1) to also use `stop`, while keeping `ask` as a deprecated synonym. Update the tests to primarily use `stop`, but also ensure that `ask` is still allowed. In a future commit, we'll be adding a new `--empty` option for git-cherry-pick(1) as well, making the consistency even more relevant. Reported-by: Elijah Newren <newren@gmail.com> Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25Merge branch 'ja/doc-formatting-fix'Junio C Hamano1-1/+1
Documentation mark-up fix. * ja/doc-formatting-fix: doc: fix some placeholders formating doc: format alternatives in synopsis
2024-03-25Merge branch 'js/bugreport-no-suffix-fix'Junio C Hamano1-3/+7
"git bugreport --no-suffix" was not supported and instead segfaulted, which has been corrected. * js/bugreport-no-suffix-fix: bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option
2024-03-25Merge branch 'rj/restore-plug-leaks'Junio C Hamano1-30/+21
Leaks from "git restore" have been plugged. * rj/restore-plug-leaks: checkout: plug some leaks in git-restore
2024-03-25cat-file: use strbuf_expand_bad_format()René Scharfe1-8/+8
Report unknown format elements and missing closing parentheses with consistent and translated messages by calling strbuf_expand_bad_format() at the very end of the combined if/else chain of expand_format() and expand_atom(). Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25factor out strbuf_expand_bad_format()René Scharfe2-18/+2
Extract a function for reporting placeholders that are not enclosed in a parenthesis or are unknown. This reduces the number of strings to translate and improves consistency across commands. Call it at the end of the if/else chain, after exhausting all accepted possibilities. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25builtin/gc: pack refs when using `git maintenance run --auto`Patrick Steinhardt1-1/+11
When running `git maintenance run --auto`, then the various subtasks will only run as needed. Thus, we for example end up only packing loose objects if we hit a certain threshold. Interestingly enough, the "pack-refs" task is actually _never_ executed when the auto-flag is set because it does not have a condition at all. As 41abfe15d9 (maintenance: add pack-refs task, 2021-02-09) mentions: The 'auto_condition' function pointer is left NULL for now. We could extend this in the future to have a condition check if pack-refs should be run during 'git maintenance run --auto'. It is not quite clear from that quote whether it is actually intended that the task doesn't run at all in this mode. Also, no test was added to verify this behaviour. Ultimately though, it feels quite surprising that `git maintenance run --auto --task=pack-refs` would quietly never do anything at all. In any case, now that we do have the logic in place to let ref backends decide whether or not to repack refs, it does make sense to wire it up accordingly. With the "reftable" backend we will thus now perform auto-compaction, which optimizes the refdb as needed. But for the "files" backend we now unconditionally pack refs as it does not yet know to handle the "auto" flag. Arguably, this can be seen as a bug fix given that previously the task never did anything at all. Eventually though we should amend the "files" backend to use some heuristics for auto compaction, as well. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25builtin/gc: forward git-gc(1)'s `--auto` flag when packing refsPatrick Steinhardt1-9/+12
Forward the `--auto` flag to git-pack-refs(1) when it has been invoked with this flag itself. This does not change anything for the "files" backend, which will continue to eagerly pack refs. But it does ensure that the "reftable" backend only compacts refs as required. This change does not impact git-maintenance(1) because this command will in fact never run the pack-refs task when run with `--auto`. This issue will be addressed in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25builtin/gc: move `struct maintenance_run_opts`Patrick Steinhardt1-27/+26
We're about to start using `struct maintenance_run_opts` in `maintenance_task_pack_refs()`. Move its definition up to prepare for this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25builtin/pack-refs: introduce new "--auto" flagPatrick Steinhardt1-1/+2
Calling git-pack-refs(1) will unconditionally cause it to pack all requested refs regardless of the current state of the ref database. For example: - With the "files" backend we will end up rewriting the complete "packed-refs" file even if only a single ref would require compaction. - With the "reftable" backend we will end up always compacting all tables into a single table. This behaviour can be completely unnecessary depending on the backend and is thus wasteful. With the introduction of the `PACK_REFS_AUTO` flag in the preceding commit we can improve this and let the backends decide for themselves whether to pack refs in the first place. Expose this functionality via a new "--auto" flag in git-pack-refs(1), which mirrors the same flag in both git-gc(1) and git-maintenance(1). Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25builtin/pack-refs: release allocated memoryPatrick Steinhardt1-8/+15
Some of the command line options in `cmd_pack_refs()` require us to allocate memory. This memory is never released and thus leaking, but we paper over this leak by declaring the respective variables as `static` function-level variables, which is somewhat awkward. Refactor the code to release the allocated memory and drop the `static` declaration. While at it, remove the useless `flags` variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-25refs: remove `PACK_REFS_ALL` flagPatrick Steinhardt1-2/+3
The intent of the `PACK_REFS_ALL` flag is to ask the backend to compact all refs instead of only a subset of them. Thus, this flag gets passed down to `refs_pack_refs()` via `struct pack_refs_opts::flags`. But starting with 4fe42f326e (pack-refs: teach pack-refs --include option, 2023-05-12), the flag's semantics have changed. Instead of being handled by the respective backends, this flag is now getting handled by the callers of `refs_pack_refs()` which will add a single glob ("*") to the list of refs-to-be-packed. Thus, the flag serves no purpose to the ref backends anymore. Remove the flag and replace it with a local variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-22rebase: use child_process_clear() to cleanJeff King1-3/+3
In the run_am() function, we set up a child_process struct to run "git-am", allocating memory for its args and env strvecs. These are normally cleaned up when we call run_command(). But if we encounter certain errors, we exit the function early and try to clean up ourselves by clearing the am.args field. This leaks the "env" strvec. We should use child_process_clear() instead, which covers both. And more importantly, it future proofs us against the struct ever growing more allocated fields. These are unlikely errors to happen in practice, so they don't actually trigger the leak sanitizer in the tests. But we can add a new test which does exercise one of the paths (and fails SANITIZE=leak without this patch). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-19format-patch: return an allocated string from log_write_email_headers()Jeff King1-0/+1
When pretty-printing a commit in the email format, we have to fill in the "after subject" field of the pretty_print_context with any extra headers the user provided (e.g., from "--to" or "--cc" options) plus any special MIME headers. We return an out-pointer that sometimes points to a newly heap-allocated string and sometimes not. To avoid leaking, we store the allocated version in a buffer with static lifetime, which is ugly. Worse, as we extend the header feature, we'll end up having to repeat this ugly pattern. Instead, let's have our out-pointer pass ownership back to the caller, and duplicate the string when necessary. This does mean one extra allocation per commit when you use extra headers, but in the context of format-patch which is showing diffs, I don't think that's even measurable. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-19pretty: drop print_email_subject flagJeff King2-1/+1
With one exception, the print_email_subject flag is set if and only if the commit format is email based: - in make_cover_letter() we set it along with CMIT_FMT_EMAIL explicitly - in show_log(), we set it if cmit_fmt_is_mail() is true. That covers format-patch as well as "git log --format=email" (or mboxrd). The one exception is "rev-list --format=email", which somewhat nonsensically prints the author and date as email headers, but no subject, like: $ git rev-list --format=email HEAD commit 64fc4c2cdd4db2645eaabb47aa4bac820b03cdba From: Jeff King <peff@peff.net> Date: Tue, 19 Mar 2024 19:39:26 -0400 this is the subject this is the body It's doubtful that this is a useful format at all (the "commit" lines replace the "From" lines that would make it work as an actual mbox). But I think that printing the subject as a header (like this patch does) is the least surprising thing to do. So let's drop this field, making the code a little simpler and easier to reason about. Note that we do need to set the "rev" field of the pretty_print_context in rev-list, since that is used to check for subject_prefix, etc. It's not possible to set those fields via rev-list, so we'll always just print "Subject: ". But unless we pass in our rev_info, fmt_output_email_subject() would segfault trying to figure it out. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-19pretty: split oneline and email subject printingJeff King1-1/+1
The pp_title_line() function is used for two formats: the oneline format and the subject line of the email format. But most of the logic in the function does not make any sense for oneline; it is about special formatting of email headers. Lumping the two formats together made sense long ago in 4234a76167 (Extend --pretty=oneline to cover the first paragraph, 2007-06-11), when there was a lot of manual logic to paste lines together. But later, 88c44735ab (pretty: factor out format_subject(), 2008-12-27) pulled that logic into its own function. We can implement the oneline format by just calling that one function. This makes the intention of the code much more clear, as we know we only need to worry about those extra email options when dealing with actual email. While the intent here is cleanup, it is possible to trigger these cases in practice by running format-patch with an explicit --oneline option. But if you did, the results are basically nonsense. For example, with the preserve_subject flag: $ printf "%s\n" one two three | git commit --allow-empty -F - $ git format-patch -1 --stdout -k | grep ^Subject Subject: =?UTF-8?q?one=0Atwo=0Athree?= $ git format-patch -1 --stdout -k --oneline --no-signature 2af7fbe one two three Or with extra headers: $ git format-patch -1 --stdout --cc=me --oneline --no-signature 2af7fbe one two three Cc: me So I'd actually consider this to be an improvement, though you are probably crazy to use other formats with format-patch in the first place (arguably it should forbid non-email formats entirely, but that's a bigger change). As a bonus, it eliminates some pointless extra allocations for the oneline output. The email code, since it has to deal with wrapping, formats into an extra auxiliary buffer. The speedup is tiny, though like "rev-list --no-abbrev --format=oneline" seems to improve by a consistent 1-2% for me. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-19shortlog: stop setting pp.print_email_subjectJeff King1-1/+0
When shortlog processes a commit using its internal traversal, it may pretty-print the subject line for the summary view. When we do so, we set the "print_email_subject" flag in the pretty-print context. But this flag does nothing! Since we are using CMIT_FMT_USERFORMAT, we skip most of the usual formatting code entirely. This flag is there due to commit 6d167fd7cc (pretty: use fmt_output_email_subject(), 2017-03-01). But that just switched us away from setting an empty "subject" header field, which was similarly useless. That was added by dd2e794a21 (Refactor pretty_print_commit arguments into a struct, 2009-10-19). Before using the struct, we had to pass _something_ as the argument, so we passed the empty string (a NULL would have worked equally well). So this setting has never done anything, and we can drop the line. That shortens the code, but more importantly, makes it easier to reason about and refactor the other users of this flag. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-18Merge branch 'pw/rebase-i-ignore-cherry-pick-help-environment'Junio C Hamano1-11/+3
Code simplification by getting rid of code that sets an environment variable that is no longer used. * pw/rebase-i-ignore-cherry-pick-help-environment: rebase -i: stop setting GIT_CHERRY_PICK_HELP
2024-03-18builtin/am: allow disabling conflict advicePhilippe Blain1-5/+9
When 'git am' or 'git rebase --apply' encounter a conflict, they show a message instructing the user how to continue the operation. This message can't be disabled. Use ADVICE_MERGE_CONFLICT introduced in the previous commit to allow disabling it. Update the tests accordingly, as the advice output is now on stderr instead of stdout. In t4150, redirect stdout to 'out' and stderr to 'err', since this is less confusing. In t4254, as we are testing a specific failure mode of 'git am', simply disable the advice. Note that we are not testing that this advice is shown in 'git rebase' for the apply backend since 2ac0d6273f (rebase: change the default backend from "am" to "merge", 2020-02-15). Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-16doc: fix some placeholders formatingJean-Noël Avila1-1/+1
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-16bugreport.c: fix a crash in `git bugreport` with `--no-suffix` optionJiamu Sun1-3/+7
`git bugreport` does not complain when `--no-suffix` is given, but it leads to a segmentation fault as the it is not prepared to see a NULL assigned to the option_suffix variable. Signed-off-by: Jiamu Sun <barroit@linux.com> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15config: allow tweaking whitespace between value and commentJunio C Hamano1-6/+1
Extending the previous step, this allows the whitespace placed after the value before the "# comment message" to be tweaked by tweaking the preprocessing rule to: * If the given comment string begins with one or more whitespace characters followed by '#', it is passed intact. * If the given comment string begins with '#', a Space is prepended. * Otherwise, " # " (Space, '#', Space) is prefixed. * A string with LF in it cannot be used as a comment string. Unlike the previous step, which unconditionally added a space after the value before writing the "# comment string", because the above preprocessing already gives a whitespace before the '#', the resulting string is written immediately after copying the value. And the sanity checking rule becomes * comment string after the above massaging that comes into git_config_set_multivar_in_file_gently() must - begin with zero or more whitespace characters followed by '#'. - not have a LF in it. I personally think this is over-engineered, but since I thought things through anyway, here it is in the patch form. The logic to tweak end-user supplied comment string is encapsulated in a new helper function, git_config_prepare_comment_string(), so if new front-end callers would want to use the same massaging rules, it is easily reused. Unfortunately I do not think of a way to tweak the preprocessing rules further to optionally allow having no blank after the value, i.e. to produce [section] variable = value#comment (which is a valid way to say section.variable=value, by the way) without sacrificing the ergonomics for the more usual case, so this time I really stop here. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15config: fix --comment formattingJunio C Hamano1-4/+11
When git adds comments itself (like "rebase -i" todo list and "commit -e" log message editor), it always gives a comment introducer "#" followed by a Space before the message, except for the recently introduced "git config --comment", where the users are forced to say " this is my comment" if they want to add their comment in this usual format; otherwise their comment string will end up without a space after the "#". Make it more ergonomic, while keeping it possible to also use this unusual style, by massaging the comment string at the UI layer with a set of simple rules: * If the given comment string begins with '#', it is passed intact. * Otherwise, "# " is prefixed. * A string with LF in it cannot be used as a comment string. Right now there is only one "front-end" that accepts end-user comment string and calls the underlying machinery to add or modify configuration file with comments, but to make sure that the future callers perform similar massaging as they see fit, add a sanity check logic in git_config_set_multivar_in_file_gently(), which is the single choke point in the codepaths that consumes the comment string. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15Merge branch 'jc/xwrite-cleanup'Junio C Hamano3-23/+7
Uses of xwrite() helper have been audited and updated for better error checking and simpler code. * jc/xwrite-cleanup: repack: check error writing to pack-objects subprocess sideband: avoid short write(2) unpack: replace xwrite() loop with write_in_full()
2024-03-15Merge branch 'as/option-names-in-messages'Junio C Hamano2-2/+2
Error message updates. * as/option-names-in-messages: revision.c: trivial fix to message builtin/clone.c: trivial fix of message builtin/remote.c: trivial fix of error message transport-helper.c: trivial fix of error message
2024-03-15Merge branch 'kh/branch-ref-syntax-advice'Junio C Hamano1-2/+6
When git refuses to create a branch because the proposed branch name is not a valid refname, an advice message is given to refer the user to exact naming rules. * kh/branch-ref-syntax-advice: branch: advise about ref syntax rules advice: use double quotes for regular quoting advice: use backticks for verbatim advice: make all entries stylistically consistent t3200: improve test style
2024-03-15config: add --comment option to add a commentRalph Seichter4-12/+20
Introduce the ability to append comments to modifications made using git-config. Example usage: git config --comment "changed via script" \ --add safe.directory /home/alice/repo.git based on the proposed patch, the output produced is: [safe] directory = /home/alice/repo.git #changed via script Users need to be able to distinguish between config entries made using automation and entries made by a human. Automation can add comments containing a URL pointing to explanations for the change made, avoiding questions from users as to why their config file was changed by a third party. The implementation ensures that a # character is unconditionally prepended to the provided comment string, and that the comment text is appended as a suffix to the changed key-value-pair in the same line of text. Multi-line comments (i.e. comments containing linefeed) are rejected as errors, causing Git to exit without making changes. Comments are aimed at humans who inspect or change their Git config using a pager or editor. Comments are not meant to be read or displayed by git-config at a later time. Signed-off-by: Ralph Seichter <github@seichter.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14Merge branch 'la/trailer-api'Junio C Hamano1-2/+99
Trailer API updates. Acked-by: Christian Couder <christian.couder@gmail.com> cf. <CAP8UFD1Zd+9q0z1JmfOf60S2vn5-sD3SafDvAJUzRFwHJKcb8A@mail.gmail.com> * la/trailer-api: format_trailers_from_commit(): indirectly call trailer_info_get() format_trailer_info(): move "fast path" to caller format_trailers(): use strbuf instead of FILE trailer_info_get(): reorder parameters trailer: move interpret_trailers() to interpret-trailers.c trailer: reorder format_trailers_from_commit() parameters trailer: rename functions to use 'trailer' shortlog: add test for de-duplicating folded trailers trailer: free trailer_info _after_ all related usage
2024-03-14Merge branch 'so/clean-dry-run-without-force'Junio C Hamano1-15/+5
The implementation in "git clean" that makes "-n" and "-i" ignore clean.requireForce has been simplified, together with the documentation. * so/clean-dry-run-without-force: clean: further clean-up of implementation around "--force" clean: improve -n and -f implementation and documentation
2024-03-14checkout: plug some leaks in git-restoreRubén Justo1-30/+21
In git-restore we need to free the pathspec and pathspec_from_file values from the struct checkout_opts. A simple fix could be to free them in cmd_restore, after the call to checkout_main returns, like we are doing [1][2] in the sibling function cmd_checkout. However, we can do even better. We have git-switch and git-restore, both of them spin-offs[3][4] of git-checkout. All three are implemented as thin wrappers around checkout_main. Considering this, it makes a lot of sense to do the cleanup closer to checkout_main. Move the cleanups, including the new_branch_info variable, to checkout_main. As a consequence, mark: t2070, t2071, t2072 and t6418 as leak-free. [1] 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16) [2] 7ce4088ab7 (parse-options: consistently allocate memory in fix_filename(), 2023-03-04) [3] d787d311db (checkout: split part of it to new command 'switch', 2019-03-29) [4] 46e91b663b (checkout: split part of it to new command 'restore', 2019-04-25) Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14checkout: fix interaction between --conflict and --mergePhillip Wood1-3/+7
When using "git checkout" to recreate merge conflicts or merge uncommitted changes when switching branch "--conflict" sensibly implies "--merge". Unfortunately the way this is implemented means that "git checkout --conflict=diff3 --no-merge" implies "--merge" violating the usual last-one-wins rule. Fix this by only overriding the value of opts->merge if "--conflicts" comes after "--no-merge" or "-[-no]-merge" is not given on the command line. The behavior of "git checkout --merge --no-conflict" is unchanged and will still merge on the basis that the "-[-no]-conflict" options are primarily intended to affect the conflict style and so "--no-conflict" should cancel a previous "--conflict" but not override "--merge". Of the four new tests the second one tests the behavior change introduced by this commit, the other three check that this commit does not regress the existing behavior. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14checkout: cleanup --conflict=<style> parsingPhillip Wood1-19/+32
Passing an invalid conflict style name such as "--conflict=bad" gives the error message error: unknown style 'bad' given for 'merge.conflictstyle' which is unfortunate as it talks about a config setting rather than the option given on the command line. This happens because the implementation calls git_xmerge_config() to set the conflict style using the value given on the command line. Use the newly added parse_conflict_style_name() instead and pass the value down the call chain to override the config setting. This also means we can avoid setting up a struct config_context required for calling git_xmerge_config(). The option is now parsed in a callback to avoid having to store the option name. This is a change in behavior as now git checkout --conflict=bad --conflict=diff3 will error out when parsing "--conflict=bad" whereas before this change it would succeed because it would only try to parse the value of the last "--conflict" option given on the command line. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-14merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood1-2/+1
Introduce a macro to initialize `struct ll_merge_options` in preparation for the next commit that will add a new member that needs to be initialized to a non-zero value. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13status: allow --untracked=false and friendsJunio C Hamano1-2/+11
It is natural to expect that the "--untracked" option and the status.showuntrackedFiles configuration variable to take a Boolean value ("do you want me to show untracked files?"), but the current code takes nothing but "no" as "no, please do not show any". Allow the usual Boolean values to be given, and treat 'true' as "normal", and 'false' as "no". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-13status: unify parsing of --untracked= and status.showUntrackedFilesJunio C Hamano1-17/+27
There are two code paths that take a string and parse it to enum untracked_status_type. Introduce a helper function and use it. As these two places handle an error differently, add an additional invalid value to the enum, and have the caller of the helper handle the error condition, instead of dying or emitting error message from the helper. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12prefer comment_line_str to comment_line_char for printingJeff King4-14/+14
As part of our transition to multi-byte comment characters, we should use the string variable rather than the historical character variable. All of the sites adjusted here are just swapping out "%c" for "%s" in format strings, or strbuf_addch() for strbuf_addstr(). The type system and printf-attribute give the compiler enough information to make sure our formats and variable changes all match (especially important for cases where the format string is defined far away from its use, like prepare_to_commit() in commit.c). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_add_commented_lines()Jeff King2-5/+5
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_add_commented_lines() rather than a single character. All of the callers have to be adjusted; most can just pass comment_line_str rather than comment_line_char. And now our "cheat" in strbuf_commented_addf() can go away, as we can take the full string from it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_commented_addf()Jeff King3-7/+7
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_commented_addf() rather than a single character. All of the callers have to be adjusted, but they can just pass comment_line_str rather than comment_line_char. Note that we rely on strbuf_add_commented_lines() under the hood, so we'll cheat a bit to squeeze our string into a single character (for now the two are equivalent, and we'll address this TODO in the next patch). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12strbuf: accept a comment string for strbuf_stripspace()Jeff King8-9/+9
As part of our transition to multi-byte comment characters, let's take a NUL-terminated string pointer for strbuf_stripspace(), rather than a single character. We can continue to support its feature of ignoring comments by accepting a NULL pointer (as opposed to the current behavior of a NUL byte). All of the callers have to be adjusted, but they can all just pass comment_line_str (or NULL). Inside the function we detect comments by comparing the first byte of a line to the comment character. We'll adjust that to use starts_with(), which will match multiple bytes (though for now, of course, we still only allow a single byte, so it's academic). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12environment: store comment_line_char as a stringJeff King1-2/+2
We'd like to eventually support multi-byte comment prefixes, but the comment_line_char variable is referenced in many spots, making the transition difficult. Let's start by storing the character in a NUL-terminated string. That will let us switch code over incrementally to the string format, and we can easily support the existing code with a macro wrapper (since we'll continue to allow only a single-byte prefix, this will behave identically). Once all references to the "char" variable have been converted, we can drop it and enable longer strings. We'll still have to touch all of the spots that create or set the variable in this patch, but there are only a few (reading the config, and the "auto" character selector). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12commit: refactor base-case of adjust_comment_line_char()Jeff King1-2/+3
When core.commentChar is set to "auto", we check a set of candidate characters against the proposed buffer to see which if any can be used without ambiguity. But before we do that, we optimize for the common case that the default "#" is fine by just seeing if it is present in the buffer at all. The way we do this is a bit subtle, though: we assign the candidate character to comment_line_char preemptively, then check if it works, and return if it does. The subtle part is that sometimes setting comment_line_char is important (after we return, the important outcome is the fact that we have set the variable) and sometimes it is useless (if our optimization fails, we go on to do the more careful checks and eventually assign something else instead). To make it more clear what is happening (and to make further refactoring of comment_line_char easier), let's check our candidate character directly, and then assign as part of returning if it worked out. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-11Merge branch 'js/merge-base-with-missing-commit'Junio C Hamano11-42/+90
Make sure failure return from merge_bases_many() is properly caught. * js/merge-base-with-missing-commit: merge-ort/merge-recursive: do report errors in `merge_submodule()` merge-recursive: prepare for `merge_submodule()` to report errors commit-reach(repo_get_merge_bases_many_dirty): pass on errors commit-reach(repo_get_merge_bases_many): pass on "missing commits" errors commit-reach(get_octopus_merge_bases): pass on "missing commits" errors commit-reach(repo_get_merge_bases): pass on "missing commits" errors commit-reach(get_merge_bases_many_0): pass on "missing commits" errors commit-reach(merge_bases_many): pass on "missing commits" errors commit-reach(paint_down_to_common): start reporting errors commit-reach(paint_down_to_common): prepare for handling shallow commits commit-reach(repo_in_merge_bases_many): report missing commits commit-reach(repo_in_merge_bases_many): optionally expect missing commits commit-reach(paint_down_to_common): plug two memory leaks
2024-03-07Merge branch 'jk/upload-pack-bounded-resources'Junio C Hamano1-0/+2
Various parts of upload-pack has been updated to bound the resource consumption relative to the size of the repository to protect from abusive clients. * jk/upload-pack-bounded-resources: upload-pack: free tree buffers after parsing upload-pack: use PARSE_OBJECT_SKIP_HASH_CHECK in more places upload-pack: always turn off save_commit_buffer upload-pack: disallow object-info capability by default upload-pack: accept only a single packfile-uri line upload-pack: use a strmap for want-ref lines upload-pack: use oidset for deepen_not list upload-pack: switch deepen-not list to an oid_array upload-pack: drop separate v2 "haves" array
2024-03-07Merge branch 'ps/remote-helper-repo-initialization-fix'Junio C Hamano1-0/+46
A custom remote helper no longer cannot access the newly created repository during "git clone", which is a regression in Git 2.44. This has been corrected. * ps/remote-helper-repo-initialization-fix: builtin/clone: allow remote helpers to detect repo
2024-03-07Merge branch 'eg/add-uflags'Junio C Hamano1-1/+1
Code clean-up practice. * eg/add-uflags: add: use unsigned type for collection of bits
2024-03-07Merge branch 'jt/commit-redundant-scissors-fix'Junio C Hamano1-7/+5
"git commit -v --cleanup=scissors" used to add the scissors line twice in the log message buffer, which has been corrected. * jt/commit-redundant-scissors-fix: commit: unify logic to avoid multiple scissors lines when merging commit: avoid redundant scissor line with --cleanup=scissors -v
2024-03-07Merge branch 'js/merge-tree-3-trees'Junio C Hamano6-23/+57
"git merge-tree" has learned that the three trees involved in the 3-way merge only need to be trees, not necessarily commits. * js/merge-tree-3-trees: fill_tree_descriptor(): mark error message for translation cache-tree: avoid an unnecessary check Always check `parse_tree*()`'s return value t4301: verify that merge-tree fails on missing blob objects merge-ort: do check `parse_tree()`'s return value merge-tree: fail with a non-zero exit code on missing tree objects merge-tree: accept 3 trees as arguments
2024-03-07Merge branch 'cc/rev-list-allow-missing-tips'Junio C Hamano1-1/+17
"git rev-list --missing=print" has learned to optionally take "--allow-missing-tips", which allows the objects at the starting points to be missing. * cc/rev-list-allow-missing-tips: revision: fix --missing=[print|allow*] for annotated tags rev-list: allow missing tips with --missing=[print|allow*] t6022: fix 'test' style and 'even though' typo oidset: refactor oidset_insert_from_set() revision: clarify a 'return NULL' in get_reference()
2024-03-05builtin/clone.c: trivial fix of messageAlexander Shopov1-1/+1
bare in that context is an option, not purely an adjective Mark it properly Signed-off-by: Alexander Shopov <ash@kambanaria.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05builtin/remote.c: trivial fix of error messageAlexander Shopov1-1/+1
Mark --mirror as option rather than command Signed-off-by: Alexander Shopov <ash@kambanaria.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05branch: advise about ref syntax rulesKristoffer Haugsbakk1-2/+6
git-branch(1) will error out if you give it a bad ref name. But the user might not understand why or what part of the name is illegal. The user might know that there are some limitations based on the *loose ref* format (filenames), but there are also further rules for easier integration with shell-based tools, pathname expansion, and playing well with reference name expressions. The man page for git-check-ref-format(1) contains these rules. Let’s advise about it since that is not a command that you just happen upon. Also make this advise configurable since you might not want to be reminded every time you make a little typo. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-05Merge branch 'kn/for-all-refs'Junio C Hamano1-3/+7
"git for-each-ref" learned "--include-root-refs" option to show even the stuff outside the 'refs/' hierarchy. * kn/for-all-refs: for-each-ref: add new option to include root refs ref-filter: rename 'FILTER_REFS_ALL' to 'FILTER_REFS_REGULAR' refs: introduce `refs_for_each_include_root_refs()` refs: extract out `loose_fill_ref_dir_regular_file()` refs: introduce `is_pseudoref()` and `is_headref()`
2024-03-05Merge branch 'rs/name-rev-with-mempool'Junio C Hamano1-19/+20
Many small allocations "git name-rev" makes have been updated to allocate from a mem-pool. * rs/name-rev-with-mempool: name-rev: use mem_pool_strfmt() mem-pool: add mem_pool_strfmt()
2024-03-05Merge branch 'rs/fetch-simplify-with-starts-with'Junio C Hamano1-3/+2
Code simplification. * rs/fetch-simplify-with-starts-with: fetch: convert strncmp() with strlen() to starts_with()
2024-03-04clean: further clean-up of implementation around "--force"Junio C Hamano1-7/+2
We clarified how "clean.requireForce" interacts with the "--dry-run" option in the previous commit, both in the implementation and in the documentation. Even when "git clean" (without other options) is required to be used with "--force" (i.e. either clean.requireForce is unset, or explicitly set to true) to protect end-users from casual invocation of the command by mistake, "--dry-run" does not require "--force" to be used, because it is already its own protection mechanism by being a no-op to the working tree files. The previous commit, however, missed another clean-up opportunity around the same area. Just like in the "--dry-run" mode, the command in the "--interactive" mode does not require "--force", either. This is because by going interactive and giving the end user one more chance to confirm, the mode itself is serving as its own protection mechanism. Let's take things one step further, and unify the code that defines interaction between "--force" and these two other options. Just like we added explanation for the reason why "--dry-run" does not honor "clean.requireForce", give an explanation for the reason why "--interactive" makes "clean.requireForce" to be ignored. Finally, add some tests to show the interaction between "--force" and "--interactive". We already have tests that show interaction between "--force" and "--dry-run", but didn't test "--interactive". Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-03clean: improve -n and -f implementation and documentationSergey Organov1-14/+9
What -n actually does in addition to its documented behavior is ignoring of configuration variable clean.requireForce, that makes sense provided -n prevents files removal anyway. So, first, document this in the manual, and then modify implementation to make this more explicit in the code. Improved implementation also stops to share single internal variable 'force' between command-line -f option and configuration variable clean.requireForce, resulting in more clear logic. Two error messages with slightly different text depending on if clean.requireForce was explicitly set or not, are merged into a single one. The resulting error message now does not mention -n as well, as it neither matches intended clean.requireForce usage nor reflects clarified implementation. Documentation of clean.requireForce is changed accordingly. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-02repack: check error writing to pack-objects subprocessJunio C Hamano1-2/+3
When "git repack" repacks promisor objects, it starts a pack-objects subprocess and uses xwrite() to send object names over the pipe to it, but without any error checking. An I/O error or short write (even though a short write is unlikely for such a small amount of data) can result in a packfile that lacks certain objects we wanted to put in there, leading to a silent repository corruption. Use write_in_full(), instead of xwrite(), to mitigate short write risks, check errors from it, and abort if we see a failure. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-02unpack: replace xwrite() loop with write_in_full()Junio C Hamano2-21/+4
We have two packfile stream consumers, index-pack and unpack-objects, that allow excess payload after the packfile stream data. Their code to relay excess data hasn't changed significantly since their original implementation that appeared in 67e5a5ec (git-unpack-objects: re-write to read from stdin, 2005-06-28) and 9bee2478 (mimic unpack-objects when --stdin is used with index-pack, 2006-10-25). These code blocks contain hand-rolled loops using xwrite(), written before our write_in_full() helper existed. This helper now provides the same functionality. Replace these loops with write_in_full() for shorter, clearer code. Update related variables accordingly. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-01Merge branch 'hs/rebase-not-in-progress' into HEADJunio C Hamano1-1/+1
Error message update. * hs/rebase-not-in-progress: rebase: make warning less passive aggressive
2024-03-01Merge branch 'ps/reflog-list' into HEADJunio C Hamano2-5/+36
"git reflog" learned a "list" subcommand that enumerates known reflogs. * ps/reflog-list: builtin/reflog: introduce subcommand to list reflogs refs: stop resolving ref corresponding to reflogs refs: drop unused params from the reflog iterator callback refs: always treat iterators as ordered refs/files: sort merged worktree and common reflogs refs/files: sort reflogs returned by the reflog iterator dir-iterator: support iteration in sorted order dir-iterator: pass name to `prepare_next_entry_data()` directly
2024-03-01format_trailers(): use strbuf instead of FILELinus Arver1-1/+5
This is another preparatory refactor to unify the trailer formatters. Make format_trailers() also write to a strbuf, to align with format_trailers_from_commit() which also does the same. Doing this makes format_trailers() behave similar to format_trailer_info() (which will soon help us replace one with the other). Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-01trailer: move interpret_trailers() to interpret-trailers.cLinus Arver1-0/+93
The interpret-trailers.c builtin is the only place we need to call interpret_trailers(), so move its definition there (together with a few helper functions called only by it) and remove its external declaration from <trailer.h>. Several helper functions that are called by interpret_trailers() remain in trailer.c because other callers in the same file still call them. Declare them in <trailer.h> so that interpret_trailers() (now in builtin/interpret-trailers.c) can continue calling them as a trailer API user. This enriches <trailer.h> with a more granular API, which can then be unit-tested in the future (because interpret_trailers() by itself does too many things to be able to be easily unit-tested). Take this opportunity to demote some file-handling functions out of the trailer API implementation, as these have nothing to do with trailers. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-01trailer: rename functions to use 'trailer'Linus Arver1-2/+2
Rename process_trailers() to interpret_trailers(), because it matches the name for the builtin command of the same name (git-interpret-trailers), which is the sole user of process_trailers(). In a following commit, we will move "interpret_trailers" from trailer.c to builtin/interpret-trailers.c. That move will necessitate the growth of the trailer.h API, forcing us to expose some additional functions in trailer.h. Rename relevant functions so that they include the term "trailer" in their name, so that clients of the API will be able to easily identify them by their "trailer" moniker, just like all the other functions already exposed by trailer.h. Rename `struct list_head *head` to `struct list_head *trailers` because "head" conveys no additional information beyond the "list_head" type. Reorder parameters for format_trailers_from_commit() to prefer const struct process_trailer_options *opts as the first parameter, because these options are intimately tied to formatting trailers. Parameters like `FILE *outfile` should be last because they are a kind of 'out' parameter, so put such parameters at the end. This will be the pattern going forward in this series. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29add: use unsigned type for collection of bitsEugenio Gigante1-1/+1
The 'refresh' function in 'builtin/add.c' declares 'flags' as signed, and passes it as an argument to the 'refresh_index' function, which though expects an unsigned value. Since in this case 'flags' represents a bag of bits, whose MSB is not used in special ways, change the type of 'flags' to unsigned. Signed-off-by: Eugenio Gigante <giganteeugenio2@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29commit-reach(repo_get_merge_bases_many_dirty): pass on errorsJohannes Schindelin1-3/+6
(Actually, this commit is only about passing on "missing commits" errors, but adding that to the commit's title would have made it too long.) The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases_many_dirty()` function is aware of that, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29commit-reach(repo_get_merge_bases_many): pass on "missing commits" errorsJohannes Schindelin1-6/+7
The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases_many()` function is aware of that, too. Naturally, there are a lot of callers that need to be adjusted now, too. Next stop: `repo_get_merge_bases_dirty()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29commit-reach(get_octopus_merge_bases): pass on "missing commits" errorsJohannes Schindelin3-5/+14
The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases()` function (which is also surfaced via the `get_merge_bases()` macro) is aware of that, too. Naturally, the callers need to be adjusted now, too. Next step: adjust `repo_get_merge_bases_many()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-29commit-reach(repo_get_merge_bases): pass on "missing commits" errorsJohannes Schindelin5-20/+28
The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases()` function (which is also surfaced via the `repo_get_merge_bases()` macro) is aware of that, too. Naturally, there are a lot of callers that need to be adjusted now, too. Next step: adjust the callers of `get_octopus_merge_bases()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-28upload-pack: always turn off save_commit_bufferJeff King1-0/+2
When the client sends us "want $oid" lines, we call parse_object($oid) to get an object struct. It's important to parse the commits because we need to traverse them in the negotiation phase. But of course we don't need to hold on to the commit messages for each one. We've turned off the save_commit_buffer flag in get_common_commits() for a long time, since f0243f26f6 (git-upload-pack: More efficient usage of the has_sha1 array, 2005-10-28). That helps with the commits we see while actually traversing. But: 1. That function is only used by the v0 protocol. I think the v2 protocol's code path leaves the flag on (and thus pays the extra memory penalty), though I didn't measure it specifically. 2. If the client sends us a bunch of "want" lines, that happens before the negotiation phase. So we'll hold on to all of those commit messages. Generally the number of "want" lines scales with the refs, not with the number of objects in the repo. But a malicious client could send a lot in order to waste memory. As an example of (2), if I generate a request to fetch all commits in git.git like this: pktline() { local msg="$*" printf "%04x%s\n" $((1+4+${#msg})) "$msg" } want_commits() { pktline command=fetch printf 0001 git cat-file --batch-all-objects --batch-check='%(objectname) %(objecttype)' | while read oid type; do test "$type" = "commit" || continue pktline want $oid done pktline done printf 0000 } want_commits | GIT_PROTOCOL=version=2 valgrind --tool=massif git-upload-pack . >/dev/null before this patch upload-pack peaks at ~125MB, and after at ~35MB. The difference is not coincidentally about the same as the sum of all commit object sizes as computed by: git cat-file --batch-all-objects --batch-check='%(objecttype) %(objectsize)' | perl -alne '$v += $F[1] if $F[0] eq "commit"; END { print $v }' In a larger repository like linux.git, that number is ~1GB. In a repository with a full commit-graph file this will have no impact (and the commit graph would save us from parsing at all, so is a much better solution!). But it's easy to do, might help a little in real-world cases (where even if you have a commit graph it might not be fully up to date), and helps a lot for a worst-case malicious request. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-28commit-reach(repo_in_merge_bases_many): report missing commitsJohannes Schindelin7-8/+35
Some functions in Git's source code follow the convention that returning a negative value indicates a fatal error, e.g. repository corruption. Let's use this convention in `repo_in_merge_bases()` to report when one of the specified commits is missing (i.e. when `repo_parse_commit()` reports an error). Also adjust the callers of `repo_in_merge_bases()` to handle such negative return values. Note: As of this patch, errors are returned only if any of the specified merge heads is missing. Over the course of the next patches, missing commits will also be reported by the `paint_down_to_common()` function, which is called by `repo_in_merge_bases_many()`, and those errors will be properly propagated back to the caller at that stage. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-27Merge branch 'rj/tag-column-fix'Junio C Hamano1-1/+2
"git tag --column" failed to check the exit status of its "git column" invocation, which has been corrected. * rj/tag-column-fix: tag: error when git-column fails
2024-02-27builtin/clone: allow remote helpers to detect repoPatrick Steinhardt1-0/+46
In 18c9cb7524 (builtin/clone: create the refdb with the correct object format, 2023-12-12), we have changed git-clone(1) so that it delays creation of the refdb until after it has learned about the remote's object format. This change was required for the reftable backend, which encodes the object format into the tables. So if we pre-initialized the refdb with the default object format, but the remote uses a different object format than that, then the resulting tables would have encoded the wrong object format. This change unfortunately breaks remote helpers which try to access the repository that is about to be created. Because the refdb has not yet been initialized at the point where we spawn the remote helper, we also don't yet have "HEAD" or "refs/". Consequently, any Git commands ran by the remote helper which try to access the repository would fail because it cannot be discovered. This is essentially a chicken-and-egg problem: we cannot initialize the refdb because we don't know about the object format. But we cannot learn about the object format because the remote helper may be unable to access the partially-initialized repository. Ideally, we would address this issue via capabilities. But the remote helper protocol is not structured in a way that guarantees that the capability announcement happens before the remote helper tries to access the repository. Instead, fix this issue by partially initializing the refdb up to the point where it becomes discoverable by Git commands. Reported-by: Mike Hommey <mh@glandium.org> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-27rebase -i: stop setting GIT_CHERRY_PICK_HELPPhillip Wood1-11/+3
Setting this environment variable causes the sequencer to display a custom message when it stops for the user to resolve conflicts and remove CHERRY_PICK_HEAD. Setting it in "git rebase" is a vestige of the scripted implementation, now that it is a builtin command we do not need to communicate with the sequencer machinery via environment variables. Move the conflicts advice to use when rebasing into sequencer.c so we do not need to pass it via the environment. Note that we retain the changes in e4301f73fff (sequencer: unset GIT_CHERRY_PICK_HELP for 'exec' commands, 2024-02-02) just in case GIT_CHERRY_PICK_HELP is set in the environment when "git rebase" is run. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-27commit: unify logic to avoid multiple scissors lines when mergingJosh Triplett1-5/+3
prepare_to_commit has some logic to figure out whether merge already added a scissors line, and therefore it shouldn't add another. Now that wt_status_add_cut_line has built-in state for whether it has already added a previous line, just set that state instead, and then remove that condition from subsequent calls to wt_status_add_cut_line. Signed-off-by: Josh Triplett <josh@joshtriplett.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-27commit: avoid redundant scissor line with --cleanup=scissors -vJosh Triplett1-2/+2
`git commit --cleanup=scissors -v` prints two scissors lines: one at the start of the comment lines, and the other right before the diff. This is redundant, and pushes the diff further down in the user's editor than it needs to be. Make wt_status_add_cut_line() remember if it has added a cut line before, and avoid adding a redundant one. Add a test for this. Signed-off-by: Josh Triplett <josh@joshtriplett.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-26Merge branch 'gt/at-is-synonym-for-head-in-add-patch'Junio C Hamano2-2/+6
Teach "git checkout -p" and friends that "@" is a synonym for "HEAD". * gt/at-is-synonym-for-head-in-add-patch: add -p tests: remove PERL prerequisites add-patch: classify '@' as a synonym for 'HEAD'
2024-02-26Merge branch 'kh/column-reject-negative-padding'Junio C Hamano1-0/+2
"git column" has been taught to reject negative padding value, as it would lead to nonsense behaviour including division by zero. * kh/column-reject-negative-padding: column: guard against negative padding column: disallow negative padding
2024-02-26Merge branch 'rs/use-xstrncmpz'Junio C Hamano3-6/+3
Code clean-up. * rs/use-xstrncmpz: use xstrncmpz()
2024-02-26name-rev: use mem_pool_strfmt()René Scharfe1-19/+20
1c56fc2084 (name-rev: pre-size buffer in get_parent_name(), 2020-02-04) got a big performance boost in an unusual repository by calculating the name length in advance. This is a bit awkward, as it references the name components twice. Use a memory pool to store the strings for the struct rev_name member tip_name. Using mem_pool_strfmt() allows efficient allocation without explicit size calculation. This simplifies the formatting part of the code without giving up performance: Benchmark 1: ./git_2.44.0 -C ../chromium/src name-rev --all Time (mean ± σ): 1.231 s ± 0.013 s [User: 1.082 s, System: 0.136 s] Range (min … max): 1.214 s … 1.252 s 10 runs Benchmark 2: ./git -C ../chromium/src name-rev --all Time (mean ± σ): 1.220 s ± 0.020 s [User: 1.083 s, System: 0.130 s] Range (min … max): 1.197 s … 1.254 s 10 runs Don't bother discarding the memory pool just before exiting. The effort for that would be very low, but actually measurable in the above example, with no benefit to users. At least UNLEAK it to calm down leak checkers. This addresses the leaks that 45a14f578e (Revert "name-rev: release unused name strings", 2022-04-22) brought back. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-26fetch: convert strncmp() with strlen() to starts_with()René Scharfe1-3/+2
Using strncmp() and strlen() to check whether a string starts with another one requires repeating the prefix candidate. Use starts_with() instead, which reduces repetition and is more readable. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-23for-each-ref: add new option to include root refsKarthik Nayak1-3/+7
The git-for-each-ref(1) command doesn't provide a way to print root refs i.e pseudorefs and HEAD with the regular "refs/" prefixed refs. This commit adds a new option "--include-root-refs" to git-for-each-ref(1). When used this would also print pseudorefs and HEAD for the current worktree. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-23ref-filter: rename 'FILTER_REFS_ALL' to 'FILTER_REFS_REGULAR'Karthik Nayak1-1/+1
The flag 'FILTER_REFS_ALL' is a bit ambiguous, where ALL doesn't specify if it means to contain refs from all worktrees or whether all types of refs (regular, HEAD & pseudorefs) or all of the above. Since here it is actually referring to all refs with the "refs/" prefix, let's rename it to 'FILTER_REFS_REGULAR' to indicate that this is specifically for regular refs. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-23Always check `parse_tree*()`'s return valueJohannes Schindelin6-6/+32
Otherwise we may easily run into serious crashes: For example, if we run `init_tree_desc()` directly after a failed `parse_tree()`, we are accessing uninitialized data or trying to dereference `NULL`. Note that the `parse_tree()` function already takes care of showing an error message. The `parse_tree_indirectly()` and `repo_get_commit_tree()` functions do not, therefore those latter call sites need to show a useful error message while the former do not. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-21builtin/reflog: introduce subcommand to list reflogsPatrick Steinhardt1-0/+34
While the git-reflog(1) command has subcommands to show reflog entries or check for reflog existence, it does not have any subcommands that would allow the user to enumerate all existing reflogs. This makes it quite hard to discover which reflogs a repository has. While this can be worked around with the "files" backend by enumerating files in the ".git/logs" directory, users of the "reftable" backend don't enjoy such a luxury. Introduce a new subcommand `git reflog list` that lists all reflogs the repository knows of to fill this gap. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-21refs: drop unused params from the reflog iterator callbackPatrick Steinhardt2-5/+2
The ref and reflog iterators share much of the same underlying code to iterate over the corresponding entries. This results in some weird code because the reflog iterator also exposes an object ID as well as a flag to the callback function. Neither of these fields do refer to the reflog though -- they refer to the corresponding ref with the same name. This is quite misleading. In practice at least the object ID cannot really be implemented in any other way as a reflog does not have a specific object ID in the first place. This is further stressed by the fact that none of the callbacks except for our test helper make use of these fields. Split up the infrastucture so that ref and reflog iterators use separate callback signatures. This allows us to drop the nonsensical fields from the reflog iterator. Note that internally, the backends still use the same shared infra to iterate over both types. As the backends should never end up being called directly anyway, this is not much of a problem and thus kept as-is for simplicity's sake. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-21rebase: make warning less passive aggressiveHarmen Stoppels1-1/+1
When you run `git rebase --continue` when no rebase is in progress, git outputs `fatal: No rebase in progress?` which is not a question but a statement. Make it appear as a statement, and use lowercase to align with error message style. Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-14Merge branch 'js/check-null-from-read-object-file'Junio C Hamano3-4/+14
The code paths that call repo_read_object_file() have been tightened to react to errors. * js/check-null-from-read-object-file: Always check the return value of `repo_read_object_file()`
2024-02-14Merge branch 'rs/receive-pack-remove-find-header'Junio C Hamano1-35/+13
Code simplification. * rs/receive-pack-remove-find-header: receive-pack: use find_commit_header() in check_nonce() receive-pack: use find_commit_header() in check_cert_push_options()
2024-02-14tag: error when git-column failsRubén Justo1-1/+2
If the user asks for the list of tags to be displayed in columns ("--columns"), a child git-column process is used to format the output as expected. In a rare situation where we encounter a problem spawning that child process, we will work erroneously. Make noticeable we're having a problem executing git-column, so the user can act accordingly. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-14rev-list: allow missing tips with --missing=[print|allow*]Christian Couder1-1/+17
In 9830926c7d (rev-list: add commit object support in `--missing` option, 2023-10-27) we fixed the `--missing` option in `git rev-list` so that it works with with missing commits, not just blobs/trees. Unfortunately, such a command would still fail with a "fatal: bad object <oid>" if it is passed a missing commit, blob or tree as an argument (before the rev walking even begins). When such a command is used to find the dependencies of some objects, for example the dependencies of quarantined objects (see the "QUARANTINE ENVIRONMENT" section in the git-receive-pack(1) documentation), it would be better if the command would instead consider such missing objects, especially commits, in the same way as other missing objects. If, for example `--missing=print` is used, it would be nice for some use cases if the missing tips passed as arguments were reported in the same way as other missing objects instead of the command just failing. We could introduce a new option to make it work like this, but most users are likely to prefer the command to have this behavior as the default one. Introducing a new option would require another dumb loop to look for that option early, which isn't nice. Also we made `git rev-list` work with missing commits very recently and the command is most often passed commits as arguments. So let's consider this as a bug fix related to these recent changes. While at it let's add a NEEDSWORK comment to say that we should get rid of the existing ugly dumb loops that parse the `--exclude-promisor-objects` and `--missing=...` options early. Helped-by: Linus Arver <linusa@google.com> Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>