[pull] master from git:master #63
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit b9bee11 (gitk: Only restore window size from ~/.gitk, not position, 2008-03-10). The earlier commit e9937d2 (Make gitk work reasonably well on Cygwin, 2007-02-01) reworked the window layout considerably. Much of this became irrelevant around 2011 after Cygwin gained an X11 server and switched to a supportable port of the Unix/X11 Tcl/Tk (it is now on the current 8.6 code base). Part of the necessary change was to restore the window size across sessions, but the position was also restored. This raised complaints on the mailing list[*], because Gitk was opened on the wrong monitor. b9bee11 was the compromise, because it was only the size that mattered for the Cygwin layout engine to work. I personally, find it annoying when Gitk pops up on a random location on the screen, in particular, since many other applications restore the window positions across sessions, so why not Gitk as well? (I do not operate multi-monitor setups, so I cannot test the case.) [*] https://lore.kernel.org/git/47AAA254.2020008@thorn.ws/ Helped-by: Mark Levedahl <mlevedahl@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The Tags and Heads window always opens at a default position and size, requiring users to reposition it each time. Remember its geometry between sessions in the config file as `geometry(showrefs)`. Note that the existing configuration is sourced in proc savestuff right before new settings are written. This makes the old settings available as local variables(!) and does not overwrite the current settings. Since we need access to the global geometry(showrefs), it is necessary to unset the local variable. Helped-by: Michael Rappazzo <rappazzo@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
When 5de460a (gitk: Refactor per-line part of getblobdiffline and its support) moved the body of a loop into a separate function, several 'continue' statements were changed to 'return'. But one instance was missed. Fix it now. Signed-off-by: Johannes Sixt <j6t@kdbg.org>
When a file is selected in the file list, the diff window scrolls to the corresponding section. The administrative data needed for this purpose is extracted from the 'rename from', 'rename to', and 'copy to' lines. Escaped file names are unescaped for this purpose. However, the lines shown in the diff window are left in the escaped form. This is not very pleasing. Replace the escaped form by the unescaped form. Add a section to treat the 'copy from' case. Signed-off-by: Johannes Sixt <j6t@kdbg.org>
If a file is renamed between commits and an external diff is started through gitk on the original or the renamed file name, gitk is unable to open the renamed file in the external diff editor. It fails to fetch the renamed file from git, because it fetches it using its original path in contrast to using the renamed path of the file. Detect the rename and open the external diff with the original and the renamed file instead of no file (fetch the renamed file path and name from git) no matter if the original or the renamed file is selected in gitk. Signed-off-by: Tobias Boesch <tobias.boesch@miele.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience and histogram diffs, not for the minimal one. This means that when reseting the diff algorithm to the default one, one needs to separately clear the bit for the minimal diff. There are places in the code that fail to do that: merge-ort.c and builtin/merge-file.c. Add the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate clearing of this bit in the places where it hasn't been forgotten. Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diff algorithm used in 'git-blame(1)' is set to 'myers', without the possibility to change it aside from the `--minimal` option. There has been long-standing interest in changing the default diff algorithm to "histogram", and Git 3.0 was floated as a possible occasion for taking some steps towards that: https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/ As a preparation for this move, it is worth making sure that the diff algorithm is configurable where useful. Make it configurable in the `git-blame(1)` command by introducing the `--diff-algorithm` option and make honor the `diff.algorithm` config variable. Keep Myers diff as the default. Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call xmkstemp_mode() instead of duplicating its error handling code. This switches the implementation from the system's mkstemp(3) to our own git_mkstemp_mode(), which works just as well. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A comment at the top of t6429 mentions why the test doesn't exercise git rebase or git cherry-pick. However, it claims that it uses `test-tool fast-rebase`. That was true when the comment was written, but commit f920b02 (replay: introduce new builtin, 2023-11-24) changed it to use git replay without updating this comment. We could potentially just strike this second comment, since git replay is a bona fide built-in, but perhaps the explanation about why it focuses on git replay is still useful. Update the comment to make it accurate again. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
While developing commit a16e8ef (merge-ort: fix merge.directoryRenames=false, 2025-03-13), I was testing things out and had an extra condition on one of the if-blocks that I occasionally swapped between '&& 0' and '&& 1' to see the effects of the changes. I forgot to remove it before submitting and it wasn't caught in review. Remove it now. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
At GitHub, we had a repository that was triggering git: merge-ort.c:3032: process_renames: Assertion `newinfo && !newinfo->merged.clean` failed. during git replay. This sounds similar to the somewhat recent f6ecb60 (merge-ort: fix directory rename on top of source of other rename/delete, 2025-08-06), but the cause is different. Unlike that case, there are no rename-to-self situations arising in this case, and new to this case it can only be triggered during a replay operation on the 2nd or later commit being replayed, never on the first merge in the sequence. To trigger, the repository needs: * an upstream which: * renames a file to a different directory, e.g. old/file -> new/file * leaves other files remaining in the original directory (so that e.g. "old/" still exists upstream even though file has been removed from it and placed elsewhere) * a topic branch being rebased where: * a commit in the sequence: * modifies old/file * a subsequent commit in the sequence being replayed: * does NOT touch *anything* under new/ * does NOT touch old/file * DOES modify other paths under old/ * does NOT have any relevant renames that we need to detect _anywhere_ elsewhere in the tree (meaning this interacts interestingly with both directory renames and cached renames) In such a case, the assertion will trigger. The fix turns out to be surprisingly simple. I have a very vague recollection that I actually considered whether to add such an if-check years ago when I added the very similar one for oldinfo in 1b6b902 (merge-ort: process_renames() now needs more defensiveness, 2021-01-19), but I think I couldn't figure out a possible way to trigger it and was worried at the time that if I didn't know how to trigger it then I wasn't so sure that simply skipping it was correct. Waiting did give me a chance to put more thorough tests and checks into place for the rename-to-self cases a few months back, which I might not have found as easily otherwise. Anyway, put the check in place now and add a test that demonstrates the fix. Note that this bug, as demonstrated by the conditions listed above, runs at the intersection of relevant renames, trivial directory resolutions, and cached renames. All three of those optimizations are ones that unfortunately make the code (and testcases!) a bit more complex, and threading all three makes it a bit more so. However, the testcase isn't crazy enough that I'd expect no one to ever hit it in practice, and was confused why we didn't see it before. After some digging, I discovered that merge.directoryRenames=false is a workaround to this bug, and GitHub used that setting until recently (it was a "temporary" match-what-libgit2-does piece of code that lasted years longer than intended). Since the conditions I gave above for triggering this bug rule out the possibility of there being directory renames, one might assume that it shouldn't matter whether you try to detect such renames if there aren't any. However, due to commit a16e8ef (merge-ort: fix merge.directoryRenames=false, 2025-03-13), the heavy hammer used there means that merge.directoryRenames=false ALSO turns off rename caching, which is critical to triggering the bug. This becomes a bit more than an aside since... Re-reading that old commit, a16e8ef (merge-ort: fix merge.directoryRenames=false, 2025-03-13), it appears that the solution to this latest bug might have been at least a partial alternative solution to that old commit. And it may have been an improved alternative (or at least help implement one), since it may be able to avoid the heavy-handed disabling of rename cache. That might be an interesting future thing to investigate, but is not critical for the current fix. However, since I spent time digging it all up, at least leave a small comment tweak breadcrumb to help some future reader (myself or others) who wants to dig further to connect the dots a little quicker. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
`wcsncpy_s()` wants to write the terminating null character so we need to allocate one more space for it in the target memory block. This should fix crashes when trying to read passwords. When this happened, the password/token wouldn't print out and Git would therefore ask for a new password every time. Signed-off-by: David Macek <david.macek.0@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the `en/make-libgit-a` topic branch, more precisely in the commits f3b4c89 (make: delete REFTABLE_LIB, add reftable to LIB_OBJS, 2025-10-02) and cf680cd (make: delete XDIFF_LIB, add xdiff to LIB_OBJS, 2025-10-02), the strategy to build three static libraries was rethought, and instead only one static library is now built. This is good. However, the CMake definition was not changed accordingly, and now CMake-based builds fail thusly: [...] Generating hook-list.h CMake Error at CMakeLists.txt:122 (string): string sub-command REPLACE requires at least four arguments. Call Stack (most recent call first): CMakeLists.txt:711 (parse_makefile_for_sources) CMake Error at CMakeLists.txt:122 (string): string sub-command REPLACE requires at least four arguments. Call Stack (most recent call first): CMakeLists.txt:717 (parse_makefile_for_sources) -- Configuring incomplete, errors occurred! Fix that by removing the parts that expect the reftable and xdiff objects to be defined separately in the Makefile, still. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The pattern `return errno = ..., -1;` is observed several times in
`compat/mingw.c`. It has served us well over the years, but now clang
starts complaining:
compat/mingw.c:723:24: error: possible misuse of comma operator here [-Werror,-Wcomma]
723 | return errno = ENOSYS, -1;
| ^
See for example this failing workflow run:
https://github.com/git-for-windows/git-sdk-arm64/actions/runs/15457893907/job/43513458823#step:8:201
Let's appease clang (and also reduce the use of the no longer common
comma operator).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bumps actions/setup-go from 5 to 6. This upgrade includes dependency updates that incorporate a fix for a critical vulnerability. [Originally opened at git-for-windows#5811] - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v5...v6) Originally-authored-by: dependabot[bot] <support@github.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the same spirit as 9faf396 (t: introduce compatibility options to clar-based tests, 2024-12-13), we should ignore --no-chain-lint passed to our clar tests, since it may appear in GIT_TEST_OPTS to be used with other tests. This is particularly important on Windows CI, where --no-chain-lint is added to the test options by default, and the meson build will pass all options to the unit tests. The only reason our meson Windows CI job does not run into this currently is that it is not respecting GIT_TEST_OPTS at all! So ignoring this option is a prerequisite to fixing that situation. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The GitHub windows-meson-test jobs directly run "meson test" with the
--slice option. This means they skip all of the ci/lib.sh
infrastructure, and in particular:
1. They do not actually set any GIT_TEST_OPTS like --verbose-log or
-x.
2. They do not do the usual handle_failed_tests() magic to print test
failures or tar up failed directories.
As a result, you get almost no feedback at all when a test fails in this
job, making debugging rather tricky.
Let's try to make this behave more like the other CI jobs. Because we're
on Windows, we can't just use the normal run-build-and-tests.sh script.
Our build runs as a separate job (like the non-meson Windows job), and
then we parallelize the tests across several job slices. So we need
something like the run-test-slice.sh script that the "windows-test" job
uses.
In theory we could just swap out the "make" invocation there for
"meson". But it doesn't quite work, because "make" knows how to pull
GIT_TEST_OPTS out of GIT-BUILD-OPTIONS automatically. But for meson, we
have to extract them into the --test-args option ourselves. I tried
making the logic in run-test-slice.sh conditional, but there ended up
being hardly any common code at all (and there are some tricky ordering
constraints). So I added up with a new meson-specific test-slice runner.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We test xmkstemp() in our helper by just calling:
xmkstemp(xstrdup(argv[1]));
This leaks both the copied string as well as the descriptor returned by
the function. In practice this isn't a big deal, since we immediately
exit the program, but:
1. LSan will complain about the memory leak. The only reason we did
not notice this in our leak-checking builds is that both of the
callers in the test suite (both in t0070) pass a broken template
(and expect failure). So the function calls die() before we can
actually leak.
But it's an accident waiting to happen if anybody adds a call which
succeeds.
2. Coverity complains about the descriptor leak. There's a long list
of uninteresting or false positives in Coverity's results, but
since we're here we might as well fix it, too.
I didn't bother adding a new test that triggers the leak. It's not even
in real production code, but just in the test-helper itself.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The output of "git worktree list" displays a table containing the worktree path, HEAD OID and branch name for each worktree. The code aligns the columns by measuring the visual width of the worktree path when it is printed. Unfortunately it fails to use the visual width when calculating the width of the column so, if any of the paths contain a multibyte character, we can end up with excess padding between columns. The simplest fix would be to replace strlen() with utf8_strwidth() in measure_widths(). However that leaves us measuring the visual width twice and the byte length once. By caching the visual width and printing the padding separately to the worktree path, we only need to calculate the visual width once and do not need the byte length at all. The visual widths are stored in an arrays of structs rather than an array of ints as the next commit will add more struct members. Even if there are no multibyte characters in any of the paths we still print an extra space between the path and the object id as the field width is calculated as one plus the length of the path and we print an explicit space as well. This is fixed by not printing the extra space. The tests are updated to include multibyte characters in one of the worktree paths and to check the spacing of the columns. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a worktree path contains newlines or other control characters it messes up the output of "git worktree list". Fix this by using quote_path() to display the worktree path. The output of "git worktree list" is designed for human consumption, scripts should be using the "--porcelain" option so this change should not break them. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If it fails, return an error. Signed-off-by: Greg Funni <gfunni234@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* js/persist-ref-window-geometry: gitk: persist position and size of the Tags and Heads window Revert "gitk: Only restore window size from ~/.gitk, not position"
* tb/external-diff-renamed: gitk: add external diff file rename detection
* 'master' of https://github.com/j6t/gitk: gitk: add external diff file rename detection gitk: show unescaped file names on 'rename' and 'copy' lines gitk: fix a 'continue' statement outside a loop to 'return' gitk: persist position and size of the Tags and Heads window Revert "gitk: Only restore window size from ~/.gitk, not position"
Yet another corner case fix around renames in the "ort" merge strategy. * en/ort-rename-another-fix: merge-ort: fix failing merges in special corner case merge-ort: remove debugging crud t6429: update comment to mention correct tool
"git blame" learns "--diff-algorithm=<algo>" option. * ad/blame-diff-algorithm: blame: make diff algorithm configurable xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASK
Code simplification. * rs/xmkstemp-simplify: wrapper: simplify xmkstemp()
Test leakfix. * jk/test-mktemp-leakfix: test-mktemp: plug memory and descriptor leaks
Update a version of action used at the GitHub Actrions CI. * js/ci-github-setup-go-update: ci: bump actions/setup-go from 5 to 6
The "return errno = EFOO, -1" construct, which is heavily used in compat/mingw.c and triggers warnings under "-Wcomma", has been rewritten to avoid the warnings. * js/mingw-assign-comma-fix: mingw: avoid the comma operator
Makefile based build have recently been updated to build a libgit.a that also has reftable and xdiff objects; CMake based build procedure has been updated to match. * js/cmake-libgit-fix: cmake: stop trying to build the reftable and xdiff libraries
Under-allocation fix. * js/wincred-get-credential-alloc-fix: wincred: avoid memory corruption
"git worktree list" attempts to show paths to worktrees while aligning them, but miscounted display columns for the paths when non-ASCII characters were involved, which has been corrected. * pw/worktree-list-display-width-fix: worktree list: quote paths worktree list: fix column spacing
"Windows+meson" job at the GitHub Actions CI was hard to debug, as it did not show and save failed test artifacts, which has been corrected. * jk/ci-windows-meson-test-fix: ci(windows-meson-test): handle options and output like other test jobs unit-test: ignore --no-chain-lint
Emulation code clean-up. * gf/win32-pthread-cond-wait-err: win32: return error if SleepConditionVariableCS fails
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
11 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )