aboutsummaryrefslogtreecommitdiffstats
path: root/config.c
AgeCommit message (Collapse)AuthorFilesLines
13 daysMerge branch 'jc/optional-path'Junio C Hamano1-1/+2
"git config get --path" segfaulted on an ":(optional)path" that does not exist, which has been corrected. * jc/optional-path: config: really treat missing optional path as not configured config: really pretend missing :(optional) value is not there config: mark otherwise unused function as file-scope static
2025-11-24config: really pretend missing :(optional) value is not thereJunio C Hamano1-0/+1
Earlier we added support for a value spelled as ":(optional)path" for configuration variables whose values are of type "path", with the documented semantics "if the path is missing, behave as if such a variable definition is not even there." This has worked OK for code paths that reads configuration files and stores the configured value as a string, where NULL in such a string is treated as if the setting is not there, left as the default. However, there are other code paths that do not _ignore_ such NULL values and misbehave. "git config get --path" is one of them. When git_config_pathname() helper function finds that the value of the variable is an optional path *and* the path is missing, it leaves the destination pointer intact (which usually is left to NULL) and returns 0 to signal a success. format_config() helper however assumed that the destination pointer always gets a string, which no longer is the case, and segfaulted. Make sure that git_config_pathname() clears the destination pointer in such a case, and teach format_config() to react to the condition by returning 1 (which is different from 0 that is a normal success and negative that is an error) to its callers. Adjust the callers to react to this new return value that tells them to pretend as if they did not even see this partcular <key, value> pair. Reported-by: Han Jiang <jhcarl0814@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-20config: mark otherwise unused function as file-scope staticJunio C Hamano1-1/+1
git_configset_get_pathname() is only used once inside config.c; we do not have to expose it as a public function. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-06Merge branch 'dk/parseopt-optional-filename-fixes'Junio C Hamano1-1/+1
A recently added configuration variable and command line option syntax ":(optional)" for values that are of filename type inconsistently behaved on an empty file (configuration took it happily, while the command line option pretended as if it did not exist), which has been corrected. * dk/parseopt-optional-filename-fixes: parseopt: remove unreachable code parseopt: restore const qualifier to parsed filename config: use boolean type for a simple flag parseopt: use boolean type for a simple flag doc: clarify command equivalence comment parseopt: fix :(optional) at command line to only ignore missing files
2025-11-04config: use boolean type for a simple flagD. Ben Knoble1-1/+1
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-10-14Merge branch 'jc/optional-path'Junio C Hamano1-2/+14
Configuration variables that take a pathname as a value (e.g. blame.ignorerevsfile) can be marked as optional by prefixing ":(optoinal)" before its value. * jc/optional-path: parseopt: values of pathname type can be prefixed with :(optional) config: values of pathname type can be prefixed with :(optional) t7500: fix GIT_EDITOR shell snippet t7500: make each piece more independent
2025-10-07config: values of pathname type can be prefixed with :(optional)Junio C Hamano1-2/+14
Sometimes people want to specify additional configuration data as "best effort" basis. Maybe commit.template configuration file points at somewhere in ~/template/ but on a particular system, the file may not exist and the user may be OK without using the template in such a case. When the value given to a configuration variable whose type is pathname wants to signal such an optional file, it can be marked by prepending ":(optional)" in front of it. Such a setting that is marked optional would avoid getting the command barf for a missing file, as an optional configuration setting that names a missing file is not even seen. cf. <xmqq5ywehb69.fsf@gitster.g> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-18Merge branch 'pw/3.0-commentchar-auto-deprecation'Junio C Hamano1-2/+295
"core.commentChar=auto" that attempts to dynamically pick a suitable comment character is non-workable, as it is too much trouble to support for little benefit, and is marked as deprecated. * pw/3.0-commentchar-auto-deprecation: commit: print advice when core.commentString=auto config: warn on core.commentString=auto breaking-changes: deprecate support for core.commentString=auto
2025-08-26commit: print advice when core.commentString=autoPhillip Wood1-6/+188
Add some advice on how to change the config settings when "core.commentString=auto" or "core.commentChar=auto". The advice includes instructions for clearing the config setting or setting a fixed comment string. To try and be as specific as possible, the advice is customized based on the user's config. If "core.commentString=auto" is set in the system config and the user does not have write access then the advice omits the instructions to clear the config and recommends changing the global config instead. An alternative approach would be to advise the user to run "git config --show-origin" and leave them to figure out how to fix it themselves but that seems rather unfriendly. As we're forcing them to update their config we should try and make that as easy as possible. In order to generate this advice we need to record each file where either of the config keys is set and whether a key occurs more that once in a given file. This lets us generate the list of commands to remove all the keys and also tells us which key the "auto" setting comes from. As we want the user to update their config we do not provide a way for this advice to be disabled other than changing the value of "core.commentChar" or "core.commentString". Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-26config: warn on core.commentString=autoPhillip Wood1-2/+113
As support for this setting was deprecated in the last commit print a warning (or die when WITH_BREAKING_CHANGES is enabled) if it is set. Avoid bombarding the user with warnings by only printing it (a) when running commands that call "git commit" and (b) only once per command. Some scaffolding is added to repo_read_config() to allow it to detect deprecated config settings and warn about them. As both "core.commentChar" and "core.commentString" set the comment character we record which one of them is used and tailor the warning message appropriately. Note the odd combination of die_message() followed by die(NULL) is to allow the next commit to insert a call to advise() in the middle. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-21Merge branch 'jc/strbuf-split'Junio C Hamano1-13/+10
Arrays of strbuf is often a wrong data structure to use, and strbuf_split*() family of functions that create them often have better alternatives. Update several code paths and replace strbuf_split*(). * jc/strbuf-split: trace2: do not use strbuf_split*() trace2: trim_trailing_newline followed by trim is a no-op sub-process: do not use strbuf_split*() environment: do not use strbuf_split*() config: do not use strbuf_split() notes: do not use strbuf_split*() merge-tree: do not use strbuf_split*() clean: do not use strbuf_split*() [part 2] clean: do not pass the whole structure when it is not necessary clean: do not use strbuf_split*() [part 1] clean: do not pass strbuf by value wt-status: avoid strbuf_split*()
2025-08-04Merge branch 'ps/config-wo-the-repository'Junio C Hamano1-533/+22
The config API had a set of convenience wrapper functions that implicitly use the_repository instance; they have been removed and inlined at the calling sites. * ps/config-wo-the-repository: (21 commits) config: fix sign comparison warnings config: move Git config parsing into "environment.c" config: remove unused `the_repository` wrappers config: drop `git_config_set_multivar()` wrapper config: drop `git_config_get_multivar_gently()` wrapper config: drop `git_config_set_multivar_in_file_gently()` wrapper config: drop `git_config_set_in_file_gently()` wrapper config: drop `git_config_set()` wrapper config: drop `git_config_set_gently()` wrapper config: drop `git_config_set_in_file()` wrapper config: drop `git_config_get_bool()` wrapper config: drop `git_config_get_ulong()` wrapper config: drop `git_config_get_int()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string_multi()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get()` wrapper config: drop `git_config_clear()` wrapper ...
2025-08-02config: do not use strbuf_split()Junio C Hamano1-13/+10
When parsing an old-style GIT_CONFIG_PARAMETERS environment variable, the code parses key=value pairs by splitting them at '=' into an array of strbuf's. As strbuf_split() leaves the delimiter at the end of the split piece, the code has to manually trim it. If we split with string_list_split(), that becomes unnecessary. Retire the use of strbuf_split() from this code path. Note that the max parameter of string_list_split() is of an ergonomically iffy design---it specifies the maximum number of times the function is allowed to split, which means that in order to split a text into up to 2 pieces, you have to pass 1, not 2. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-31Merge branch 'ps/config-wo-the-repository' into ↵Junio C Hamano1-533/+22
pw/3.0-commentchar-auto-deprecation * ps/config-wo-the-repository: (21 commits) config: fix sign comparison warnings config: move Git config parsing into "environment.c" config: remove unused `the_repository` wrappers config: drop `git_config_set_multivar()` wrapper config: drop `git_config_get_multivar_gently()` wrapper config: drop `git_config_set_multivar_in_file_gently()` wrapper config: drop `git_config_set_in_file_gently()` wrapper config: drop `git_config_set()` wrapper config: drop `git_config_set_gently()` wrapper config: drop `git_config_set_in_file()` wrapper config: drop `git_config_get_bool()` wrapper config: drop `git_config_get_ulong()` wrapper config: drop `git_config_get_int()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string_multi()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get()` wrapper config: drop `git_config_clear()` wrapper ...
2025-07-28Merge branch 'ac/auto-comment-char-fix'Junio C Hamano1-2/+4
"git commit" that concludes a conflicted merge failed to notice and remove existing comment added automatically (like "# Conflicts:") when the core.commentstring is set to 'auto'. * ac/auto-comment-char-fix: config: set comment_line_str to "#" when core.commentChar=auto commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
2025-07-24Merge branch 'pw/config-kvi-remove-path'Junio C Hamano1-15/+13
Remove a redundant member from kvi struct. * pw/config-kvi-remove-path: config: remove unneeded struct field
2025-07-23config: fix sign comparison warningsPatrick Steinhardt1-19/+21
There are a couple of -Wsign-compare warnings in "config.c": - `prepare_include_condition_pattern()` is returns a signed integer, where it either returns a negative error code or the index of the last dir separator in a path. That index will always be a non-negative number, but we cannot just change the return type to a `size_t` due to it being re-used as error code. This is fixed by splitting up concerns: the return value is only used as error code, and the prefix is now returned via an out-pointer. This fixes a sign comparison warning when comparing `text.len < prefix`, - We treat `struct config_store_data::seen` as signed integer in several places even though it's unsigned. - There are multiple trivial sign comparison warnings where we use a signed loop index to iterate through an unsigned number of items. Fix all of these issues and drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-23config: move Git config parsing into "environment.c"Patrick Steinhardt1-511/+0
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-07-23config: drop `git_config_set()` wrapperPatrick Steinhardt1-1/+1
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_set()`. All callsites are adjusted so that they use `repo_config_set(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-07-16config: set comment_line_str to "#" when core.commentChar=autoAyush Chandekar1-2/+4
If conflict comments already use a comment character that isn't "#", and core.commentChar is set "auto", Git will ignore these lines during the scan using ignored_log_message_bytes() and pick a new comment character based on the rest of the message. The newly chosen character may be different from the one used in the conflict comments and therefore, these are no longer treated as comments and end up in the final commit message. For example, during a rebase if the user previously set core.commentChar=% and then encounters a conflict, conflict comments like "% Conflicts:" are generated. If the user subsequently sets core.commentChar=auto before running `rebase --continue`, Git parses the "auto" setting and begins scanning. It first uses the existing 'comment_line_str' (which is '%') to detect and ignore conflict comments via ignored_log_message_bytes(). Then, Git scans the rest of the message (excluding conflict comments), sees that none of the remaining lines start with '#' and decides to set comment_line_str to '#'. Since the final commit character differs from the one used in the conflict comments, those lines are no longer considered comments and get included in the final commit message. Set 'comment_line_str' to '#' when core.commentChar is set to 'auto' to reset any previously set value. While this does not solve the issue of conflict comment inclusion and the user visible behaviour stays tha same, it standardizes the behaviour of the code by always resetting 'comment_line_str' to '#' when core.commentChar=auto is parsed. The patch text is based on Phillip Wood's message: https://lore.kernel.org/git/9e96aaab-79a2-4632-94cd-d016d4a63b30@gmail.com/ and the commit log message is wriiten by me. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-15Merge branch 'ps/object-store'Junio C Hamano1-2/+2
Code clean-up around object access API. * ps/object-store: odb: rename `read_object_with_reference()` odb: rename `pretend_object_file()` odb: rename `has_object()` odb: rename `repo_read_object_file()` odb: rename `oid_object_info()` odb: trivial refactorings to get rid of `the_repository` odb: get rid of `the_repository` when handling submodule sources odb: get rid of `the_repository` when handling the primary source odb: get rid of `the_repository` in `for_each()` functions odb: get rid of `the_repository` when handling alternates odb: get rid of `the_repository` in `odb_mkstemp()` odb: get rid of `the_repository` in `assert_oid_type()` odb: get rid of `the_repository` in `find_odb()` odb: introduce parent pointers object-store: rename files to "odb.{c,h}" object-store: rename `object_directory` to `odb_source` object-store: rename `raw_object_store` to `object_database`
2025-07-15config: remove unneeded struct fieldPhillip Wood1-15/+13
As well as receiving the config key and value, config callbacks also receive a "struct key_value_info" containing information about the source of the key-value pair. Accessing the "path" field of this struct from a callback passed to repo_config() results in a use-after-free. This happens because repo_config() first populates a configset by calling config_with_options() and then iterates over the configset with the callback passed by the caller. When the configset is constructed it takes a shallow copy of the "struct key_value_info" for each config setting. This leads to the use-after-free as the "path" member is freed before config_with_options() returns. We could fix this by interning the "path" field as we do for the "filename" field but the "path" field is not actually needed. It is populated with a copy of the "path" field from "struct config_source". That field was added in d14d42440d8 (config: disallow relative include paths from blobs, 2014-02-19) to distinguish between relative include directives in files and those in blobs. However, since 1b8132d99d8 (i18n: config: unfold error messages marked for translation, 2016-07-28) we can differentiate these by looking at the "origin_type" field in "struct key_value_info". So let's remove the "path" members from "struct config_source" and "struct key_value_info" and instead use a combination of the "filename" and "origin_type" fields to determine the absolute path of relative includes. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-07Sync with Git 2.50.1Junio C Hamano1-1/+1
2025-07-01odb: rename `repo_read_object_file()`Patrick Steinhardt1-1/+1
Rename `repo_read_object_file()` to `odb_read_object()` to match other functions related to the object database and our modern coding guidelines. Introduce a compatibility wrapper so that any in-flight topics will continue to compile. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-01object-store: rename files to "odb.{c,h}"Patrick Steinhardt1-1/+1
In the preceding commits we have renamed the structures contained in "object-store.h" to `struct object_database` and `struct odb_backend`. As such, the code files "object-store.{c,h}" are confusingly named now. Rename them to "odb.{c,h}" accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-15Sync with 2.49.1Junio C Hamano1-1/+1
2025-06-12Sync with 2.48.2Junio C Hamano1-1/+1
* maint-2.48: Git 2.48.2 Git 2.47.3 Git 2.46.4 Git 2.45.4 Git 2.44.4 Git 2.43.7 wincred: avoid buffer overflow in wcsncat() bundle-uri: fix arbitrary file writes via parameter injection config: quote values containing CR character git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: override exec and open only on Windows gitk: sanitize 'open' arguments: revisit recently updated 'open' calls git-gui: use git_read in githook_read git-gui: sanitize $PATH on all platforms git-gui: break out a separate function git_read_nice git-gui: assure PATH has only absolute elements. git-gui: remove option --stderr from git_read git-gui: cleanup git-bash menu item git-gui: sanitize 'exec' arguments: background git-gui: avoid auto_execok in do_windows_shortcut git-gui: sanitize 'exec' arguments: simple cases git-gui: avoid auto_execok for git-bash menu item git-gui: treat file names beginning with "|" as relative paths git-gui: remove unused proc is_shellscript git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove special treatment of Windows from open_cmd_pipe git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: use only the configured shell git-gui: remove Tcl 8.4 workaround on 2>@1 redirection git-gui: make _shellpath usable on startup git-gui: use [is_Windows], not bad _shellpath git-gui: _which, only add .exe suffix if not present gitk: encode arguments correctly with "open" gitk: sanitize 'open' arguments: command pipeline gitk: collect construction of blameargs into a single conditional gitk: sanitize 'open' arguments: simple commands, readable and writable gitk: sanitize 'open' arguments: simple commands with redirections gitk: sanitize 'open' arguments: simple commands gitk: sanitize 'exec' arguments: redirect to process gitk: sanitize 'exec' arguments: redirections and background gitk: sanitize 'exec' arguments: redirections gitk: sanitize 'exec' arguments: 'eval exec' gitk: sanitize 'exec' arguments: simple cases gitk: have callers of diffcmd supply pipe symbol when necessary gitk: treat file names beginning with "|" as relative paths Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-10environment: remove the global variable 'core_preload_index'Ayush Chandekar1-5/+0
The global variable 'core_preload_index' is used in a single function named 'preload_index()' in "preload-index.c". Move its declaration inside that function, removing unnecessary global state. This change is part of an ongoing effort to eliminate global variables, improve modularity and help libify the codebase. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-28Sync with 2.47.3Taylor Blau1-1/+1
* maint-2.47: Git 2.47.3 Git 2.46.4 Git 2.45.4 Git 2.44.4 Git 2.43.7 wincred: avoid buffer overflow in wcsncat() bundle-uri: fix arbitrary file writes via parameter injection config: quote values containing CR character git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: override exec and open only on Windows gitk: sanitize 'open' arguments: revisit recently updated 'open' calls git-gui: use git_read in githook_read git-gui: sanitize $PATH on all platforms git-gui: break out a separate function git_read_nice git-gui: assure PATH has only absolute elements. git-gui: remove option --stderr from git_read git-gui: cleanup git-bash menu item git-gui: sanitize 'exec' arguments: background git-gui: avoid auto_execok in do_windows_shortcut git-gui: sanitize 'exec' arguments: simple cases git-gui: avoid auto_execok for git-bash menu item git-gui: treat file names beginning with "|" as relative paths git-gui: remove unused proc is_shellscript git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove special treatment of Windows from open_cmd_pipe git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: use only the configured shell git-gui: remove Tcl 8.4 workaround on 2>@1 redirection git-gui: make _shellpath usable on startup git-gui: use [is_Windows], not bad _shellpath git-gui: _which, only add .exe suffix if not present gitk: encode arguments correctly with "open" gitk: sanitize 'open' arguments: command pipeline gitk: collect construction of blameargs into a single conditional gitk: sanitize 'open' arguments: simple commands, readable and writable gitk: sanitize 'open' arguments: simple commands with redirections gitk: sanitize 'open' arguments: simple commands gitk: sanitize 'exec' arguments: redirect to process gitk: sanitize 'exec' arguments: redirections and background gitk: sanitize 'exec' arguments: redirections gitk: sanitize 'exec' arguments: 'eval exec' gitk: sanitize 'exec' arguments: simple cases gitk: have callers of diffcmd supply pipe symbol when necessary gitk: treat file names beginning with "|" as relative paths
2025-05-28Sync with 2.46.4Taylor Blau1-1/+1
* maint-2.46: Git 2.46.4 Git 2.45.4 Git 2.44.4 Git 2.43.7 wincred: avoid buffer overflow in wcsncat() bundle-uri: fix arbitrary file writes via parameter injection config: quote values containing CR character git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: override exec and open only on Windows gitk: sanitize 'open' arguments: revisit recently updated 'open' calls git-gui: use git_read in githook_read git-gui: sanitize $PATH on all platforms git-gui: break out a separate function git_read_nice git-gui: assure PATH has only absolute elements. git-gui: remove option --stderr from git_read git-gui: cleanup git-bash menu item git-gui: sanitize 'exec' arguments: background git-gui: avoid auto_execok in do_windows_shortcut git-gui: sanitize 'exec' arguments: simple cases git-gui: avoid auto_execok for git-bash menu item git-gui: treat file names beginning with "|" as relative paths git-gui: remove unused proc is_shellscript git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove special treatment of Windows from open_cmd_pipe git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: use only the configured shell git-gui: remove Tcl 8.4 workaround on 2>@1 redirection git-gui: make _shellpath usable on startup git-gui: use [is_Windows], not bad _shellpath git-gui: _which, only add .exe suffix if not present gitk: encode arguments correctly with "open" gitk: sanitize 'open' arguments: command pipeline gitk: collect construction of blameargs into a single conditional gitk: sanitize 'open' arguments: simple commands, readable and writable gitk: sanitize 'open' arguments: simple commands with redirections gitk: sanitize 'open' arguments: simple commands gitk: sanitize 'exec' arguments: redirect to process gitk: sanitize 'exec' arguments: redirections and background gitk: sanitize 'exec' arguments: redirections gitk: sanitize 'exec' arguments: 'eval exec' gitk: sanitize 'exec' arguments: simple cases gitk: have callers of diffcmd supply pipe symbol when necessary gitk: treat file names beginning with "|" as relative paths Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-28Sync with 2.45.4Taylor Blau1-1/+1
* maint-2.45: Git 2.45.4 Git 2.44.4 Git 2.43.7 wincred: avoid buffer overflow in wcsncat() bundle-uri: fix arbitrary file writes via parameter injection config: quote values containing CR character git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: override exec and open only on Windows gitk: sanitize 'open' arguments: revisit recently updated 'open' calls git-gui: use git_read in githook_read git-gui: sanitize $PATH on all platforms git-gui: break out a separate function git_read_nice git-gui: assure PATH has only absolute elements. git-gui: remove option --stderr from git_read git-gui: cleanup git-bash menu item git-gui: sanitize 'exec' arguments: background git-gui: avoid auto_execok in do_windows_shortcut git-gui: sanitize 'exec' arguments: simple cases git-gui: avoid auto_execok for git-bash menu item git-gui: treat file names beginning with "|" as relative paths git-gui: remove unused proc is_shellscript git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove special treatment of Windows from open_cmd_pipe git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: use only the configured shell git-gui: remove Tcl 8.4 workaround on 2>@1 redirection git-gui: make _shellpath usable on startup git-gui: use [is_Windows], not bad _shellpath git-gui: _which, only add .exe suffix if not present gitk: encode arguments correctly with "open" gitk: sanitize 'open' arguments: command pipeline gitk: collect construction of blameargs into a single conditional gitk: sanitize 'open' arguments: simple commands, readable and writable gitk: sanitize 'open' arguments: simple commands with redirections gitk: sanitize 'open' arguments: simple commands gitk: sanitize 'exec' arguments: redirect to process gitk: sanitize 'exec' arguments: redirections and background gitk: sanitize 'exec' arguments: redirections gitk: sanitize 'exec' arguments: 'eval exec' gitk: sanitize 'exec' arguments: simple cases gitk: have callers of diffcmd supply pipe symbol when necessary gitk: treat file names beginning with "|" as relative paths Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-28Sync with 2.44.4Taylor Blau1-1/+1
* maint-2.44: Git 2.44.4 Git 2.43.7 wincred: avoid buffer overflow in wcsncat() bundle-uri: fix arbitrary file writes via parameter injection config: quote values containing CR character git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: override exec and open only on Windows gitk: sanitize 'open' arguments: revisit recently updated 'open' calls git-gui: use git_read in githook_read git-gui: sanitize $PATH on all platforms git-gui: break out a separate function git_read_nice git-gui: assure PATH has only absolute elements. git-gui: remove option --stderr from git_read git-gui: cleanup git-bash menu item git-gui: sanitize 'exec' arguments: background git-gui: avoid auto_execok in do_windows_shortcut git-gui: sanitize 'exec' arguments: simple cases git-gui: avoid auto_execok for git-bash menu item git-gui: treat file names beginning with "|" as relative paths git-gui: remove unused proc is_shellscript git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove special treatment of Windows from open_cmd_pipe git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: use only the configured shell git-gui: remove Tcl 8.4 workaround on 2>@1 redirection git-gui: make _shellpath usable on startup git-gui: use [is_Windows], not bad _shellpath git-gui: _which, only add .exe suffix if not present gitk: encode arguments correctly with "open" gitk: sanitize 'open' arguments: command pipeline gitk: collect construction of blameargs into a single conditional gitk: sanitize 'open' arguments: simple commands, readable and writable gitk: sanitize 'open' arguments: simple commands with redirections gitk: sanitize 'open' arguments: simple commands gitk: sanitize 'exec' arguments: redirect to process gitk: sanitize 'exec' arguments: redirections and background gitk: sanitize 'exec' arguments: redirections gitk: sanitize 'exec' arguments: 'eval exec' gitk: sanitize 'exec' arguments: simple cases gitk: have callers of diffcmd supply pipe symbol when necessary gitk: treat file names beginning with "|" as relative paths Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-28Sync with 2.43.7Taylor Blau1-1/+1
* maint-2.43: Git 2.43.7 wincred: avoid buffer overflow in wcsncat() bundle-uri: fix arbitrary file writes via parameter injection config: quote values containing CR character git-gui: sanitize 'exec' arguments: convert new 'cygpath' calls git-gui: do not mistake command arguments as redirection operators git-gui: introduce function git_redir for git calls with redirections git-gui: pass redirections as separate argument to git_read git-gui: pass redirections as separate argument to _open_stdout_stderr git-gui: convert git_read*, git_write to be non-variadic git-gui: override exec and open only on Windows gitk: sanitize 'open' arguments: revisit recently updated 'open' calls git-gui: use git_read in githook_read git-gui: sanitize $PATH on all platforms git-gui: break out a separate function git_read_nice git-gui: assure PATH has only absolute elements. git-gui: remove option --stderr from git_read git-gui: cleanup git-bash menu item git-gui: sanitize 'exec' arguments: background git-gui: avoid auto_execok in do_windows_shortcut git-gui: sanitize 'exec' arguments: simple cases git-gui: avoid auto_execok for git-bash menu item git-gui: treat file names beginning with "|" as relative paths git-gui: remove unused proc is_shellscript git-gui: remove git config --list handling for git < 1.5.3 git-gui: remove special treatment of Windows from open_cmd_pipe git-gui: remove HEAD detachment implementation for git < 1.5.3 git-gui: use only the configured shell git-gui: remove Tcl 8.4 workaround on 2>@1 redirection git-gui: make _shellpath usable on startup git-gui: use [is_Windows], not bad _shellpath git-gui: _which, only add .exe suffix if not present gitk: encode arguments correctly with "open" gitk: sanitize 'open' arguments: command pipeline gitk: collect construction of blameargs into a single conditional gitk: sanitize 'open' arguments: simple commands, readable and writable gitk: sanitize 'open' arguments: simple commands with redirections gitk: sanitize 'open' arguments: simple commands gitk: sanitize 'exec' arguments: redirect to process gitk: sanitize 'exec' arguments: redirections and background gitk: sanitize 'exec' arguments: redirections gitk: sanitize 'exec' arguments: 'eval exec' gitk: sanitize 'exec' arguments: simple cases gitk: have callers of diffcmd supply pipe symbol when necessary gitk: treat file names beginning with "|" as relative paths Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23config: quote values containing CR characterJustin Tobler1-1/+1
When reading the config, values that contain a trailing CRLF are stripped. If the value itself has a trailing CR, the normal LF that follows results in the CR being unintentionally stripped. This may lead to unintended behavior due to the config value written being different when it gets read. One such issue involves a repository with a submodule path containing a trailing CR. When the submodule gets initialized, the submodule is cloned without being checked out and has "core.worktree" set to the submodule path. The git-checkout(1) that gets spawned later reads the "core.worktree" config value, but without the trailing CR, and consequently attempts to checkout to a different path than intended. If the repository contains a matching path that is a symlink, it is possible for the submodule repository to be checked out in arbitrary locations. This is extra bad when the symlink points to the submodule hooks directory and the submodule repository contains an executable "post-checkout" hook. Once the submodule repository checkout completes, the "post-checkout" hook immediately executes. To prevent mismatched config state due to misinterpreting a trailing CR, wrap config values containing CR in double quotes when writing the entry. This ensures a trailing CR is always separated for an LF and thus prevented from getting stripped. Note that this problem cannot be addressed by just quoting each CR with "\r". The reading side of the config interprets only a few backslash escapes, and "\r" is not among them. This fix is sufficient though because it only affects the CR at the end of a line and any literal CR in the interior is already preserved. Co-authored-by: David Leadbeater <dgl@dgl.cx> Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-04-15object-store: merge "object-store-ll.h" and "object-store.h"Patrick Steinhardt1-1/+1
The "object-store-ll.h" header has been introduced to keep transitive header dependendcies and compile times at bay. Now that we have created a new "object-store.c" file though we can easily move the last remaining additional bit of "object-store.h", the `odb_path_map`, out of the header. Do so. As the "object-store.h" header is now equivalent to its low-level alternative we drop the latter and inline it into the former. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-04-08Merge branch 'ps/object-wo-the-repository' into ps/object-file-cleanupJunio C Hamano1-5/+0
* ps/object-wo-the-repository: hash: stop depending on `the_repository` in `null_oid()` hash: fix "-Wsign-compare" warnings object-file: split out logic regarding hash algorithms delta-islands: stop depending on `the_repository` object-file-convert: stop depending on `the_repository` pack-bitmap-write: stop depending on `the_repository` pack-revindex: stop depending on `the_repository` pack-check: stop depending on `the_repository` environment: move access to "core.bigFileThreshold" into repo settings pack-write: stop depending on `the_repository` and `the_hash_algo` object: stop depending on `the_repository` csum-file: stop depending on `the_repository`
2025-03-26Merge branch 'ua/some-builtins-wo-the-repository'Junio C Hamano1-0/+4
A handful of built-in command implementations have been rewritten to use the repository instance supplied by git.c:run_builtin(), its caller. * ua/some-builtins-wo-the-repository: builtin/checkout-index: stop using `the_repository` builtin/for-each-ref: stop using `the_repository` builtin/ls-files: stop using `the_repository` builtin/pack-refs: stop using `the_repository` builtin/send-pack: stop using `the_repository` builtin/verify-commit: stop using `the_repository` builtin/verify-tag: stop using `the_repository` config: teach repo_config to allow `repo` to be NULL
2025-03-10environment: move access to "core.bigFileThreshold" into repo settingsPatrick Steinhardt1-5/+0
The "core.bigFileThreshold" setting is stored in a global variable and populated via `git_default_core_config()`. This may cause issues in the case where one is handling multiple different repositories in a single process with different values for that config key, as we may or may not see the correct value in that case. Furthermore, global state blocks our path towards libification. Refactor the code so that we instead store the value in `struct repo_settings`, where the value is computed as-needed and cached. Note that this change requires us to adapt one test in t1050 that verifies that we die when parsing an invalid "core.bigFileThreshold" value. The exercised Git command doesn't use the value at all, and thus it won't hit the new code path that parses the value. This is addressed by using git-hash-object(1) instead, which does read the value. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-07config: teach repo_config to allow `repo` to be NULLUsman Akinyemi1-0/+4
The `repo` value can be NULL if a builtin command is run outside any repository. The current implementation of `repo_config()` will fail if `repo` is NULL. If the `repo` is NULL the `repo_config()` can ignore the repository configuration but it should read the other configuration sources like the system-side configuration instead of failing. Teach the `repo_config()` to allow `repo` to be NULL by calling the `read_very_early_config()` which read config but only enumerate system and global settings. This will be useful in the following commits. Suggested-by: Junio C Hamano <gitster@pobox.com> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-06Merge branch 'tz/doc-txt-to-adoc-fixes'Junio C Hamano1-8/+8
Fallouts from recent renaming of documentation files from .txt suffix to the new .adoc suffix have been corrected. * tz/doc-txt-to-adoc-fixes: (38 commits) xdiff: *.txt -> *.adoc fixes unpack-trees.c: *.txt -> *.adoc fixes transport.h: *.txt -> *.adoc fixes trace2/tr2_sysenv.c: *.txt -> *.adoc fixes trace2.h: *.txt -> *.adoc fixes t6434: *.txt -> *.adoc fixes t6012: *.txt -> *.adoc fixes t/helper/test-rot13-filter.c: *.txt -> *.adoc fixes simple-ipc.h: *.txt -> *.adoc fixes setup.c: *.txt -> *.adoc fixes refs.h: *.txt -> *.adoc fixes pseudo-merge.h: *.txt -> *.adoc fixes parse-options.h: *.txt -> *.adoc fixes object-name.c: *.txt -> *.adoc fixes list-objects-filter-options.h: *.txt -> *.adoc fixes fsck.h: *.txt -> *.adoc fixes diffcore.h: *.txt -> *.adoc fixes diff.h: *.txt -> *.adoc fixes contrib/long-running-filter: *.txt -> *.adoc fixes config.c: *.txt -> *.adoc fixes ...
2025-03-05Merge branch 'ps/path-sans-the-repository'Junio C Hamano1-5/+0
The path.[ch] API takes an explicit repository parameter passed throughout the callchain, instead of relying on the_repository singleton instance. * ps/path-sans-the-repository: path: adjust last remaining users of `the_repository` environment: move access to "core.sharedRepository" into repo settings environment: move access to "core.hooksPath" into repo settings repo-settings: introduce function to clear struct path: drop `git_path()` in favor of `repo_git_path()` rerere: let `rerere_path()` write paths into a caller-provided buffer path: drop `git_common_path()` in favor of `repo_common_path()` worktree: return allocated string from `get_worktree_git_dir()` path: drop `git_path_buf()` in favor of `repo_git_path_replace()` path: drop `git_pathdup()` in favor of `repo_git_path()` path: drop unused `strbuf_git_path()` function path: refactor `repo_submodule_path()` family of functions submodule: refactor `submodule_to_gitdir()` to accept a repo path: refactor `repo_worktree_path()` family of functions path: refactor `repo_git_path()` family of functions path: refactor `repo_common_path()` family of functions
2025-03-03config.c: *.txt -> *.adoc fixesTodd Zullinger1-8/+8
Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-28environment: move access to "core.hooksPath" into repo settingsPatrick Steinhardt1-5/+0
The "core.hooksPath" setting is stored in a global variable and populated via the `git_default_core_config`. This may cause issues in the case where one is handling multiple different repositories in a single process with different values for that config key, as we may or may not see the correct value in that case. Furthermore, global state blocks our path towards libification. Refactor the code so that we instead store the value in `struct repo_settings`. The value is computed as-needed and cached. The result should be functionally the same as there aren't ever any code paths where we'd execute hooks outside the context of a repository. Note that this requires us to change the passed-in repository in the `repo_git_path()` family of functions to be non-constant, as we call `adjust_git_path()` there. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-01-28git-compat-util: move include of "compat/zlib.h" into "git-zlib.h"Patrick Steinhardt1-0/+1
We include "compat/zlib.h" in "git-compat-util.h", which is unnecessarily broad given that we only have a small handful of files that use the zlib library. Move the header into "git-zlib.h" instead and adapt users of zlib to include that header. One exception is the reftable library, as we don't want to use the Git-specific wrapper of zlib there, so we include "compat/zlib.h" instead. Furthermore, we move the include into "reftable/system.h" so that users of the library other than Git can wire up zlib themselves. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-23Merge branch 'ps/build-sign-compare'Junio C Hamano1-0/+1
Start working to make the codebase buildable with -Wsign-compare. * ps/build-sign-compare: t/helper: don't depend on implicit wraparound scalar: address -Wsign-compare warnings builtin/patch-id: fix type of `get_one_patchid()` builtin/blame: fix type of `length` variable when emitting object ID gpg-interface: address -Wsign-comparison warnings daemon: fix type of `max_connections` daemon: fix loops that have mismatching integer types global: trivial conversions to fix `-Wsign-compare` warnings pkt-line: fix -Wsign-compare warning on 32 bit platform csum-file: fix -Wsign-compare warning on 32-bit platform diff.h: fix index used to loop through unsigned integer config.mak.dev: drop `-Wno-sign-compare` global: mark code units that generate warnings with `-Wsign-compare` compat/win32: fix -Wsign-compare warning in "wWinMain()" compat/regex: explicitly ignore "-Wsign-compare" warnings git-compat-util: introduce macros to disable "-Wsign-compare" warnings
2024-12-06global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt1-0/+1
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-12-04config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak1-17/+0
The variables `packed_git_window_size` and `packed_git_limit` are global config variables used in the `packfile.c` file. Since it is only used in this file, let's change it from being a global config variable to a local variable for the subsystem. With this, we rid `packfile.c` from all global variable usage and this means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file. Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-04config: make `delta_base_cache_limit` a non-global variableKarthik Nayak1-5/+0
The `delta_base_cache_limit` variable is a global config variable used by multiple subsystems. Let's make this non-global, by adding this variable independently to the subsystems where it is used. First, add the setting to the `repo_settings` struct, this provides access to the config in places where the repository is available. Use this in `packfile.c`. In `index-pack.c` we add it to the `pack_idx_option` struct and its constructor. While the repository struct is available here, it may not be set because `git index-pack` can be used without a repository. In `gc.c` add it to the `gc_config` struct and also the constructor function. The gc functions currently do not have direct access to a repository struct. These changes are made to remove the usage of `delta_base_cache_limit` as a global variable in `packfile.c`. This brings us one step closer to removing the `USE_THE_REPOSITORY_VARIABLE` definition in `packfile.c` which we complete in the next patch. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-24config: fix evaluating "onbranch" with nonexistent git dirPatrick Steinhardt1-6/+9
The `include_by_branch()` function is responsible for evaluating whether or not a specific include should be pulled in based on the currently checked out branch. Naturally, his condition can only be evaluated when we have a properly initialized repository with a ref store in the first place. This is why the function guards against the case when either `data->repo` or `data->repo->gitdir` are `NULL` pointers. But the second check is insufficient: the `gitdir` may be set even though the repository has not been initialized. Quoting "setup.c": NEEDSWORK: currently we allow bogus GIT_DIR values to be set in some code paths so we also need to explicitly setup the environment if the user has set GIT_DIR. It may be beneficial to disallow bogus GIT_DIR values at some point in the future. So when either the GIT_DIR environment variable or the `--git-dir` global option are set by the user then `the_repository` may end up with an initialized `gitdir` variable. And this happens even when the dir is invalid, like for example when it doesn't exist. It follows that only checking for whether or not `gitdir` is `NULL` is not sufficient for us to determine whether the repository has been properly initialized. This issue can lead to us triggering a BUG: when using a config with an "includeIf.onbranch:" condition outside of a repository while using the `--git-dir` option pointing to an invalid Git directory we may end up trying to evaluate the condition even though the ref storage format has not been set up. This bisects to 173761e21b (setup: start tracking ref storage format, 2023-12-29), but that commit really only starts to surface the issue that has already existed beforehand. The code to check for `gitdir` was introduced via 85fe0e800c (config: work around bug with includeif:onbranch and early config, 2019-07-31), which tried to fix similar issues when we didn't yet have a repository set up. But the fix was incomplete as it missed the described scenario. As the quoted comment mentions, we'd ideally refactor the code to not set up `gitdir` with an invalid value in the first place, but that may be a bigger undertaking. Instead, refactor the code to use the ref storage format as an indicator of whether or not the ref store has been set up to fix the bug. Reported-by: Ronan Pigott <ronan@rjp.ie> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: stop storing "core.notesRef" globallyPatrick Steinhardt1-8/+0
Stop storing the "core.notesRef" config value globally. Instead, retrieve the value in `default_notes_ref()`. The code is never called in a hot loop anyway, so doing this on every invocation should be perfectly fine. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: stop storing "core.warnAmbiguousRefs" globallyPatrick Steinhardt1-5/+0
Same as the preceding commits, storing the "core.warnAmbiguousRefs" value globally is misdesigned as this setting may be set per repository. Move the logic into the repo-settings subsystem. The usual pattern here is that users are expected to call `prepare_repo_settings()` before they access the settings themselves. This seems somewhat fragile though, as it is easy to miss and leads to somewhat ugly code patterns at the call sites. Instead, introduce a new function that encapsulates this logic for us. This also allows us to change how exactly the lazy initialization works in the future, e.g. by only partially initializing values as requested by the caller. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: stop storing "core.preferSymlinkRefs" globallyPatrick Steinhardt1-5/+0
Same as the preceding commit, storing the "core.preferSymlinkRefs" value globally is misdesigned as this setting may be set per repository. There is only a single user of this value anyway, namely the "files" backend. So let's just remove the global variable and read the value of this setting when initializing the backend. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: stop storing "core.logAllRefUpdates" globallyPatrick Steinhardt1-10/+0
The value of "core.logAllRefUpdates" is being stored in the global variable `log_all_ref_updates`. This design is somewhat aged nowadays, where it is entirely possible to access multiple repositories in the same process which all have different values for this setting. So using a single global variable to track it is plain wrong. Remove the global variable. Instead, we now provide a new function part of the repo-settings subsystem that parses the value for a specific repository. While that may require us to read the value multiple times, we work around this by reading it once when the ref backends are set up and caching the value there. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: guard state depending on a repositoryPatrick Steinhardt1-0/+2
In "environment.h" we have quite a lot of functions and variables that either explicitly or implicitly depend on `the_repository`. The implicit set of stateful declarations includes for example variables which get populated when parsing a repository's Git configuration. This set of variables is broken by design, as their state often depends on the last repository config that has been parsed. So they may or may not represent the state of `the_repository`. Fixing that is quite a big undertaking, and later patches in this series will demonstrate a solution for a first small set of those variables. So for now, let's guard these with `USE_THE_REPOSITORY_VARIABLE` so that callers are aware of the implicit dependency. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12config: make dependency on repo in `read_early_config()` explicitPatrick Steinhardt1-6/+4
The `read_early_config()` function can be used to read configuration where a repository has not yet been set up. As such, it is optional whether or not `the_repository` has already been initialized. If it was initialized we use its commondir and gitdir. If not, the function will try to detect the Git directories by itself and, if found, also parse their config files. This means that we implicitly rely on `the_repository`. Make this dependency explicit by passing a `struct repository`. This allows us to again drop the `USE_THE_REPOSITORY_VARIABLE` define in "config.c". Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12config: document `read_early_config()` and `read_very_early_config()`Patrick Steinhardt1-4/+0
It's not clear what `read_early_config()` and `read_very_early_config()` do differently compared to `repo_read_config()` from just looking at their names. Document both of these in the header file to clarify their intent. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12environment: make `get_git_common_dir()` accept a repositoryPatrick Steinhardt1-1/+1
The `get_git_common_dir()` function retrieves the path to the common 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-12environment: make `get_git_dir()` accept a repositoryPatrick Steinhardt1-1/+3
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-08-26Merge branch 'ps/maintenance-detach-fix'Junio C Hamano1-2/+3
Maintenance tasks other than "gc" now properly go background when "git maintenance" runs them. * ps/maintenance-detach-fix: run-command: fix detaching when running auto maintenance builtin/maintenance: add a `--detach` flag builtin/gc: add a `--detach` flag builtin/gc: stop processing log file on signal builtin/gc: fix leaking config values builtin/gc: refactor to read config into structure config: fix constness of out parameter for `git_config_get_expiry()`
2024-08-26Merge branch 'tb/config-fixed-value-with-valueless-true' into maint-2.46Junio C Hamano1-1/+1
"git config --value=foo --fixed-value section.key newvalue" barfed when the existing value in the configuration file used the valueless true syntax, which has been corrected. * tb/config-fixed-value-with-valueless-true: config.c: avoid segfault with --fixed-value and valueless config
2024-08-23Merge branch 'ps/config-wo-the-repository'Junio C Hamano1-149/+76
Use of API functions that implicitly depend on the_repository object in the config subsystem has been rewritten to pass a repository object through the callchain. * ps/config-wo-the-repository: config: hide functions using `the_repository` by default global: prepare for hiding away repo-less config functions config: don't depend on `the_repository` with branch conditions config: don't have setters depend on `the_repository` config: pass repo to functions that rename or copy sections config: pass repo to `git_die_config()` config: pass repo to `git_config_get_expiry_in_days()` config: pass repo to `git_config_get_expiry()` config: pass repo to `git_config_get_max_percent_split_change()` config: pass repo to `git_config_get_split_index()` config: pass repo to `git_config_get_index_threads()` config: expose `repo_config_clear()` config: introduce missing setters that take repo as parameter path: hide functions using `the_repository` by default path: stop relying on `the_repository` in `worktree_git_path()` path: stop relying on `the_repository` when reporting garbage hooks: remove implicit dependency on `the_repository` editor: do not rely on `the_repository` for interactive edits path: expose `do_git_common_path()` as `repo_common_pathv()` path: expose `do_git_path()` as `repo_git_pathv()`
2024-08-23Merge branch 'ps/leakfixes-part-4'Junio C Hamano1-1/+2
More leak fixes. * ps/leakfixes-part-4: (22 commits) builtin/diff: free symmetric diff members diff: free state populated via options builtin/log: fix leak when showing converted blob contents userdiff: fix leaking memory for configured diff drivers builtin/format-patch: fix various trivial memory leaks diff: fix leak when parsing invalid ignore regex option unpack-trees: clear index when not propagating it sequencer: release todo list on error paths merge-ort: unconditionally release attributes index builtin/fast-export: plug leaking tag names builtin/fast-export: fix leaking diff options builtin/fast-import: plug trivial memory leaks builtin/notes: fix leaking `struct notes_tree` when merging notes builtin/rebase: fix leaking `commit.gpgsign` value config: fix leaking comment character config submodule-config: fix leaking name entry when traversing submodules read-cache: fix leaking hashfile when writing index fails bulk-checkin: fix leaking state TODO object-name: fix leaking symlink paths in object context object-file: fix memory leak when reading corrupted headers ...
2024-08-16config: fix constness of out parameter for `git_config_get_expiry()`Patrick Steinhardt1-2/+2
The type of the out parameter of `git_config_get_expiry()` is a pointer to a constant string, which creates the impression that ownership of the returned data wasn't transferred to the caller. This isn't true though and thus quite misleading. Adapt the parameter to be of type `char **` and adjust callers accordingly. While at it, refactor `get_shared_index_expire_date()` to drop the static `shared_index_expire` variable. It is only used in that function, and furthermore we would only hit the code where we parse the expiry date a single time because we already use a static `prepared` variable to track whether we did parse it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-14Merge branch 'tb/config-fixed-value-with-valueless-true'Junio C Hamano1-1/+1
"git config --value=foo --fixed-value section.key newvalue" barfed when the existing value in the configuration file used the valueless true syntax, which has been corrected. * tb/config-fixed-value-with-valueless-true: config.c: avoid segfault with --fixed-value and valueless config
2024-08-14config: fix leaking comment character configPatrick Steinhardt1-1/+2
When the comment line character has been specified multiple times in the configuration, then `git_default_core_config()` will cause a memory leak because it unconditionally copies the string into `comment_line_str` without free'ing the previous value. In fact, it can't easily free the value in the first place because it may contain a string constant. Refactor the code such that we track allocated comment character strings via a separate non-constant variable `comment_line_str_to_free`. Adapt sites that set `comment_line_str` to set both and free the old value that was stored in `comment_line_str_to_free`. This memory leak is being hit in t3404. As there are still other memory leaks in that file we cannot yet mark it as passing with leak checking enabled. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: hide functions using `the_repository` by defaultPatrick Steinhardt1-130/+0
The config subsystem provides a bunch of legacy functions that read or set configuration for `the_repository`. The use of those functions is discouraged, and it is easy to miss the implicit dependency on `the_repository` that calls to those functions may cause. Move all config-related functions that use `the_repository` into a block that gets only conditionally compiled depending on whether or not the macro has been defined. This also removes all dependencies on that variable in "config.c", allowing us to remove the definition of said preprocessor macro. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: don't depend on `the_repository` with branch conditionsPatrick Steinhardt1-4/+5
When computing branch "includeIf" conditions we use `the_repository` to obtain the main ref store. We really shouldn't depend on this global repository though, but should instead use the repository that is being passed to us via `struct config_include_data`. Otherwise, when parsing configuration of e.g. submodules, we may end up evaluating the condition the via the wrong refdb. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: don't have setters depend on `the_repository`Patrick Steinhardt1-8/+8
Some of the setters that accept a `struct repository` still implicitly rely on `the_repository` via `git_config_set_multivar_in_file()`. While this function would typically use the caller-provided path, it knows to fall back to using the configuration path indicated by `the_repository`. Adapt those functions to instead use the caller-provided repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to functions that rename or copy sectionsPatrick Steinhardt1-14/+16
Refactor functions that rename or copy config sections to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the functions accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to `git_die_config()`Patrick Steinhardt1-6/+6
Refactor `git_die_config()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to `git_config_get_expiry_in_days()`Patrick Steinhardt1-2/+3
Refactor `git_config_get_expiry_in_days()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to `git_config_get_expiry()`Patrick Steinhardt1-2/+2
Refactor `git_config_get_expiry()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to `git_config_get_max_percent_split_change()`Patrick Steinhardt1-2/+2
Refactor `git_config_get_max_percent_split_change()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to `git_config_get_split_index()`Patrick Steinhardt1-2/+2
Refactor `git_config_get_split_index()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: pass repo to `git_config_get_index_threads()`Patrick Steinhardt1-2/+2
Refactor `git_config_get_index_threads()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: expose `repo_config_clear()`Patrick Steinhardt1-1/+1
While we already have `repo_config_clear()` as an alternative to `git_config_clear()` that doesn't rely on `the_repository`, it is not exposed to callers outside of the config subsystem. Do so. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13config: introduce missing setters that take repo as parameterPatrick Steinhardt1-20/+73
While we already provide some of the config-setting interfaces with a `struct repository` as parameter, others only have a variant that implicitly depends on `the_repository`. Fill in those gaps such that we can start to deprecate the repo-less variants. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-01config.c: avoid segfault with --fixed-value and valueless configTaylor Blau1-1/+1
When using `--fixed-value` with a key whose value is left empty (implied as being "true"), 'git config' may crash when invoked like either of: $ git config set --file=config --value=value --fixed-value \ section.key pattern $ git config --file=config --fixed-value section.key value pattern The original bugreport[1] bisects to 00bbdde141 (builtin/config: introduce "set" subcommand, 2024-05-06), which is a red-herring, since the original bugreport uses the new 'git config set' invocation. The behavior likely bisects back to c90702a1f6 (config: plumb --fixed-value into config API, 2020-11-25), which introduces the new --fixed-value option in the first place. Looking at the relevant frame from a failed process's coredump, the crash appears in config.c::matches() like so: (gdb) up #1 0x000055b3e8b06022 in matches (key=0x55b3ea894360 "section.key", value=0x0, store=0x7ffe99076eb0) at config.c:2884 2884 return !strcmp(store->fixed_value, value); where we are trying to compare the `--fixed-value` argument to `value`, which is NULL. Avoid attempting to match `--fixed-value` for configuration keys with no explicit value. A future patch could consider the empty value to mean "true", "yes", "on", etc. when invoked with `--type=bool`, but let's punt on that for now in the name of avoiding the segfault. [1]: https://lore.kernel.org/git/CANrWfmTek1xErBLrnoyhHN+gWU+rw14y6SQ+abZyzGoaBjmiKA@mail.gmail.com/ Reported-by: Han Jiang <jhcarl0814@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-08Merge branch 'ps/leakfixes-more'Junio C Hamano1-0/+1
More memory leaks have been plugged. * ps/leakfixes-more: (29 commits) builtin/blame: fix leaking ignore revs files builtin/blame: fix leaking prefixed paths blame: fix leaking data for blame scoreboards line-range: plug leaking find functions merge: fix leaking merge bases builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` sequencer: fix memory leaks in `make_script_with_merges()` builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` apply: fix leaking string in `match_fragment()` sequencer: fix leaking string buffer in `commit_staged_changes()` commit: fix leaking parents when calling `commit_tree_extended()` config: fix leaking "core.notesref" variable rerere: fix various trivial leaks builtin/stash: fix leak in `show_stash()` revision: free diff options builtin/log: fix leaking commit list in git-cherry(1) merge-recursive: fix memory leak when finalizing merge builtin/merge-recursive: fix leaking object ID bases builtin/difftool: plug memory leaks in `run_dir_diff()` object-name: free leaking object contexts ...
2024-07-02Merge branch 'ps/use-the-repository'Junio C Hamano1-0/+3
A CPP macro USE_THE_REPOSITORY_VARIABLE is introduced to help transition the codebase to rely less on the availability of the singleton the_repository instance. * ps/use-the-repository: hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE` t/helper: remove dependency on `the_repository` in "proc-receive" t/helper: fix segfault in "oid-array" command without repository t/helper: use correct object hash in partial-clone helper compat/fsmonitor: fix socket path in networked SHA256 repos replace-object: use hash algorithm from passed-in repository protocol-caps: use hash algorithm from passed-in repository oidset: pass hash algorithm when parsing file http-fetch: don't crash when parsing packfile without a repo hash-ll: merge with "hash.h" refs: avoid include cycle with "repository.h" global: introduce `USE_THE_REPOSITORY_VARIABLE` macro hash: require hash algorithm in `empty_tree_oid_hex()` hash: require hash algorithm in `is_empty_{blob,tree}_oid()` hash: make `is_null_oid()` independent of `the_repository` hash: convert `oidcmp()` and `oideq()` to compare whole hash global: ensure that object IDs are always padded hash: require hash algorithm in `oidread()` and `oidclr()` hash: require hash algorithm in `hasheq()`, `hashcmp()` and `hashclr()` hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions
2024-06-24Merge branch 'tb/pseudo-merge-reachability-bitmap'Junio C Hamano1-0/+9
The pseudo-merge reachability bitmap to help more efficient storage of the reachability bitmap in a repository with too many refs has been added. * tb/pseudo-merge-reachability-bitmap: (26 commits) pack-bitmap.c: ensure pseudo-merge offset reads are bounded Documentation/technical/bitmap-format.txt: add missing position table t/perf: implement performance tests for pseudo-merge bitmaps pseudo-merge: implement support for finding existing merges ewah: `bitmap_equals_ewah()` pack-bitmap: extra trace2 information pack-bitmap.c: use pseudo-merges during traversal t/test-lib-functions.sh: support `--notick` in `test_commit_bulk()` pack-bitmap: implement test helpers for pseudo-merge ewah: implement `ewah_bitmap_popcount()` pseudo-merge: implement support for reading pseudo-merge commits pack-bitmap.c: read pseudo-merge extension pseudo-merge: scaffolding for reads pack-bitmap: extract `read_bitmap()` function pack-bitmap-write.c: write pseudo-merge table pseudo-merge: implement support for selecting pseudo-merge commits config: introduce `git_config_double()` pack-bitmap: make `bitmap_writer_push_bitmapped_commit()` public pack-bitmap: implement `bitmap_writer_has_bitmapped_object_id()` pack-bitmap-write: support storing pseudo-merge commits ...
2024-06-20Merge branch 'ps/abbrev-length-before-setup-fix'Junio C Hamano1-2/+2
Setting core.abbrev too early before the repository set-up (typically in "git clone") caused segfault, which as been corrected. * ps/abbrev-length-before-setup-fix: object-name: don't try to abbreviate to lengths greater than hexsz parse-options-cb: stop clamping "--abbrev=" to hash length config: fix segfault when parsing "core.abbrev" without repo
2024-06-17Merge branch 'jc/varargs-attributes'Junio C Hamano1-1/+0
Varargs functions that are unannotated as printf-like or execl-like have been annotated as such. * jc/varargs-attributes: __attribute__: add a few missing format attributes __attribute__: mark some functions with LAST_ARG_MUST_BE_NULL __attribute__: remove redundant attribute declaration for git_die_config() __attribute__: trace2_region_enter_printf() is like "printf"
2024-06-14global: introduce `USE_THE_REPOSITORY_VARIABLE` macroPatrick Steinhardt1-0/+3
Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-12config: fix segfault when parsing "core.abbrev" without repoPatrick Steinhardt1-2/+2
The "core.abbrev" config allows the user to specify the minimum length when abbreviating object hashes. Next to the values "auto" and "no", this config also accepts a concrete length that needs to be bigger or equal to the minimum length and smaller or equal to the hash algorithm's hex length. While the former condition is trivial, the latter depends on the object format used by the current repository. It is thus a variable upper boundary that may either be 40 (SHA-1) or 64 (SHA-256). This has two major downsides. First, the user that specifies this config must be aware of the object hashes that its repository use. If they want to configure the value globally, then they cannot pick any value in the range `[41, 64]` if they have any repository that uses SHA-1. If they did, Git would error out when parsing the config. Second, and more importantly, parsing "core.abbrev" crashes when outside of a Git repository because we dereference `the_hash_algo` to figure out its hex length. Starting with c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07) though, we stopped initializing `the_hash_algo` outside of Git repositories. Fix both of these issues by not making it an error anymore when the given length exceeds the hash length. Instead, leave the abbreviated length intact. `repo_find_unique_abbrev_r()` handles this just fine except for a performance penalty which we will fix in a subsequent commit. Reported-by: Kyle Lippincott <spectral@google.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-11config: fix leaking "core.notesref" variablePatrick Steinhardt1-0/+1
The variable used to track the "core.notesref" config is not getting freed before we assign to it and thus leaks. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10__attribute__: remove redundant attribute declaration for git_die_config()Junio C Hamano1-1/+0
The convention is to declare the function attribute to an extern function together with its declaration in the header file, without repeating the attribute declaration with its definition in the .c source file (a file-scope static function declares its attribute together with its definition in the .c file it is defined, as there is no other place to do so). The definition of git_die_config() in config.c did not follow the convention and had its attribute declared with both its declaration in the header and its definition in the .c source file. Remove the one in the config.c to match everybody else. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-06Merge branch 'ps/leakfixes'Junio C Hamano1-17/+38
Leakfixes. * ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
2024-06-03Merge branch 'ps/leakfixes' into ps/leakfixes-moreJunio C Hamano1-17/+38
* ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
2024-05-30Merge branch 'jc/fix-2.45.1-and-friends-for-maint'Junio C Hamano1-12/+1
Adjust jc/fix-2.45.1-and-friends-for-2.39 for more recent maintenance track. * jc/fix-2.45.1-and-friends-for-maint: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-28Merge branch 'ps/builtin-config-cleanup'Junio C Hamano1-2/+2
Code clean-up to reduce inter-function communication inside builtin/config.c done via the use of global variables. * ps/builtin-config-cleanup: (21 commits) builtin/config: pass data between callbacks via local variables builtin/config: convert flags to a local variable builtin/config: track "fixed value" option via flags only builtin/config: convert `key` to a local variable builtin/config: convert `key_regexp` to a local variable builtin/config: convert `regexp` to a local variable builtin/config: convert `value_pattern` to a local variable builtin/config: convert `do_not_match` to a local variable builtin/config: move `respect_includes_opt` into location options builtin/config: move default value into display options builtin/config: move type options into display options builtin/config: move display options into local variables builtin/config: move location options into local variables builtin/config: refactor functions to have common exit paths config: make the config source const builtin/config: check for writeability after source is set up builtin/config: move actions into `cmd_config_actions()` builtin/config: move legacy options into `cmd_config()` builtin/config: move subcommand options into `cmd_config()` builtin/config: move legacy mode into its own function ...
2024-05-27config: plug various memory leaksPatrick Steinhardt1-9/+27
Now that memory ownership rules around `git_config_string()` and `git_config_pathname()` are clearer, it also got easier to spot that the returned memory needs to be free'd. Plug a subset of those cases and mark now-passing tests as leak free. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27config: clarify memory ownership in `git_config_string()`Patrick Steinhardt1-3/+3
The out parameter of `git_config_string()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt1-2/+4
The `check_roundtrip_encoding` variable is tracked in a `const char *` even though it may contain allocated strings at times. The result is that those strings may be leaking because we never free them. Refactor the code to always store allocated strings in this variable. The default value is handled in `check_roundtrip()` now, which is the only user of the variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt1-5/+5
The out parameter of `git_config_pathname()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-24Merge branch 'fixes/2.45.1/2.44' into jc/fix-2.45.1-and-friends-for-maintJunio C Hamano1-12/+1
* fixes/2.45.1/2.44: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-24Merge branch 'fixes/2.45.1/2.43' into fixes/2.45.1/2.44Junio C Hamano1-12/+1
* fixes/2.45.1/2.43: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-24Merge branch 'fixes/2.45.1/2.42' into fixes/2.45.1/2.43Junio C Hamano1-12/+1
* fixes/2.45.1/2.42: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-24Merge branch 'fixes/2.45.1/2.41' into fixes/2.45.1/2.42Junio C Hamano1-12/+1
* fixes/2.45.1/2.41: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-24Merge branch 'fixes/2.45.1/2.40' into fixes/2.45.1/2.41Junio C Hamano1-12/+1
* fixes/2.45.1/2.40: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-24Merge branch 'jc/fix-2.45.1-and-friends-for-2.39' into fixes/2.45.1/2.40Junio C Hamano1-12/+1
Revert overly aggressive "layered defence" that went into 2.45.1 and friends, which broke "git-lfs", "git-annex", and other use cases, so that we can rebuild necessary counterparts in the open. * jc/fix-2.45.1-and-friends-for-2.39: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack
2024-05-24config: introduce `git_config_double()`Taylor Blau1-0/+9
Future commits will want to parse a double-precision floating point value from configuration, but we have no way to parse such a value prior to this patch. The core of the routine is implemented in git_parse_double(). Unlike git_parse_unsigned() and git_parse_signed(), however, the function implemented here only works on type "double", and not related types like "float", or "long double". This is because "float" and "long double" use different functions to convert from ASCII strings to floating point values (strtof() and strtold(), respectively). Likewise, there is no pointer type that can assign to any of these values (except for "void *"), so the only way to define this trio of functions would be with a macro expansion that is parameterized over the floating point type and conversion function. That is all doable, but likely to be overkill given our current needs, which is only to parse double-precision floats. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-21Revert "core.hooksPath: add some protection while cloning"Johannes Schindelin1-12/+1
This defense-in-depth was intended to protect the clone operation against future escalations where bugs in `git clone` would allow attackers to write arbitrary files in the `.git/` directory would allow for Remote Code Execution attacks via maliciously-placed hooks. However, it turns out that the `core.hooksPath` protection has unintentional side effects so severe that they do not justify the benefit of the protections. For example, it has been reported in https://lore.kernel.org/git/FAFA34CB-9732-4A0A-87FB-BDB272E6AEE8@alchemists.io/ that the following invocation, which is intended to make `git clone` safer, is itself broken by that protective measure: git clone --config core.hooksPath=/dev/null <url> Since it turns out that the benefit does not justify the cost, let's revert 20f3588efc6 (core.hooksPath: add some protection while cloning, 2024-03-30). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-16Merge branch 'ps/refs-without-the-repository'Junio C Hamano1-1/+2
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-15Merge branch 'ps/config-subcommands'Junio C Hamano1-10/+6
The operation mode options (like "--get") the "git config" command uses have been deprecated and replaced with subcommands (like "git config get"). * ps/config-subcommands: builtin/config: display subcommand help builtin/config: introduce "edit" subcommand builtin/config: introduce "remove-section" subcommand builtin/config: introduce "rename-section" subcommand builtin/config: introduce "unset" subcommand builtin/config: introduce "set" subcommand builtin/config: introduce "get" subcommand builtin/config: introduce "list" subcommand builtin/config: pull out function to handle `--null` builtin/config: pull out function to handle config location builtin/config: use `OPT_CMDMODE()` to specify modes builtin/config: move "fixed-value" option to correct group builtin/config: move option array around config: clarify memory ownership when preparing comment strings
2024-05-15config: make the config source constPatrick Steinhardt1-2/+2
The `struct git_config_source` passed to `config_with_options()` is never modified. Let's mark it as `const` to clarify. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-07cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt1-1/+2
Apply the rules that rewrite callers of "refs" interfaces to explicitly pass `struct ref_store`. The resulting patch has been applied with the `--whitespace=fix` option. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-06config: clarify memory ownership when preparing comment stringsPatrick Steinhardt1-10/+6
The ownership of memory returned when preparing a comment string is quite intricate: when the returned value is different than the passed value, then the caller is responsible to free the memory. This is quite subtle, and it's even easier to miss because the returned value is in fact a `const char *`. Adapt the function to always return either `NULL` or a newly allocated string. The function is called at most once per git-config(1), so it's not like this micro-optimization really matters. Thus, callers are now always responsible for freeing the value. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-29Sync with 2.44.1Johannes Schindelin1-1/+12
* maint-2.44: (41 commits) Git 2.44.1 Git 2.43.4 Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel ...
2024-04-19Sync with 2.43.4Johannes Schindelin1-1/+12
* maint-2.43: (40 commits) Git 2.43.4 Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories ...
2024-04-19Sync with 2.42.2Johannes Schindelin1-1/+12
* maint-2.42: (39 commits) Git 2.42.2 Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' ...
2024-04-19Sync with 2.41.1Johannes Schindelin1-1/+12
* maint-2.41: (38 commits) Git 2.41.1 Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' docs: document security issues around untrusted .git dirs ...
2024-04-19Sync with 2.40.2Johannes Schindelin1-1/+12
* maint-2.40: (39 commits) Git 2.40.2 Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' docs: document security issues around untrusted .git dirs upload-pack: disable lazy-fetching by default ...
2024-04-19Sync with 2.39.4Johannes Schindelin1-1/+12
* maint-2.39: (38 commits) Git 2.39.4 fsck: warn about symlink pointing inside a gitdir core.hooksPath: add some protection while cloning init.templateDir: consider this config setting protected clone: prevent hooks from running during a clone Add a helper function to compare file contents init: refactor the template directory discovery into its own function find_hook(): refactor the `STRIP_EXTENSION` logic clone: when symbolic links collide with directories, keep the latter entry: report more colliding paths t5510: verify that D/F confusion cannot lead to an RCE submodule: require the submodule path to contain directories only clone_submodule: avoid using `access()` on directories submodules: submodule paths must not contain symlinks clone: prevent clashing git dirs when cloning submodule in parallel t7423: add tests for symlinked submodule directories has_dir_name(): do not get confused by characters < '/' docs: document security issues around untrusted .git dirs upload-pack: disable lazy-fetching by default fetch/clone: detect dubious ownership of local repositories ...
2024-04-19core.hooksPath: add some protection while cloningJohannes Schindelin1-1/+12
Quite frequently, when vulnerabilities were found in Git's (quite complex) clone machinery, a relatively common way to escalate the severity was to trick Git into running a hook which is actually a script that has just been laid on disk as part of that clone. This constitutes a Remote Code Execution vulnerability, the highest severity observed in Git's vulnerabilities so far. Some previously-fixed vulnerabilities allowed malicious repositories to be crafted such that Git would check out files not in the worktree, but in, say, a submodule's `<git>/hooks/` directory. A vulnerability that "merely" allows to modify the Git config would allow a related attack vector, to manipulate Git into looking in the worktree for hooks, e.g. redirecting the location where Git looks for hooks, via setting `core.hooksPath` (which would be classified as CWE-427: Uncontrolled Search Path Element and CWE-114: Process Control, for more details see https://cwe.mitre.org/data/definitions/427.html and https://cwe.mitre.org/data/definitions/114.html). To prevent that attack vector, let's error out and complain loudly if an active `core.hooksPath` configuration is seen in the repository-local Git config during a `git clone`. There is one caveat: This changes Git's behavior in a slightly backwards-incompatible manner. While it is probably a rare scenario (if it exists at all) to configure `core.hooksPath` via a config in the Git templates, it _is_ conceivable that some valid setup requires this to work. In the hopefully very unlikely case that a user runs into this, there is an escape hatch: set the `GIT_CLONE_PROTECTION_ACTIVE=false` environment variable. Obviously, this should be done only with utmost caution. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2024-04-15Merge branch 'jc/unleak-core-excludesfile'Junio C Hamano1-1/+3
The variable that holds the value read from the core.excludefile configuration variable used to leak, which has been corrected. * jc/unleak-core-excludesfile: config: do not leak excludes_file
2024-04-08config: do not leak excludes_fileJunio C Hamano1-1/+3
The excludes_file variable is marked "const char *", but all the assignments to it are made with a piece of memory allocated just for it, and the variable is responsible for owning it. When "core.excludesfile" is read, the code just lost the previous value, leaking memory. Plug it. The real problem is that the variable is mistyped; our convention is to never make a variable that owns the piece of memory pointed by it as "const". Fixing that would reduce the chance of this kind of bug happening, and also would make it unnecessary to cast the constness away while free()ing it, but that would be a much larger follow-up effort. Reported-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-05Merge branch 'jk/core-comment-string'Junio C Hamano1-4/+7
core.commentChar used to be limited to a single byte, but has been updated to allow an arbitrary multi-byte sequence. * jk/core-comment-string: config: add core.commentString config: allow multi-byte core.commentChar environment: drop comment_line_char compatibility macro wt-status: drop custom comment-char stringification sequencer: handle multi-byte comment characters when writing todo list find multi-byte comment chars in unterminated buffers find multi-byte comment chars in NUL-terminated strings prefer comment_line_str to comment_line_char for printing strbuf: accept a comment string for strbuf_add_commented_lines() strbuf: accept a comment string for strbuf_commented_addf() strbuf: accept a comment string for strbuf_stripspace() environment: store comment_line_char as a string strbuf: avoid shadowing global comment_line_char name commit: refactor base-case of adjust_comment_line_char() strbuf: avoid static variables in strbuf_add_commented_lines() strbuf: simplify comment-handling in add_lines() helper config: forbid newline as core.commentChar
2024-04-05Merge branch 'rs/config-comment'Junio C Hamano1-8/+72
"git config" learned "--comment=<message>" option to leave a comment immediately after the "variable = value" on the same line in the configuration file. * rs/config-comment: config: allow tweaking whitespace between value and comment config: fix --comment formatting config: add --comment option to add a comment
2024-04-01Merge branch 'ds/config-internal-whitespace-fix'Junio C Hamano1-5/+10
"git config" corrupted literal HT characters written in the configuration file as part of a value, which has been corrected. * ds/config-internal-whitespace-fix: config.txt: describe handling of whitespace further t1300: add more tests for whitespace and inline comments config: really keep value-internal whitespace verbatim config: minor addition of whitespace
2024-03-27config: add core.commentStringJeff King1-3/+4
The core.commentChar code recently learned to accept more than a single ASCII character. But using it is annoying with multiple versions of Git, since older ones will reject it outright: $ git.v2.44.0 -c core.commentchar=foo stripspace -s error: core.commentChar should only be one ASCII character fatal: unable to parse 'core.commentchar' from command-line config Let's add an alias core.commentString. That's arguably a better name anyway, since we now can handle strings, and it makes it possible to have a config that works reasonably with both old and new versions of Git (see the example in the documentation). This is strictly an alias, so there's not much point in adding duplicate tests; I added a single one to t0030 that exercises the alias code. Note also that the error messages for invalid values will now show the variable the config parser handed us, and thus will be normalized to lowercase (rather than camelcase). A few tests in t0030 are adjusted to match. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-21config: really keep value-internal whitespace verbatimDragan Simic1-4/+9
Fix a bug in function parse_value() that prevented whitespace characters (i.e. spaces and horizontal tabs) found inside configuration option values from being parsed and returned in their original form. The bug caused any number of consecutive whitespace characters to be wrongly "squashed" into the same number of space characters. This bug was introduced back in July 2009, in commit ebdaae372b46 ("config: Keep inner whitespace verbatim"). Further investigation showed that setting a configuration value, by invoking git-config(1), converts value-internal horizontal tabs into "\t" escape sequences, which the buggy value-parsing logic in function parse_value() didn't "squash" into spaces. That's why the test included in the ebdaae37 commit passed, which presumably made the bug remain undetected for this long. On the other hand, value-internal literal horizontal tab characters, found in a configuration file edited by hand, do get "squashed" by the value-parsing logic, so the right choice was to fix this bug by making the value-internal whitespace characters preserved verbatim. Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-21config: minor addition of whitespaceDragan Simic1-1/+1
In general, binary operators should be enclosed in a pair of leading and trailing space (SP) characters. Thus, clean up one spotted expression that for some reason had a "bunched up" operator. Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15config: allow tweaking whitespace between value and commentJunio C Hamano1-11/+58
Extending the previous step, this allows the whitespace placed after the value before the "# comment message" to be tweaked by tweaking the preprocessing rule to: * If the given comment string begins with one or more whitespace characters followed by '#', it is passed intact. * If the given comment string begins with '#', a Space is prepended. * Otherwise, " # " (Space, '#', Space) is prefixed. * A string with LF in it cannot be used as a comment string. Unlike the previous step, which unconditionally added a space after the value before writing the "# comment string", because the above preprocessing already gives a whitespace before the '#', the resulting string is written immediately after copying the value. And the sanity checking rule becomes * comment string after the above massaging that comes into git_config_set_multivar_in_file_gently() must - begin with zero or more whitespace characters followed by '#'. - not have a LF in it. I personally think this is over-engineered, but since I thought things through anyway, here it is in the patch form. The logic to tweak end-user supplied comment string is encapsulated in a new helper function, git_config_prepare_comment_string(), so if new front-end callers would want to use the same massaging rules, it is easily reused. Unfortunately I do not think of a way to tweak the preprocessing rules further to optionally allow having no blank after the value, i.e. to produce [section] variable = value#comment (which is a valid way to say section.variable=value, by the way) without sacrificing the ergonomics for the more usual case, so this time I really stop here. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15config: fix --comment formattingJunio C Hamano1-6/+14
When git adds comments itself (like "rebase -i" todo list and "commit -e" log message editor), it always gives a comment introducer "#" followed by a Space before the message, except for the recently introduced "git config --comment", where the users are forced to say " this is my comment" if they want to add their comment in this usual format; otherwise their comment string will end up without a space after the "#". Make it more ergonomic, while keeping it possible to also use this unusual style, by massaging the comment string at the UI layer with a set of simple rules: * If the given comment string begins with '#', it is passed intact. * Otherwise, "# " is prefixed. * A string with LF in it cannot be used as a comment string. Right now there is only one "front-end" that accepts end-user comment string and calls the underlying machinery to add or modify configuration file with comments, but to make sure that the future callers perform similar massaging as they see fit, add a sanity check logic in git_config_set_multivar_in_file_gently(), which is the single choke point in the codepaths that consumes the comment string. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-15config: add --comment option to add a commentRalph Seichter1-8/+17
Introduce the ability to append comments to modifications made using git-config. Example usage: git config --comment "changed via script" \ --add safe.directory /home/alice/repo.git based on the proposed patch, the output produced is: [safe] directory = /home/alice/repo.git #changed via script Users need to be able to distinguish between config entries made using automation and entries made by a human. Automation can add comments containing a URL pointing to explanations for the change made, avoiding questions from users as to why their config file was changed by a third party. The implementation ensures that a # character is unconditionally prepended to the provided comment string, and that the comment text is appended as a suffix to the changed key-value-pair in the same line of text. Multi-line comments (i.e. comments containing linefeed) are rejected as errors, causing Git to exit without making changes. Comments are aimed at humans who inspect or change their Git config using a pager or editor. Comments are not meant to be read or displayed by git-config at a later time. Signed-off-by: Ralph Seichter <github@seichter.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12config: allow multi-byte core.commentCharJeff King1-5/+5
Now that all of the code handles multi-byte comment characters, it's safe to allow users to set them. There is one special case I kept: we still will not allow an empty string for the commentChar. While it might make sense in some contexts (e.g., output where you don't want any comment prefix), there are plenty where it will behave badly (e.g., all of our starts_with() checks will indicate that every line is a comment!). It might be reasonable to assign some meaningful semantics, but it would probably involve checking how each site behaves. In the interim let's forbid it and we can loosen things later. Likewise, the "commentChar cannot be a newline" rule is now extended to "it cannot contain a newline" (for the same reason: it can confuse our parsing loops). Since comment_line_str is used in many parts of the code, it's hard to cover all possibilities with tests. We can convert the existing double-semicolon prefix test to show that "git status" works. And we'll give it a more challenging case in t7507, where we confirm that git-commit strips out the commit template along with any --verbose text when reading the edited commit message back in. That covers the basics, though it's possible there could be issues in more exotic spots (e.g., the sequencer todo list uses its own code). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12environment: store comment_line_char as a stringJeff King1-1/+1
We'd like to eventually support multi-byte comment prefixes, but the comment_line_char variable is referenced in many spots, making the transition difficult. Let's start by storing the character in a NUL-terminated string. That will let us switch code over incrementally to the string format, and we can easily support the existing code with a macro wrapper (since we'll continue to allow only a single-byte prefix, this will behave identically). Once all references to the "char" variable have been converted, we can drop it and enable longer strings. We'll still have to touch all of the spots that create or set the variable in this patch, but there are only a few (reading the config, and the "auto" character selector). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-12config: forbid newline as core.commentCharJeff King1-0/+2
Since we usually look for a comment char while parsing line-oriented files, setting core.commentChar to a single newline can confuse our code quite a bit. For example, using it with "git commit" causes us to fail to recognize any of the template as comments, including it in the config message. Which kind of makes sense, since the template content is on its own line (so no line can "start" with a newline). In other spots I would not be surprised if you can create more mischief (e.g., violating loop assumptions) but I didn't dig into it. Since comment characters are a local preference, to some degree this is a case of "if it hurts, don't do it". But given that this would be a silly and pointless thing to do, and that it makes it harder to reason about code parsing comment lines, let's just forbid it. There are other cases that are perhaps questionable (e.g., setting the comment char to a single space), but they seem to behave reasonably (at least a simple "git commit" will correctly identify and strip the template lines). So I haven't worried about going on a hunt for every stupid thing a user might do to themselves, and just focused on the most confusing case. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-08Merge branch 'en/header-cleanup' into maint-2.43Junio C Hamano1-3/+0
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
2024-01-18config: factor out global config file retrievalKristoffer Haugsbakk1-0/+20
Factor out code that retrieves the global config file so that we can use it in `gc.c` as well. Use the old name from the previous commit since this function acts functionally the same as `git_system_config` but for “global”. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-01-18config: rename global config functionKristoffer Haugsbakk1-2/+2
Rename this function to a more descriptive name since we want to use the existing name for a new function. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-01-18config: format newlinesKristoffer Haugsbakk1-2/+0
Remove unneeded newlines according to `clang-format`. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-01-08Merge branch 'en/header-cleanup'Junio C Hamano1-3/+0
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 Newren1-3/+0
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-12-09config: use git_config_string() for core.checkRoundTripEncodingJeff King1-6/+2
Since this code path was recently converted to check for a NULL value, it now behaves exactly like git_config_string(). We can shorten the code a bit by using that helper. Note that git_config_string() takes a const pointer, but our storage variable is non-const. We're better off making this "const", though, since the default value points to a string literal (and thus it would be an error if anybody tried to write to it). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-09config: reject bogus values for core.checkstatJeff King1-0/+3
If you feed nonsense config like: git -c core.checkstat=foobar status we'll silently ignore the unknown value, rather than reporting an error. This goes all the way back to c08e4d5b5c (Enable minimal stat checking, 2013-01-22). Detecting and complaining now is technically a backwards-incompatible change, but I don't think anybody has any reason to use an invalid value here. There are no historical values we'd want to allow for backwards compatibility or anything like that. We are better off loudly telling the user that their config may not be doing what they expect. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-09config: handle NULL value when parsing non-boolsJeff King1-0/+8
When the config parser sees an "implicit" bool like: [core] someVariable it passes NULL to the config callback. Any callback code which expects a string must check for NULL. This usually happens via helpers like git_config_string(), etc, but some custom code forgets to do so and will segfault. These are all fairly vanilla cases where the solution is just the usual pattern of: if (!value) return config_error_nonbool(var); though note that in a few cases we have to split initializers like: int some_var = initializer(); into: int some_var; if (!value) return config_error_nonbool(var); some_var = initializer(); There are still some broken instances after this patch, which I'll address on their own in individual patches after this one. Reported-by: Carlos Andrés Ramírez Cataño <antaigroupltda@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-30Merge branch 'jc/attr-tree-config'Junio C Hamano1-0/+16
The attribute subsystem learned to honor `attr.tree` configuration that specifies which tree to read the .gitattributes files from. * jc/attr-tree-config: attr: add attr.tree for setting the treeish to read attributes from attr: read attributes from HEAD when bare repo
2023-10-13attr: add attr.tree for setting the treeish to read attributes fromJohn Cai1-0/+16
44451a2 (attr: teach "--attr-source=<tree>" global option to "git", 2023-05-06) provided the ability to pass in a treeish as the attr source. In the context of serving Git repositories as bare repos like we do at GitLab however, it would be easier to point --attr-source to HEAD for all commands by setting it once. Add a new config attr.tree that allows this. Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-10Merge branch 'cw/prelim-cleanup'Junio C Hamano1-172/+1
Shuffle some bits across headers and sources to prepare for libification effort. * cw/prelim-cleanup: parse: separate out parsing functions from config.h config: correct bad boolean env value error message wrapper: reduce scope of remove_or_warn() hex-ll: separate out non-hash-algo functions
2023-09-29parse: separate out parsing functions from config.hCalvin Wan1-179/+1
The files config.{h,c} contain functions that have to do with parsing, but not config. In order to further reduce all-in-one headers, separate out functions in config.c that do not operate on config into its own file, parse.h, and update the include directives in the .c files that need only such functions accordingly. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-29config: correct bad boolean env value error messageCalvin Wan1-1/+8
An incorrectly defined boolean environment value would result in the following error message: bad boolean config value '%s' for '%s' This is a misnomer since environment value != config value. Instead of calling git_config_bool() to parse the environment value, mimic the functionality inside of git_config_bool() but with the correct error message. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-08-31add core.maxTreeDepth configJeff King1-0/+5
Most of our tree traversal algorithms use recursion to visit sub-trees. For pathologically large trees, this can cause us to run out of stack space and abort in an uncontrolled way. Let's put our own limit here so that we can fail gracefully rather than segfaulting. In similar cases where we recursed along the commit graph, we rewrote the algorithms to avoid recursion and keep any stack data on the heap. But the commit graph is meant to grow without bound, whereas it's not an imposition to put a limit on the maximum size of tree we'll handle. And this has a bonus side effect: coupled with a limit on individual tree entry names, this limits the total size of a path we may encounter. This gives us an extra protection against code handling long path names which may suffer from integer overflows in the size (which could then be exploited by malicious trees). The default of 4096 is set to be much longer than anybody would care about in the real world. Even with single-letter interior tree names (like "a/b/c"), such a path is at least 8191 bytes. While most operating systems will let you create such a path incrementally, trying to reference the whole thing in a system call (as Git would do when actually trying to access it) will result in ENAMETOOLONG. Coupled with the recent fsck.largePathname warning, the maximum total pathname Git will handle is (by default) 16MB. This config option doesn't do anything yet; future patches will convert various algorithms to respect the limit. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-17Merge branch 'cw/compat-util-header-cleanup'Junio C Hamano1-2/+0
Further shuffling of declarations across header files to streamline file dependencies. * cw/compat-util-header-cleanup: git-compat-util: move alloc macros to git-compat-util.h treewide: remove unnecessary includes for wrapper.h kwset: move translation table from ctype sane-ctype.h: create header for sane-ctype macros git-compat-util: move wrapper.c funcs to its header git-compat-util: move strbuf.c funcs to its header
2023-07-06Merge branch 'gc/config-context'Junio C Hamano1-326/+220
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-07-06Merge branch 'gc/config-partial-submodule-kvi-fix'Junio C Hamano1-6/+0
Partially revert a sanity check that the rest of the config code was not ready, to avoid triggering it in a corner case. * gc/config-partial-submodule-kvi-fix: config: don't BUG when both kvi and source are set
2023-07-05git-compat-util: move alloc macros to git-compat-util.hCalvin Wan1-1/+0
alloc_nr, ALLOC_GROW, and ALLOC_GROW_BY are commonly used macros for dynamic array allocation. Moving these macros to git-compat-util.h with the other alloc macros focuses alloc.[ch] to allocation for Git objects and additionally allows us to remove inclusions to alloc.h from files that solely used the above macros. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-05treewide: remove unnecessary includes for wrapper.hCalvin Wan1-1/+0
Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-29Merge branch 'en/header-split-cache-h-part-3'Junio C Hamano1-1/+4
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: pass source to config_parser_event_fn_tGlen Choo1-64/+13
..so that the callback can use a "struct config_source" parameter instead of "config_reader.source". "struct config_source" is internal to config.c, so we are adding a pointer to a struct defined in config.c into a public function signature defined in config.h, but this is okay because this function has only ever been (and probably ever will be) used internally by config.c. As a result, the_reader isn't used anywhere, so "struct config_reader" is obsolete (it was only intended to be used with the_reader). Remove them. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: add kvi.path, use it to evaluate includesGlen Choo1-21/+19
Include directives are evaluated using the path of the config file. To reduce the dependence on "config_reader.source", add a new "key_value_info.path" member and use that instead of "config_source.path". This allows us to remove a "struct config_reader *" field from "struct config_include_data", which will subsequently allow us to remove "struct config_reader" entirely. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config.c: remove config_reader from configsetsGlen Choo1-34/+11
Remove the last usage of "struct config_reader" from configsets by copying the "kvi" arg instead of recomputing "kvi" from config_reader.source. Since we no longer need to pass both "struct config_reader" and "struct config_set" in a single "void *cb", remove "struct configset_add_data" too. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: pass kvi to die_bad_number()Glen Choo1-95/+61
Plumb "struct key_value_info" through all code paths that end in die_bad_number(), which lets us remove the helper functions that read analogous values from "struct config_reader". As a result, nothing reads config_reader.config_kvi any more, so remove that too. In config.c, this requires changing the signature of git_configset_get_value() to 'return' "kvi" in an out parameter so that git_configset_get_<type>() can pass it to git_config_<type>(). Only numeric types will use "kvi", so for non-numeric types (e.g. git_configset_get_string()), pass NULL to indicate that the out parameter isn't needed. Outside of config.c, config callbacks now need to pass "ctx->kvi" to any of the git_config_<type>() functions that parse a config string into a number type. Included is a .cocci patch to make that refactor. The only exceptional case is builtin/config.c, where git_config_<type>() is called outside of a config callback (namely, on user-provided input), so config source information has never been available. In this case, die_bad_number() defaults to a generic, but perfectly descriptive message. Let's provide a safe, non-NULL for "kvi" anyway, but make sure not to change the message. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28trace2: plumb config kviGlen Choo1-46/+0
There is a code path starting from trace2_def_param_fl() that eventually calls current_config_scope(), and thus it needs to have "kvi" plumbed through it. Additional plumbing is also needed to get "kvi" to trace2_def_param_fl(), which gets called by two code paths: - Through tr2_cfg_cb(), which is a config callback, so it trivially receives "kvi" via the "struct config_context ctx" parameter. - Through tr2_list_env_vars_fl(), which is a high level function that lists environment variables for tracing. This has been secretly behaving like git_config_from_parameters() (in that it parses config from environment variables/the CLI), but does not set config source information. Teach tr2_list_env_vars_fl() to be well-behaved by using kvi_from_param(), which is used elsewhere for CLI/environment variable-based config. As a result, current_config_scope() has no more callers, so remove it. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config.c: pass ctx with CLI configGlen Choo1-39/+41
Pass config_context when parsing CLI config. To provide the .kvi member, refactor out kvi_from_param() from the logic that caches CLI config in configsets. Now that config_context and config_context.kvi is always present when config machinery calls config callbacks, plumb "kvi" so that we can remove all calls of current_config_scope() except for trace2/*.c (which will be handled in a later commit), and remove all other current_config_*() (the functions themselves and their calls). Note that this results in .kvi containing a different, more complete set of information than the mocked up "struct config_source" in git_config_from_parameters(). Plumbing "kvi" reveals a few places where we've been doing the wrong thing: * git_config_parse_parameter() hasn't been setting config source information, so plumb "kvi" there too. * Several sites in builtin/config.c have been calling current_config_*() functions outside of config callbacks (indirectly, via the format_config() helper), which means they're reading state that isn't set correctly: * "git config --get-urlmatch --show-scope" iterates config to collect values, but then attempts to display the scope after config iteration, causing the "unknown" scope to be shown instead of the config file's scope. It's clear that this wasn't intended: we knew that "--get-urlmatch" couldn't show config source metadata, which is why "--show-origin" was marked incompatible with "--get-urlmatch" when it was introduced [1]. It was most likely a mistake that we allowed "--show-scope" to sneak through. Fix this by copying the "kvi" value in the collection phase so that it can be read back later. This means that we can now support "git config --get-urlmatch --show-origin", but that is left unchanged for now. * "git config --default" doesn't have config source metadata when displaying the default value, so "--show-scope" also results in "unknown", and "--show-origin" results in a BUG(). Fix this by treating the default value as if it came from the command line (e.g. like we do with "git -c" or "git config --file"), using kvi_from_param(). [1] https://lore.kernel.org/git/20160205112001.GA13397@sigill.intra.peff.net/ Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: pass ctx with config filesGlen Choo1-35/+70
Pass config_context to config_callbacks when parsing config files. To provide the .kvi member, refactor out the configset logic that caches "struct config_source" and "enum config_scope" as a "struct key_value_info". Make the "enum config_scope" available to the config file machinery by plumbing an additional arg through git_config_from_file_with_options(). We do not exercise ctx yet because the remaining current_config_*() callers may be used with config_with_options(), which may read config from parameters, but parameters don't pass ctx yet. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config.c: pass ctx in configsetsGlen Choo1-19/+16
Pass config_context to config callbacks in configset_iter(), trivially setting the .kvi member to the cached key_value_info. Then, in config callbacks that are only used with configsets, use the .kvi member to replace calls to current_config_*(), and delete current_config_line() because it has no remaining callers. This leaves builtin/config.c and config.c as the only remaining users of current_config_*(). Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-28config: add ctx arg to config_fn_tGlen Choo1-14/+24
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-26config: don't BUG when both kvi and source are setGlen Choo1-6/+0
When iterating through config, we read config source metadata from global values - either a "struct config_source + enum config_scope" or a "struct key_value_info", using the current_config* functions. Prior to the series starting from 0c60285147 (config.c: create config_reader and the_reader, 2023-03-28), we weren't very picky about which values we should read in which situation; we did note that both groups of values generally shouldn't be set together, but if both were set, current_config* preferentially reads key_value_info. When that series added more structure, we enforced that either the former (when parsing a config source) can be set, or the latter (when iterating a config set), but *never* both at the same time. See 9828453ff0 (config.c: remove current_config_kvi, 2023-03-28) and 5cdf18e7cd (config.c: remove current_parsing_scope, 2023-03-28). That was a good simplifying constraint that helped us reason about the global state, but it turns out that there is at least one situation where we need both to be set at the same time: in a blobless partial clone where .gitmodules is missing. "git fetch" in such a repo will start a config parse over .gitmodules (setting the config_source), and Git will attempt to lazy-fetch it from the promisor remote. However, when we try to read the promisor configuration, we start iterating a config set (setting the key_value_info), and we BUG() out because that's not allowed any more. Teaching config_reader to gracefully handle this is somewhat complicated, but fortunately, there are proposed changes to the config.c machinery to get rid of this global state, and make the BUG() obsolete [1]. We should rely on that as the eventual solution, and avoid doing yet another refactor in the meantime. Therefore, fix the bug by removing the BUG() check. We're reverting to an older, less safe state, but that's generally okay since key_value_info is always preferentially read, so we'd always read the correct values when we iterate a config set in the middle of a config parse (like we are here). The reverse would be wrong, but extremely unlikely to happen since very few callers parse config without going through a config set. [1] https://lore.kernel.org/git/pull.1497.v3.git.git.1687290231.gitgitgadget@gmail.com Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-23Merge branch 'rj/leakfixes'Junio C Hamano1-0/+2
Leakfixes * rj/leakfixes: tests: mark as passing with SANITIZE=leak config: fix a leak in git_config_copy_or_rename_section_in_file branch: fix a leak in cmd_branch branch: fix a leak in setup_tracking rev-parse: fix a leak with --abbrev-ref branch: fix a leak in setup_tracking branch: fix a leak in check_tracking_branch branch: fix a leak in inherit_tracking branch: fix a leak in dwim_and_setup_tracking remote: fix a leak in query_matches_negative_refspec config: fix a leak in git_config_copy_or_rename_section_in_file
2023-06-22Merge branch 'ds/disable-replace-refs'Junio C Hamano1-5/+0
Introduce a mechanism to disable replace refs globally and per repository. * ds/disable-replace-refs: repository: create read_replace_refs setting replace-objects: create wrapper around setting repository: create disable_replace_refs()
2023-06-21object-store-ll.h: split this header out of object-store.hElijah Newren1-1/+1
The vast majority of files including object-store.h did not need dir.h nor khash.h. Split the header into two files, and let most just depend upon object-store-ll.h, while letting the two callers that need it depend on the full object-store.h. After this patch: $ git grep -h include..object-store | sort | uniq -c 2 #include "object-store.h" 129 #include "object-store-ll.h" Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21git-compat-util.h: remove unneccessary include of wildmatch.hElijah Newren1-0/+1
The include of wildmatch.h in git-compat-util.h was added in cebcab189aa (Makefile: add USE_WILDMATCH to use wildmatch as fnmatch, 2013-01-01) as a way to be able to compile-time force any calls to fnmatch() to instead invoke wildmatch(). The defines and inline function were removed in 70a8fc999d9 (stop using fnmatch (either native or compat), 2014-02-15), and this include in git-compat-util.h has been unnecessary ever since. Remove the include from git-compat-util.h, but add it to the .c files that had omitted the direct #include they needed. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21repository: remove unnecessary include of path.hElijah Newren1-0/+1
This also made it clear that several .c files that depended upon path.h were missing a #include for it; add the missing includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-21read-cache*.h: move declarations for read-cache.c functions from cache.hElijah Newren1-0/+1
For the functions defined in read-cache.c, move their declarations from cache.h to a new header, read-cache-ll.h. Also move some related inline functions from cache.h to read-cache.h. The purpose of the read-cache-ll.h/read-cache.h split is that about 70% of the sites don't need the inline functions and the extra headers they include. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-17config: fix a leak in git_config_copy_or_rename_section_in_fileRubén Justo1-0/+1
A branch can have its configuration spread over several configuration sections. This situation was already foreseen in 52d59cc645 (branch: add a --copy (-c) option to go with --move (-m), 2017-06-18) when "branch -c" was introduced. Unfortunately, a leak was also introduced: $ git branch foo $ cat >> .git/config <<EOF [branch "foo"] some-key-a = a value [branch "foo"] some-key-b = b value [branch "foo"] some-key-c = c value EOF $ git branch -c foo bar Direct leak of 130 byte(s) in 2 object(s) allocated from: ... in xrealloc wrapper.c ... in strbuf_grow strbuf.c ... in strbuf_vaddf strbuf.c ... in strbuf_addf strbuf.c ... in store_create_section config.c ... in git_config_copy_or_rename_section_in_file config.c ... in git_config_copy_section_in_file config.c ... in git_config_copy_section config.c ... in copy_or_rename_branch builtin/branch.c ... in cmd_branch builtin/branch.c ... in run_builtin git.c Let's fix it. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12config: fix a leak in git_config_copy_or_rename_section_in_fileRubén Justo1-0/+1
In 52d59cc645 (branch: add a --copy (-c) option to go with --move (-m), 2017-06-18) a new strbuf variable was introduced, but not released. Thus, when copying a branch that has any configuration, we have a leak. $ git branch foo $ git config branch.foo.some-key some_value $ git branch -c foo bar Direct leak of 65 byte(s) in 1 object(s) allocated from: ... in xrealloc wrapper.c ... in strbuf_grow strbuf.c ... in strbuf_vaddf strbuf.c ... in strbuf_addf strbuf.c ... in store_create_section config.c ... in git_config_copy_or_rename_section_in_file config.c ... in git_config_copy_section_in_file config.c ... in git_config_copy_section config.c ... in copy_or_rename_branch builtin/branch.c ... in cmd_branch builtin/branch.c ... in run_builtin git.c Let's fix that leak. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12repository: create read_replace_refs settingDerrick Stolee1-5/+0
The 'read_replace_refs' global specifies whether or not we should respect the references of the form 'refs/replace/<oid>' to replace which object we look up when asking for '<oid>'. This global has caused issues when it is not initialized properly, such as in b6551feadfd (merge-tree: load default git config, 2023-05-10). To make this more robust, move its config-based initialization out of git_default_config and into prepare_repo_settings(). This provides a repository-scoped version of the 'read_replace_refs' global. The global still has its purpose: it is disabled process-wide by the GIT_NO_REPLACE_OBJECTS environment variable or by a call to disable_replace_refs() in some specific Git commands. Since we already encapsulated the use of the constant inside replace_refs_enabled(), we can perform the initialization inside that method, if necessary. This solves the problem of forgetting to check the config, as we will check it before returning this value. Due to this encapsulation, the global can move to be static within replace-object.c. There is an interesting behavior change possible here: we now have a repository-scoped understanding of this config value. Thus, if there was a command that recurses into submodules and might follow replace refs, then it would now respect the core.useReplaceRefs config value in each repository. 'git grep --recurse-submodules' is such a command that recurses into submodules in-process. We can demonstrate the granularity of this config value via a test in t7814. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-26repository: move 'repository_format_worktree_config' to repo scopeVictoria Dye1-3/+4
Move 'repository_format_worktree_config' out of the global scope and into the 'repository' struct. This change is similar to how 'repository_format_partial_clone' was moved in ebaf3bcf1ae (repository: move global r_f_p_c to repo struct, 2021-06-17), adding it to the 'repository' struct and updating 'setup.c' & 'repository.c' functions to assign the value appropriately. The primary goal of this change is to be able to load the worktree config of a submodule depending on whether that submodule - not its superproject - has 'extensions.worktreeConfig' enabled. To ensure 'do_git_config_sequence()' has access to the newly repo-scoped configuration, add a 'struct repository' argument to 'do_git_config_sequence()' and pass it the 'repo' value from 'config_with_options()'. Finally, add/update tests in 't3007-ls-files-recurse-submodules.sh' to verify 'extensions.worktreeConfig' is read an used independently by superprojects and submodules. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-26config: pass 'repo' directly to 'config_with_options()'Victoria Dye1-7/+9
Add a 'struct repository' argument to 'config_with_options()' and remove the 'repo' field from 'struct git_config_source'. A 'struct repository' instance was originally added to the config source in e3e8bf046e9 (submodule-config: pass repo upon blob config read, 2021-08-16) to improve how submodule blob config content was accessed. At the time, this was the only use for a 'repository' instance, so it was naturally added only where it was needed: to 'struct git_config_source'. However, in upcoming patches, 'config_with_options()' will need the repository instance to access extension information (regardless of whether a 'config_source' exists). To make the 'struct repository' instance more easily accessible, move it into the function's arguments. Update all callers of 'config_with_options()' to pass the appropriate 'repo' value: * in 'builtin/config.c', use 'the_repository' * in 'submodule--config.c', use the 'repo' arg in 'config_from_gitmodules()' * in 'read_[very_]early_config()' & 'read_protected_config()', set 'repo' to NULL (repository instances aren't available there) * in 'populate_remote_urls()', use the repo instance that has been added to the 'struct config_include_data' * in 'repo_read_config()', use the given 'repo' arg Finally, note that this patch eliminates the fallback to 'the_repository' that previously existed for the 'config_source' repo instance if it was NULL. The fallback is no longer necessary, as the 'repo' is set explicitly in all cases where it is needed. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-26config: use gitdir to get worktree configVictoria Dye1-9/+19
Update 'do_git_config_sequence()' to read the worktree config from 'config.worktree' in 'opts->git_dir' rather than the gitdir of 'the_repository'. The worktree config is loaded from the path returned by 'git_pathdup("config.worktree")', the 'config.worktree' relative to the gitdir of 'the_repository'. If loading the config for a submodule, this path is incorrect, since 'the_repository' is the superproject. 'opts->git_dir' is the gitdir of the submodule being configured, so the config file in that location should be read instead. To ensure the use of 'opts->git_dir' is safe, require that 'opts->git_dir' is set if-and-only-if 'opts->commondir' is set (rather than "only-if" as it is now). In all current usage of 'config_options', these values are set together, so the stricter check does not change any behavior. Finally, add tests to 't3007-ls-files-recurse-submodules.sh' to verify the corrected config is loaded. Use 'ls-files' to test this because, unlike some other '--recurse-submodules' commands, 'ls-files' parses the config of the submodule in the same process as the superproject (via 'show_submodule()' -> 'repo_read_index()' -> 'prepare_repo_settings()'). As a result, 'the_repository' points to the config of the superproject but the commondir/gitdir in the config sequence will be that of the submodule, providing the exact scenario needed to verify this patch. The first test ('--recurse-submodules parses submodule repo config') checks that the submodule's *repo* config is read when running 'ls-files' on the superproject; this confirms already-working behavior, serving as a reference for how worktree config parsing should behave. The second test ('--recurse-submodules parses submodule worktree config') tests the same scenario as the previous but instead using the *worktree* config, demonstrating the corrected behavior. The 'test_config' helper is extended for this case so that it properly applies the '--worktree' option to the configure/unconfigure operations it performs. Note that, although the submodule worktree config is now parsed instead of the superproject's, 'extensions.worktreeConfig' in the superproject still controls whether or not the worktree config is enabled at all in the submodule. This will be fixed in a later patch. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-05-09Merge branch 'en/header-split-cache-h-part-2'Junio C Hamano1-1/+2
More header clean-up. * en/header-split-cache-h-part-2: (22 commits) reftable: ensure git-compat-util.h is the first (indirect) include diff.h: reduce unnecessary includes object-store.h: reduce unnecessary includes commit.h: reduce unnecessary includes fsmonitor: reduce includes of cache.h cache.h: remove unnecessary headers treewide: remove cache.h inclusion due to previous changes cache,tree: move basic name compare functions from read-cache to tree cache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c hash-ll.h: split out of hash.h to remove dependency on repository.h tree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h dir.h: move DTYPE defines from cache.h versioncmp.h: move declarations for versioncmp.c functions from cache.h ws.h: move declarations for ws.c functions from cache.h match-trees.h: move declarations for match-trees.c functions from cache.h pkt-line.h: move declarations for pkt-line.c functions from cache.h base85.h: move declarations for base85.c functions from cache.h copy.h: move declarations for copy.c functions from cache.h server-info.h: move declarations for server-info.c functions from cache.h packfile.h: move pack_window and pack_entry from cache.h ...
2023-04-25Merge branch 'en/header-split-cache-h'Junio C Hamano1-0/+6
Header clean-up. * en/header-split-cache-h: (24 commits) protocol.h: move definition of DEFAULT_GIT_PORT from cache.h mailmap, quote: move declarations of global vars to correct unit treewide: reduce includes of cache.h in other headers treewide: remove double forward declaration of read_in_full cache.h: remove unnecessary includes treewide: remove cache.h inclusion due to pager.h changes pager.h: move declarations for pager.c functions from cache.h treewide: remove cache.h inclusion due to editor.h changes editor: move editor-related functions and declarations into common file treewide: remove cache.h inclusion due to object.h changes object.h: move some inline functions and defines from cache.h treewide: remove cache.h inclusion due to object-file.h changes object-file.h: move declarations for object-file.c functions from cache.h treewide: remove cache.h inclusion due to git-zlib changes git-zlib: move declarations for git-zlib functions from cache.h treewide: remove cache.h inclusion due to object-name.h changes object-name.h: move declarations for object-name.c functions from cache.h treewide: remove unnecessary cache.h inclusion treewide: be explicit about dependence on mem-pool.h treewide: be explicit about dependence on oid-array.h ...
2023-04-24Sync with Git 2.40.1Junio C Hamano1-11/+25
2023-04-24treewide: remove cache.h inclusion due to previous changesElijah Newren1-1/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-24ws.h: move declarations for ws.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-17Sync with 2.39.3Johannes Schindelin1-11/+25
* maint-2.39: (34 commits) Git 2.39.3 Git 2.38.5 Git 2.37.7 Git 2.36.6 Git 2.35.8 Makefile: force -O0 when compiling with SANITIZE=leak Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix range-diff: use ssize_t for parsed "len" in read_patches() ...
2023-04-17Sync with 2.38.5Johannes Schindelin1-11/+25
* maint-2.38: (32 commits) Git 2.38.5 Git 2.37.7 Git 2.36.6 Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion ...
2023-04-17Sync with 2.37.7Johannes Schindelin1-11/+25
* maint-2.37: (31 commits) Git 2.37.7 Git 2.36.6 Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 ...
2023-04-17Sync with 2.36.6Johannes Schindelin1-11/+25
* maint-2.36: (30 commits) Git 2.36.6 Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix ...
2023-04-17Sync with 2.35.8Johannes Schindelin1-11/+25
* maint-2.35: (29 commits) Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR ...
2023-04-17Sync with 2.34.8Johannes Schindelin1-11/+25
* maint-2.34: (28 commits) Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION ...
2023-04-17Sync with 2.33.8Johannes Schindelin1-11/+25
* maint-2.33: (27 commits) Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ...
2023-04-17Sync with 2.32.7Johannes Schindelin1-11/+25
* maint-2.32: (26 commits) Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ci: install python on ubuntu ...
2023-04-17Sync with 2.31.8Johannes Schindelin1-11/+25
* maint-2.31: (25 commits) Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ci: install python on ubuntu ci: use the same version of p4 on both Linux and macOS ...
2023-04-17Sync with 2.30.9Johannes Schindelin1-11/+25
* maint-2.30: (23 commits) Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ci: install python on ubuntu ci: use the same version of p4 on both Linux and macOS ci: remove the pipe after "p4 -V" to catch errors github-actions: run gcc-8 on ubuntu-20.04 image ...
2023-04-17config.c: disallow overly-long lines in `copy_or_rename_section_in_file()`Taylor Blau1-0/+13
As a defense-in-depth measure to guard against any potentially-unknown buffer overflows in `copy_or_rename_section_in_file()`, refuse to work with overly-long lines in a gitconfig. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
2023-04-17config.c: avoid integer truncation in `copy_or_rename_section_in_file()`Taylor Blau1-5/+5
There are a couple of spots within `copy_or_rename_section_in_file()` that incorrectly use an `int` to track an offset within a string, which may truncate or wrap around to a negative value. Historically it was impossible to have a line longer than 1024 bytes anyway, since we used fgets() with a fixed-size buffer of exactly that length. But the recent change to use a strbuf permits us to read lines of arbitrary length, so it's possible for a malicious input to cause us to overflow past INT_MAX and do an out-of-bounds array read. Practically speaking, however, this should never happen, since it requires 2GB section names or values, which are unrealistic in non-malicious circumstances. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2023-04-17config: avoid fixed-sized buffer when renaming/deleting a sectionTaylor Blau1-6/+7
When renaming (or deleting) a section of configuration, Git uses the function `git_config_copy_or_rename_section_in_file()` to rewrite the configuration file after applying the rename or deletion to the given section. To do this, Git repeatedly calls `fgets()` to read the existing configuration data into a fixed size buffer. When the configuration value under `old_name` exceeds the size of the buffer, we will call `fgets()` an additional time even if there is no newline in the configuration file, since our read length is capped at `sizeof(buf)`. If the first character of the buffer (after zero or more characters satisfying `isspace()`) is a '[', Git will incorrectly treat it as beginning a new section when the original section is being removed. In other words, a configuration value satisfying this criteria can incorrectly be considered as a new secftion instead of a variable in the original section. Avoid this issue by using a variable-width buffer in the form of a strbuf rather than a fixed-with region on the stack. A couple of small points worth noting: - Using a strbuf will cause us to allocate arbitrary sizes to match the length of each line. In practice, we don't expect any reasonable configuration files to have lines that long, and a bandaid will be introduced in a later patch to ensure that this is the case. - We are using strbuf_getwholeline() here instead of strbuf_getline() in order to match `fgets()`'s behavior of leaving the trailing LF character on the buffer (as well as a trailing NUL). This could be changed later, but using strbuf_getwholeline() changes the least about this function's implementation, so it is picked as the safest path. - It is temping to want to replace the loop to skip over characters matching isspace() at the beginning of the buffer with a convenience function like `strbuf_ltrim()`. But this is the wrong approach for a couple of reasons: First, it involves a potentially large and expensive `memmove()` which we would like to avoid. Second, and more importantly, we also *do* want to preserve those spaces to avoid changing the output of other sections. In all, this patch is a minimal replacement of the fixed-width buffer in `git_config_copy_or_rename_section_in_file()` to instead use a `struct strbuf`. Reported-by: André Baptista <andre@ethiack.com> Reported-by: Vítor Pinho <vitor@ethiack.com> Helped-by: Patrick Steinhardt <ps@pks.im> Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2023-04-11mailmap, quote: move declarations of global vars to correct unitElijah Newren1-0/+1
Since earlier commits removed the inclusion of cache.h from mailmap.c and quote.c, it feels odd to have the extern declarations of global variables in cache.h rather than the actual header included by the source file. Move these global variable extern declarations from cache.h to mailmap.c and quote.c. Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-11pager.h: move declarations for pager.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-11object-name.h: move declarations for object-name.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-11treewide: be explicit about dependence on convert.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-11treewide: be explicit about dependence on advice.hElijah Newren1-0/+1
Dozens of files made use of advice functions, without explicitly including advice.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include advice.h if they are using it. Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-11treewide: be explicit about dependence on trace.h & trace2.hElijah Newren1-0/+1
Dozens of files made use of trace and trace2 functions, without explicitly including trace.h or trace2.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include trace.h or trace2.h if they are using them. Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-06Merge branch 'en/header-split-cleanup'Junio C Hamano1-1/+6
Split key function and data structure definitions out of cache.h to new header files and adjust the users. * en/header-split-cleanup: csum-file.h: remove unnecessary inclusion of cache.h write-or-die.h: move declarations for write-or-die.c functions from cache.h treewide: remove cache.h inclusion due to setup.h changes setup.h: move declarations for setup.c functions from cache.h treewide: remove cache.h inclusion due to environment.h changes environment.h: move declarations for environment.c functions from cache.h treewide: remove unnecessary includes of cache.h wrapper.h: move declarations for wrapper.c functions from cache.h path.h: move function declarations for path.c functions from cache.h cache.h: remove expand_user_path() abspath.h: move absolute path functions from cache.h environment: move comment_line_char from cache.h treewide: remove unnecessary cache.h inclusion from several sources treewide: remove unnecessary inclusion of gettext.h treewide: be explicit about dependence on gettext.h treewide: remove unnecessary cache.h inclusion from a few headers
2023-04-06Merge branch 'gc/config-parsing-cleanup'Junio C Hamano1-245/+343
Config API clean-up to reduce its dependence on static variables * gc/config-parsing-cleanup: config.c: rename "struct config_source cf" config: report cached filenames in die_bad_number() config.c: remove current_parsing_scope config.c: remove current_config_kvi config.c: plumb the_reader through callbacks config.c: create config_reader and the_reader config.c: don't assign to "cf_global" directly config.c: plumb config_source through static fns
2023-04-06Merge branch 'ab/config-multi-and-nonbool'Junio C Hamano1-17/+92
Assorted config API updates. * ab/config-multi-and-nonbool: for-each-repo: with bad config, don't conflate <path> and <cmd> config API: add "string" version of *_value_multi(), fix segfaults config API users: test for *_get_value_multi() segfaults for-each-repo: error on bad --config config API: have *_multi() return an "int" and take a "dest" versioncmp.c: refactor config reading next commit config API: add and use a "git_config_get()" family of functions config tests: add "NULL" tests for *_get_value_multi() config tests: cover blind spots in git_die_config() tests
2023-03-28config.c: rename "struct config_source cf"Glen Choo1-131/+131
The "cf" name is a holdover from before 4d8dd1494e (config: make parsing stack struct independent from actual data source, 2013-07-12), when the struct was named config_file. Since that acronym no longer makes sense, rename "cf" to "cs". In some places, we have "struct config_set cs", so to avoid conflict, rename those "cs" to "set" ("config_set" would be more descriptive, but it's much longer and would require us to rewrap several lines). Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>