[v1,0/2] git-gui: reduce Tcl version requirement from 8.6 to 8.5
mbox series

Message ID 20200314224159.14174-1-me@yadavpratyush.com
Headers show
Series
  • git-gui: reduce Tcl version requirement from 8.6 to 8.5
Related show

Message

Pratyush Yadav March 14, 2020, 10:41 p.m. UTC
Hi,

Some MacOS distributions ship with Tcl 8.5. This means we can't use
TclOO. So, use our homegrown class.tcl instead.

While here, fix a potential variable name collision by creating a
separate namespace for a chord's script evaluation.

Jonathan,

Can you please test the patches the same way you tested your original
series just to be sure we don't break anything? A review would also be
nice.

Pratyush Yadav (2):
  git-gui: reduce Tcl version requirement from 8.6 to 8.5
  git-gui: create a new namespace for chord script evaluation

 git-gui.sh    |  4 ++--
 lib/chord.tcl | 56 +++++++++++++++++++++++++--------------------------
 lib/index.tcl | 10 +++++----
 3 files changed, 35 insertions(+), 35 deletions(-)

--
2.26.0.rc1.11.g30e9940356

Comments

Eric Sunshine March 15, 2020, 6:54 p.m. UTC | #1
On Sat, Mar 14, 2020 at 9:47 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> Some MacOS distributions ship with Tcl 8.5. This means we can't use
> TclOO. So, use our homegrown class.tcl instead.

It should be mentioned that this patch series fixes a regression in
Git v2.25 in which git-gui could not even be launched on Mac OS. The
problem was reported here[1] a couple months ago.

I performed some rudimentary testing of this patch series on Mac OS,
and it appears to be working as expected; it certainly fixes the
problem of git-gui not launching on Mac OS. (I did notice a
misbehavior related to the original patch which caused git-gui to be
unusable on Mac OS in v2.25, but I suspect that misbehavior is not
related to or caused by this patch series, thus shouldn't prevent its
acceptance.)

[1]: https://github.com/prati0100/git-gui/issues/26
Junio C Hamano March 16, 2020, 3:48 p.m. UTC | #2
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sat, Mar 14, 2020 at 9:47 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>> Some MacOS distributions ship with Tcl 8.5. This means we can't use
>> TclOO. So, use our homegrown class.tcl instead.
>
> It should be mentioned that this patch series fixes a regression in
> Git v2.25 in which git-gui could not even be launched on Mac OS. The
> problem was reported here[1] a couple months ago.
>
> I performed some rudimentary testing of this patch series on Mac OS,
> and it appears to be working as expected; it certainly fixes the
> problem of git-gui not launching on Mac OS. (I did notice a
> misbehavior related to the original patch which caused git-gui to be
> unusable on Mac OS in v2.25, but I suspect that misbehavior is not
> related to or caused by this patch series, thus shouldn't prevent its
> acceptance.)

I was actually hesitant to see this kind of change for the first
time this late in the cycle (the code may work with old Tcl/Tk but
do we know it does with newer ones?)  

I'll pull git-gui updates when Pratyush tells me to, which would
happen before the final (scheduled on 22nd).  I'll trust git-gui
maintainer's decision to include these changes in it, or to cook
longer to wait for the 2.27 cycle.  Comments like yours that help
Pratyush make the right judgment is greatly appreciated.

Thanks.
Pratyush Yadav March 17, 2020, 12:49 p.m. UTC | #3
On 16/03/20 08:48AM, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Sat, Mar 14, 2020 at 9:47 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> >> Some MacOS distributions ship with Tcl 8.5. This means we can't use
> >> TclOO. So, use our homegrown class.tcl instead.
> >
> > It should be mentioned that this patch series fixes a regression in
> > Git v2.25 in which git-gui could not even be launched on Mac OS. The
> > problem was reported here[1] a couple months ago.
> >
> > I performed some rudimentary testing of this patch series on Mac OS,
> > and it appears to be working as expected; it certainly fixes the
> > problem of git-gui not launching on Mac OS. (I did notice a
> > misbehavior related to the original patch which caused git-gui to be
> > unusable on Mac OS in v2.25, but I suspect that misbehavior is not
> > related to or caused by this patch series, thus shouldn't prevent its
> > acceptance.)
> 
> I was actually hesitant to see this kind of change for the first
> time this late in the cycle

*sigh* I intended to finish the change much sooner, but one thing after 
another, and I kept putting it on the backburner :-(

> (the code may work with old Tcl/Tk but do we know it does with newer 
> ones?)  

It is actually the other way around. My system has Tcl 8.6 installed, 
and it works well with it. The problem is checking if it works with 8.5. 
My distro's package manager doesn't show Tcl 8.5 as an option (only 
8.6), so I have to try and build 8.5 from source to test it. It is more 
work than I can afford right now. So I am relying on Eric and other 
MacOS users' reports.

But IIUC, unless there are any hidden gotchas in Tcl 8.5, there is a 
fairly little chance that this patch breaks anything significant that 
wasn't already broken before. The patch reverts from using a new 
8.6-specific feature to pure, backward-compatible Tcl that should work 
with older versions too. No, a larger concern for me is whether I missed 
something somewhere that breaks the feature for _all_ versions of 
git-gui.
 
> I'll pull git-gui updates when Pratyush tells me to, which would
> happen before the final (scheduled on 22nd).  I'll trust git-gui
> maintainer's decision to include these changes in it, or to cook
> longer to wait for the 2.27 cycle.  Comments like yours that help
> Pratyush make the right judgment is greatly appreciated.

Honestly, I'd like to cook it a bit longer but the reality is that very 
few people, if any, actually track and test my tree. Most people 
actually discover bugs when the changes hit a new Git release.

I cooked the original series that bumped the version requirement for 
quite some time because I suspected some platforms might still be on 
older versions. But I got absolutely no feedback/complaints, so I went 
ahead and sent you a pull request.

The change hit my 'master' on December 6. Git v2.25.0 was tagged on 
January 13th. MacOS breakage was reported on January 15th.

So, I have a choice between waiting for the 2.27 cycle keeping git-gui 
broken on MacOS for another few months in hopes that someone comes in 
and discovers a bug, or have it merged in v2.26 fixing git-gui for MacOS 
but risk (though it shouldn't be too high) introducing a bug in _all_ 
platforms. Neither choice is ideal, but I'm leaning towards the latter. 
Having _most_ things working is better than having nothing working at 
all.
Eric Sunshine March 19, 2020, 3:25 p.m. UTC | #4
On Tue, Mar 17, 2020 at 8:49 AM Pratyush Yadav <me@yadavpratyush.com> wrote:
> On 16/03/20 08:48AM, Junio C Hamano wrote:
> > I'll pull git-gui updates when Pratyush tells me to, which would
> > happen before the final (scheduled on 22nd).  I'll trust git-gui
> > maintainer's decision to include these changes in it, or to cook
> > longer to wait for the 2.27 cycle. [...]
>
> Honestly, I'd like to cook it a bit longer but the reality is that very
> few people, if any, actually track and test my tree. Most people
> actually discover bugs when the changes hit a new Git release.

Yep, that's the big issue. I track Junio's "next" branch pretty
closely but don't track the git-gui repository at all, so it wasn't
until Junio pulled from you, and after I pulled from Junio, that I
noticed the problem. So, in the longer run, asking Junio to pull more
often -- and earlier -- may be a good way forward.