git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ramsay Jones <ramsay@ramsay1.demon.co.uk>,
	GIT Mailing-list <git@vger.kernel.org>,
	Pat Thoyts <patthoyts@users.sourceforge.net>
Subject: Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file
Date: Wed, 22 Jan 2014 17:42:16 +0100	[thread overview]
Message-ID: <52DFF4E8.8060605@viscovery.net> (raw)
In-Reply-To: <xmqq38kgyozt.fsf@gitster.dls.corp.google.com>

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

  reply	other threads:[~2014-01-22 16:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-20 20:22 [PATCH/RFC] Makefile: Fix compilation of windows resource file Ramsay Jones
2014-01-21 21:04 ` Junio C Hamano
2014-01-21 21:36   ` Junio C Hamano
2014-01-21 22:51     ` Ramsay Jones
2014-01-21 23:48       ` Junio C Hamano
2014-01-22  6:55         ` Johannes Sixt
2014-01-22 16:12           ` Junio C Hamano
2014-01-22 16:42             ` Johannes Sixt [this message]
2014-01-22 17:38               ` Junio C Hamano
2014-01-22 20:14                 ` Junio C Hamano
2014-01-23  7:28                   ` [PATCH v2] Makefile: Fix compilation of Windows " Johannes Sixt
2014-01-23 12:02                     ` Pat Thoyts
2014-01-23 14:16                       ` Johannes Sixt
2014-01-23 15:19                         ` Pat Thoyts
2014-01-23 18:02                           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52DFF4E8.8060605@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=patthoyts@users.sourceforge.net \
    --cc=ramsay@ramsay1.demon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).