git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: gitster@pobox.com
Cc: git@vger.kernel.org, Johannes.Schindelin@gmx.de,
	barkalow@iabervon.org, davvid@gmail.com
Subject: [RFCv4 7/5] More fixes to the git-remote-cvs installation procedure
Date: Tue, 18 Aug 2009 02:41:00 +0200	[thread overview]
Message-ID: <200908180241.00544.johan@herland.net> (raw)
In-Reply-To: <1250480161-21933-1-git-send-email-johan@herland.net>

- Makefile: Make sure git-remote-cvs is rebuilt when 'prefix' changes
  (by adding a dependency on GIT-CFLAGS). This prevents a regular 'make'
  followed by a 'make prefix=/somewhere/else install' from installing a
  non-working git-remote-cvs.

- Makefile: When mangling git-remote-cvs to add the git_remote_cvs install
  location to the Python search path, _replace_ the initial 'current dir'
  entry in sys.path (instead of merely prepending the install location).
  Hence, if the git_remote_cvs package is not installed at the correct
  location (and also not present in any of Python's default package dirs),
  then git-remote-cvs will fail loudly instead of silently falling back on
  the git_remote_cvs subdir in git.git.

- Allow for the git_remote_cvs install path to be overridden by the
  $GITPYTHONLIB environment variable.

- t/test-lib.sh: Set $GITPYTHONLIB (unless $GIT_TEST_INSTALLED is enabled)
  so that we test the currently built version of git_remote_cvs  (the one
  that is built in git_remote_cvs/build/lib) instead of a previously
  installed version.

- Another minor check and a line length fix.

Found-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---

On Monday 17 August 2009, Junio C Hamano wrote:
> With your Makefile patch the test still failed.  It turns out that I had
> a broken version from the previous round installed in my PATH, and the
> test failed until I removed the faulty ones manually from the installed
> location.  This is not good.
>
> While running tests, we really should import from the build directory,
> preferrably ignoring the installed directory but at least giving
> precedence to the build directory over the installed directory, to avoid
> problems like this.

Agreed. This patch puts the build directory first in Python's search path
when running the testsuite (unless $GIT_TEST_INSTALLED is enabled). The
install directory is ignored (unless the install dir happens to be in
Python's default search path, in which case the build dir will still have
precedence).

> In my case, it was a "bad installed version masking the version we are
> testing", but a more problematic would be the other way around.  If you
> have a good version installed, and if somebody breaks it in the updated
> source, "make test" won't catch the breakage and then you "make install"
> a broken version without noticing.

Yes. This is also taken care of in this patch ('make test' sets
$GITPYTHONLIB, which forces git-remote-cvs to look for its package
in the build dir instead of the install dir).

This patch (and the previous 6/5) will be folded into the next iteration
of the jh/cvs-helper patch series.


...Johan

 Makefile      |   11 +++++++++--
 t/test-lib.sh |    9 +++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index b9a7f25..1d7bf80 100644
--- a/Makefile
+++ b/Makefile
@@ -1477,13 +1477,20 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
 endif # NO_PERL
 
 ifndef NO_PYTHON
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS
 $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
 	$(QUIET_GEN)$(RM) $@ $@+ && \
-	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_cvs -s --no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' instlibdir` && \
+	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_cvs -s \
+		--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \
+		instlibdir` && \
 	sed -e '1{' \
 	    -e '	s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
 	    -e '}' \
-	    -e 's|^import sys.*|&; sys.path.insert(0, "@@INSTLIBDIR@@")|' \
+	    -e 's|^import sys.*|&; \\\
+	           import os; \\\
+	           sys.path[0] = os.environ.has_key("GITPYTHONLIB") and \\\
+	                         os.environ["GITPYTHONLIB"] or \\\
+	                         "@@INSTLIBDIR@@"|' \
 	    -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
 	    $@.py >$@+ && \
 	chmod +x $@+ && \
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 01ea386..a7fbfef 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -638,6 +638,15 @@ test -d ../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
+if test -z "$GIT_TEST_INSTALLED"
+then
+	GITPYTHONLIB="$(pwd)/../git_remote_cvs/build/lib"
+	export GITPYTHONLIB
+	test -d ../git_remote_cvs/build || {
+		error "You haven't built git_remote_cvs yet, have you?"
+	}
+fi
+
 if ! test -x ../test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
-- 
1.6.4.304.g1365c.dirty

      parent reply	other threads:[~2009-08-18  0:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-17  3:35 [RFCv4 0/5] CVS remote helper Johan Herland
2009-08-17  3:35 ` [RFCv4 1/5] Basic build infrastructure for Python scripts Johan Herland
2009-08-17  3:35 ` [RFCv4 2/5] 1/2: Add Python support library for CVS remote helper Johan Herland
2009-08-17  3:35 ` [RFCv4 3/5] 2/2: " Johan Herland
2009-10-05 13:31   ` Sverre Rabbelier
2009-10-05 14:41     ` Johan Herland
2009-10-05 14:50       ` Sverre Rabbelier
2009-10-05 15:35         ` Johan Herland
2009-08-17  3:36 ` [RFCv4 4/5] git-remote-cvs: Remote helper program for CVS repositories Johan Herland
2009-08-17  3:36 ` [RFCv4 5/5] Add simple selftests of git-remote-cvs functionality Johan Herland
2009-08-17 16:27 ` [RFCv4 6/5] Fix the Makefile-generated path to the git_remote_cvs package in git-remote-cvs Johan Herland
2009-08-18  0:41 ` Johan Herland [this message]

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=200908180241.00544.johan@herland.net \
    --to=johan@herland.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=barkalow@iabervon.org \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).