Re: [GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 17, 2014 at 3:49 PM, Jens Lehmann <Jens.Lehmann@xxxxxx> wrote:
> Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
> the following error:
>
>    No working directory ../../../<path>
>
>    couldn't change working directory
>    to "../../../<path>": no such file or
>    directory
>
> This is because "git rev-parse --show-toplevel" is only run when git gui
> sees a git version of at least 1.7.0 (which is the version in which the
> --show-toplevel option was introduced). But "package vsatisfies" returns
> false when the major version changes, which is not what we want here.
>
> Fix that for both places where the git version is checked using vsatifies

s/vsatifies/vsatisfies/

> by appending a '-' to the version number. This tells vsatisfies that a
> change of the major version is not considered to be a problem, as long as
> the new major version is larger. This is done for both the place that
> caused the reported bug and another spot where the git version is tested
> for another feature.
>
> Reported-by: Chris Packham <judge.packham@xxxxxxxxx>
> Reported-by: Yann Dirson <ydirson@xxxxxxx>
> Helped-by: Pat Thoyts <patthoyts@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@xxxxxx>
> ---
>
> Am 17.05.2014 14:23, schrieb Pat Thoyts:
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>
>>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>>
>>>> Jens Lehmann <Jens.Lehmann@xxxxxx> writes:
>>>>
>>>>> Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
>>>>> git gui will not work inside submodules anymore due to the major version
>>>>> number change from 1 to 2. I'd like to hear Pat's opinion on this; even
>>>>> though I think my patch is less risky (as it doesn't change behavior for
>>>>> pre-2 versions), he might like Chris' proposal better.
>>>>
>>>> Thanks; I share the same feeling.
>>>
>>> So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
>>> not missing any commit from there, I tentatively created a fork of
>>> it, applied your patch and merged it somewhere on 'pu' that is close
>>> to 'next'.  We may want to fast-track it to 2.0 without waiting for
>>> an Ack from Pat but let's give him one more day to respond.
>>>
>>
>> The analysis about the major version number being significant is
>> correct. By default vsatisfies assumes that a major version number
>> change means all lesser versions are incompatible. However, you can
>> prevent that assumption using an unlimited check by appending a - (minus
>> sign) to the version to yield an open ended range. Or by giving another
>> range. So the only change required is to append a minus.
>>
>>   package vsatisfies $::_git_version 1.7.0-
>>
>> will suffice.
>>
>>   package vsatisfies $::_git_version 1.7.0 2.0.0
>>
>> would work but would cause failures when we arrive at git 3.0
>
> Thanks for the review! In this version I added the '-' to the version
> passed to vsatisfies and updated the commit message accordingly. I
> tested the result and it fixes the regression.
>
> Junio, please replace my old version with this. In the first version
> I forgot to add a ">= 0" after the vcompare, which results in all
> versions that are /different/ than the one checked against pass the
> test. While that fixes the 2.0.0 regression, it will fail for git
> versions older than the version that is tested for. So my first
> attempt wasn't /that/ different from Chris' proposal ... :-/
>
>
>  git-gui/git-gui.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index cf2209b..6a8907e 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -1283,7 +1283,7 @@ load_config 0
>  apply_config
>
>  # v1.7.0 introduced --show-toplevel to return the canonical work-tree
> -if {[package vsatisfies $_git_version 1.7.0]} {
> +if {[package vsatisfies $_git_version 1.7.0-]} {
>         if { [is_Cygwin] } {
>                 catch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}
>         } else {
> @@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
>                 close $fd
>         }
>
> -       if {[package vsatisfies $::_git_version 1.6.3]} {
> +       if {[package vsatisfies $::_git_version 1.6.3-]} {
>                 set ls_others [list --exclude-standard]
>         } else {
>                 set ls_others [list --exclude-per-directory=.gitignore]
> --
> 1.8.3.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]