From: Ramsay Jones <ramsay@ramsay1•demon.co.uk>
To: Junio C Hamano <gitster@pobox•com>
Cc: GIT Mailing-list <git@vger•kernel.org>
Subject: Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file
Date: Tue, 21 Jan 2014 22:51:30 +0000 [thread overview]
Message-ID: <52DEF9F2.1000905@ramsay1.demon.co.uk> (raw)
In-Reply-To: <xmqqppnlt3u3.fsf@gitster.dls.corp.google.com>
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
> .
>
next prev parent reply other threads:[~2014-01-21 22:51 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 [this message]
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
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=52DEF9F2.1000905@ramsay1.demon.co.uk \
--to=ramsay@ramsay1$(echo .)demon.co.uk \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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