All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Joachim Schmitz" <jojo@schmitz-digital.de>
To: git@vger.kernel.org
Subject: Re: Test failures with python versions when building git 1.8.1
Date: Wed, 2 Jan 2013 10:09:17 +0100	[thread overview]
Message-ID: <kc0tg0$suo$1@ger.gmane.org> (raw)
In-Reply-To: 20130102085935.GB9328@sigill.intra.peff.net

Jeff King wrote:
> On Tue, Jan 01, 2013 at 11:18:46PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> [1] This symlink is doubly wrong, because any use of symbolic links
>>>     in the test scripts needs to depend on the SYMLINKS prereq, and
>>>     this does not.
>>
>> Yeah, I think we have discussed this once already in
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/210688/focus=210714
>
> Thanks for the pointer; it looks like nothing productive came of the
> earlier discussion. To give a hat trick of failure to this line of
> code, I notice that the existing code also does not properly put
> quotes around $GIT_BUILD_DIR.
>
>>> [2] In both the current code and what I showed above, the test
>>>     scripts depend on things in contrib/. This is probably a bad
>>>     idea in general, as the quality of what goes into contrib is
>>>     not as closely watched (especially with respect to things like
>>>     portability). Certainly I would not have known to look more
>>>     carefully at a patch to contrib/svn-fe for breakage to the test
>>> suite.
>>
>> As long as such tests are made skippable with appropriate
>> prerequisites, I do not think it is bad to have their tests in t/; I
>> would say it is rather better than having them in contrib/ and leave
>> it not run by anybody, which happened to some of the stuff in
>> contrib/ already.
>
> Good point. While my sense of decorum wants to keep contrib totally
> split out, from a practical point of view, it is better to have more
> people run the tests and report failures than not.
>
> Whether we end up doing something with contrib and tests or not, the
> patch below gives a minimal fix in the meantime. Dan, does it fix your
> problem?
>
> -- >8 --
> Subject: [PATCH] t9020: don't run python from $PATH
>
> In t9020, we symlink in a python script from contrib to help
> with the testing. However, we don't munge its #!-line, which
> means we may run the wrong python (we want the one in
> PYTHON_PATH). On top of this, we use a symlink without
> checking the SYMLINKS prereq, and we fail to properly quote
> GIT_BUILD_DIR, which may have spaces.
>
> Instead of symlinking, let's just write a small script which
> will feed the contrib script to PYTHON_PATH. To avoid
> quoting issues, we just export the variables the script
> needs to run.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t9020-remote-svn.sh | 5 ++++-
> t/test-lib.sh         | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
> index 4f2dfe0..416623b 100755
> --- a/t/t9020-remote-svn.sh
> +++ b/t/t9020-remote-svn.sh
> @@ -14,7 +14,10 @@ export PATH="$HOME:$PATH"
>
> # We override svnrdump by placing a symlink to the svnrdump-emulator
> in . export PATH="$HOME:$PATH"

With this patch that comment is no longer true.

> -ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
> +export GIT_BUILD_DIR
> +write_script svnrdump <<\EOF
> +exec "$PYTHON_PATH" "$GIT_BUILD_DIR"/contrib/svn-fe/svnrdump_sim.py
> "$@" +EOF
>
> init_git () {
>  rm -fr .git &&

  reply	other threads:[~2013-01-02  9:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-02  4:12 Test failures with python versions when building git 1.8.1 Dan McGee
2013-01-02  5:19 ` Junio C Hamano
2013-01-02  5:45   ` Eric S. Raymond
2013-01-02  6:53   ` Jeff King
2013-01-02  7:18     ` Junio C Hamano
2013-01-02  8:59       ` Jeff King
2013-01-02  9:09         ` Joachim Schmitz [this message]
2013-01-02 14:18         ` Dan McGee
2013-01-02 16:35           ` Junio C Hamano
2013-01-02 21:25             ` Dan McGee
2013-01-02 16:34         ` Junio C Hamano
2013-01-02 20:54           ` Jeff King

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='kc0tg0$suo$1@ger.gmane.org' \
    --to=jojo@schmitz-digital.de \
    --cc=git@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.