[PATCH/RFC] Makefile: Fix compilation of windows resource file

[PATCH/RFC] Makefile: Fix compilation of windows resource file

[Ramsay Jones]

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]

In order to fix the syntax error, we replace any rcX with zero and
include some additional 'zero' padding to the version number list.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Junio,

This patch is marked RFC because, as I was just about to send this
email, I realized it wouldn't always work:

    $ touch git.rc
    $ make V=1 GIT_VERSION=1.9.dirty git.res
    windres -O coff \
              -DMAJOR=1 -DMINOR=9 -DPATCH=dirty \
              -DGIT_VERSION="\\\"1.9.dirty\\\"" git.rc -o git.res
    C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
    make: *** [git.res] Error 1
    $

:-D

I suspect it would be easier to change GIT-VERSION-GEN to also set, say,
GIT_VERSION_MAJOR, GIT_VERSION_MINOR and GIT_VERSION_PATCH ...

ATB,
Ramsay Jones

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b4af1e2..308baaa 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= -DPATCH=, $(wordlist 1,3,$(patsubst rc%,0,$(subst -, ,$(subst ., ,$(GIT_VERSION))) 0 0))) \
 	  -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
 
 ifndef NO_PERL
-- 
1.8.5

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> 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]

Thanks for a report.  I've been wondering how many distros and
packagers would have an issue like this when we go to 2-digit
release naming.  Of course we knew everybody can grok 3-or-4 ;-)

> In order to fix the syntax error, we replace any rcX with zero and
> include some additional 'zero' padding to the version number list.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> Hi Junio,
>
> This patch is marked RFC because, as I was just about to send this
> email, I realized it wouldn't always work:

Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
not even working for maintenance releases.  Does it differenciate
between 1.8.5.1 and 1.8.5.2, for example?.  Or does "windres" always
assume that a package version is always 3-dewey-decimal (not 2, not
4)?

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> 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]
>
> Thanks for a report.  I've been wondering how many distros and
> packagers would have an issue like this when we go to 2-digit
> release naming.  Of course we knew everybody can grok 3-or-4 ;-)
>
>> In order to fix the syntax error, we replace any rcX with zero and
>> include some additional 'zero' padding to the version number list.
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> ---
>>
>> Hi Junio,
>>
>> This patch is marked RFC because, as I was just about to send this
>> email, I realized it wouldn't always work:
>
> Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
> not even working for maintenance releases.  Does it differenciate
> between 1.8.5.1 and 1.8.5.2, for example?.  Or does "windres" always
> assume that a package version is always 3-dewey-decimal (not 2, not
> 4)?

Perhaps like this?  Just grab digit-only segments that are separated
with either dot or dash (and stop when we see a non-digit like
'dirty' or 'rcX'), and make them separated with comma.

Note that I am merely guessing that "short-digit" version numbers
are acceptable by now after seeing

    https://sourceware.org/ml/binutils/2012-07/msg00199.html

without knowing the current state of affairs.  If that is not the
case you may have to count the iteration of the loop and append or
chop the resulting string as necessary.

 Makefile              |  2 +-
 gen-version-string.sh | 13 +++++++++++++
 git.rc                |  4 ++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index b4af1e2..329f942 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))))) \
+		-DVERSIONSTRING=$$(./gen-version-string.sh $(GIT_VERSION)) \
 	  -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
 
 ifndef NO_PERL
diff --git a/gen-version-string.sh b/gen-version-string.sh
new file mode 100755
index 0000000..00af718
--- /dev/null
+++ b/gen-version-string.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+IFS=.- result=
+for v in $1
+do
+	if expr "$v" : '[0-9][0-9]*$' >/dev/null
+	then
+		result=$result${result:+,}$v
+	else
+		break
+	fi
+done
+echo "$result"
diff --git a/git.rc b/git.rc
index bce6db9..6f2a8d2 100644
--- a/git.rc
+++ b/git.rc
@@ -1,6 +1,6 @@
 1 VERSIONINFO
-FILEVERSION     MAJOR,MINOR,PATCH,0
-PRODUCTVERSION  MAJOR,MINOR,PATCH,0
+FILEVERSION     VERSIONSTRING,0
+PRODUCTVERSION  VERSIONSTRING,0
 BEGIN
   BLOCK "StringFileInfo"
   BEGIN

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Ramsay Jones]

On 21/01/14 21:36, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> 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]
>>
>> Thanks for a report.  I've been wondering how many distros and
>> packagers would have an issue like this when we go to 2-digit
>> release naming.  Of course we knew everybody can grok 3-or-4 ;-)
>>
>>> In order to fix the syntax error, we replace any rcX with zero and
>>> include some additional 'zero' padding to the version number list.
>>>
>>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>>> ---
>>>
>>> Hi Junio,
>>>
>>> This patch is marked RFC because, as I was just about to send this
>>> email, I realized it wouldn't always work:
>>
>> Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
>> not even working for maintenance releases.  Does it differenciate
>> between 1.8.5.1 and 1.8.5.2, for example?.  Or does "windres" always
>> assume that a package version is always 3-dewey-decimal (not 2, not
>> 4)?

I'm no expert on '.rc' file syntax, but the code certainly does not
(currently) support four digit versions.

> Perhaps like this?  Just grab digit-only segments that are separated
> with either dot or dash (and stop when we see a non-digit like
> 'dirty' or 'rcX'), and make them separated with comma.

Oh, this is *much* better than my new (unsent) attempt to fix this! ;-)

> 
> Note that I am merely guessing that "short-digit" version numbers
> are acceptable by now after seeing
> 
>     https://sourceware.org/ml/binutils/2012-07/msg00199.html

Ah, nice find!

I will test your patch (below) and let you know soon, but it looks
good to me. (I can't test it tonight, unfortunately.)

ATB,
Ramsay Jones

> 
> without knowing the current state of affairs.  If that is not the
> case you may have to count the iteration of the loop and append or
> chop the resulting string as necessary.
> 
>  Makefile              |  2 +-
>  gen-version-string.sh | 13 +++++++++++++
>  git.rc                |  4 ++--
>  3 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index b4af1e2..329f942 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))))) \
> +		-DVERSIONSTRING=$$(./gen-version-string.sh $(GIT_VERSION)) \
>  	  -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
>  
>  ifndef NO_PERL
> diff --git a/gen-version-string.sh b/gen-version-string.sh
> new file mode 100755
> index 0000000..00af718
> --- /dev/null
> +++ b/gen-version-string.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +IFS=.- result=
> +for v in $1
> +do
> +	if expr "$v" : '[0-9][0-9]*$' >/dev/null
> +	then
> +		result=$result${result:+,}$v
> +	else
> +		break
> +	fi
> +done
> +echo "$result"
> diff --git a/git.rc b/git.rc
> index bce6db9..6f2a8d2 100644
> --- a/git.rc
> +++ b/git.rc
> @@ -1,6 +1,6 @@
>  1 VERSIONINFO
> -FILEVERSION     MAJOR,MINOR,PATCH,0
> -PRODUCTVERSION  MAJOR,MINOR,PATCH,0
> +FILEVERSION     VERSIONSTRING,0
> +PRODUCTVERSION  VERSIONSTRING,0
>  BEGIN
>    BLOCK "StringFileInfo"
>    BEGIN
> .
> 

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

>> Note that I am merely guessing that "short-digit" version numbers
>> are acceptable by now after seeing
>> 
>>     https://sourceware.org/ml/binutils/2012-07/msg00199.html
>
> Ah, nice find!
>
> I will test your patch (below) and let you know soon, but it looks
> good to me. (I can't test it tonight, unfortunately.)

One thing to note is that I don't know why the existing code dropped
the fourth digit from the maintenance series.  The updated one will
give you "1,8,5,3,0" (because I just have a hardcoded ",0" at the
end for no good reason there), and if the missing fourth digit in
the original was a deliberate workaround for this file having an
upper limit of the number of digits (like "four"), this change will
break it, so if that is the case, you may have to count and stop the
loop early, perhaps like...

>> diff --git a/gen-version-string.sh b/gen-version-string.sh
>> new file mode 100755
>> index 0000000..00af718
>> --- /dev/null
>> +++ b/gen-version-string.sh
>> @@ -0,0 +1,13 @@
>> +#!/bin/sh
>> +
>> +IFS=.- result=

Add

	num_digits=0

here, and...

>> +for v in $1
>> +do
>> +	if expr "$v" : '[0-9][0-9]*$' >/dev/null
>> +	then
>> +		result=$result${result:+,}$v

... insert these here.

                num_digits=$(( $num_digits + 1 ))
                if test $num_digits = 4
                then
                        break
                fi

>> +	else
>> +		break
>> +	fi
>> +done
>> +echo "$result"
>> diff --git a/git.rc b/git.rc
>> index bce6db9..6f2a8d2 100644
>> --- a/git.rc
>> +++ b/git.rc
>> @@ -1,6 +1,6 @@
>>  1 VERSIONINFO
>> -FILEVERSION     MAJOR,MINOR,PATCH,0
>> -PRODUCTVERSION  MAJOR,MINOR,PATCH,0
>> +FILEVERSION     VERSIONSTRING,0
>> +PRODUCTVERSION  VERSIONSTRING,0
>>  BEGIN
>>    BLOCK "StringFileInfo"
>>    BEGIN
>> .
>> 

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Johannes Sixt]

[Cc Pat, who added git.rc]

Am 1/22/2014 0:48, schrieb Junio C Hamano:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>>> Note that I am merely guessing that "short-digit" version numbers
>>> are acceptable by now after seeing
>>>
>>>     https://sourceware.org/ml/binutils/2012-07/msg00199.html
>>
>> Ah, nice find!
>>
>> I will test your patch (below) and let you know soon, but it looks
>> good to me. (I can't test it tonight, unfortunately.)
> 
> One thing to note is that I don't know why the existing code dropped
> the fourth digit from the maintenance series.

I don't know either. But it does not really matter. When there are 4
digits in the FILEVERSION and PRODUCTVERSION statements, then the user
does not see them as-are, but, for example, 1.8.1283 for
FILEVERSION 1,8,5,3 (1283 = 5*256+3). Therefore, I think that there is
no point in providing 4 numbers, and the patch below should be
sufficient.

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

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> [Cc Pat, who added git.rc]
>
> Am 1/22/2014 0:48, schrieb Junio C Hamano:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> 
>>>> Note that I am merely guessing that "short-digit" version numbers
>>>> are acceptable by now after seeing
>>>>
>>>>     https://sourceware.org/ml/binutils/2012-07/msg00199.html
>>>
>>> Ah, nice find!
>>>
>>> I will test your patch (below) and let you know soon, but it looks
>>> good to me. (I can't test it tonight, unfortunately.)
>> 
>> One thing to note is that I don't know why the existing code dropped
>> the fourth digit from the maintenance series.
>
> I don't know either. But it does not really matter. When there are 4
> digits in the FILEVERSION and PRODUCTVERSION statements, then the user
> does not see them as-are, but, for example, 1.8.1283 for
> FILEVERSION 1,8,5,3 (1283 = 5*256+3). Therefore, I think that there is
> no point in providing 4 numbers, and the patch below should be
> sufficient.

Would that work well when we do 1.9.1, the first maintenance/bugfix
release for 1.9?

> 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

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Johannes Sixt]

Am 1/22/2014 17:12, schrieb Junio C Hamano:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> [Cc Pat, who added git.rc]
>>
>> Am 1/22/2014 0:48, schrieb Junio C Hamano:
>>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>>
>>>>> Note that I am merely guessing that "short-digit" version numbers
>>>>> are acceptable by now after seeing
>>>>>
>>>>>     https://sourceware.org/ml/binutils/2012-07/msg00199.html
>>>>
>>>> Ah, nice find!
>>>>
>>>> I will test your patch (below) and let you know soon, but it looks
>>>> good to me. (I can't test it tonight, unfortunately.)
>>>
>>> One thing to note is that I don't know why the existing code dropped
>>> the fourth digit from the maintenance series.
>>
>> I don't know either. But it does not really matter. When there are 4
>> digits in the FILEVERSION and PRODUCTVERSION statements, then the user
>> does not see them as-are, but, for example, 1.8.1283 for
>> FILEVERSION 1,8,5,3 (1283 = 5*256+3). Therefore, I think that there is

I just noticed that I'm wrong here: The user will see "1.8.5.3". But I
think it makes no difference. Read on.

>> no point in providing 4 numbers, and the patch below should be
>> sufficient.
> 
> Would that work well when we do 1.9.1, the first maintenance/bugfix
> release for 1.9?

Define "work well".

The numbers defined in {FILE,PRODUCT}VERSION statements are intended for
machine consumption and are always 4 positions (if the source contains
fewer, they are padded with zeros). They can be used by installers to
decide whether a file that already exists in the system should be
overwritten by a newer version.

Unfortunately, these numbers are visible when the user invokes Properties
from the context menu of git.exe in the file manager and then switches to
the "Version" tab. All 4 positions are always listed. Therefore, the user
will see "1.9.0.0" for the first release of the 1.9 series, which is
"wrong", because you will call "1.9", not "1.9.0.0", I assume.

With sufficient effort, we could achieve that version 1.9.1 is listed as
"1.9.1.0". That is still "wrong".

Since we can't get this display right, I suggest that we just punt (as per
my patch). That should work out nicely because we can fairly safely assume
that there are no installers around that look at these particular version
numbers.

BTW, that same "Version" tab will have another entry, called "Product
Version" later in the list. This one lists the string that we pass in
-DGIT_VERSION (see quoted context below). It is the truely correct version
that *users* should be interested in.

> 
>> 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

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> The numbers defined in {FILE,PRODUCT}VERSION statements are intended for
> machine consumption and are always 4 positions (if the source contains
> fewer, they are padded with zeros). They can be used by installers to
> decide whether a file that already exists in the system should be
> overwritten by a newer version.

OK, that makes sense.  If you package 1.9 (padded as 1.9.0.0) and
then 1.9.1 (padded as 1.9.1.0), you can update from 1.9 to 1.9.1
just fine.

> Unfortunately, these numbers are visible when the user invokes Properties
> from the context menu of git.exe in the file manager and then switches to
> the "Version" tab. All 4 positions are always listed. Therefore, the user
> will see "1.9.0.0" for the first release of the 1.9 series, which is
> "wrong", because you will call "1.9", not "1.9.0.0", I assume.
>
> With sufficient effort, we could achieve that version 1.9.1 is listed as
> "1.9.1.0". That is still "wrong".

I would not be worried about showing 1.9.1.0 for 1.9.1 and/or
1.9.0.0 for 1.9 at all.

But if the (receiving) system expects these to be monotonically
increasing, I suspect the script I posted would not "work well"
under that expectation.  When you package 1.9.2.g43218765.dirty,
that would become "1.9.2.0", and become indistinguishable from the
package taken from v1.9.2 tag, which is not good at all.  So the
script should strip [0-9]*\.g[0-9a-f]*\(\.dirty\)? from the end
first.

But even without complications from the "N-commit after the tag" it
won't "work well" if you cut packages from anything that is not
tagged anyway.  The only thing we know about any package taken from
the tip of 'master' past v1.9 is that it is newer than the package
taken from v1.9 tag. Sometimes it should be considered newer than a
package taken from v1.9.x tag (i.e. the contents of the maintenance
relase is fully included in 'master'), but not always (i.e. the tip
of 'master' when the package was made may contain up to v1.9.3 but
v1.9.4 may be newer than that).

If you truncate down to only two, like your patch does, anything
past v1.9 and before v1.10 (or v2.0) would have 1.9.0.0 and that is
no worse than giving 1.9.3.0 for v1.9.3 and giving 1.9.0.0 for
anything based on 'master'.  Your user may have installed a package
made from v1.9.1 and would want to update to the one taken from
'master' when it contained everything up to v1.9.3---under my
earlier "take numbers" approach, we would be "updating" from 1.9.1.0
to 1.9.0.0, which does not look like updating at all to the system.
The installers can use this to decide "a file that already exists in
the system" is newer, which is wrong, if I am reading your earlier
explanation corretly.

With your "we just take the first two numbers always", you would be
sidegrading between two 1.9.0.0, which may fare better.

> Since we can't get this display right, I suggest that we just punt (as per
> my patch). That should work out nicely because we can fairly safely assume
> that there are no installers around that look at these particular version
> numbers.
>
> BTW, that same "Version" tab will have another entry, called "Product
> Version" later in the list. This one lists the string that we pass in
> -DGIT_VERSION (see quoted context below). It is the truely correct version
> that *users* should be interested in.
>
>> 
>>> 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

Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Sixt <j.sixt@viscovery.net> writes:
> ...
>> ..., I suggest that we just punt (as per
>> my patch). That should work out nicely because we can fairly safely assume
>> that there are no installers around that look at these particular version
>> numbers.

OK.  I do not think we care too deeply about how a "forced to be
four dewey-decimal numbers" looks compared to 2 or 3 numbers in the
$(GIT_VERSION), as I think we always had that (non-)issue, but not
being able to compile is not very nice.

So can you, Pat or Ramsay send a tested patch with a proposed log
message?   Preferrably by -rc1 but I think the change is low impact
that it can be in -rc2, leaving -rc1 broken.

Thanks.

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

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

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 <j6t@kdbg.org>
---
 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

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

[Pat Thoyts]

On 23 January 2014 07:28, Johannes Sixt <j.sixt@viscovery.net> wrote:
> From: Johannes Sixt <j6t@kdbg.org>
>
> 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 <j6t@kdbg.org>
> ---
>  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

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

[Johannes Sixt]

Am 1/23/2014 13:02, schrieb Pat Thoyts:
> On 23 January 2014 07:28, Johannes Sixt <j.sixt@viscovery.net> 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

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

[Pat Thoyts]

On 23 January 2014 14:16, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 1/23/2014 13:02, schrieb Pat Thoyts:
>> On 23 January 2014 07:28, Johannes Sixt <j.sixt@viscovery.net> 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 <patthoyts@users.sourceforge.net>

Cheers,
Pat.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

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

[Junio C Hamano]

Pat Thoyts <patthoyts@gmail.com> writes:

>>> 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 <patthoyts@users.sourceforge.net>

Thanks.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.