On 18/05/14 07:49, Jens Lehmann 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
> 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]
>
Works for me.
--
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