aboutsummaryrefslogtreecommitdiffstats
path: root/builtin/stash.c (follow)
AgeCommit message (Collapse)AuthorFilesLines
2025-09-29Merge branch 'dk/stash-apply-index'Junio C Hamano1-6/+11
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 Hamano1-2/+2
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 Hamano1-9/+5
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 Hamano1-1/+4
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 King1-3/+2
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 King1-1/+2
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 Knoble1-2/+7
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 Knoble1-4/+4
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é Scharfe1-9/+5
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 King1-1/+4
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 Hamano1-2/+4
"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 Steinhardt1-2/+4
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 Hamano1-11/+45
"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 Michalak1-11/+45
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 Steinhardt1-2/+2
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 Hamano1-11/+449
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 Hamano1-2/+8
"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. carlson1-0/+167
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. carlson1-0/+260
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. carlson1-11/+22
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 Jayatheerth1-2/+8
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 Wood1-3/+7
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 Wood1-1/+1
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 Newren1-1/+0
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 Steinhardt1-5/+2
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 Steinhardt1-0/+2
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 Nayak1-14/+25
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 Steinhardt1-1/+3
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 Hamano1-1/+5
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 Hamano1-8/+9
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 Hamano1-3/+3
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 Cai1-0/+1
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 Cai1-1/+4
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 Steinhardt1-1/+1
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 Steinhardt1-6/+6
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 Steinhardt1-1/+2
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 King1-1/+1
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 Hamano1-1/+22
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 Steinhardt1-1/+22
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>
2024-08-14Merge branch 'ps/leakfixes-part-3'Junio C Hamano1-2/+16
More leakfixes. * ps/leakfixes-part-3: (24 commits) commit-reach: fix trivial memory leak when computing reachability convert: fix leaking config strings entry: fix leaking pathnames during delayed checkout object-name: fix leaking commit list items t/test-repository: fix leaking repository builtin/credential-cache: fix trivial leaks builtin/worktree: fix leaking derived branch names builtin/shortlog: fix various trivial memory leaks builtin/rerere: fix various trivial memory leaks builtin/credential-store: fix leaking credential builtin/show-branch: fix several memory leaks builtin/rev-parse: fix memory leak with `--parseopt` builtin/stash: fix various trivial memory leaks builtin/remote: fix various trivial memory leaks builtin/remote: fix leaking strings in `branch_list` builtin/ls-remote: fix leaking `pattern` strings builtin/submodule--helper: fix leaking buffer in `is_tip_reachable` builtin/submodule--helper: fix leaking clone depth parameter builtin/name-rev: fix various trivial memory leaks builtin/describe: fix trivial memory leak when describing blob ...
2024-08-09remerge-diff: clean up temporary objdir at a central placeJunio C Hamano1-3/+3
After running a diff between two things, or a series of diffs while walking the history, the diff computation is concluded by a call to diff_result_code() to extract the exit status of the diff machinery. The function can work on "struct diffopt", but all the callers historically and currently pass "struct diffopt" that is embedded in the "struct rev_info" that is used to hold the remerge_diff bit and the remerge_objdir variable that points at the temporary object directory in use. Redefine diff_result_code() to take the whole "struct rev_info" to give it an access to these members related to remerge-diff, so that it can get rid of the temporary object directory for any and all callers that used the feature. We can lose the equivalent code to do so from the code paths for individual commands, diff-tree, diff, and log. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-01builtin/stash: fix various trivial memory leaksPatrick Steinhardt1-2/+16
There are multiple trivial memory leaks in git-stash(1). Fix those. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13merge-recursive: honor diff.algorithmAntonin Delpeuch1-1/+1
The documentation claims that "recursive defaults to the diff.algorithm config setting", but this is currently not the case. This fixes it, ensuring that diff.algorithm is used when -Xdiff-algorithm is not supplied. This affects the following porcelain commands: "merge", "rebase", "cherry-pick", "pull", "stash", "log", "am" and "checkout". It also affects the "merge-tree" ancillary interrogator. This change refactors the initialization of merge options to introduce two functions, "init_merge_ui_options" and "init_merge_basic_options" instead of just one "init_merge_options". This design follows the approach used in diff.c, providing initialization methods for porcelain and plumbing commands respectively. Thanks to that, the "replay" and "merge-recursive" plumbing commands remain unaffected by diff.algorithm. Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11commit: fix leaking parents when calling `commit_tree_extended()`Patrick Steinhardt1-5/+4
When creating commits via `commit_tree_extended()`, the caller passes in a string list of parents. This call implicitly transfers ownership of that list to the function, which is quite surprising to begin with. But to make matters worse, `commit_tree_extended()` doesn't even bother to free the list of parents in error cases. The result is a memory leak, and one that the caller cannot fix by themselves because they do not know whether parts of the string list have already been released. Refactor the code such that callers can keep ownership of the list of parents, which is getting indicated by parameter being a constant pointer now. Free the lists at the calling site and add a common exit path to those sites as required. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11builtin/stash: fix leak in `show_stash()`Patrick Steinhardt1-0/+2
We leak the `revision_args()` variable. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11object-name: free leaking object contextsPatrick Steinhardt1-3/+9
While it is documented in `struct object_context::path` that this variable needs to be released by the caller, this fact is rather easy to miss given that we do not ever provide a function to release the object context. And of course, while some callers dutifully release the path, many others don't. Introduce a new `object_context_release()` function that releases the path. Convert callsites that used to free the path to use that new function and add missing calls to callsites that were leaking memory. Refactor those callsites as required to have a single return path, only. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-16Merge branch 'ps/refs-without-the-repository'Junio C Hamano1-10/+13
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`
2024-05-08Merge branch 'ps/the-index-is-no-more'Junio C Hamano1-23/+22
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-07cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt1-10/+13
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-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