| Message ID | 20250401030102.297272-1-mlevedahl@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | gitk: override PATH search only on Windows | expand |
Hi Mark, On Mon, 31 Mar 2025, Mark Levedahl wrote: > Restrict overrides of exec/open to Windows only, as > the need for this is Tcl adding the current working directory > to $PATH on Windows. Recent modifications to this render > gitk unusable on Cygwin, isolating these overrides to Windows only > both fixes that breakage andk reduces the liklihood of similar > issues in the future. I agree that this is the right thing to do, and apologize for the breakage by copying the code from Git GUI to gitk and then contributing it _years_ later without double-checking whether it is still needed for Cygwin. > patch summary: > 1 - modifies the existing code to restrict the overrides > to Windows, restoring other platorms to native exec/open. > 2 - remove now superflous variable _search_exe. > 3 - fix the override code to avoid path search given a > relative path like foo/bar. > > --- > Changes since v1 - fixed commit ID reference for git-gui, otherwise > improved commit message in patch 1. > Added patches 2 and 3. > > Mark Levedahl (3): > gitk: override $PATH search only on Windows I really wish that the reviewing process offered better tools than a fixed diff for this patch; Inspecting it with `-w` would probably make it much more obvious what it does (and make it substantially easier to verify that it does not do anything inadvertently). In any case, these patches look good to me, thank you for working on them so carefully. Ciao, Johannes > gitk: _search_exe is no longer needed > gitk: limit PATH search to bare executable names > > gitk | 147 +++++++++++++++++++++++------------------------------------ > 1 file changed, 58 insertions(+), 89 deletions(-) > > -- > 2.49.0.99.31 I want that version. It probably has fixed all the bugs.
Am 01.04.25 um 05:00 schrieb Mark Levedahl: > Restrict overrides of exec/open to Windows only, as > the need for this is Tcl adding the current working directory > to $PATH on Windows. Recent modifications to this render > gitk unusable on Cygwin, isolating these overrides to Windows only > both fixes that breakage andk reduces the liklihood of similar > issues in the future. > > patch summary: > 1 - modifies the existing code to restrict the overrides > to Windows, restoring other platorms to native exec/open. > 2 - remove now superflous variable _search_exe. > 3 - fix the override code to avoid path search given a > relative path like foo/bar. > > --- > Changes since v1 - fixed commit ID reference for git-gui, otherwise > improved commit message in patch 1. > Added patches 2 and 3. > > Mark Levedahl (3): > gitk: override $PATH search only on Windows > gitk: _search_exe is no longer needed > gitk: limit PATH search to bare executable names Thank you for following up. These all look good. Patch 3 is a good find! I've inserted a patch 0.5/3 that replaces the existing TAB characters with four SP in the indentation. Thanks, -- Hannes
On 4/1/25 12:10 PM, Johannes Schindelin wrote: > I really wish that the reviewing process offered better tools than a > fixed diff for this patch; Inspecting it with `-w` would probably make it > much more obvious what it does (and make it substantially easier to verify > that it does not do anything inadvertently). Agreed. The first patch is nasty due to indentation changes, git diff -w is indespensible, perhaps that should be available similar to a diffstat. Thanks for the review. Mark
Restrict overrides of exec/open to Windows only, as the need for this is Tcl adding the current working directory to $PATH on Windows. Recent modifications to this render gitk unusable on Cygwin, isolating these overrides to Windows only both fixes that breakage andk reduces the liklihood of similar issues in the future. patch summary: 1 - modifies the existing code to restrict the overrides to Windows, restoring other platorms to native exec/open. 2 - remove now superflous variable _search_exe. 3 - fix the override code to avoid path search given a relative path like foo/bar. --- Changes since v1 - fixed commit ID reference for git-gui, otherwise improved commit message in patch 1. Added patches 2 and 3. Mark Levedahl (3): gitk: override $PATH search only on Windows gitk: _search_exe is no longer needed gitk: limit PATH search to bare executable names gitk | 147 +++++++++++++++++++++++------------------------------------ 1 file changed, 58 insertions(+), 89 deletions(-)