Skip to content

Backport Git for Windows' gitk patches #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 20, 2025
Merged

Backport Git for Windows' gitk patches #4

merged 5 commits into from
Feb 20, 2025

Conversation

dscho
Copy link
Contributor

@dscho dscho commented Jan 11, 2025

Some of them hail from all the way back to 2012, and even the youngest is a week and a day shy of its second birthday.

These patches have been carried in Git for Windows, being ported patiently to every new Git version, mainly because the lack of activity on other gitk patches on the Git mailing list did not entice me to upstream them and see just how far I can stretch my frustration tolerance.

Until now! Now, gitk has an active maintainer and I could not be more delighted to contribute these patches so that the wider community can benefit from them.

dscho and others added 5 commits January 11, 2025 18:17
Just like CVE-2022-41953 for Git GUI, there exists a vulnerability of
`gitk` where it looks for `taskkill.exe` in the current directory before
searching `PATH`.

Note that the many `exec git` calls are unaffected, due to an obscure
quirk in Tcl's `exec` function. Typically, `git.exe` lives next to
`wish.exe` (i.e. the program that is run to execute `gitk` or Git GUI)
in Git for Windows, and that is the saving grace for `git.exe because
`exec` searches the directory where `wish.exe` lives even before the
current directory, according to
https://www.tcl-lang.org/man/tcl/TclCmd/exec.htm#M24:

	If a directory name was not specified as part of the application
	name, the following directories are automatically searched in
	order when attempting to locate the application:

	    The directory from which the Tcl executable was loaded.

	    The current directory.

	    The Windows 32-bit system directory.

	    The Windows home directory.

	    The directories listed in the path.

The same is not true, however, for `taskkill.exe`: it lives in the
Windows system directory (never mind the 32-bit, Tcl's documentation is
outdated on that point, it really means `C:\Windows\system32`).

Signed-off-by: Johannes Schindelin <[email protected]>
Assumes file names in git tree objects are UTF-8 encoded.

On most unix systems, the system encoding (and thus the TCL system
encoding) will be UTF-8, so file names will be displayed correctly.

On Windows, it is impossible to set the system encoding to UTF-8.
Changing the TCL system encoding (via 'encoding system ...', e.g. in the
startup code) is explicitly discouraged by the TCL docs.

Change gitk functions dealing with file names to always convert
from and to UTF-8.

Signed-off-by: Karsten Blees <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Git for Windows now ships with the new Git icon from git-scm.com. Use that
icon file if it exists instead of the old procedurally drawn one.

This patch was sent upstream but so far no decision on its inclusion was
made, so commit it to our fork.

Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Tcl/Tk 8.6 introduced new events for the cursor left/right keys and
apparently changed the behavior of the previous event.

Let's work around that by using the new events when we are running with
Tcl/Tk 8.6 or later.

This fixes git-for-windows/git#495

Signed-off-by: Johannes Schindelin <[email protected]>
When using remotes (with git-flow especially), the remote reference names
are almost always wordwrapped in the "list references" window because it's
somewhat narrow by default. It's possible to resize it with a mouse,
but it's annoying to have to do this every time, especially on Windows 10,
where the window border seems to be only one (1) pixel wide, thus making
the grabbing of the window border tricky.

Signed-off-by: James J. Raden <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
@j6t
Copy link
Owner

j6t commented Jan 12, 2025

All changes look good. Will pick them up.

@j6t j6t merged commit 9990b58 into j6t:master Feb 20, 2025
@avih
Copy link
Contributor

avih commented Feb 20, 2025

I'm wondering out loud whether there's an overlap between the commit here "gitk: unicode file names support", which does an explicit conversion from utf-8 in several places, and the first commit in #7 "Fixing file name encoding issues", which adds only one line:

encoding system utf-8

I can't judge whether it's an overlap because I don't know how to reproduce an issue which the commit here fixes (and then check whether the other commit also fixes it).

But at least as far as my reported test results go at #7, there's at least one issue which the commit here doesn't fix on Windows, but the commit at #7 does fix on Windows - the missing diff output when invoking gitk <japanese file name>.

@dscho dscho deleted the g4w-gitk branch February 20, 2025 10:45
@j6t
Copy link
Owner

j6t commented Feb 20, 2025

@avih Sure, commit 5eb02dd does assume that file names are encoded in UTF-8, but it has been battle-tested in the Git-for-Windows distribution.

Let's continue the discussion about encoding system utf-8 in #7.

@avih
Copy link
Contributor

avih commented Feb 23, 2025

I think the first commit windows: ... executables in worktree changes some level of indirection or form of encapsulation of the arguments or something along those lines.

In the new open wrapper, if I add puts stderr $args at the first line and also right before real_open, like so:

proc open {args} {
	puts stderr $args
	# sanitize args ...
	puts stderr $args
	uplevel 1 real_open $args
}

Then for one of the calls, which apparently uses multi-line stdin, the stdin argument prints differently, but I don't think stdin is supposed to change between the printouts:

{| git log --no-color -z --pretty=raw --show-notes --parents --boundary --stdin <<f3431b1756435a4bddb3627064c9ab34fbd54dd9\n--\nã‚ã„ã†.txt} r
{| D:/path/to/cmd/git.exe log --no-color -z --pretty=raw --show-notes --parents --boundary --stdin {<<f3431b1756435a4bddb3627064c9ab34fbd54dd9
--
ã‚ã„ã†.txt}} r

(ignore the string encoding issue, it's an unrelated bug)

stdin of the original arguments is printed with newlines as escape sequence \n, but at the modified arguments it's printed as a list with literal newlines.

I'm unable to judge whether these are 100% equivalent (or only equivalent for open?), but if not, then it might suggest a bug in some circumstances (or a fix of a bug...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants