summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2025-05-23git-gui: use [is_Windows], not bad _shellpathMark Levedahl1-1/+1
Commit 7d076d56757c (git-gui: handle shell script text filters when loading for blame, 2011-12-09) added open_cmd_pipe, with special handling for Windows detected by seeing that _shellpath does not point to an executable shell. That is bad practice, and is broken by the next commit that assures _shellpath is valid on all platforms. Fix this by using [is_Windows] as done for all Windows specific code. Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23git-gui: _which, only add .exe suffix if not presentMark Levedahl1-0/+3
The _which function finds executables on $PATH, and adds .exe on Windows unless -script was given. However, win32.tcl executes "wscript.exe" and "cscript.exe", both of which fail as _which adds .exe to both. This is already fixed in git-gui released by Git for Windows. Do so here. Signed-off-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23Merge branch 'js/fix-open-exec-2.40.0' into js/fix-open-execTaylor Blau1-85/+163
Branch js/fix-open-exec-2.40.0 converts `open` and `exec` calls to call wrappers that sanitze the command arguments. This side branch updates three `open` calls that are in conflict with the fix in the preceding commit. To keep the intended operation of the 'open' calls, this merge does not try to merge and resolve the conflicts, but ignores the conversions that are brought in by the side branch, taking "ours" side of the code in these three cases. New fixes are the topic of the next commit. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: encode arguments correctly with "open"Avi Halachmi (:avih)1-16/+3
While "exec" uses a normal arguments list which is applied as command + arguments (and redirections, etc), "open" uses a single argument which is this command+arguments, where the command and arguments are a list inside this one argument to "open". Commit bb5cb23 (gitk: prevent overly long command lines 2023-05-08) changed several values from individual arguments in that list (hashes and file names), to a single value which is fed to git via redirection to its stdin using "open" [1]. However, it didn't ensure correctly that this aggregate value in this string is interpreted as a single element in this command+args list. It did just enough so that newlines (which is how these elements are concatenated) don't split this single list element. A followup commit at the same patchset: 7dd272e (gitk: escape file paths before piping to git log 2023-05-08) added a bit more, by escaping backslahes and spaces at the file names, so that at least it doesn't break when such file names get used there. But these are not enough. At the very least tab is missing, and more, and trying to manually escape every possible thing which can affect how this string is interpreted in a list is a sub-par approach. The solution is simply to tell tcl "this is a single list element". which we can do by aggregating this value completely normally (hashes and files separated by newlines), and then do [list $value]. So this is what this commit does, for all 3 places where bb5cb23 changed individual elements into an aggregate value. [1] That was not a fully accurate description. The accurate version is that this string originally included two lists: hashes and files. When used with "open" these lists correctly become the individual elements of these lists, even if they contain spaces etc, so the arguments which were used at this "git" commands were correct. Commit bb5cb23 couldn't use these two lists as-is, because it needed to process the individual elements in them (one element per line of the aggregate value), and the issue is that ensuring this aggregate is indeed interpreted as a single list element was sub-par. Note: all the (double) quotes before/after the modification are not required and with zero effect, even for \n. But this commit preserves the original quoting form intentionally. It can be cleaned up later. Signed-off-by: Avi Halachmi (:avih) <avihpit@yahoo.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'open' arguments: command pipelineJohannes Sixt1-4/+15
As in the earlier commits, introduce a function that constructs a pipeline of commands after sanitizing the arguments. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: collect construction of blameargs into a single conditionalJohannes Sixt1-8/+6
The command line to invoke 'git blame' for a single line is constructed using several if-conditionals, each with the same condition {$from_index new {}}. Merge all of them into a single conditional. This requires to duplicate significant parts of the command, but it helps the next change, where we will have to deal with a nested list structure. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'open' arguments: simple commands, readable and writableJohannes Sixt1-2/+9
As in the previous commits, introduce a function that sanitizes arguments and also keeps the returned file handle writable to pass data to stdin. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'open' arguments: simple commands with redirectionsJohannes Sixt1-5/+13
As in the previous commits, introduce a function that sanitizes arguments intended for the process and in addition allows to pass redirections, which are passed to Tcl's 'open' verbatim. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'open' arguments: simple commandsJohannes Sixt1-26/+33
Tcl 'open' treats the second argument as a command when it begins with |. The remainder of the argument is a list comprising the command and its arguments. It assigns special meaning to these arguments when they begin with a redirection, pipe or background operator. There are a number of invocations of 'open' which construct arguments that are taken from the Git repository or a user input. However, when file names or ref names are taken from the repository, it is possible to find names which have these special forms. They must not be interpreted by 'open' lest it redirects input or output, or attempts to build a pipeline using a command name controlled by the repository. Introduce a helper function that identifies such arguments and prepends "./" to force such a name to be regarded as a relative file name. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'exec' arguments: redirect to processJohannes Sixt1-2/+2
Convert one 'exec' call that sends output to a process (pipeline). Fortunately, the command does not contain any variables. For this reason, just treat it as a "redirection". Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'exec' arguments: redirections and backgroundJohannes Sixt1-3/+2
Convert 'exec' calls that both redirect output to a file and run the process in the background. 'safe_exec_redirect' can take both these "redirections" in the second argument simultaneously. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'exec' arguments: redirectionsJohannes Sixt1-8/+17
As in the previous commits, introduce a function that sanitizes arguments intended for the process and in addition allows to pass redirections verbatim, which are interpreted by Tcl's 'exec'. Redirections can include the background operator '&'. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'exec' arguments: 'eval exec'Johannes Sixt1-6/+6
Convert calls of 'exec' where the arguments are already available in a list and 'eval' is used to unpack the list. Use 'concat' to unite the arguments into a single list before passing them to 'safe_exec'. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: sanitize 'exec' arguments: simple casesJohannes Sixt1-18/+48
Tcl 'exec' assigns special meaning to its argument when they begin with redirection, pipe or background operator. There are a number of invocations of 'exec' which construct arguments that are taken from the Git repository or a user input. However, when file names or ref names are taken from the repository, it is possible to find names with have these special forms. They must not be interpreted by 'exec' lest it redirects input or output, or attempts to build a pipeline using a command name controlled by the repository. Introduce a helper function that identifies such arguments and prepends "./" to force such a name to be regarded as a relative file name. Convert those 'exec' calls where the arguments can simply be packed into a list. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: have callers of diffcmd supply pipe symbol when necessaryJohannes Sixt1-10/+6
Function 'diffcmd' derives which of git diff-files, git diff-index, or git diff-tree must be invoked depending on the ids provided. It puts the pipe symbol as the first element of the returned command list. Note though that of the four callers only two use the command with Tcl 'open' and need the pipe symbol. The other two callers pass the command to Tcl 'exec' and must remove the pipe symbol. Do not include the pipe symbol in the constructed command list, but let the call sites decide whether to add it or not. Note that Tcl 'open' inspects only the first character of the command list, which is also the first character of the first element in the list. For this reason, it is valid to just tack on the pipe symbol with |$cmd and it is not necessary to use [concat | $cmd]. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23gitk: treat file names beginning with "|" as relative pathsJohannes Sixt1-5/+18
The Tcl 'open' function has a vary wide interface. It can open files as well as pipes to external processes. The difference is made only by the first character of the file name: if it is "|", an process is spawned. We have a number of calls of Tcl 'open' that take a file name from the environment in which Gitk is running. Be prepared that insane values are injected. In particular, when we intend to open a file, do not mistake a file name that happens to begin with "|" as a request to run a process. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-22midx docs: clarify tie breakingPhillip Wood1-4/+7
Clarify what happens when an object exists in more than one pack, but not in the preferred pack. "git multi-pack-index repack" relies on ties for objects that are not in the preferred pack being resolved in favor of the newest pack that contains a copy of the object. If ties were resolved in favor of the oldest pack as the current documentation suggests the multi-pack index would not reference any of the objects in the pack created by "git multi-pack-index repack". Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22midx: avoid negative array indexPhillip Wood1-2/+2
nth_midxed_pack_int_id() returns the index of the pack file in the multi pack index's list of packfiles that the specified object. The index is returned as a uint32_t. Storing this in an int will make the index negative if the most significant bit is set. Fix this by using uint32_t as the rest of the code does. This is unlikely to be a practical problem as it requires the multipack index to reference 2^31 packfiles. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22midx repack: avoid potential integer overflow on 64 bit systemsPhillip Wood1-2/+8
On a 64 bit system the calculation p->pack_size * pack_info[i].referenced_objects could overflow. If a pack file contains 2^28 objects with an average compressed size of 1KB then the pack size will be 2^38B. If all of the objects are referenced by the multi-pack index the sum above will overflow. Avoid this by using shifted integer arithmetic and changing the order of the calculation so that the pack size is divided by the total number of objects in the pack before multiplying by the number of objects referenced by the multi-pack index. Using a shift of 14 bits should give reasonable accuracy while avoiding overflow for pack sizes less that 1PB. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22midx repack: avoid integer overflow on 32 bit systemsPhillip Wood2-4/+24
On a 32 bit system "git multi-pack-index --repack --batch-size=120M" failed with fatal: size_t overflow: 6038786 * 1289 The calculation to estimated size of the objects in the pack referenced by the multi-pack-index uses st_mult() to multiply the pack size by the number of referenced objects before dividing by the total number of objects in the pack. As size_t is 32 bits on 32 bit systems this calculation easily overflows. Fix this by using 64bit arithmetic instead. Also fix a potential overflow when caluculating the total size of the objects referenced by the multipack index with a batch size larger than SIZE_MAX / 2. In that case total_size += estimated_size can overflow as both total_size and estimated_size can be greater that SIZE_MAX / 2. This is addressed by using saturating arithmetic for the addition. Although estimated_size is of type uint64_t by the time we reach this sum it is bounded by the batch size which is of type size_t and so casting estimated_size to size_t does not truncate the value. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22diff --no-index: support limiting by pathspecJacob Keller4-20/+150
The --no-index option of git-diff enables using the diff machinery from git while operating outside of a repository. This mode of git diff is able to compare directories and produce a diff of their contents. When operating git diff in a repository, git has the notion of "pathspecs" which can specify which files to compare. In particular, when using git to diff two trees, you might invoke: $ git diff-tree -r <treeish1> <treeish2>. where the treeish could point to a subdirectory of the repository. When invoked this way, users can limit the selected paths of the tree by using a pathspec. Either by providing some list of paths to accept, or by removing paths via a negative refspec. The git diff --no-index mode does not support pathspecs, and cannot limit the diff output in this way. Other diff programs such as GNU difftools have options for excluding paths based on a pattern match. However, using git diff as a diff replacement has several advantages over many popular diff tools, including coloring moved lines, rename detections, and similar. Teach git diff --no-index how to handle pathspecs to limit the comparisons. This will only be supported if both provided paths are directories. For comparisons where one path isn't a directory, the --no-index mode already has some DWIM shortcuts implemented in the fixup_paths() function. Modify the fixup_paths function to return 1 if both paths are directories. If this is the case, interpret any extra arguments to git diff as pathspecs via parse_pathspec. Use parse_pathspec to load the remaining arguments (if any) to git diff --no-index as pathspec items. Disable PATHSPEC_ATTR support since we do not have a repository to do attribute lookup. Disable PATHSPEC_FROMTOP since we do not have a repository root. All pathspecs are treated as rooted at the provided comparison paths. After loading the pathspec data, calculate skip offsets for skipping past the root portion of the paths. This is required to ensure that pathspecs start matching from the provided path, rather than matching from the absolute path. We could instead pass the paths as prefix values to parse_pathspec. This is slightly problematic because the paths come from the command line and don't necessarily have the proper trailing slash. Additionally, that would require parsing pathspecs multiple times. Pass the pathspec object and the skip offsets into queue_diff, which in-turn must pass them along to read_directory_contents. Modify read_directory_contents to check against the pathspecs when scanning the directory. Use the skip offset to skip past the initial root of the path, and only match against portions that are below the intended directory structure being compared. The search algorithm for finding paths is recursive with read_dir. To make pathspec matching work properly, we must set both DO_MATCH_DIRECTORY and DO_MATCH_LEADING_PATHSPEC. Without DO_MATCH_DIRECTORY, paths like "a/b/c/d" will not match against pathspecs like "a/b/c". This is usually achieved by setting the is_dir parameter of match_pathspec. Without DO_MATCH_LEADING_PATHSPEC, paths like "a/b/c" would not match against pathspecs like "a/b/c/d". This is crucial because we recursively iterate down the directories. We could simply avoid checking pathspecs at subdirectories, but this would force recursion down directories which would simply be skipped. If we always passed DO_MATCH_LEADING_PATHSPEC, then we will incorrectly match in certain cases such as matching 'a/c' against ':(glob)**/d'. The match logic will see that a matches the leading part of the **/ and accept this even tho c doesn't match. To avoid this, use the match_leading_pathspec() variant recently introduced. This sets both flags when is_dir is set, but leaves them both cleared when is_dir is 0. Add test cases and documentation covering the new functionality. Note for the documentation I opted not to move the placement of '--' which is sometimes used to disambiguate arguments. The diff --no-index mode requires exactly 2 arguments determining what to compare. Any additional arguments are interpreted as pathspecs and must come afterwards. Use of '--' would not actually disambiguate anything, since there will never be ambiguity over which arguments represent paths or pathspecs. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22pathspec: add flag to indicate operation without repositoryJacob Keller3-4/+16
A following change will add support for pathspecs to the git diff --no-index command. This mode of git diff does not load any repository. Add a new PATHSPEC_NO_REPOSITORY flag indicating that we're parsing pathspecs without a repository. Both PATHSPEC_ATTR and PATHSPEC_FROMTOP require a repository to function. Thus, verify that both of these are set in magic_mask to ensure they won't be accepted when PATHSPEC_NO_REPOSITORY is set. Check PATHSPEC_NO_REPOSITORY when warning about paths outside the directory tree. When the flag is set, do not look for a git repository when generating the warning message. Finally, add a BUG in match_pathspec_item if the istate is NULL but the pathspec has PATHSPEC_ATTR set. Callers which support PATHSPEC_ATTR should always pass a valid istate, and callers which don't pass a valid istate should have set PATHSPEC_ATTR in the magic_mask field to disable support for attribute-based pathspecs. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22pathspec: add match_leading_pathspec variantJacob Keller2-0/+16
The do_match_pathspec() function has the DO_MATCH_LEADING_PATHSPEC option to allow pathspecs to match when matching "src" against a pathspec like "src/path/...". This support is not exposed by match_pathspec, and the internal flags to do_match_pathspec are not exposed outside of dir.c The upcoming support for pathspecs in git diff --no-index need the LEADING matching behavior when iterating down through a directory with readdir. We could try to expose the match_pathspec_with_flags to the public API. However, DO_MATCH_EXCLUDES really shouldn't be public, and its a bit weird to only have a few of the flags become public. Instead, add match_leading_pathspec() as a function which sets both DO_MATCH_DIRECTORY and DO_MATCH_LEADING_PATHSPEC when is_dir is true. This will be used in a following change to support pathspec matching in git diff --no-index. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-22Merge branch 'top-panel-search-highlight' of github.com:bnfour/gitkJohannes Sixt1-2/+2
* 'top-panel-search-highlight' of github.com:bnfour/gitk: gitk: do not hard-code color of search results in commit list Signed-off-by: Johannes Sixt <j6t@kdbg.org>
2025-05-21name-hash: don't add sparse directories in threaded lazy initAlex Mironov1-2/+4
Ensure that logic added in 5f11669586 (name-hash: don't add directories to name_hash, 2021-04-12) also applies in multithreaded hashtable init path. As per the original single-threaded change above: sparse directory entries represent a directory that is outside the sparse-checkout definition. These are not paths to blobs, so should not be added to the name_hash table. Instead, they should be added to the directory hashtable when 'ignore_case' is true. Add a condition to avoid placing sparse directories into the name_hash hashtable. This avoids filling the table with extra entries that will never be queried. Signed-off-by: Alex Mironov <alexandrfox@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-20t: remove unexpected SANITIZE_LEAK variablesKarthik Nayak3-8/+0
As of 1fc7ddf35b (test-lib: unconditionally enable leak checking, 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the leak checks are enabled by default. However, some newly added tests include them by mistake. Let's clean this up. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Acked-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-20builtin/receive-pack: add option to skip connectivity checkJustin Tobler3-18/+56
During git-receive-pack(1), connectivity of the object graph is validated to ensure that the received packfile does not leave the repository in a broken state. This is done via git-rev-list(1) and walking the objects, which can be expensive for large repositories. Generally, this check is critical to avoid an incomplete received packfile from corrupting a repository. Server operators may have additional knowledge though around exactly how Git is being used on the server-side which can be used to facilitate more efficient connectivity computation of incoming objects. For example, if it can be ensured that all objects in a repository are connected and do not depend on any missing objects, the connectivity of newly written objects can be checked by walking the object graph containing only the new objects from the updated tips and identifying the missing objects which represent the boundary between the new objects and the repository. These boundary objects can be checked in the canonical repository to ensure the new objects connect as expected and thus avoid walking the rest of the object graph. Git itself cannot make the guarantees required for such an optimization as it is possible for a repository to contain an unreachable object that references a missing object without the repository being considered corrupt. Introduce the --skip-connectivity-check option for git-receive-pack(1) which bypasses this connectivity check to give more control to the server-side. Note that without proper server-side validation of newly received objects handled outside of Git, usage of this option risks corrupting a repository. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-20t5410: test receive-pack connectivity checkJustin Tobler2-2/+23
As part of git-recieve-pack(1), the connectivity of objects is checked. Add a test validating that git-receive-pack(1) fails due to an incoming packfile that would leave the repository with missing objects. Instead of creating a new test file, "t5410" is generalized for receive-pack testing. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-20Merge branch 'yh/fix-non-themed-combobox'Johannes Sixt1-4/+2
* yh/fix-non-themed-combobox: gitk: Legacy widgets doesn't have combobox
2025-05-19The sixteenth batchJunio C Hamano1-0/+15
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19Merge branch 'ps/reftable-read-block-perffix'Junio C Hamano4-17/+19
Performance regression in not-yet-released code has been corrected. * ps/reftable-read-block-perffix: reftable: fix perf regression when reading blocks of unwanted type
2025-05-19Merge branch 'ly/reftable-writer-leakfix'Junio C Hamano1-2/+6
Leakfix. * ly/reftable-writer-leakfix: reftable/writer: fix memory leak when `writer_index_hash()` fails reftable/writer: fix memory leak when `padded_write()` fails
2025-05-19Merge branch 'jk/oidmap-cleanup'Junio C Hamano11-18/+21
Code cleanup. * jk/oidmap-cleanup: raw_object_store: drop extra pointer to replace_map oidmap: add size function oidmap: rename oidmap_free() to oidmap_clear()
2025-05-19Merge branch 'rc/t1001-test-path-is-file'Junio C Hamano1-1/+1
Test update. * rc/t1001-test-path-is-file: t1001: replace 'test -f' with 'test_path_is_file'
2025-05-19Merge branch 'ly/am-split-stgit-leakfix'Junio C Hamano1-1/+3
Leakfix. * ly/am-split-stgit-leakfix: builtin/am: fix memory leak in `split_mail_stgit_series`
2025-05-19Merge branch 'bc/make-avoid-unneeded-rebuild-with-compdb-dir'Junio C Hamano1-1/+1
Build performance fix. * bc/make-avoid-unneeded-rebuild-with-compdb-dir: Makefile: avoid constant rebuilds with compilation database
2025-05-19Merge branch 'ag/doc-send-email'Junio C Hamano3-9/+66
The `send-email` documentation has been updated with OAuth2.0 related examples. * ag/doc-send-email: docs: add credential helper for outlook and gmail in OAuth list of helpers docs: improve send-email documentation send-mail: improve checks for valid_fqdn
2025-05-19Merge branch 'sc/bundle-uri-use-all-refs-in-bundle'Junio C Hamano3-94/+124
Bundle-URI feature did not use refs recorded in the bundle other than normal branches as anchoring points to optimize the follow-up fetch during "git clone"; now it is told to utilize all. * sc/bundle-uri-use-all-refs-in-bundle: bundle-uri: add test for bundle-uri clones with tags bundle-uri: copy all bundle references ino the refs/bundle space
2025-05-19Merge branch 'pw/sequencer-reflog-use-after-free'Junio C Hamano2-60/+67
Use-after-free fix in the sequencer. * pw/sequencer-reflog-use-after-free: sequencer: rework reflog message handling sequencer: move reflog message functions
2025-05-19configure.ac: upgrade to a compilation check for sysinfoRamsay Jones1-3/+22
Commit f5e3c6c57d ("meson: do a full usage-based compile check for sysinfo", 2025-04-25) updated the 'sysinfo()' check, as part of the meson build, due to the failure of the check on Solaris. Prior to that commit, the meson build only checked the availability of the '<sys/sysinfo.h>' header file. On Solaris, both the header and the 'sysinfo()' function exist, but are completely unrelated to the same function on Linux (and cygwin). Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf build. This check looked for the 'sysinfo()' function itself, rather than just the header, but it will fail (incorrectly set HAVE_SYSINFO) for the same reason. In order to correctly identify the 'sysinfo()' function we require as part of 'git-gc' (used in the 'total_ram() function), we also upgrade to a compilation check, in a similar way to the meson commit. Note that since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields of the 'struct sysinfo' are used, so the new check includes both of those fields in the compile check. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19meson.build: correct setting of GIT_EXEC_PATHRamsay Jones1-2/+10
For the non-'runtime prefix' case, the meson build sets the GIT_EXEC_PATH build variable to an absolute path equivalent to <prefix>/libexec/git-core. In comparison, the default make build sets it to a relative path equivalent to 'libexec/git-core'. Indeed, the make build requires the use of some means outside of the Makefile (eg. config.mak[.*] or the command-line) to set GIT_EXEC_PATH to anything other than 'libexec/git-core'. For example, the make invocation: $ make gitexecdir=/some/other/bin all install will build git with GIT_EXEC_PATH set to '/some/other/bin' and install the 'library' executables to that location. However, without setting the 'gitexecdir' make variable, irrespective of the 'runtime prefix' setting, the GIT_EXEC_PATH is always set to 'libexec/git-core'. The meson built-in 'libexecdir' option can be used to provide a similar configurability. The default value for the option is 'libexec'. Attempting to set the option to '' on the command-line, will reset it to the '.' string, presumably to ensure a relative path value. This commit allows the meson build, similar to the above, to configure the project like: $ meson setup --buildtype=debugoptimized -Dprefix=$HOME -Dpcre2=disabled \ -Dlibexecdir=/some/other/bin build so that the GIT_EXEC_PATH is set to '/some/other/bin'. Absent the -Dlibexecdir argument, the GIT_EXEC_PATH is set to 'libexec/git-core'. In order to correct the value of GIT_EXEC_PATH, default the value to the static string value 'libexec/git-core', and only override if the value of the 'libexecdir' option has a value different to 'libexec' or '.'. Also, like the Makefile, add a check for an absolute path when the runtime prefix option is true (and if so, error out). Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19meson: correct path to system config/attribute filesRamsay Jones2-6/+18
The path to the system-wide config and attributes files are not being set correctly in the meson build. Unless explicitly overridden on the command line during setup, the 'gitconfig' and 'gitattributes' options are defaulting to absolute paths in the '/etc' system directory. This is only appropriate if the <prefix> is set specifically to '/usr'. The directory in which these files are placed is generally referred to as the 'system configuration directory' or 'sysconfdir' for short. When the prefix is '/usr' then the sysconfdir is usually set to '/etc', but any other value for prefix results in the relative directory value 'etc' instead. (eg if prefix is '/usr/local', then the 'etc' relative value results in a system configuration directory of '/usr/local/etc'). When setting the 'sysconfdir' builtin option value, the meson system uses exactly this algorithm, so we can use get_option('sysconfdir') directly when setting the (non-overridden) build variables. In order to allow for overriding from the command line, remove the default values specified for the 'gitconfig' and 'gitattributes' options in the 'meson_options.txt' file. This allows the user to specify any pathname for those options, while being able to test for the unset (empty) value. An absolute pathname will be used unchanged and a relative pathname will be appended to '<prefix>/'. These values are then used to set the 'ETC_GITCONFIG' and 'ETC_GITATTRIBUTES' build variables which are, in turn, passed to the compiler as '-D' arguments. When the 'gitconfig' or 'gitattributes' options are not used, then use the built-in 'sysconfdir' and set the ETC_GITCONFIG build variable to the string "<sysconfdir>/gitconfig". Similarly, set ETC_ATTRIBUTES to "<sysconfdir>/gitattributes". Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19meson: correct install location of YAML.pmRamsay Jones1-1/+1
When executing an 'meson install' the YAML.pm file is incorrectly placed in the <prefix>/share/perl5/Git/SVN directory. The YAML.pm file should be placed in a 'Memoize' subdirectory instead. In order to correct the location, update the 'install_dir' of the relevant target in the 'perl/Git/SVN/Memoize/meson.build' file. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19meson.build: quote the GITWEBDIR build configurationRamsay Jones1-1/+1
The build configuration options with (non-empty) values, for example filesystem paths potentially containing spaces, have been set using the '.set_quoted()' method. However, the GITWEBDIR value has been set using the '.set()' method instead. In order to correctly quote the GITWEBDIR value, replace the '.set()' method with '.set_quoted()'. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19meson: reformat default options to workaround bug in `meson configure`Eli Schwartz1-8/+6
Since 13cb20fc46 ("meson: fix compilation with Visual Studio", 2025-01-22) it has not been possible to list build options via `meson configure`. This is due to Meson's static analysis of build options failing to handle constant folding, and thinking we set a totally invalid default `-std=`. This is reported upstream but we anyways need to work with existing versions. It turns out there is a simple solution: turn the entire default option into a conditional branch, which means Meson sees either nothing, or everything. As a result, Git users can once again see pretty-printed options before building. Reported-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Bug: https://github.com/mesonbuild/meson/issues/14623 Signed-off-by: Eli Schwartz <eschwartz@gentoo.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19receive-pack: use batched reference updatesKarthik Nayak3-23/+56
The reference updates performed as a part of 'git-receive-pack(1)', take place one at a time. For each reference update, a new transaction is created and committed. This is necessary to ensure we can allow individual updates to fail without failing the entire command. The command also supports an 'atomic' mode, which uses a single transaction to update all of the references. But this mode has an all-or-nothing approach, where if a single update fails, all updates would fail. In 23fc8e4f61 (refs: implement batch reference update support, 2025-04-08), we introduced a new mechanism to batch reference updates. Under the hood, this uses a single transaction to perform a batch of reference updates, while allowing only individual updates to fail. Utilize this newly introduced batch update mechanism in 'git-receive-pack(1)'. This provides a significant bump in performance, especially when dealing with repositories with large number of references. With the reftable backend there is a 18x performance improvement, when performing receive-pack with 10000 refs: Benchmark 1: receive: many refs (refformat = reftable, refcount = 10000, revision = master) Time (mean ± σ): 4.276 s ± 0.078 s [User: 0.796 s, System: 3.318 s] Range (min … max): 4.185 s … 4.430 s 10 runs Benchmark 2: receive: many refs (refformat = reftable, refcount = 10000, revision = HEAD) Time (mean ± σ): 235.4 ms ± 6.9 ms [User: 75.4 ms, System: 157.3 ms] Range (min … max): 228.5 ms … 254.2 ms 11 runs Summary receive: many refs (refformat = reftable, refcount = 10000, revision = HEAD) ran 18.16 ± 0.63 times faster than receive: many refs (refformat = reftable, refcount = 10000, revision = master) In similar conditions, the files backend sees a 1.21x performance improvement: Benchmark 1: receive: many refs (refformat = files, refcount = 10000, revision = master) Time (mean ± σ): 1.121 s ± 0.021 s [User: 0.128 s, System: 0.975 s] Range (min … max): 1.097 s … 1.156 s 10 runs Benchmark 2: receive: many refs (refformat = files, refcount = 10000, revision = HEAD) Time (mean ± σ): 927.9 ms ± 22.6 ms [User: 99.0 ms, System: 815.2 ms] Range (min … max): 903.1 ms … 978.0 ms 10 runs Summary receive: many refs (refformat = files, refcount = 10000, revision = HEAD) ran 1.21 ± 0.04 times faster than receive: many refs (refformat = files, refcount = 10000, revision = master) As using batched updates requires the error handling to be moved to the end of the flow, create and use a 'struct strset' to track the failed refs and attribute the correct errors to them. This change also uncovers an issue when a client provides multiple updates to the same reference. For example: $ git send-pack remote.git A:foo B:foo Enumerating objects: 3, done. Counting objects: 100% (3/3), done. Delta compression using up to 20 threads Compressing objects: 100% (2/2), done. Writing objects: 100% (3/3), 226 bytes | 226.00 KiB/s, done. Total 3 (delta 1), reused 0 (delta 0), pack-reused 0 (from 0) remote: error: cannot lock ref 'refs/heads/foo': reference already exists To remote.git ! [remote rejected] A -> foo (failed to update ref) ! [remote failure] B -> foo (remote failed to report status) As you can see, the remote runs into an error because it cannot lock the target reference for the second update. Furthermore, the remote complains that the first update has been rejected whereas the second update didn't receive any status update because we failed to lock it. Reading this status message alone a user would probably expect that `foo` has not been updated at all. But that's not the case: while we claim that the ref wasn't updated, it surprisingly points to `A` now. One could argue that this is merely an error in how we report the result of this push. But ultimately, the user's request itself is already broken and doesn't make any sense in the first place and cannot ever lead to a sensible outcome that honors the full request. The conversion to batched transactions fixes the issue because we now try to queue both updates in the same transaction. As such, the transaction itself will notice this conflict and refuse the update altogether before we commit any of the values. Note that this requires changes to a couple of tests in t5408 that happened to exercise this behaviour. Given that the generated output is misleading and given that the user request cannot ever be fully honored this really feels more like a bug than properly designed behaviour. As such, changing the behaviour feels like the right thing to do. Since now reference updates are batched, the 'reference-transaction' hook will be invoked with all updates together. Currently git will 'die' when the hook returns with a non-zero exit status in the 'prepared' stage. For 'git-receive-pack(1)', this allowed users to reject an individual reference update, git would have applied previous updates but immediately abort further execution. This is definitely an incorrect usage of this hook, since the right place to do this would be the 'update' hook. This patch retains the latter behavior, but 'reference-transaction' hook now changes to a all-or-nothing behavior when a non-zero exit status is returned in the 'prepared' stage, since batch updates use a transaction under the hood. This explains the change in 't1416'. Helped-by: Jeff King <peff@peff.net> Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19send-pack: fix memory leak around duplicate refsKarthik Nayak2-0/+13
The 'git-send-pack(1)' allows users to push objects to a remote repository and explicitly list the references to be pushed. The status of each reference pushed is captured into a list mapped by refname. If a reference fails to be updated, its error message is captured in the `ref->remote_status` field. While the command allows duplicate ref inputs, the list doesn't accommodate this behavior as a particular refname is linked to a single `struct ref*` element. So if the user inputs a reference twice like: git send-pack remote.git A:foo B:foo where the user is trying to update the same reference 'foo' twice and the reference fails to be updated, we first fill `ref->remote_status` with error message for the input 'A:foo' then we override the same field with the error message for 'B:foo'. This override happens without first free'ing the previous value. Fix this leak. The current tests already incorporate the above example, but in the test 'A:foo' succeeds while 'B:foo' fails, meaning that the memory leak isn't triggered. Add a new test with multiple duplicates. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19fetch: use batched reference updatesKarthik Nayak1-54/+73
The reference updates performed as a part of 'git-fetch(1)', take place one at a time. For each reference update, a new transaction is created and committed. This is necessary to ensure we can allow individual updates to fail without failing the entire command. The command also supports an '--atomic' mode, which uses a single transaction to update all of the references. But this mode has an all-or-nothing approach, where if a single update fails, all updates would fail. In 23fc8e4f61 (refs: implement batch reference update support, 2025-04-08), we introduced a new mechanism to batch reference updates. Under the hood, this uses a single transaction to perform a batch of reference updates, while allowing only individual updates to fail. Utilize this newly introduced batch update mechanism in 'git-fetch(1)'. This provides a significant bump in performance, especially when dealing with repositories with large number of references. Adding support for batched updates is simply modifying the flow to also create a batch update transaction in the non-atomic flow. With the reftable backend there is a 22x performance improvement, when performing 'git-fetch(1)' with 10000 refs: Benchmark 1: fetch: many refs (refformat = reftable, refcount = 10000, revision = master) Time (mean ± σ): 3.403 s ± 0.775 s [User: 1.875 s, System: 1.417 s] Range (min … max): 2.454 s … 4.529 s 10 runs Benchmark 2: fetch: many refs (refformat = reftable, refcount = 10000, revision = HEAD) Time (mean ± σ): 154.3 ms ± 17.6 ms [User: 102.5 ms, System: 56.1 ms] Range (min … max): 145.2 ms … 220.5 ms 18 runs Summary fetch: many refs (refformat = reftable, refcount = 10000, revision = HEAD) ran 22.06 ± 5.62 times faster than fetch: many refs (refformat = reftable, refcount = 10000, revision = master) In similar conditions, the files backend sees a 1.25x performance improvement: Benchmark 1: fetch: many refs (refformat = files, refcount = 10000, revision = master) Time (mean ± σ): 605.5 ms ± 9.4 ms [User: 117.8 ms, System: 483.3 ms] Range (min … max): 595.6 ms … 621.5 ms 10 runs Benchmark 2: fetch: many refs (refformat = files, refcount = 10000, revision = HEAD) Time (mean ± σ): 485.8 ms ± 4.3 ms [User: 91.1 ms, System: 396.7 ms] Range (min … max): 477.6 ms … 494.3 ms 10 runs Summary fetch: many refs (refformat = files, refcount = 10000, revision = HEAD) ran 1.25 ± 0.02 times faster than fetch: many refs (refformat = files, refcount = 10000, revision = master) With this we'll either be using a regular transaction or a batch update transaction. This helps cleanup some code which is no longer needed as we'll now always have some type of 'ref_transaction' object being propagated. One big change is that earlier, each individual update would propagate a failure. Whereas now, the `ref_transaction_for_each_rejected_update` function is called at the end of the flow to capture the exit status for 'git-fetch(1)' and also to print F/D conflict errors. This does change the order of the errors being printed, but the behavior stays the same. Since transaction errors are now explicitly defined as part of 76e760b999 (refs: introduce enum-based transaction error types, 2025-04-08), utilize them and get rid of custom errors defined within 'builtin/fetch.c'. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19refs: add function to translate errors to stringsKarthik Nayak3-24/+26
The commit 76e760b999 (refs: introduce enum-based transaction error types, 2025-04-08) introduced enum-based transaction error types. The refs transaction logic was also modified to propagate these errors. For clients of the ref transaction system, it would be beneficial to provide human readable messages for these errors. There is already an existing mapping in 'builtin/update-ref.c', move it to 'refs.c' as `ref_transaction_error_msg()` and use the same within the 'builtin/update-ref.c'. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19docs: replace git_config to repo_configK Jayatheerth1-9/+10
Since this document was written, the built-in API has been updated a few times, but the document was left stale. Adjust to the current best practices by calling repo_config() on the repository instance the subcommand implementation receives as a parameter, instead of calling git_config() that used to be the common practice. Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>