aboutsummaryrefslogtreecommitdiff
path: root/builtin/stash.c
AgeCommit message (Collapse)Author
2026-03-30docs: fix "git stash [push]" documentationQuentin Bernet
Both the synopsis and explanation are incorrect and contradict each other. The synopsis claims "push" can only be omitted when you do not give any options and arguments. The explanation correctly claims that non-option arguments are not allowed, except pathspec elements preceded by double hyphens. But it also adds "-p" to the list of exceptions, even though it is an option argument. Signed-off-by: Quentin Bernet <quentin.bernet@bluewin.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-03-24Merge branch 'dd/cocci-do-not-pass-strbuf-by-value'Junio C Hamano
Add a coccinelle rule to break the build when "struct strbuf" gets passed by value. * dd/cocci-do-not-pass-strbuf-by-value: stash: do not pass strbuf by value coccinelle: detect struct strbuf passed by value
2026-03-15stash: do not pass strbuf by valueDeveshi Dwivedi
save_untracked_files() takes its 'files' parameter as struct strbuf by value. Passing a strbuf by value copies the struct but shares the underlying buffer between caller and callee, risking a dangling pointer and double-free if the callee reallocates. The function needs both the buffer and its length for pipe_command(), so a plain const char * is not sufficient here. Switch the parameter to struct strbuf * and update the caller to pass a pointer. Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-03-03add-patch: allow disabling editing of hunksPatrick Steinhardt
The "add-patch" mode allows the user to edit hunks to apply custom changes. This is incompatible with a new `git history split` command that we're about to introduce in a subsequent commit, so we need a way to disable this mode. Add a new flag to disable editing hunks. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-03-03add-patch: split out `struct interactive_options`Patrick Steinhardt
The `struct add_p_opt` is reused both by our infra for "git add -p" and "git add -i". Users of `run_add_i()` for example are expected to pass `struct add_p_opt`. This is somewhat confusing and raises the question of which options apply to what part of the stack. But things are even more confusing than that: while callers are expected to pass in `struct add_p_opt`, these options ultimately get used to initialize a `struct add_i_state` that is used by both subsystems. So we are basically going full circle here. Refactor the code and split out a new `struct interactive_options` that hosts common options used by both. These options are then applied to a `struct interactive_config` that hosts common configuration. This refactoring doesn't yet fully detangle the two subsystems from one another, as we still end up calling `init_add_i_state()` in the "git add -p" subsystem. This will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-03-03Merge branch 'aa/add-p-no-auto-advance'Junio C Hamano
"git add -p" learned a new mode that allows the user to revisit a file that was already dealt with. * aa/add-p-no-auto-advance: add-patch: allow interfile navigation when selecting hunks add-patch: allow all-or-none application of patches add-patch: modify patch_update_file() signature interactive -p: add new `--auto-advance` flag
2026-02-17interactive -p: add new `--auto-advance` flagAbraham Samuel Adekunle
When using the interactive add, reset, stash or checkout machinery, we do not have the option of reworking with a file when selecting hunks, because the session automatically advances to the next file or ends if we have just one file. Introduce the flag `--auto-advance` which auto advances by default, when interactively selecting patches with the '--patch' option. However, the `--no-auto-advance` option does not auto advance, thereby allowing users the option to rework with files. Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-02-13Merge branch 'ps/commit-list-functions-renamed'Junio C Hamano
Rename three functions around the commit_list data structure. * ps/commit-list-functions-renamed: commit: rename `free_commit_list()` to conform to coding guidelines commit: rename `reverse_commit_list()` to conform to coding guidelines commit: rename `copy_commit_list()` to conform to coding guidelines
2026-01-15commit: rename `free_commit_list()` to conform to coding guidelinesPatrick Steinhardt
Our coding guidelines say that: Functions that operate on `struct S` are named `S_<verb>()` and should generally receive a pointer to `struct S` as first parameter. While most of the functions related to `struct commit_list` already follow that naming schema, `free_commit_list()` doesn't. Rename the function to address this and adjust all of its callers. Add a compatibility wrapper for the old function name to ease the transition and avoid any semantic conflicts with in-flight patch series. This wrapper will be removed once Git 2.53 has been released. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-01-15commit: rename `reverse_commit_list()` to conform to coding guidelinesPatrick Steinhardt
Our coding guidelines say that: Functions that operate on `struct S` are named `S_<verb>()` and should generally receive a pointer to `struct S` as first parameter. While most of the functions related to `struct commit_list` already follow that naming schema, `reverse_commit_list()` doesn't. Rename the function to address this and adjust all of its callers. Add a compatibility wrapper for the old function name to ease the transition and avoid any semantic conflicts with in-flight patch series. This wrapper will be removed once Git 2.53 has been released. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2026-01-09cocci: convert parse_tree functions to repo_ variantsRené Scharfe
Add and apply a semantic patch to convert calls to parse_tree() and friends to the corresponding variant that takes a repository argument, to allow the functions that implicitly use the_repository to be retired once all potential in-flight topics are settled and converted as well. The changes in .c files were generated by Coccinelle, but I fixed a whitespace bug it would have introduced to builtin/commit.c. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-29Merge branch 'dk/stash-apply-index'Junio C Hamano
The stash.index configuration variable can be set to make "git stash pop/apply" pretend that it was invoked with "--index". * dk/stash-apply-index: stash: honor stash.index in apply, pop modes stash: refactor private config globals t3905: remove unneeded blank line t3903: reduce dependencies on previous tests
2025-09-29Merge branch 'jk/setup-revisions-freefix'Junio C Hamano
There are double frees and leaks around setup_revisions() API used in "git stash show", which has been fixed, and setup_revisions() API gained a wrapper to make it more ergonomic when using it with strvec-manged argc/argv pairs. * jk/setup-revisions-freefix: revision: retain argv NULL invariant in setup_revisions() treewide: pass strvecs around for setup_revisions_from_strvec() treewide: use setup_revisions_from_strvec() when we have a strvec revision: add wrapper to setup_revisions() from a strvec revision: manage memory ownership of argv in setup_revisions() stash: tell setup_revisions() to free our allocated strings
2025-09-23Merge branch 'rs/get-oid-with-flags-cleanup'Junio C Hamano
Code clean-up. * rs/get-oid-with-flags-cleanup: use repo_get_oid_with_flags()
2025-09-23Merge branch 'jk/add-i-color'Junio C Hamano
Some among "git add -p" and friends ignored color.diff and/or color.ui configuration variables, which is an old regression, which has been corrected. * jk/add-i-color: contrib/diff-highlight: mention interactive.diffFilter add-interactive: manually fall back color config to color.ui add-interactive: respect color.diff for diff coloring stash: pass --no-color to diff plumbing child processes
2025-09-22revision: add wrapper to setup_revisions() from a strvecJeff King
The setup_revisions() function was designed to take the argc/argv pair from the operating system. But we sometimes construct our own argv using a strvec and pass that in. There are a few gotchas that callers need to deal with here: 1. You should always pass the free_removed_argv_elements option via setup_revision_opt. Otherwise, entries may be leaked if setup_revisions() re-shuffles options. 2. After setup_revisions() returns, the strvec state is odd. We get a reduced argc from setup_revisions() telling us how many unknown options were left in place. Entries after that in argv may be retained, or may be NULL (depending on how the reshuffling happened). But the strvec's "nr" field still represents the original value, and some of the entries it thinks it is still storing may be NULL. Callers must be careful with how they access it. Some callers deal with (1), but not all. In practice they are OK because they do not pass any options that would cause setup_revisions() to re-shuffle (namely unknown options which may be relayed from the user, and the use of the "--" separator). But it's probably a good idea to consistently pass this option anyway to future-proof ourselves against the details of setup_revisions() changing. No callers address (2), though I don't think there any visible bugs. Most of them simply call strvec_clear() and never otherwise look at the result. And in fact, if they naively set foo.nr to the argc returned by setup_revisions(), that would cause leaks! Because setup_revisions() does not free consumed options[1], we have to leave the "nr" field of the strvec at its original value to find and free them during strvec_clear(). So I don't think there are any bugs to fix here, but we can make things safer and simpler for callers. Let's introduce a helper function that sets the free_removed_argv_elements automatically and shrinks the strvec to represent the retained options afterwards (taking care to free the now-obsolete entries). We'll start by converting all of the call-sites which use the free_removed_argv_elements option. There should be no behavior change for them, except that their "shrunken" entries are cleaned up immediately, rather than waiting for a strvec_clear() call. [1] Arguably setup_revisions() should be doing this step for us if we told it to free removed options, but there are many existing callers which will be broken if it did. Introducing this helper is a possible first step towards that. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-22stash: tell setup_revisions() to free our allocated stringsJeff King
In "git stash show", we do a first pass of parsing our command line options by splitting them into revision args and stash args. These are stored in strvecs, and we pass the revision args to setup_revisions(). But setup_revisions() may modify the argv we pass it, causing us to leak some of the entries. In particular, if it sees a "--" string, that will be dropped from argv. This is the same as other cases addressed by f92dbdbc6a (revisions API: don't leak memory on argv elements that need free()-ing, 2022-08-02), and we should fix it the same way: by passing the free_removed_argv_elements option to setup_revisions(). The added test here is run only with SANITIZE=leak, without checking its output, because the behavior of stash with "--" is a little odd: 1. Running "git stash show" will show --stat output. But running "git stash show --" will show --patch. 2. I'd expect a non-option after "--" to be treated as a pathspec, so: git stash show -p 1 -- foo would look treat "1" as a stash (a synonym for stash@{1}) and restrict the resulting diff to "foo". But it doesn't. We split the revision/stash args without any regard to "--". So in the example above both "1" and "foo" are stashes. Which is an error, but also: git stash show -- foo treats "foo" as a stash, not a pathspec. These are both oddities that we may want to address (or may not, if we want to retain historical quirks). But they are well outside the scope of this patch. So for now we'll just let the tests confirm we aren't leaking without otherwise expecting any behavior. If we later address either of those points and end up with another test that covers "stash show --", we can drop this leak-only test. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-21stash: honor stash.index in apply, pop modesD. Ben Knoble
With stash.index=true, git-stash(1) command now tries to reinstate the index by default in the "apply" and "pop" modes. Not doing so creates a common trap [1], [2]: "git stash apply" is not the reverse of "git stash push" because carefully staged indices are lost and have to be manually recreated. OTOH, this mode is not always desirable and may create more conflicts when applying stashes. As usual, "--no-index" will disable this behavior if you set "stash.index". [1]: https://lore.kernel.org/git/CAPx1GvcxyDDQmCssMjEnt6JoV6qPc5ZUpgPLX3mpUC_4PNYA1w@mail.gmail.com/ [2]: https://lore.kernel.org/git/c5a811ac-8cd3-c389-ac6d-29020a648c87@gmail.com/ Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-21stash: refactor private config globalsD. Ben Knoble
A subsequent commit will access a new config variable in the stash subcommand implementations, which requires the variables to be declared before the relevant functions. Prep with a pure refactoring change to consolidate config-related globals with the rest of the globals. Best-viewed-with: --color-moved Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-10use repo_get_oid_with_flags()René Scharfe
get_oid_with_context() allows specifying flags and reports object details via a passed-in struct object_context. Some callers just want to specify flags, but don't need any details back. Convert them to repo_get_oid_with_flags(), which provides just that and frees them from dealing with the context structure. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-08stash: pass --no-color to diff plumbing child processesJeff King
After a partial stash, we may clear out the working tree by capturing the output of diff-tree and piping it into git-apply (and likewise we may use diff-index to restore the index). So we most definitely do not want color diff output from that diff-tree process. And it normally would not produce any, since its stdout is not going to a tty, and the default value of color.ui is "auto". However, if GIT_PAGER_IN_USE is set in the environment, that overrides the tty check, and we'll produce a colorized diff that chokes git-apply: $ echo y | GIT_PAGER_IN_USE=1 git stash -p [...] Saved working directory and index state WIP on main: 4f2e2bb foo error: No valid patches in input (allow with "--allow-empty") Cannot remove worktree changes Setting this variable is a relatively silly thing to do, and not something most users would run into. But we sometimes do it in our tests to stimulate color. And it is a user-visible bug, so let's fix it rather than work around it in the tests. The root issue here is that diff-tree (and other diff plumbing) should probably not ever produce color by default. It does so not by parsing color.ui, but because of the baked-in "auto" default from 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10). But changing that is risky; we've had discussions back and forth on the topic over the years. E.g.: https://lore.kernel.org/git/86D0A377-8AFD-460D-A90E-6327C6934DFC@gmail.com/. So let's accept that as the status quo for now and protect ourselves by passing --no-color to the child processes. This is the same thing we did for add-interactive itself in 1c6ffb546b (add--interactive.perl: specify --no-color explicitly, 2020-09-07). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-21Merge branch 'ps/remote-rename-fix'Junio C Hamano
"git remote rename origin upstream" failed to move origin/HEAD to upstream/HEAD when origin/HEAD is unborn and performed other renames extremely inefficiently, which has been corrected. * ps/remote-rename-fix: builtin/remote: only iterate through refs that are to be renamed builtin/remote: rework how remote refs get renamed builtin/remote: determine whether refs need renaming early on builtin/remote: fix sign comparison warnings refs: simplify logic when migrating reflog entries refs: pass refname when invoking reflog entry callback
2025-08-06refs: pass refname when invoking reflog entry callbackPatrick Steinhardt
With `refs_for_each_reflog_ent()` callers can iterate through all the reflog entries for a given reference. The callback that is being invoked for each such entry does not receive the name of the reference that we are currently iterating through. This isn't really a limiting factor, as callers can simply pass the name via the callback data. But this layout sometimes does make for a bit of an awkward calling pattern. One example: when iterating through all reflogs, and for each reflog we iterate through all refnames, we have to do some extra book keeping to track which reference name we are currently yielding reflog entries for. Change the signature of the callback function so that the reference name of the reflog gets passed through to it. Adapt callers accordingly and start using the new parameter in trivial cases. The next commit will refactor the reference migration logic to make use of this parameter so that we can simplify its logic a bit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-04Merge branch 'lm/add-p-context'Junio C Hamano
"git add/etc -p" now honor the diff.context configuration variable, and also they learn to honor the -U<n> command-line option. * lm/add-p-context: add-patch: add diff.context command line overrides add-patch: respect diff.context configuration t: use test_config in t4055 t: use test_grep in t3701 and t4055
2025-07-29add-patch: add diff.context command line overridesLeon Michalak
This patch compliments the previous commit, where builtins that use add-patch infrastructure now respect diff.context and diff.interHunkContext file configurations. In particular, this patch helps users who don't want to set persistent context configurations or just want a way to override them on a one-time basis, by allowing the relevant builtins to accept corresponding command line options that override the file configurations. This mimics commands such as diff and log, which allow for both context file configuration and command line overrides. Signed-off-by: Leon Michalak <leonmichalak6@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-23config: drop `git_config()` wrapperPatrick Steinhardt
In 036876a1067 (config: hide functions using `the_repository` by default, 2024-08-13) we have moved around a bunch of functions in the config subsystem that depend on `the_repository`. Those function have been converted into mere wrappers around their equivalent function that takes in a repository as parameter, and the intent was that we'll eventually remove those wrappers to make the dependency on the global repository variable explicit at the callsite. Follow through with that intent and remove `git_config()`. All callsites are adjusted so that they use `repo_config(the_repository, ...)` instead. While some callsites might already have a repository available, this mechanical conversion is the exact same as the current situation and thus cannot cause any regression. Those sites should eventually be cleaned up in a later patch series. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-30Merge branch 'bc/stash-export-import'Junio C Hamano
An interchange format for stash entries is defined, and subcommand of "git stash" to import/export has been added. * bc/stash-export-import: builtin/stash: provide a way to import stashes from a ref builtin/stash: provide a way to export stashes to a ref builtin/stash: factor out revision parsing into a function object-name: make get_oid quietly return an error
2025-06-24Merge branch 'kj/stash-onbranch-submodule-fix'Junio C Hamano
"git stash" recorded a wrong branch name when submodules are present in the current checkout, which has been corrected. * kj/stash-onbranch-submodule-fix: stash: fix incorrect branch name in stash message
2025-06-12builtin/stash: provide a way to import stashes from a refbrian m. carlson
Now that we have a way to export stashes to a ref, let's provide a way to import them from such a ref back to the stash. This works much the way the export code does, except that we strip off the first parent chain commit and then store each resulting commit back to the stash. We don't clear the stash first and instead add the specified stashes to the top of the stash. This is because users may want to export just a few stashes, such as to share a small amount of work in progress with a colleague, and it would be undesirable for the receiving user to lose all of their data. For users who do want to replace the stash, it's easy to do to: simply run "git stash clear" first. We specifically rely on the fact that we'll produce identical stash commits on both sides in our tests. This provides a cheap, straightforward check for our tests and also makes it easy for users to see if they already have the same data in both repositories. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-12builtin/stash: provide a way to export stashes to a refbrian m. carlson
A common user problem is how to sync in-progress work to another machine. Users currently must use some sort of transfer of the working tree, which poses security risks and also necessarily causes the index to become dirty. The experience is suboptimal and frustrating for users. A reasonable idea is to use the stash for this purpose, but the stash is stored in the reflog, not in a ref, and as such it cannot be pushed or pulled. This also means that it cannot be saved into a bundle or preserved elsewhere, which is a problem when using throwaway development environments. In addition, users often want to replicate stashes across machines, such as when they must use multiple machines or when they use throwaway dev environments, such as those based on the Devcontainer spec, where they might otherwise lose various in-progress work. Let's solve this problem by allowing the user to export the stash to a ref (or, to just write it into the repository and print the hash, à la git commit-tree). Introduce git stash export, which writes a chain of commits where the first parent is always a chain to the previous stash, or to a single, empty commit (for the final item) and the second is the stash commit normally written to the reflog. Iterate over each stash from top to bottom, looking up the data for each one, and then create the chain from the single empty commit back up in reverse order. Generate a predictable empty commit so our behavior is reproducible. Create a useful commit message, preserving the author and committer information, to help users identify stash commits when viewing them as normal commits. If the user has specified specific stashes they'd like to export instead, use those instead of iterating over all of the stashes. As part of this, specifically request quiet behavior when looking up the OID for a revision because we will eventually hit a revision that doesn't exist and we don't want to die when that occurs. When exporting stashes, be sure to verify that they look like valid stashes and don't contain invalid data. This will help avoid failures on import or problems due to attempting to export invalid refs that are not stashes. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-12builtin/stash: factor out revision parsing into a functionbrian m. carlson
We allow several special forms of stash names in this code. In the future, we'll want to allow these same forms without parsing a stash commit, so let's refactor this code out into a function for reuse. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-11stash: fix incorrect branch name in stash messageK Jayatheerth
When creating a stash, Git uses the current branch name of the superproject to construct the stash commit message. However, in repositories with submodules, the message may mistakenly display the submodule branch name instead. This is because `refs_resolve_ref_unsafe()` returns a pointer to a static buffer. Subsequent calls to the same function overwrite the buffer, corrupting the originally fetched `branch_name` used for the stash message. Use `xstrdup()` to duplicate the branch name immediately after resolving it, so that later buffer overwrites do not affect the stash message. Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-07stash: allow "git stash [<options>] --patch <pathspec>" to assume pushPhillip Wood
The support for assuming "push" when "-p" is given introduced in 9e140909f61 (stash: allow pathspecs in the no verb form, 2017-02-28) is very narrow, neither "git stash -m <message> -p <pathspec>" nor "git stash --patch <pathspec>" imply "push" and die instead. Relax this by passing PARSE_OPT_STOP_AT_NON_OPTION when push is being assumed and then setting "force_assume" if "--patch" was present. This means "git stash <pathspec> -p" still dies so that it does not assume the user meant "push" if they mistype a subcommand name but "git stash -m <message> -p <pathspec>" will now succeed. The test added in the last commit is adjusted to check that push is still assumed when "--patch" comes after other options on the command-line. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-07stash: allow "git stash -p <pathspec>" to assume push againPhillip Wood
Historically "git stash [<options>]" was assumed to mean "git stash save [<options>]". Since 1ada5020b38 (stash: use stash_push for no verb form, 2017-02-28) it is assumed to mean "git stash push [<options>]". As the push subcommand supports pathspecs, 9e140909f61 (stash: allow pathspecs in the no verb form, 2017-02-28) allowed "git stash -p <pathspec>" to mean "git stash push -p <pathspec>". This was broken in 8c3713cede7 (stash: eliminate crude option parsing, 2020-02-17) which failed to account for "push" being added to the start of argv in cmd_stash() before it calls push_stash() and kept looking in argv[0] for "-p" after moving the code to push_stash(). Fix this by regression by checking argv[1] instead of argv[0] and add a couple of tests to prevent future regressions. Helped-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-17stash: remove merge-recursive.h includeElijah Newren
stash was modified to use merge_ort_nonrecursive() instead of merge_recursive_generic() back in commit 874cf2a60444 (stash: apply stash using 'merge_ort_nonrecursive()', 2022-05-10). That makes the inclusion of merge-recursive.h unnecessary. In preparation for the removal of merge-recursive.h, remove the unnecessary include. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06global: trivial conversions to fix `-Wsign-compare` warningsPatrick Steinhardt
We have a bunch of loops which iterate up to an unsigned boundary using a signed index, which generates warnigs because we compare a signed and unsigned value in the loop condition. Address these sites for trivial cases and enable `-Wsign-compare` warnings for these code units. This patch only adapts those code units where we can drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro in the same step. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt
Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-26builtin: pass repository to sub commandsKarthik Nayak
In 9b1cb5070f (builtin: add a repository parameter for builtin functions, 2024-09-13) the repository was passed down to all builtin commands. This allowed the repository to be passed down to lower layers without depending on the global `the_repository` variable. Continue this work by also passing down the repository parameter from the command to sub-commands. This will help pass down the repository to other subsystems and cleanup usage of global variables like 'the_repository' and 'the_hash_algo'. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-30builtin/stash: fix leaking `pathspec_from_file`Patrick Steinhardt
The `OPT_PATHSPEC_FROM_FILE()` option maps to `OPT_FILENAME()`, which we know will always allocate memory when passed. We never free the memory though, causing a memory leak. Plug it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-23Merge branch 'jc/pass-repo-to-builtins'Junio C Hamano
The convention to calling into built-in command implementation has been updated to pass the repository, if known, together with the prefix value. * jc/pass-repo-to-builtins: add: pass in repo variable instead of global the_repository builtin: remove USE_THE_REPOSITORY for those without the_repository builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.h builtin: add a repository parameter for builtin functions
2024-09-23Merge branch 'ps/environ-wo-the-repository'Junio C Hamano
Code clean-up. * ps/environ-wo-the-repository: (21 commits) environment: stop storing "core.notesRef" globally environment: stop storing "core.warnAmbiguousRefs" globally environment: stop storing "core.preferSymlinkRefs" globally environment: stop storing "core.logAllRefUpdates" globally refs: stop modifying global `log_all_ref_updates` variable branch: stop modifying `log_all_ref_updates` variable repo-settings: track defaults close to `struct repo_settings` repo-settings: split out declarations into a standalone header environment: guard state depending on a repository environment: reorder header to split out `the_repository`-free section environment: move `set_git_dir()` and related into setup layer environment: make `get_git_namespace()` self-contained environment: move object database functions into object layer config: make dependency on repo in `read_early_config()` explicit config: document `read_early_config()` and `read_very_early_config()` environment: make `get_git_work_tree()` accept a repository environment: make `get_graft_file()` accept a repository environment: make `get_index_file()` accept a repository environment: make `get_object_directory()` accept a repository environment: make `get_git_common_dir()` accept a repository ...
2024-09-16Merge branch 'jc/range-diff-lazy-setup'Junio C Hamano
Code clean-up. * jc/range-diff-lazy-setup: remerge-diff: clean up temporary objdir at a central place remerge-diff: lazily prepare temporary objdir on demand
2024-09-13builtin: remove USE_THE_REPOSITORY_VARIABLE from builtin.hJohn Cai
Instead of including USE_THE_REPOSITORY_VARIABLE by default on every builtin, remove it from builtin.h and add it to all the builtins that include builtin.h (by definition, that means all builtins/*.c). Also, remove the include statement for repository.h since it gets brought in through builtin.h. The next step will be to migrate each builtin from having to use the_repository. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-13builtin: add a repository parameter for builtin functionsJohn Cai
In order to reduce the usage of the global the_repository, add a parameter to builtin functions that will get passed a repository variable. This commit uses UNUSED on most of the builtin functions, as subsequent commits will modify the actual builtins to pass the repository parameter down. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: make `get_git_work_tree()` accept a repositoryPatrick Steinhardt
The `get_git_work_tree()` function retrieves the path of the work tree of `the_repository`. Make it accept a `struct repository` such that it can work on arbitrary repositories and make it part of the repository subsystem. This reduces our reliance on `the_repository` and clarifies scope. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: make `get_index_file()` accept a repositoryPatrick Steinhardt
The `get_index_file()` function retrieves the path to the index file of `the_repository`. Make it accept a `struct repository` such that it can work on arbitrary repositories and make it part of the repository subsystem. This reduces our reliance on `the_repository` and clarifies scope. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: make `get_git_dir()` accept a repositoryPatrick Steinhardt
The `get_git_dir()` function retrieves the path to the Git directory for `the_repository`. Make it accept a `struct repository` such that it can work on arbitrary repositories and make it part of the repository subsystem. This reduces our reliance on `the_repository` and clarifies scope. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-05drop trailing newline from warning/error/die messagesJeff King
Our error reporting routines append a trailing newline, and the strings we pass to them should not include them (otherwise we get an extra blank line after the message). These cases were all found by looking at the results of: git grep -P '[^_](error|error_errno|warning|die|die_errno)\(.*\\n"[,)]' '*.c' Note that we _do_ sometimes include a newline in the middle of such messages, to create multiline output (hence our grep matching "," or ")" after we see the newline, so we know we're at the end of the string). It's possible that one or more of these cases could intentionally be including a blank line at the end, but having looked at them all manually, I think these are all just mistakes. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-23Merge branch 'ps/stash-keep-untrack-empty-fix'Junio C Hamano
A corner case bug in "git stash" was fixed. * ps/stash-keep-untrack-empty-fix: builtin/stash: fix `--keep-index --include-untracked` with empty HEAD
2024-08-16builtin/stash: fix `--keep-index --include-untracked` with empty HEADPatrick Steinhardt
It was reported that creating a stash with `--keep-index --include-untracked` causes an error when HEAD points to a commit whose tree is empty: $ git stash push --keep-index --include-untracked error: pathspec ':/' did not match any file(s) known to git This error comes from `git checkout --no-overlay $i_tree -- :/`, which we execute to reset the working tree to the state in our index. As the tree generated from the index is empty in our case, ':/' does not match any files and thus causes git-checkout(1) to error out. Fix the issue by skipping the checkout when the index tree is empty. As explained in the in-code comment, this should be the correct thing to do as there is nothing that we'd have to reset in the first place. Reported-by: Piotr Siupa <piotrsiupa@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>