aboutsummaryrefslogtreecommitdiff
path: root/builtin/add.c
AgeCommit message (Collapse)Author
2026-03-24Merge branch 'ps/history-split'Junio C Hamano
"git history" learned the "split" subcommand. * ps/history-split: builtin/history: implement "split" subcommand builtin/history: split out extended function to create commits cache-tree: allow writing in-memory index as tree add-patch: allow disabling editing of hunks add-patch: add support for in-memory index patching add-patch: remove dependency on "add-interactive" subsystem add-patch: split out `struct interactive_options` add-patch: split out header from "add-interactive.h"
2026-03-09Merge branch 'cs/add-skip-submodule-ignore-all'Junio C Hamano
"git add <submodule>" has been taught to honor submodule.<name>.ignore that is set to "all" (and requires "git add -f" to override it). * cs/add-skip-submodule-ignore-all: Documentation: update add --force option + ignore=all config tests: fix existing tests when add an ignore=all submodule tests: t2206-add-submodule-ignored: ignore=all and add --force tests read-cache: submodule add need --force given ignore=all configuration read-cache: update add_files_to_cache take param ignored_too
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-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-06read-cache: update add_files_to_cache take param ignored_tooClaus Schneider(Eficode)
The ignored_too parameter is added to the function add_files_to_cache for usage of explicit updating the index for the updated submodule using the explicit patchspec to the submodule. Signed-off-by: Claus Schneider(Eficode) <claus.schneider@eficode.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-10-02Merge branch 'jt/odb-transaction'Junio C Hamano
The work to build on the bulk-checkin infrastructure to create many objects at once in a transaction and to abstract it into the generic object layer continues. * jt/odb-transaction: odb: add transaction interface object-file: update naming from bulk-checkin object-file: relocate ODB transaction code bulk-checkin: drop flush_odb_transaction() builtin/update-index: end ODB transaction when --verbose is specified bulk-checkin: remove ODB transaction nesting
2025-09-29Merge branch 'jk/color-variable-fixes'Junio C Hamano
Some places in the code confused a variable that is *not* a boolean to enable color but is an enum that records what the user requested to do about color. A couple of bugs of this sort have been fixed, while the code has been cleaned up to prevent similar bugs in the future. * jk/color-variable-fixes: config: store want_color() result in a separate bool add-interactive: retain colorbool values longer color: return bool from want_color() color: use git_colorbool enum type to store colorbools pretty: use format_commit_context.auto_color as colorbool diff: stop passing ecbdata->use_color as boolean diff: pass o->use_color directly to fill_metainfo() diff: don't use diff_options.use_color as a strict bool diff: simplify color_moved check when flushing grep: don't treat grep_opt.color as a strict bool color: return enum from git_config_colorbool() color: use GIT_COLOR_* instead of numeric constants
2025-09-16color: use GIT_COLOR_* instead of numeric constantsJeff King
Long ago Git's decision to show color for a subsytem was stored in a tri-state variable: it could be true (1), false (0), or unknown (-1). But since daa0c3d971 (color: delay auto-color decision until point of use, 2011-08-17) we want to carry around a new state, "auto", which bases the decision on the tty-ness of stdout (rather than collapsing that "auto" state to a true/false immediately). That commit introduced a set of GIT_COLOR_* defines to represent each state: UNKNOWN, ALWAYS, NEVER, and AUTO. But it only used the AUTO value, and left alone code using bare 0/1/-1 values. And of course since then we've grown many new spots that use those bare values. Let's switch all of these to use the named constants. That should make the code a bit easier to read, as it is more obvious that we're representing a color decision. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-16odb: add transaction interfaceJustin Tobler
Transactions are managed via the {begin,end}_odb_transaction() function in the object-file subsystem and its implementation is specific to the files object source. Introduce odb_transaction_{begin,commit}() in the odb subsystem to provide an eventual object source agnostic means to manage transactions. Update call sites to instead manage transactions through the odb subsystem. Also rename {begin,end}_odb_transaction() functions to object_file_transaction_{begin,commit}() to clarify the object source it supports. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-16object-file: relocate ODB transaction codeJustin Tobler
The bulk-checkin subsystem provides various functions to manage ODB transactions. Apart from {begin,end}_odb_transaction(), these functions are only used by the object-file subsystem to manage aspects of a transaction implementation specific to the files object source. Relocate all the transaction code in bulk-checkin to object-file. This simplifies the exposed transaction interface by reducing it to only {begin,end}_odb_transaction(). Function and type names are adjusted in the subsequent commit to better fit the new location. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-25bulk-checkin: remove global transaction stateJustin Tobler
Object database transactions in the bulk-checkin subsystem rely on global state to track transaction status. Stop relying on global state and instead store the transaction in the `struct object_database`. Functions that operate on transactions are updated to now wire transaction state. Signed-off-by: Justin Tobler <jltobler@gmail.com> 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: move Git config parsing into "environment.c"Patrick Steinhardt
In "config.c" we host both the business logic to read and write config files as well as the logic to parse specific Git-related variables. On the one hand this is mixing concerns, but even more importantly it means that we cannot easily remove the dependency on `the_repository` in our config parsing logic. Move the logic into "environment.c". This file is a grab bag of all kinds of global state already, so it is quite a good fit. Furthermore, it also hosts most of the global variables that we're parsing the config values into, making this an even better fit. Note that there is one hidden change: in `parse_fsync_components()` we use an `int` to iterate through `ARRAY_SIZE(fsync_component_names)`. But as -Wsign-compare warnings are enabled in this file this causes a compiler warning. The issue is fixed by using a `size_t` instead. This change allows us to drop the `USE_THE_REPOSITORY_VARIABLE` declaration. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-27Merge branch 'ds/sparse-apply-add-p'Junio C Hamano
"git apply" and "git add -i/-p" code paths no longer unnecessarily expand sparse-index while working. * ds/sparse-apply-add-p: p2000: add performance test for patch-mode commands reset: integrate sparse index with --patch git add: make -p/-i aware of sparse index apply: integrate with the sparse index
2025-05-16git add: make -p/-i aware of sparse indexDerrick Stolee
It is slow to expand a sparse index in-memory due to parsing of trees. We aim to minimize that performance cost when possible. 'git add -p' uses 'git apply' child processes to modify the index, but still there are some expansions that occur. It turns out that control flows out of cmd_add() in the interactive cases before the lines that confirm that the builtin is integrated with the sparse index. Moving that integration point earlier in cmd_add() allows 'git add -i' and 'git add -p' to operate without expanding a sparse index to a full one. Add test cases that confirm that these interactive add options work with the sparse index. Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-04-20builtin/add: remove unnecessary if statementUsman Akinyemi
Since we already teach the `repo_config()` in "f29f1990b5 (config: teach repo_config to allow `repo` to be NULL, 2025-03-08)" to allow `repo` to be NULL, no need to check if `repo` is NULL before calling `repo_config()`. Suggested-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Usman Akinyemi <usmanakinyemi202@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-10-11git: pass in repo to builtin based on setup_git_directory_gentlyJohn Cai
The current code in run_builtin() passes in a repository to the builtin based on whether cmd_struct's option flag has RUN_SETUP. This is incorrect, however, since some builtins that only have RUN_SETUP_GENTLY can potentially take a repository. setup_git_directory_gently() tells us whether or not a command is being run inside of a repository. Use the output of setup_git_directory_gently() to help determine whether or not there is a repository to pass to the builtin. If not, then we just pass NULL. As part of this patch, we need to modify add to check for a NULL repo before calling repo_git_config(), since add -h can be run outside of a repository. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-13add: pass in repo variable instead of global the_repositoryJohn Cai
With the repository variable available in the builtin function as an argument, pass this down into helper functions instead of using the global the_repository. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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-06-05add-i: finally retire add.interactive.useBuiltinJunio C Hamano
The configuration variable stopped doing anything (other than announcing itself as a variable that does not do anything useful, when it is used) in Git 2.40. At this point, it is not even worth giving the warning, which was meant to be a way to help users notice they are carrying unused cruft in their configuration files and give them a chance to clean-up. Let's remove the warning and documentation for it, and truly stop paying attention to it. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/config/add.txt | 6 ------ builtin/add.c | 6 +----- t/t3701-add-interactive.sh | 15 --------------- 3 files changed, 1 insertion(+), 26 deletions(-)
2024-05-08Merge branch 'ps/the-index-is-no-more'Junio C Hamano
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-04-22add: plug a leak on interactive_addRubén Justo
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-18builtin: stop using `the_index`Patrick Steinhardt
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-15Merge branch 'gt/add-u-commit-i-pathspec-check'Junio C Hamano
"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-09Merge branch 'rj/use-adv-if-enabled'Junio C Hamano
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-03builtin/add: error out when passing untracked path with -uGhanshyam Thakkar
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-03revision: optionally record matches with pathspec elementsJunio C Hamano
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-03-30add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPORubén Justo
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 Justo
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 Justo
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-07Merge branch 'eg/add-uflags'Junio C Hamano
Code clean-up practice. * eg/add-uflags: add: use unsigned type for collection of bits
2024-02-29add: use unsigned type for collection of bitsEugenio Gigante
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-01-08Merge branch 'en/header-cleanup'Junio C Hamano
Remove unused header "#include". * en/header-cleanup: treewide: remove unnecessary includes in source files treewide: add direct includes currently only pulled in transitively trace2/tr2_tls.h: remove unnecessary include submodule-config.h: remove unnecessary include pkt-line.h: remove unnecessary include line-log.h: remove unnecessary include http.h: remove unnecessary include fsmonitor--daemon.h: remove unnecessary includes blame.h: remove unnecessary includes archive.h: remove unnecessary include treewide: remove unnecessary includes in source files treewide: remove unnecessary includes from header files
2023-12-26treewide: remove unnecessary includes in source filesElijah Newren
Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-04attr: enable attr pathspec magic for git-add and git-stashJoanna Wang
Allow users to limit or exclude files based on file attributes during git-add and git-stash. For example, the chromium project would like to use $ git add . ':(exclude,attr:submodule)' as submodules are managed by an external tool, forbidding end users to record changes with "git add". Allowing "git add" to often records changes that users do not want in their commits. This commit does not change any attr magic implementation. It is only adding attr as an allowed pathspec in git-add and git-stash, which was previously blocked by GUARD_PATHSPEC and a pathspec mask in parse_pathspec()). However, we fix a bug in prefix_magic() where attr values were unintentionally removed. This was triggerable when parse_pathspec() is called with PATHSPEC_PREFIX_ORIGIN as a flag, which was the case for git-stash (Bug originally filed here [*]) Furthermore, while other commands hit this code path it did not result in unexpected behavior because this bug only impacts the pathspec->items->original field which is NOT used to filter paths. However, git-stash does use pathspec->items->original when building args used to call other git commands. (See add_pathspecs() usage and implementation in stash.c) It is possible that when the attr pathspec feature was first added in b0db704652 (pathspec: allow querying for attributes, 2017-03-13), "PATHSPEC_ATTR" was just unintentionally left out of a few GUARD_PATHSPEC() invocations. Later, to get a more user-friendly error message when attr was used with git-add, PATHSPEC_ATTR was added as a mask to git-add's invocation of parse_pathspec() 84d938b732 (add: do not accept pathspec magic 'attr', 2018-09-18). However, this user-friendly error message was never added for git-stash. [Reference] * https://lore.kernel.org/git/CAMmZTi-0QKtj7Q=sbC5qhipGsQxJFOY-Qkk1jfkRYwfF5FcUVg@mail.gmail.com/) Signed-off-by: Joanna Wang <jojwang@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-30Merge branch 'ni/die-message-fix-for-git-add'Junio C Hamano
Message updates. * ni/die-message-fix-for-git-add: builtin/add.c: clean up die() messages
2023-10-17builtin/add.c: clean up die() messagesNaomi Ibe
As described in the CodingGuidelines document, a single line message given to die() and its friends should not capitalize its first word, and should not add full-stop at the end. Signed-off-by: Naomi Ibe <naomi.ibeh69@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-13Merge branch 'jk/unused-post-2.42-part2'Junio C Hamano
Unused parameters to functions are marked as such, and/or removed, in order to bring us closer to -Wunused-parameter clean. * jk/unused-post-2.42-part2: parse-options: mark unused parameters in noop callback interpret-trailers: mark unused "unset" parameters in option callbacks parse-options: add more BUG_ON() annotations merge: do not pass unused opt->value parameter parse-options: mark unused "opt" parameter in callbacks parse-options: prefer opt->value to globals in callbacks checkout-index: delay automatic setting of to_tempfile format-patch: use OPT_STRING_LIST for to/cc options merge: simplify parsing of "-n" option merge: make xopts a strvec
2023-09-05parse-options: add more BUG_ON() annotationsJeff King
These callbacks are similar to the ones touched by 517fe807d6 (assert NOARG/NONEG behavior of parse-options callbacks, 2018-11-05), but were either missed in that commit (the one in add.c) or were added later (the one in log.c). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-21diff: drop useless return from run_diff_{files,index} functionsJeff King
Neither of these functions ever returns a value other than zero. Instead, they expect unrecoverable errors to exit immediately, and things like "--exit-code" are stored inside the diff_options struct to be handled later via diff_result_code(). Some callers do check the return values, but many don't bother. Let's drop the useless return values, which are misleading callers about how the functions work. This could be seen as a step in the wrong direction, as we might want to eventually "lib-ify" these to more cleanly return errors up the stack, in which case we'd have to add the return values back in. But there are some benefits to doing this now: 1. In the current code, somebody could accidentally add a "return -1" to one of the functions, which would be erroneously ignored by many callers. By removing the return code, the compiler can notice the mismatch and force the developer to decide what to do. Obviously the other option here is that we could start consistently checking the error code in every caller. But it would be dead code, and we wouldn't get any compile-time help in catching new cases. 2. It communicates the situation to callers, who may want to choose a different function. These functions are really thin wrappers for doing git-diff-files and git-diff-index within the process. But callers who care about recovering from an error here are probably better off using the underlying library functions, many of which do return errors. If somebody eventually wants to teach these functions to propagate errors, they'll have to switch back to returning a value, effectively reverting this patch. But at least then they will be starting with a level playing field: they know that they will need to inspect each caller to see how it should handle the error. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-06Merge branch 'gc/config-context'Junio C Hamano
Reduce reliance on a global state in the config reading API. * gc/config-context: config: pass source to config_parser_event_fn_t config: add kvi.path, use it to evaluate includes config.c: remove config_reader from configsets config: pass kvi to die_bad_number() trace2: plumb config kvi config.c: pass ctx with CLI config config: pass ctx with config files config.c: pass ctx in configsets config: add ctx arg to config_fn_t urlmatch.h: use config_fn_t type config: inline git_color_default_config
2023-06-29Merge branch 'en/header-split-cache-h-part-3'Junio C Hamano
Header files cleanup. * en/header-split-cache-h-part-3: (28 commits) fsmonitor-ll.h: split this header out of fsmonitor.h hash-ll, hashmap: move oidhash() to hash-ll object-store-ll.h: split this header out of object-store.h khash: name the structs that khash declares merge-ll: rename from ll-merge git-compat-util.h: remove unneccessary include of wildmatch.h builtin.h: remove unneccessary includes list-objects-filter-options.h: remove unneccessary include diff.h: remove unnecessary include of oidset.h repository: remove unnecessary include of path.h log-tree: replace include of revision.h with simple forward declaration cache.h: remove this no-longer-used header read-cache*.h: move declarations for read-cache.c functions from cache.h repository.h: move declaration of the_index from cache.h merge.h: move declarations for merge.c from cache.h diff.h: move declaration for global in diff.c from cache.h preload-index.h: move declarations for preload-index.c from elsewhere sparse-index.h: move declarations for sparse-index.c from cache.h name-hash.h: move declarations for name-hash.c from cache.h run-command.h: move declarations for run-command.c from cache.h ...
2023-06-28config: add ctx arg to config_fn_tGlen Choo
Add a new "const struct config_context *ctx" arg to config_fn_t to hold additional information about the config iteration operation. config_context has a "struct key_value_info kvi" member that holds metadata about the config source being read (e.g. what kind of config source it is, the filename, etc). In this series, we're only interested in .kvi, so we could have just used "struct key_value_info" as an arg, but config_context makes it possible to add/adjust members in the future without changing the config_fn_t signature. We could also consider other ways of organizing the args (e.g. moving the config name and value into config_context or key_value_info), but in my experiments, the incremental benefit doesn't justify the added complexity (e.g. a config_fn_t will sometimes invoke another config_fn_t but with a different config value). In subsequent commits, the .kvi member will replace the global "struct config_reader" in config.c, making config iteration a global-free operation. It requires much more work for the machinery to provide meaningful values of .kvi, so for now, merely change the signature and call sites, pass NULL as a placeholder value, and don't rely on the arg in any meaningful way. Most of the changes are performed by contrib/coccinelle/config_fn_ctx.pending.cocci, which, for every config_fn_t: - Modifies the signature to accept "const struct config_context *ctx" - Passes "ctx" to any inner config_fn_t, if needed - Adds UNUSED attributes to "ctx", if needed Most config_fn_t instances are easily identified by seeing if they are called by the various config functions. Most of the remaining ones are manually named in the .cocci patch. Manual cleanups are still needed, but the majority of it is trivial; it's either adjusting config_fn_t that the .cocci patch didn't catch, or adding forward declarations of "struct config_context ctx" to make the signatures make sense. The non-trivial changes are in cases where we are invoking a config_fn_t outside of config machinery, and we now need to decide what value of "ctx" to pass. These cases are: - trace2/tr2_cfg.c:tr2_cfg_set_fl() This is indirectly called by git_config_set() so that the trace2 machinery can notice the new config values and update its settings using the tr2 config parsing function, i.e. tr2_cfg_cb(). - builtin/checkout.c:checkout_main() This calls git_xmerge_config() as a shorthand for parsing a CLI arg. This might be worth refactoring away in the future, since git_xmerge_config() can call git_default_config(), which can do much more than just parsing. Handle them by creating a KVI_INIT macro that initializes "struct key_value_info" to a reasonable default, and use that to construct the "ctx" arg. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: inline git_color_default_configGlen Choo
git_color_default_config() is a shorthand for calling two other config callbacks. There are no other non-static functions that do this and it will complicate our refactoring of config_fn_t so inline it instead. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-22Merge branch 'ds/add-i-color-configuration-fix'Junio C Hamano
The reimplemented "git add -i" did not honor color.ui configuration. * ds/add-i-color-configuration-fix: add: test use of brackets when color is disabled add: check color.ui for interactive add