From: Michael J Gruber <git@drmicha.warpmail.net>
To: Matthias Andree <matthias.andree@gmx.de>
Cc: git@vger.kernel.org, "Junio C. Hamano" <gitster@pobox.com>
Subject: Re: [PATCH v3] To make GIT-VERSION-FILE, search for git more widely
Date: Thu, 07 May 2009 13:49:45 +0200 [thread overview]
Message-ID: <4A02CAD9.9080808@drmicha.warpmail.net> (raw)
In-Reply-To: <1241688129-31613-1-git-send-email-matthias.andree@gmx.de>
Matthias Andree venit, vidit, dixit 07.05.2009 11:22:
> Situation: sudo make install rebuilds the whole package even if you've just
> built it before. For instance:
>
> make configure
> ./configure # defaults to --prefix=/usr/local
> make all doc
> sudo make install install-doc install-html # REBUILDS HAPPEN HERE
>
> This causes the "sudo make install" to rebuild everything because it believes
> the version had changed.
> sudo strips $PATH for security reasons.
>
> The underlying problem flow is:
>
> 1 - Makefile has "include GIT-VERSION-FILE", thus gmake builds
> GIT-VERSION-FILE early.
>
> 2 - GIT-VERSION-FILE depends on a .PHONY target (.FORCE-GIT-VERSION-FILE)
> 3 - Thus, GNU make *always* executes GIT-VERSION-GEN
> 4 - GIT-VERSION-GEN now, under the stripped $PATH, cannot find "git" and
> sees a different version number.
> 5 - GIT-VERSION-GEN notes the difference in versions and regenerates
> GIT-VERSION-FILE, with up-to-date timestamp.
> 6 - GNU make rebuilds everything because GIT-VERSION-FILE is new.
>
> The patch makes GIT-VERSION-GEN look for the current built git$X executable,
> and in $(prefix)/bin/git, before falling back to plain "git" and thus to the
> default version in GIT-VERSION-GEN.
Thanks for the detailed analysis, now I g[oi]t it!
According to the analysis, the problem would also appear with a standard
make run (without configure) as long as git is not in the sudoer's $PATH
($prefix isn't, no distro git in /usr).
> Signed-off-by: Matthias Andree <matthias.andree@gmx.de>
> ---
> GIT-VERSION-GEN | 9 ++++-----
> Makefile | 6 +++++-
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index 39cde78..d0dfef3 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -2,6 +2,7 @@
>
> GVF=GIT-VERSION-FILE
> DEF_VER=v1.6.3.GIT
> +test -x "$GIT" || GIT=git
>
> LF='
> '
> @@ -12,12 +13,12 @@ if test -f version
> then
> VN=$(cat version) || VN="$DEF_VER"
> elif test -d .git -o -f .git &&
> - VN=$(git describe --abbrev=4 HEAD 2>/dev/null) &&
> + VN=$($GIT describe --abbrev=4 HEAD 2>/dev/null) &&
> case "$VN" in
> *$LF*) (exit 1) ;;
> v[0-9]*)
> - git update-index -q --refresh
> - test -z "$(git diff-index --name-only HEAD --)" ||
> + $GIT update-index -q --refresh
> + test -z "$($GIT diff-index --name-only HEAD --)" ||
> VN="$VN-dirty" ;;
> esac
> then
> @@ -38,5 +39,3 @@ test "$VN" = "$VC" || {
> echo >&2 "GIT_VERSION = $VN"
> echo "GIT_VERSION = $VN" >$GVF
> }
> -
> -
> diff --git a/Makefile b/Makefile
> index 6e21643..d6be483 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -177,7 +177,11 @@ all::
> # away (some NTFS drivers seem to zero the contents in that scenario).
>
> GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
> - @$(SHELL_PATH) ./GIT-VERSION-GEN
> + @{ GIT=./git$X ; test -x "$$GIT" ; } \
> + || { GIT=$(prefix)/bin/git$X ; test -x "$$GIT" ; }\
> + || GIT=git ; \
> + export GIT ; \
> + $(SHELL_PATH) ./GIT-VERSION-GEN
> -include GIT-VERSION-FILE
>
> uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not')
Looks good to me. So, untested but reviewed by me.
Michael
next prev parent reply other threads:[~2009-05-07 11:51 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-07 9:22 [PATCH v3] To make GIT-VERSION-FILE, search for git more widely Matthias Andree
2009-05-07 11:49 ` Michael J Gruber [this message]
2009-05-07 12:04 ` Matthias Andree
2009-05-07 12:09 ` Michael J Gruber
2009-05-07 12:12 ` Matthias Andree
2009-05-08 0:05 ` Junio C Hamano
2009-05-08 8:27 ` Matthias Andree
2009-05-08 8:41 ` Junio C Hamano
2009-05-08 11:09 ` Matthias Andree
2009-05-09 16:55 ` Junio C Hamano
2009-05-09 17:10 ` Francis Galiegue
2009-05-09 18:17 ` Matthias Andree
2009-05-13 12:17 ` Matthias Andree
2009-05-13 19:32 ` Junio C Hamano
2009-06-02 10:55 ` Nanako Shiraishi
2009-06-02 15:50 ` Junio C Hamano
2009-06-02 18:35 ` Johannes Sixt
2009-06-03 7:32 ` Matthias Andree
2009-06-04 0:12 ` [PATCH v4] " Matthias Andree
2009-06-04 5:18 ` [PATCH v3] " Junio C Hamano
2009-06-04 8:35 ` Matthias Andree
2009-05-08 8:52 ` Johannes Sixt
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A02CAD9.9080808@drmicha.warpmail.net \
--to=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=matthias.andree@gmx.de \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).