git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 08/10] run test suite without dashed git-commands in PATH
Date: Sun, 25 Jan 2009 23:40:04 -0700	[thread overview]
Message-ID: <20090126064004.GA3004@comcast.net> (raw)
In-Reply-To: <alpine.DEB.1.00.0901250255250.14855@racer>

Hi,

On Sun, Jan 25, 2009 at 02:59:53AM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Sat, 24 Jan 2009, Matthew Ogilvie wrote:
> 
> >  .gitignore          |    1 +
> >  Makefile            |   42 +++++++++++++++++++++++++++++++-----------
> >  t/test-lib.sh       |   14 +++++++++++++-
> >  test-bin-wrapper.sh |   12 ++++++++++++
> >  4 files changed, 57 insertions(+), 12 deletions(-)
> >  create mode 100644 test-bin-wrapper.sh
> 
> I am strongly opposed to a patch this big, just for something as 3rd class 
> as CVS server faking.  We already have a big fallout from all that bending 
> over for Windows support, and I do not like it at all.
> 
> Note: I do not even have to look further than the diffstat to see that it 
> is wrong.
> 
> The point is: if cvsserver wants to pretend that it is in a fake bin where 
> almost none of the other Git programs are, fine, let's do that _in the 
> test for cvsserver_.
> 
> Let's not fsck up the whole test suite just for one user.
> 
> Ciao,
> Dscho

Since by default git is installed such that most of the dashed-form
commands are not in a user's default PATH, my thought was that
it would make sense for the test suite to mimick that environment
as much as possible.  This could detect regressions in any
installed/tested git script that erroneously assumes the dashed
form commands are in the PATH, not just git-cvsserver.

I can think of ways it could be made smaller, but they seem uglier than
the current patch to me:

    1. Perhaps just list the executables for the fake bin directory
separately, but then it is all too easy for the list to get out of sync
with what the final install environment will be.

    2. Perhaps strip off the $X's (.exe on windows; currently empty
elsewhere) from the words of the existing variables, in the rule
for building the "fake bin" directory.  But generally I'ld rather
not assume that pattern-matching replacement of
$X's will never conflict with a part of a script name.

    3. Perhaps just use symlinks or hardlinks instead of a wrapper
script.  This might have some promise, except that links are more
likely to fail on windows, and the wrappers generally give you
more flexibility for testing odd scenarios.

    4. The test-bin-wrapper.sh script does not actually need to
set environment variables (GIT_EXEC_DIT and templates) for
purposes of this patch.  But my thought was that in this form
you could run things straight out of the test-bin directory
to manually try out new code without needing to actually install
a build or mess with the environment variables yourself.  It could
also be extended to handle other global wrapper needs
relatively easily, such as valgrind.

Any other ideas?

--
Matthew Ogilvie   [mmogilvi_git@miniinfo.net]

  reply	other threads:[~2009-01-26  6:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-24 23:43 [PATCH 00/10] Misc. cvsserver, dashless, and test suite patches Matthew Ogilvie
2009-01-24 23:43 ` [PATCH 01/10] cvsserver: removed unused sha1Or-k mode from kopts_from_path Matthew Ogilvie
2009-01-24 23:43   ` [PATCH 02/10] cvsserver: add comments about database schema/usage Matthew Ogilvie
2009-01-24 23:43     ` [PATCH 03/10] cvsserver: remove unused functions _headrev and gethistory Matthew Ogilvie
2009-01-24 23:43       ` [PATCH 04/10] git-shell: allow running git-cvsserver, not just cvs Matthew Ogilvie
2009-01-24 23:43         ` [PATCH 05/10] cvsserver: run dashless "git command"s to access plumbing Matthew Ogilvie
2009-01-24 23:43           ` [PATCH 06/10] t2300: use documented technique to invoke git-sh-setup Matthew Ogilvie
2009-01-24 23:43             ` [PATCH 07/10] t3409: use dashless "git commit" instead of "git-commit" Matthew Ogilvie
2009-01-24 23:43               ` [PATCH 08/10] run test suite without dashed git-commands in PATH Matthew Ogilvie
2009-01-24 23:43                 ` [PATCH 09/10] Revert "adapt git-cvsserver manpage to dash-free syntax" Matthew Ogilvie
2009-01-24 23:43                   ` [PATCH 10/10] cvsserver doc: emphasize using CVS_SERVER= phrase within CVSROOT Matthew Ogilvie
2009-01-25  1:59                 ` [PATCH 08/10] run test suite without dashed git-commands in PATH Johannes Schindelin
2009-01-26  6:40                   ` Matthew Ogilvie [this message]
2009-01-26 11:06                     ` Johannes Schindelin
2009-01-27  6:13                       ` Matthew Ogilvie
2009-01-25  1:53         ` [PATCH 04/10] git-shell: allow running git-cvsserver, not just cvs Johannes Schindelin

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=20090126064004.GA3004@comcast.net \
    --to=mmogilvi_git@miniinfo.net \
    --cc=Johannes.Schindelin@gmx.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 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).