Commit Graph

77950 Commits (v2.51.0-rc0)

Author SHA1 Message Date
Johannes Sixt 67a128b91e gitk: sanitize 'open' arguments: revisit recently updated 'open' calls
The previous commits bb5cb23daf (gitk: prevent overly long command
lines, 2023-01-24) rewrote a set of the 'open' calls substantially.
These were then later updated by 7dd272eca1 (gitk: escape file paths
before piping to git log, 2023-01-24) and d5d1b91e5327 (gitk: encode
arguments correctly with "open", 2025-03-07). In the preceding merge,
the conversions to a safe_open variant were undone to ensure that the
principal operation of the new 'open' calls is not modified by accident.

Since the 'open' calls now pass a redirection from a Tcl string as
stdin, convert the calls to 'safe_open_command_redirect'.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Johannes Sixt 074c2b9d7c git-gui: use git_read in githook_read
0730a5a3a5 ("git-gui - use git-hook, honor core.hooksPath", 2023-09-17)
rewrote githook_read to use `git hook` to run a hook script. The code
that was replaced discovered the hook script file manually and invoked
it using function _open_stdout_stderr. After the rewrite, this function
is still invoked, but it calls into `git` instead of the hook scripts.

Notice though, that we have function git_read that invokes git and
prepares a pipe for the caller to read from. Replace the implementation
of githook_read to be just a wrapper around git_read. This unifies the
way in which the git executable is invoked. git_read ultimately also
calls into _open_stdout_stderr, but it modifies the path to the git
executable before doing so.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl 384b1409e8 git-gui: sanitize $PATH on all platforms
Since 8f23432b38 (windows: ignore empty `PATH` elements, 2022-11-23),
git-gui removes empty elements from $PATH, and a prior commit made this
remove all non-absolute elements from $PATH. But, this happens only on
Windows. Unsafe $PATH elements in $PATH are possible on all platforms.
Let's sanitize $PATH on all platforms to have consistent behavior. If a
user really wants the current repository on $PATH, they can add its
absolute name to $PATH.

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-23 17:04:23 -04:00
Johannes Sixt aa42e87ef4 git-gui: break out a separate function git_read_nice
There are two callers of git_read that request special treatment using
option --nice. Rewrite them to call a new function git_read_nice that
does the special treatment. Now we can remove all option treatment from
git_read.

git_write has the same capability, but there are no callers that
request --nice. Remove the feature without substitution.

This is a preparation for a later change where we want to make git_read
and friends non-variadic. Then it cannot have optional arguments.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl 8fe7861c51 git-gui: assure PATH has only absolute elements.
Since 8f23432b38 (windows: ignore empty `PATH` elements, 2022-11-23),
git-gui excises all empty paths from $PATH, but still allows '.' or
other relative paths, which can also allow executing code from the
repository. Let's remove anything except absolute elements. While here,
let's remove duplicated elements, which are very common on Windows:
only the first such item can do anything except waste time repeating a
search.

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-23 17:04:23 -04:00
Johannes Sixt 23ba43256b git-gui: remove option --stderr from git_read
Some callers of git_read want to redirect stderr of the invoked command
to stdout.  The function offers option --stderr for this purpose.
However, the option only appends 2>@1 to the commands.  The callers can
do that themselves. In lib/console.tcl we even have a caller that
already knew implictly what --stderr does behind the scenes.

This is a preparation for a later change where we want to make git_read
non-variadic. Then it cannot have optional leading arguments.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl 676c49583f git-gui: cleanup git-bash menu item
git-gui on Git for Windows creates a menu item to start a git-bash
session for the current repository. This menu-item works as desired when
git-gui is installed in the Git for Windows (g4w) distribution, but
not when run from a different location such as normally done in
development. The reason is that git-bash's location is known to be
'/git-bash' in the Unix pathname space known to MSYS, but this is not
known in the Windows pathname space. Instead, git-gui derives a pathname
for git-bash assuming it is at a known relative location.

If git-gui is run from a different directory than assumed in g4w, the
relative location changes, and git-gui resorts to running a generic bash
login session in a Windows console.

But, the MSYS system underlying Git for Windows includes the 'cygpath'
utility to convert between Unix and Windows pathnames. Let's use this so
git-bash's Windows pathname is determined directly from /git-bash.

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-23 17:04:23 -04:00
Johannes Sixt e883ceb122 git-gui: sanitize 'exec' arguments: background
As in the previous commits, introduce a function that sanitizes
arguments intended for the process, but runs the process in the
background. Convert 'exec' calls to use this new function.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl 00c7aa86e9 git-gui: avoid auto_execok in do_windows_shortcut
git-gui on Windows uses auto_execok to locate git-gui.exe,
which performs the same flawed search as does the builtin exec.
Use _which instead, performing a safe PATH lookup.

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-23 17:04:23 -04:00
Johannes Sixt 4f3e0a4bce git-gui: sanitize 'exec' arguments: simple cases
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 that 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.

Note that most commands containing the word 'exec' route through
console::exec or console::chain, which we will treat in another commit.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl 411cd493cb git-gui: avoid auto_execok for git-bash menu item
On Windows, git-gui offers to open a git-bash session for the current
repository from the menu, but uses [auto_execok start] to get the
command to actually run that shell.

The code for auto_execok, in /usr/share/tcl8.6/tcl.init, has 'start' in
the 'shellBuiltins' list for cmd.exe on Windows: as a result,
auto_execok does not actually search for start, meaning this usage is
technically ok with auto_execok now.  However, leaving this use of
auto_execok in place will just induce confusion about why a known unsafe
function is being used on Windows. Instead, let's switch to using our
known safe _which function that looks only in $PATH, excluding the
current working directory.

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-23 17:04:23 -04:00
Johannes Sixt c2e8904258 git-gui: treat file names beginning with "|" as relative paths
The Tcl 'open' function has a very 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 "|", a process is spawned.

We have a number of calls of Tcl 'open' that take a file name from the
environment in which Git GUI is running. Be prepared that insane values
are injected. In particular, when we intend to open a file, do not take
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-23 17:04:23 -04:00
Mark Levedahl 2c66188b12 git-gui: remove unused proc is_shellscript
Commit 7d076d5675 (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added is_shellscript to test if a file
is executable by the shell, used only when searching for textconv
filters. The previous commit rearranged the tests for finding such
filters, and removed the only user of is_shellscript. Remove this
function.

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-23 17:04:23 -04:00
Johannes Sixt 8255167b26 git-gui: remove git config --list handling for git < 1.5.3
git-gui uses `git config --null --list` to parse configuration. Git
versions prior to 1.5.3 do not have --null and need different treatment.
Nobody should be using such an old version anymore. (Moreover, since
0730a5a3a, git-gui requires git v2.36 or later). Keep only the code for
modern Git.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Johannes Sixt 4eb9b1157b git-gui: remove special treatment of Windows from open_cmd_pipe
Commit 7d076d5675 (git-gui: handle shell script text filters when
loading for blame, 2011-12-09) added open_cmd_pipe to run text
conversion in support of blame, with special handling for shell
scripts on Windows. To determine whether the command is a shell
script, 'lindex' is used to pick off the first token from the command.
However, cmd is actually a command string taken from .gitconfig
literally and is not necessarily a syntactically correct Tcl list.
Hence, it cannot be processed by 'lindex' and 'lrange' reliably.
Pass the command string to the shell just like on non-Windows
platforms to avoid the potentially incorrect treatment.

A use of 'auto_execok' is removed by this change. This function is
dangerous on Windows, because it searches programs in the current
directory. Delegating the path lookup to the shell is safe, because
/bin/sh and /bin/bash follow POSIX on all platforms, including the
Git for Windows port.

A possible regression is that the old code, given filter command of
'foo', could find 'foo.bat' as a script, and not just bare 'foo', or
'foo.exe'.  This rewrite requires explicitly giving the suffix if it is
not .exe.

This part of Git GUI can be exercised using

    git gui blame -- some.file

while some.file has a textconv filter configured and has unstaged
modifications.

Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl f9a2e8a38f git-gui: remove HEAD detachment implementation for git < 1.5.3
git-gui provides an implementation to detach HEAD on Git versions prior
to 1.5.3.  Nobody should be using such an old version anymore.
(Moreover, since 0730a5a3a, git-gui requires git v2.36 or later).
Keep only the code for modern Git.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
[j6t: message tweaked]
Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Signed-off-by: Taylor Blau <me@ttaylorr.com>
2025-05-23 17:04:23 -04:00
Mark Levedahl 02dd866ba9 git-gui: use only the configured shell
git-gui has a few places where a bare "sh" is passed to exec, meaning
that the first instance of "sh" on $PATH will be used rather than the
shell configured. This violates expectations that the configured shell
is being used. Let's use [shellpath] everywhere.

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-23 17:04:23 -04:00
Mark Levedahl 4774c704d2 git-gui: remove Tcl 8.4 workaround on 2>@1 redirection
Since b792230 ("git-gui: Show a progress meter for checking out files",
2007-07-08), git-gui includes a workaround for Tcl that does not support
using 2>@1 to redirect stderr to stdout. Tcl added such support in
8.4.7, released in 2004, and this is fully supported in all 8.5
releases.

As git-gui has a hard-coded requirement for Tcl >= 8.5, the workaround
is no longer needed. Delete it.

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-23 17:04:23 -04:00
Mark Levedahl 10637fc327 git-gui: make _shellpath usable on startup
Since commit d5257fb3c1 (git-gui: handle textconv filter on
Windows and in development, 2010-08-07), git-gui will search for a
usable shell if _shellpath is not configured, and on Windows may
resort to using auto_execok to find 'sh'. While this was intended for
development use, checks are insufficient to assure a proper
configuration when deployed where _shellpath is always set, but might
not give a usable shell.

Let's make this more robust by only searching if _shellpath was not
defined, and then using only our restricted search functions.
Furthermore, we should convert to a Windows path on Windows.  Always
check for a valid shell on startup, meaning an absolute path to an
executable, aborting if these conditions are not met.

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-23 17:04:23 -04:00
Johannes Sixt dcda716dbc Merge branch 'ml/git-gui-exec-path-fix'
* ml/git-gui-exec-path-fix:
  git-gui - use git-hook, honor core.hooksPath
  git-gui - re-enable use of hook scripts
2025-05-23 17:04:23 -04:00
Mark Levedahl c5c32781c9 git-gui: use [is_Windows], not bad _shellpath
Commit 7d076d5675 (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-23 17:04:23 -04:00
Mark Levedahl 37b9230226 git-gui: _which, only add .exe suffix if not present
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-23 17:04:23 -04:00
Taylor Blau d7bc50cece Merge branch 'js/fix-open-exec-2.40.0' into js/fix-open-exec
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-23 17:04:21 -04:00
Avi Halachmi (:avih) 8e3070aa5e gitk: encode arguments correctly with "open"
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-23 17:03:30 -04:00
Johannes Sixt 026c397d91 gitk: sanitize 'open' arguments: command pipeline
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-23 17:03:30 -04:00
Johannes Sixt 79a3ef5314 gitk: collect construction of blameargs into a single conditional
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-23 17:03:30 -04:00
Johannes Sixt 2aeb4484a0 gitk: sanitize 'open' arguments: simple commands, readable and writable
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-23 17:03:30 -04:00
Johannes Sixt 42a64b41a7 gitk: sanitize 'open' arguments: simple commands with redirections
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-23 17:03:30 -04:00
Johannes Sixt fe32bf31b8 gitk: sanitize 'open' arguments: simple commands
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-23 17:03:30 -04:00
Johannes Sixt 30846b4306 gitk: sanitize 'exec' arguments: redirect to process
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-23 17:03:30 -04:00
Johannes Sixt 7a0493edda gitk: sanitize 'exec' arguments: redirections and background
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-23 17:03:30 -04:00
Johannes Sixt 6b631ee8ed gitk: sanitize 'exec' arguments: redirections
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-23 17:03:30 -04:00
Johannes Sixt 88139a617f gitk: sanitize 'exec' arguments: 'eval exec'
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-23 17:03:30 -04:00
Johannes Sixt 9f0d1c2f7d gitk: sanitize 'exec' arguments: simple cases
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-23 17:03:30 -04:00
Johannes Sixt 6eb797f5d1 gitk: have callers of diffcmd supply pipe symbol when necessary
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-23 17:03:30 -04:00
Johannes Sixt b966b738e1 gitk: treat file names beginning with "|" as relative paths
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-23 17:03:30 -04:00
Phillip Wood 70b128c576 midx docs: clarify tie breaking
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-22 14:48:37 -07:00
Phillip Wood 3aa98a61da midx: avoid negative array index
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-22 14:48:37 -07:00
Phillip Wood f874c0ed90 midx repack: avoid potential integer overflow on 64 bit systems
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-22 14:48:36 -07:00
Phillip Wood b103881d4f midx repack: avoid integer overflow on 32 bit systems
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-22 14:48:36 -07:00
Jacob Keller 09fb155f11 diff --no-index: support limiting by pathspec
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-22 14:20:11 -07:00
Jacob Keller 00466c1620 pathspec: add flag to indicate operation without repository
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-22 14:20:11 -07:00
Jacob Keller 6e4fb00156 pathspec: add match_leading_pathspec variant
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-22 14:20:11 -07:00
Johannes Sixt bfb0fa7099 Merge branch 'top-panel-search-highlight' of github.com:bnfour/gitk
* '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-22 19:15:31 +02:00
Alex Mironov 2e60aabc75 name-hash: don't add sparse directories in threaded lazy init
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-21 14:51:08 -07:00
Karthik Nayak 368d8c86f7 t: remove unexpected SANITIZE_LEAK variables
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-20 15:09:33 -07:00
Justin Tobler 68cb0b5253 builtin/receive-pack: add option to skip connectivity check
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-20 11:43:36 -07:00
Justin Tobler 95262afe78 t5410: test receive-pack connectivity check
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-20 11:43:36 -07:00
Johannes Sixt 9d60ba03d6 Merge branch 'yh/fix-non-themed-combobox'
* yh/fix-non-themed-combobox:
  gitk: Legacy widgets doesn't have combobox
2025-05-20 19:42:52 +02:00
Junio C Hamano 8613c2bb6c The sixteenth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-19 16:02:48 -07:00