Re: [msysGit] [PATCH v2] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Pat Thoyts
On 23 January 2014 14:16, Johannes Sixt  wrote:
> Am 1/23/2014 13:02, schrieb Pat Thoyts:
>> On 23 January 2014 07:28, Johannes Sixt  wrote:
>>> @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
>>>
>>>  git.res: git.rc GIT-VERSION-FILE
>>> $(QUIET_RC)$(RC) \
>>> - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, 
>>> ,$(subst ., ,$(GIT_VERSION) \
>>> + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
>>> ,$(GIT_VERSION) \
>>>   -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
>>>
>>>  ifndef NO_PERL
>>
>> This was put in as a response to
>> https://github.com/msysgit/git/issues/5 where a request was made to be
>> able to check the version without actually executing the file.
>
> If I understand the request correctly, it is about manual inspection. The
> correct version string for this purpose is recorded via -DGIT_VERSION.
>
>> Given
>> that the majority of versions has the same first two digits this
>> becomes fairly useless without the patchlevel digit. So it would be
>> preferable to try to maintain all three digits. The following should
>> do this:
>>
>> GIT_VERSION=1.9.rc0
>> all:
>> echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
>> $(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
>> ,$(GIT_VERSION 0 0))
>>
>> This removes any rc* parts and appends a couple of zeros so that all
>> missing elements should appear as 0 in the final list.
>
> As Junio already pointed out, this records the wrong number in the 1.9
> track before 1.9.1 is out because the third position is the commit count,
> not the patch level.
>
> -- Hannes

OK - I cehcked and you are right in that the GIT_VERSION value is the
one showing up the properties dialog at least under Windows 7. As this
is the most likely to be examined I agree that just taking the first
two digits is the simplest fix here. So, fine by me then.

Acked-by: Pat Thoyts 

Cheers,
Pat.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [msysGit] [PATCH v2] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Johannes Sixt
Am 1/23/2014 13:02, schrieb Pat Thoyts:
> On 23 January 2014 07:28, Johannes Sixt  wrote:
>> @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
>>
>>  git.res: git.rc GIT-VERSION-FILE
>> $(QUIET_RC)$(RC) \
>> - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, 
>> ,$(subst ., ,$(GIT_VERSION) \
>> + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
>> ,$(GIT_VERSION) \
>>   -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
>>
>>  ifndef NO_PERL
>
> This was put in as a response to
> https://github.com/msysgit/git/issues/5 where a request was made to be
> able to check the version without actually executing the file.

If I understand the request correctly, it is about manual inspection. The
correct version string for this purpose is recorded via -DGIT_VERSION.

> Given
> that the majority of versions has the same first two digits this
> becomes fairly useless without the patchlevel digit. So it would be
> preferable to try to maintain all three digits. The following should
> do this:
> 
> GIT_VERSION=1.9.rc0
> all:
> echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
> $(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
> ,$(GIT_VERSION 0 0))
> 
> This removes any rc* parts and appends a couple of zeros so that all
> missing elements should appear as 0 in the final list.

As Junio already pointed out, this records the wrong number in the 1.9
track before 1.9.1 is out because the third position is the commit count,
not the patch level.

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [msysGit] [PATCH v2] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Pat Thoyts
On 23 January 2014 07:28, Johannes Sixt  wrote:
> From: Johannes Sixt 
>
> If the git version number consists of less than three period
> separated numbers, then the Windows resource file compilation
> issues a syntax error:
>
>   $ touch git.rc
>   $ make V=1 git.res
>   GIT_VERSION = 1.9.rc0
>   windres -O coff \
> -DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \
> -DGIT_VERSION="\\\"1.9.rc0\\\"" git.rc -o git.res
>   C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
>   make: *** [git.res] Error 1
>   $
>
> Note that -DPATCH=rc0.
>
> The values passed via -DMAJOR=, -DMINOR=, and -DPATCH= are used in
> FILEVERSION and PRODUCTVERSION statements, which expect up to four numeric
> values. These version numbers are intended for machine consumption. They
> are typically inspected by installers to decide whether a file to be
> installed is newer than one that exists on the system, but are not used
> for much else.
>
> We can be pretty certain that there are no tools that look at these
> version numbers, not even the installer of Git for Windows does.
> Therefore, to fix the syntax error, fill in only the first two numbers,
> which we are guaranteed to find in Git version numbers.
>
> Signed-off-by: Johannes Sixt 
> ---
>  That "not even the installer of Git for Windows uses" the FILEVERSION
>  numbers is a bold statement of mine (I did not check). If I am wrong,
>  this approach for a fix is not viable, and a better fix would be
>  needed. Otherwise, an Acked-By would be appreciated so that we can
>  have this fix in upstream ASAP.
>
>  Makefile | 2 +-
>  git.rc   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b4af1e2..99b2b89 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
>
>  git.res: git.rc GIT-VERSION-FILE
> $(QUIET_RC)$(RC) \
> - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, 
> ,$(subst ., ,$(GIT_VERSION) \
> + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
> ,$(GIT_VERSION) \
>   -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
>
>  ifndef NO_PERL
> diff --git a/git.rc b/git.rc
> index bce6db9..33aafb7 100644
> --- a/git.rc
> +++ b/git.rc
> @@ -1,6 +1,6 @@
>  1 VERSIONINFO
> -FILEVERSION MAJOR,MINOR,PATCH,0
> -PRODUCTVERSION  MAJOR,MINOR,PATCH,0
> +FILEVERSION MAJOR,MINOR,0,0
> +PRODUCTVERSION  MAJOR,MINOR,0,0
>  BEGIN
>BLOCK "StringFileInfo"
>BEGIN
> --
> 1.9.rc0.1179.g5088b55
>
This was put in as a response to
https://github.com/msysgit/git/issues/5 where a request was made to be
able to check the version without actually executing the file. Given
that the majority of versions has the same first two digits this
becomes fairly useless without the patchlevel digit. So it would be
preferable to try to maintain all three digits. The following should
do this:

GIT_VERSION=1.9.rc0
all:
echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
$(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
,$(GIT_VERSION 0 0))

This removes any rc* parts and appends a couple of zeros so that all
missing elements should appear as 0 in the final list.

Pat Thoyts
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html