git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] NO_PERL support
@ 2009-04-03  7:03 Robin H. Johnson
  2009-04-03 13:20 ` Jeff King
                   ` (3 more replies)
  0 siblings, 4 replies; 30+ messages in thread
From: Robin H. Johnson @ 2009-04-03  7:03 UTC (permalink / raw)
  To: Git Mailing List


[-- Attachment #1.1: Type: text/plain, Size: 599 bytes --]

The attached patch adds NO_PERL to the build system, in the same fashion
as NO_TCLTK.

Effects:
- No perl-based scripts or code are installed:
	$SCRIPT_PERL
	git-instaweb
	gitweb
	git-cvsserver
	git-svn
- git-add does NOT have interactive support.
- None of the tests for the disabled stuff gets run.

This patch has been a optional part of the Gentoo build of Git for a
full year now, originally introduced with v1.5.4.4.

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #1.2: 20090402-git-1.6.2.2-noperl.patch --]
[-- Type: text/plain, Size: 12384 bytes --]

Implement 95% of the NO_PERL functionality, to build Git without any Perl
support, because some Gentoo users want a Git without any Perl whatesoever
(Gentoo bug #214168).

Remaining bits are doing configure.ac as well as git-remote usage in:
t5502-quickfetch.sh
t5512-ls-remote.sh
t5520-pull.sh

Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
Updated-by: Bernd Lommerzheim <bernd@lommerzheim.com>
Bugzilla-URL: http://bugs.gentoo.org/show_bug.cgi?id=214168
Notes: Ported from 20080423-git-1.5.5.1-noperl.patch
Notes: Ported from 20080322-git-1.5.4.5-noperl.patch
Notes: Ported from 20080528-git-1.5.6.1-noperl.patch
Notes: Ported from 20080626-git-1.5.6.1-noperl.patch and 20081123-git-1.6.0.4-noperl-cvsserver.patch
Notes: Ported from 20090126-git-1.6.1.1-noperl.patch
Notes: t3701-add-interactive.sh block added 2009/04/02 as it was missed, broke
       testsuite with USE=-perl.

diff --git a/Makefile b/Makefile
index 0675c43..0adb2b5 100644
--- a/Makefile
+++ b/Makefile
@@ -139,6 +139,8 @@ all::
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
 # MakeMaker (e.g. using ActiveState under Cygwin).
 #
+# Define NO_PERL if you do not want Perl scripts at all.
+#
 # Define NO_TCLTK if you do not want Tcl/Tk GUI.
 #
 # The TCL_PATH variable governs the location of the Tcl interpreter
@@ -277,6 +279,8 @@ SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
+SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH))
+ifndef NO_PERL
 SCRIPT_PERL += git-add--interactive.perl
 SCRIPT_PERL += git-archimport.perl
 SCRIPT_PERL += git-cvsexportcommit.perl
@@ -285,10 +289,11 @@ SCRIPT_PERL += git-cvsserver.perl
 SCRIPT_PERL += git-relink.perl
 SCRIPT_PERL += git-send-email.perl
 SCRIPT_PERL += git-svn.perl
-
-SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
-	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
-	  git-instaweb
+SCRIPTS += $(patsubst %.perl,%,$(SCRIPT_PERL)) \
+			git-instaweb
+else
+SCRIPT_PERL =
+endif
 
 # Empty...
 EXTRA_PROGRAMS =
@@ -335,7 +340,10 @@ BUILT_INS += git-whatchanged$X
 ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
 
 # what 'all' will build but not install in gitexecdir
-OTHER_PROGRAMS = git$X gitweb/gitweb.cgi
+OTHER_PROGRAMS = git$X
+ifndef NO_PERL
+OTHER_PROGRAMS += gitweb/gitweb.cgi
+endif
 
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
@@ -868,6 +876,10 @@ else
 	endif
 endif
 
+ifdef NO_PERL
+	BASIC_CFLAGS += -DNO_PERL
+endif
+
 ifdef ZLIB_PATH
 	BASIC_CFLAGS += -I$(ZLIB_PATH)/include
 	EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib)
@@ -1067,6 +1079,11 @@ endif
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
 endif
+ifeq ($(PERL_PATH),)
+NO_PERL=NoThanks
+export NO_PERL
+export NO_PERL_MAKEMAKER
+endif
 
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
@@ -1142,7 +1159,9 @@ ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
 	$(QUIET_SUBDIR0)gitk-git $(QUIET_SUBDIR1) all
 endif
+ifndef NO_PERL
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
+endif
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
 please_set_SHELL_PATH_to_a_more_modern_shell:
@@ -1189,6 +1208,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	chmod +x $@+ && \
 	mv $@+ $@
 
+ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
 perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL
@@ -1248,6 +1268,7 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
+endif # NO_PERL
 
 configure: configure.ac
 	$(QUIET_GEN)$(RM) $@ $<+ && \
@@ -1455,9 +1476,12 @@ install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
-	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
+ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
+	$(INSTALL) git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'
+endif
 ifndef NO_TCLTK
 	$(MAKE) -C gitk-git install
 	$(MAKE) -C git-gui gitexecdir='$(gitexec_instdir_SQ)' install
@@ -1565,9 +1589,11 @@ clean:
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
-	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
+ifndef NO_PERL
+	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C perl clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 ifndef NO_TCLTK
diff --git a/builtin-add.c b/builtin-add.c
index 08443f2..01e18a1 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -15,7 +15,9 @@ static const char * const builtin_add_usage[] = {
 	"git add [options] [--] <filepattern>...",
 	NULL
 };
+#ifndef NO_PERL
 static int patch_interactive, add_interactive;
+#endif
 static int take_worktree_changes;
 
 static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
@@ -158,6 +160,7 @@ static const char **validate_pathspec(int argc, const char **argv, const char *p
 	return pathspec;
 }
 
+#ifndef NO_PERL
 int interactive_add(int argc, const char **argv, const char *prefix)
 {
 	int status, ac;
@@ -186,6 +189,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	free(args);
 	return status;
 }
+#endif
 
 static struct lock_file lock_file;
 
@@ -199,8 +203,10 @@ static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only),
 	OPT__VERBOSE(&verbose),
 	OPT_GROUP(""),
+#ifndef NO_PERL
 	OPT_BOOLEAN('i', "interactive", &add_interactive, "interactive picking"),
 	OPT_BOOLEAN('p', "patch", &patch_interactive, "interactive patching"),
+#endif
 	OPT_BOOLEAN('f', "force", &ignored_too, "allow adding otherwise ignored files"),
 	OPT_BOOLEAN('u', "update", &take_worktree_changes, "update tracked files"),
 	OPT_BOOLEAN('N', "intent-to-add", &intent_to_add, "record only the fact that the path will be added later"),
@@ -252,10 +258,12 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
+#ifndef NO_PERL
 	if (patch_interactive)
 		add_interactive = 1;
 	if (add_interactive)
 		exit(interactive_add(argc, argv, prefix));
+#endif
 
 	git_config(add_config, NULL);
 
diff --git a/builtin-commit.c b/builtin-commit.c
index 46e649c..944f3ac 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -100,7 +100,9 @@ static struct option builtin_commit_options[] = {
 	OPT_GROUP("Commit contents options"),
 	OPT_BOOLEAN('a', "all", &all, "commit all changed files"),
 	OPT_BOOLEAN('i', "include", &also, "add specified files to index for commit"),
+#ifndef NO_PERL
 	OPT_BOOLEAN(0, "interactive", &interactive, "interactively add files"),
+#endif
 	OPT_BOOLEAN('o', "only", &only, "commit only specified files"),
 	OPT_BOOLEAN('n', "no-verify", &no_verify, "bypass pre-commit hook"),
 	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
@@ -223,6 +225,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 	struct string_list partial;
 	const char **pathspec = NULL;
 
+#ifndef NO_PERL
 	if (interactive) {
 		interactive_add(argc, argv, prefix);
 		if (read_cache_preload(NULL) < 0)
@@ -230,6 +233,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 		commit_style = COMMIT_AS_IS;
 		return get_index_file();
 	}
+#endif
 
 	if (*argv)
 		pathspec = get_pathspec(prefix, argv);
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 67c431d..83a5535 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -9,6 +9,12 @@ then
 	test_done
 	exit
 fi
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping git-svn tests, NO_PERL defined' :
+	test_done
+	exit
+fi
 
 GIT_DIR=$PWD/.git
 GIT_SVN_DIR=$GIT_DIR/svn/git-svn
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index eb63718..e3bcf20 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -3,6 +3,12 @@
 test_description='git remote porcelain-ish'
 
 . ./test-lib.sh
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping git-cvsimport tests, NO_PERL defined' :
+	test_done
+	exit
+fi
 
 setup_repository () {
 	mkdir "$1" && (
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b4e2b4d..2e74ce5 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -38,7 +38,7 @@ test_expect_success \
 	"echo King of the bongo >file &&
 	test_must_fail git commit -m foo -a file"
 
-test_expect_success \
+[ -z "$NO_PERL" ] && test_expect_success \
 	"using paths with --interactive" \
 	"echo bong-o-bong >file &&
 	! (echo 7 | git commit -m foo --interactive file)"
@@ -119,7 +119,7 @@ test_expect_success \
 	"echo 'gak' >file && \
 	 git commit -m 'author' --author 'Rubber Duck <rduck@convoy.org>' -a"
 
-test_expect_success \
+[ -z "$NO_PERL" ] && test_expect_success \
 	"interactive add" \
 	"echo 7 | git commit --interactive | grep 'What now'"
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index cb3d183..1d5355d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2,6 +2,12 @@
 
 test_description='git send-email'
 . ./test-lib.sh
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping git-send-email tests, NO_PERL defined' :
+	test_done
+	exit
+fi
 
 PROG='git send-email'
 test_expect_success \
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 245a7c3..6cd779a 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -13,6 +13,12 @@ then
     test_done
     exit
 fi
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping git-cvsexportcommit tests, NO_PERL defined' :
+	test_done
+	exit
+fi
 
 CVSROOT=$(pwd)/cvsroot
 CVSWORK=$(pwd)/cvswork
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 6a37f71..84a0d31 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -17,6 +17,12 @@ then
     test_done
     exit
 fi
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping git-cvsserver tests, NO_PERL defined' :
+	test_done
+	exit
+fi
 perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     test_expect_success 'skipping git-cvsserver tests, Perl SQLite interface unavailable' :
     test_done
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6ed10d0..d4580a4 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -72,6 +72,13 @@ safe_chmod () {
 }
 
 . ./test-lib.sh
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping gitweb-standalone-no-errors tests, NO_PERL defined' :
+	test_done
+	exit
+fi
+
 
 perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
     test_expect_success 'skipping gitweb tests, perl version is too old' :
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index d2379e7..75bf178 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -16,6 +16,12 @@ then
 	test_done
 	exit
 fi
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping git-cvsimport tests, NO_PERL defined' :
+	test_done
+	exit
+fi
 
 cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
 case "$cvsps_version" in
diff a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
--- a/t/t3701-add-interactive.sh	2009-04-02 14:26:45.000000000 -0700
+++ b/t/t3701-add-interactive.sh	2009-04-02 23:06:20.543358154 -0700
@@ -3,6 +3,13 @@
 test_description='add -i basic tests'
 . ./test-lib.sh
 
+if test -n "$NO_PERL"
+then
+	test_expect_success 'skipping add-interactive tests, NO_PERL defined' :
+	test_done
+	exit
+fi
+
 test_expect_success 'setup (initial)' '
 	echo content >file &&
 	git add file &&

[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03  7:03 [PATCH] NO_PERL support Robin H. Johnson
@ 2009-04-03 13:20 ` Jeff King
  2009-04-03 14:51   ` Johannes Schindelin
  2009-04-03 17:54   ` Johannes Sixt
  2009-04-03 15:46 ` Jeff King
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 13:20 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Johannes Sixt, Git Mailing List

[cc'ing Johannes because of some possibly related work in the test
scripts; see below]

On Fri, Apr 03, 2009 at 12:03:50AM -0700, Robin H. Johnson wrote:

> The attached patch adds NO_PERL to the build system, in the same fashion
> as NO_TCLTK.

I think this is a good change. One of my less-abled test platforms has a
broken perl, and I end up having to manually skip a lot of tests. Being
able to set NO_PERL and have the tests themselves realize they must be
skipped will be much cleaner.

> Effects:
> - No perl-based scripts or code are installed:
> 	$SCRIPT_PERL
> 	git-instaweb
> 	gitweb
> 	git-cvsserver
> 	git-svn
> - git-add does NOT have interactive support.
> - None of the tests for the disabled stuff gets run.

Hmm. With this patch, a user with a NO_PERL build will just get:

  $ git svn
  git: 'svn' is not a git-command. See 'git --help'.
  $ git add -i
  error: unknown switch `i'
  usage: ...

I wonder if we should be a little nicer and say "we know about this
command or option, but we did not build support for it". OTOH, what you
have mirrors what NO_TCLTK does, so perhaps it is not a big deal.

> @@ -1067,6 +1079,11 @@ endif
>  ifeq ($(TCLTK_PATH),)
>  NO_TCLTK=NoThanks
>  endif
> +ifeq ($(PERL_PATH),)
> +NO_PERL=NoThanks
> +export NO_PERL
> +export NO_PERL_MAKEMAKER
> +endif

Do we have to actually export here? NO_TCLTK doesn't. If it's for the
test scripts, then should it be part of GIT-BUILD-OPTIONS?

> +if test -n "$NO_PERL"
> +then
> +	test_expect_success 'skipping git-svn tests, NO_PERL defined' :
> +	test_done
> +	exit
> +fi

This probably got copied from an older example, but I think the
recommended way to skip tests these days is to use 'say' instead of
test_expect_success (since we have statistics on passing/failing tests
now).

Also, it may make sense to integrate this with Johannes Sixt's
test_have_prereq work (which is still in next), but I haven't looked too
closely at that.

> -test_expect_success \
> +[ -z "$NO_PERL" ] && test_expect_success \

I think it is nicer to actually say "skipping test" as the other spots
do instead of just not running it.

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 13:20 ` Jeff King
@ 2009-04-03 14:51   ` Johannes Schindelin
  2009-04-03 14:53     ` Jeff King
  2009-04-03 17:54   ` Johannes Sixt
  1 sibling, 1 reply; 30+ messages in thread
From: Johannes Schindelin @ 2009-04-03 14:51 UTC (permalink / raw)
  To: Jeff King; +Cc: Robin H. Johnson, Johannes Sixt, Git Mailing List

Hi,

On Fri, 3 Apr 2009, Jeff King wrote:

> On Fri, Apr 03, 2009 at 12:03:50AM -0700, Robin H. Johnson wrote:
> 
> > Effects:
> > - No perl-based scripts or code are installed:
> > 	$SCRIPT_PERL
> > 	git-instaweb
> > 	gitweb
> > 	git-cvsserver
> > 	git-svn
> > - git-add does NOT have interactive support.
> > - None of the tests for the disabled stuff gets run.
> 
> Hmm. With this patch, a user with a NO_PERL build will just get:
> 
>   $ git svn
>   git: 'svn' is not a git-command. See 'git --help'.
>   $ git add -i
>   error: unknown switch `i'
>   usage: ...
> 
> I wonder if we should be a little nicer and say "we know about this
> command or option, but we did not build support for it". OTOH, what you
> have mirrors what NO_TCLTK does, so perhaps it is not a big deal.

Actually, "add -i" is a part of another program that does work without 
Perl, so it is quite a different kind of kettle.

It is not like "git gui" works with NO_TCLTK but "git gui -i" does not.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 14:51   ` Johannes Schindelin
@ 2009-04-03 14:53     ` Jeff King
  2009-04-03 15:02       ` Johannes Schindelin
  0 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-03 14:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Robin H. Johnson, Johannes Sixt, Git Mailing List

On Fri, Apr 03, 2009 at 04:51:09PM +0200, Johannes Schindelin wrote:

> > Hmm. With this patch, a user with a NO_PERL build will just get:
> > 
> >   $ git svn
> >   git: 'svn' is not a git-command. See 'git --help'.
> >   $ git add -i
> >   error: unknown switch `i'
> >   usage: ...
> > 
> > I wonder if we should be a little nicer and say "we know about this
> > command or option, but we did not build support for it". OTOH, what you
> > have mirrors what NO_TCLTK does, so perhaps it is not a big deal.
> 
> Actually, "add -i" is a part of another program that does work without 
> Perl, so it is quite a different kind of kettle.
> 
> It is not like "git gui" works with NO_TCLTK but "git gui -i" does not.

Yes, there are really two cases here:

  1. options to programs that will be disabled

  2. whole programs that will not be installed

Are you proposing to print "sorry, perl support not compiled in" (or
whatever) for case 1, but ignore case 2? (And I am not sure if that is a
bad idea, but I am trying to be clear on what you mean).

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 14:53     ` Jeff King
@ 2009-04-03 15:02       ` Johannes Schindelin
  2009-04-03 15:11         ` Miklos Vajna
  2009-04-03 15:16         ` Jeff King
  0 siblings, 2 replies; 30+ messages in thread
From: Johannes Schindelin @ 2009-04-03 15:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Robin H. Johnson, Johannes Sixt, Git Mailing List

Hi,

On Fri, 3 Apr 2009, Jeff King wrote:

> On Fri, Apr 03, 2009 at 04:51:09PM +0200, Johannes Schindelin wrote:
> 
> > > Hmm. With this patch, a user with a NO_PERL build will just get:
> > > 
> > >   $ git svn
> > >   git: 'svn' is not a git-command. See 'git --help'.
> > >   $ git add -i
> > >   error: unknown switch `i'
> > >   usage: ...
> > > 
> > > I wonder if we should be a little nicer and say "we know about this
> > > command or option, but we did not build support for it". OTOH, what you
> > > have mirrors what NO_TCLTK does, so perhaps it is not a big deal.
> > 
> > Actually, "add -i" is a part of another program that does work without 
> > Perl, so it is quite a different kind of kettle.
> > 
> > It is not like "git gui" works with NO_TCLTK but "git gui -i" does not.
> 
> Yes, there are really two cases here:
> 
>   1. options to programs that will be disabled
> 
>   2. whole programs that will not be installed
> 
> Are you proposing to print "sorry, perl support not compiled in" (or
> whatever) for case 1, but ignore case 2? (And I am not sure if that is a
> bad idea, but I am trying to be clear on what you mean).

1) as it is all too easy, but not 2) as that would require too much work, 
which would be overkill.

Just my 1ct,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 15:02       ` Johannes Schindelin
@ 2009-04-03 15:11         ` Miklos Vajna
  2009-04-03 15:16         ` Jeff King
  1 sibling, 0 replies; 30+ messages in thread
From: Miklos Vajna @ 2009-04-03 15:11 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jeff King, Robin H. Johnson, Johannes Sixt, Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Fri, Apr 03, 2009 at 05:02:16PM +0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 1) as it is all too easy, but not 2) as that would require too much work, 
> which would be overkill.

What about something trivial like this?

diff --git a/builtin-add.c b/builtin-add.c
index cb67d2c..a3d798e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -164,6 +164,10 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	const char **args;
 	const char **pathspec = NULL;
 
+#ifdef NO_PERL
+	return error("git was compiled without perl support.");
+#endif
+
 	if (argc) {
 		pathspec = validate_pathspec(argc, argv, prefix);
 		if (!pathspec)

This way it would be more explicit that it was disabled and the
situation is not about -i was removed in some new versions or anything.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 15:02       ` Johannes Schindelin
  2009-04-03 15:11         ` Miklos Vajna
@ 2009-04-03 15:16         ` Jeff King
  1 sibling, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 15:16 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Robin H. Johnson, Johannes Sixt, Git Mailing List

On Fri, Apr 03, 2009 at 05:02:16PM +0200, Johannes Schindelin wrote:

> > Yes, there are really two cases here:
> > 
> >   1. options to programs that will be disabled
> > 
> >   2. whole programs that will not be installed
> > 
> > Are you proposing to print "sorry, perl support not compiled in" (or
> > whatever) for case 1, but ignore case 2? (And I am not sure if that is a
> > bad idea, but I am trying to be clear on what you mean).
> 
> 1) as it is all too easy, but not 2) as that would require too much work, 
> which would be overkill.

I don't think 2 is that hard. See the patch below (on top of Robin's).
The question is whether it is more confusing for these programs to show
up in completion and then turn out to be totally broken, or for them to
simply be missing altogether. That is, as a git user who is accustomed
to the regular build, will I be more confused by:

  $ git svn
  Sorry, git was built without support for git-svn (NO_PERL).

or

  $ git svn
  git: 'svn' is not a git-command. See 'git --help'.

---
diff --git a/Makefile b/Makefile
index c4eb1ad..41abadb 100644
--- a/Makefile
+++ b/Makefile
@@ -298,7 +298,6 @@ SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH))
-ifndef NO_PERL
 SCRIPT_PERL += git-add--interactive.perl
 SCRIPT_PERL += git-archimport.perl
 SCRIPT_PERL += git-cvsexportcommit.perl
@@ -309,9 +308,6 @@ SCRIPT_PERL += git-send-email.perl
 SCRIPT_PERL += git-svn.perl
 SCRIPTS += $(patsubst %.perl,%,$(SCRIPT_PERL)) \
 			git-instaweb
-else
-SCRIPT_PERL =
-endif
 
 # Empty...
 EXTRA_PROGRAMS =
@@ -1304,6 +1300,13 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
+else # NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : missing-perl.sh
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	    missing-perl.sh >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
 endif # NO_PERL
 
 configure: configure.ac
diff --git a/missing-perl.sh b/missing-perl.sh
new file mode 100644
index 0000000..d277be9
--- /dev/null
+++ b/missing-perl.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+echo "Sorry, git was built without support for `basename $0` (NO_PERL)."
+exit 128

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03  7:03 [PATCH] NO_PERL support Robin H. Johnson
  2009-04-03 13:20 ` Jeff King
@ 2009-04-03 15:46 ` Jeff King
  2009-04-03 15:59   ` Robin H. Johnson
  2009-04-03 16:25 ` Junio C Hamano
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
  3 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-03 15:46 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

Also, a few meta-issues with this patch.

One is that the usual practice is to send patches inline rather than
attaching them.

Another is that this text:

> The attached patch adds NO_PERL to the build system, in the same fashion
> as NO_TCLTK.
> 
> Effects:
> - No perl-based scripts or code are installed:
> 	$SCRIPT_PERL
> 	git-instaweb
> 	gitweb
> 	git-cvsserver
> 	git-svn
> - git-add does NOT have interactive support.
> - None of the tests for the disabled stuff gets run.
> 
> This patch has been a optional part of the Gentoo build of Git for a
> full year now, originally introduced with v1.5.4.4.

should go with the patch as part of the commit message along with this:

> Implement 95% of the NO_PERL functionality, to build Git without any Perl
> support, because some Gentoo users want a Git without any Perl whatesoever
> (Gentoo bug #214168).
> 
> Remaining bits are doing configure.ac as well as git-remote usage in:
> t5502-quickfetch.sh
> t5512-ls-remote.sh
> t5520-pull.sh
> 
> Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
> Updated-by: Bernd Lommerzheim <bernd@lommerzheim.com>
> Bugzilla-URL: http://bugs.gentoo.org/show_bug.cgi?id=214168
> Notes: Ported from 20080423-git-1.5.5.1-noperl.patch
> Notes: Ported from 20080322-git-1.5.4.5-noperl.patch
> Notes: Ported from 20080528-git-1.5.6.1-noperl.patch
> Notes: Ported from 20080626-git-1.5.6.1-noperl.patch and 20081123-git-1.6.0.4-noperl-cvsserver.patch
> Notes: Ported from 20090126-git-1.6.1.1-noperl.patch
> Notes: t3701-add-interactive.sh block added 2009/04/02 as it was missed, broke
>        testsuite with USE=-perl.

If you are using git to prepare your patch, then "git format-patch"
should generate the right output.

And finally, these two hunks:

> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index 6a37f71..84a0d31 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -17,6 +17,12 @@ then
> [...]
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index 6ed10d0..d4580a4 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh

did not apply for me, to the current 'master' or 'next'. Furthermore,
the blobs they are based on aren't even present in my repo, so a 3-way
merge was impossible. What did you base this patch on?

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 15:46 ` Jeff King
@ 2009-04-03 15:59   ` Robin H. Johnson
  2009-04-03 16:18     ` Jeff King
  0 siblings, 1 reply; 30+ messages in thread
From: Robin H. Johnson @ 2009-04-03 15:59 UTC (permalink / raw)
  To: Git Mailing List

On Fri, Apr 03, 2009 at 11:46:08AM -0400, Jeff King wrote:
> Also, a few meta-issues with this patch.
>
> One is that the usual practice is to send patches inline rather than
> attaching them.
I usually send as attachments, as the filename of the patch is important
as well, and the majority of places that I submit to want attachments.

> Another is that this text:
...
> should go with the patch as part of the commit message along with this:
The text was specifically describing it for the Git community.

...
> If you are using git to prepare your patch, then "git format-patch"
> should generate the right output.
This patch was generated with format-patch years ago, but has been
hand-tuned since.

> And finally, these two hunks:
> > diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> > index 6a37f71..84a0d31 100755
> > --- a/t/t9400-git-cvsserver-server.sh
> > +++ b/t/t9400-git-cvsserver-server.sh
> > @@ -17,6 +17,12 @@ then
> > [...]
> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> > index 6ed10d0..d4580a4 100755
> > --- a/t/t9500-gitweb-standalone-no-errors.sh
> > +++ b/t/t9500-gitweb-standalone-no-errors.sh
> did not apply for me, to the current 'master' or 'next'. Furthermore,
> the blobs they are based on aren't even present in my repo, so a 3-way
> merge was impossible. What did you base this patch on?
The patch applies cleanly to the 1.6.2.* tarballs.

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 15:59   ` Robin H. Johnson
@ 2009-04-03 16:18     ` Jeff King
  0 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 16:18 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

On Fri, Apr 03, 2009 at 08:59:38AM -0700, Robin H. Johnson wrote:

> I usually send as attachments, as the filename of the patch is important
> as well, and the majority of places that I submit to want attachments.
> [...]
> The text was specifically describing it for the Git community.
> [...]
> This patch was generated with format-patch years ago, but has been
> hand-tuned since.

Ah, OK. Maybe I misunderstood; I thought you were submitting this for
inclusion in upstream git. In which case things like the patch filename
would become irrelevant (and the maintainer's ability to apply via "git
am" would become relevant).

> > did not apply for me, to the current 'master' or 'next'. Furthermore,
> > the blobs they are based on aren't even present in my repo, so a 3-way
> > merge was impossible. What did you base this patch on?
> The patch applies cleanly to the 1.6.2.* tarballs.

OK, thanks.

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03  7:03 [PATCH] NO_PERL support Robin H. Johnson
  2009-04-03 13:20 ` Jeff King
  2009-04-03 15:46 ` Jeff King
@ 2009-04-03 16:25 ` Junio C Hamano
  2009-04-03 17:15   ` Jeff King
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
  3 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2009-04-03 16:25 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

"Robin H. Johnson" <robbat2@gentoo.org> writes:

> diff --git a/Makefile b/Makefile
> index 0675c43..0adb2b5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -139,6 +139,8 @@ all::
>  # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
>  # MakeMaker (e.g. using ActiveState under Cygwin).
>  #
> +# Define NO_PERL if you do not want Perl scripts at all.
> +#
>  # Define NO_TCLTK if you do not want Tcl/Tk GUI.
>  #
>  # The TCL_PATH variable governs the location of the Tcl interpreter
> @@ -277,6 +279,8 @@ SCRIPT_SH += git-stash.sh
>  SCRIPT_SH += git-submodule.sh
>  SCRIPT_SH += git-web--browse.sh
>  
> +SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH))
> +ifndef NO_PERL
>  SCRIPT_PERL += git-add--interactive.perl
>  SCRIPT_PERL += git-archimport.perl
>  SCRIPT_PERL += git-cvsexportcommit.perl
> @@ -285,10 +289,11 @@ SCRIPT_PERL += git-cvsserver.perl
>  SCRIPT_PERL += git-relink.perl
>  SCRIPT_PERL += git-send-email.perl
>  SCRIPT_PERL += git-svn.perl
> -
> -SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
> -	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
> -	  git-instaweb
> +SCRIPTS += $(patsubst %.perl,%,$(SCRIPT_PERL)) \
> +			git-instaweb
> +else
> +SCRIPT_PERL =
> +endif
>  
>...
> +ifdef NO_PERL
> +	BASIC_CFLAGS += -DNO_PERL
> +endif

While I do not mind omitting whole programs in fringes like you did for
archimport and relink, I *really* do not like this particular change (and
any change to the C code).

I'd rather see something along the lines of:

	if NO_PERL
        git-add--interactive: unimplemented.sh
        	rm -f $@+ $@
                cat $? >$@+
                chmod +x $@+
                mv $@+ $@
	else
        git-add--interactive: git-add--interactive.perl
        	... usual .perl to script rule applies
	endif

and ship:

	#!/bin/sh
        echo >&2 "Sorry $0 not available here"
        exit 1

in unimplemented.sh, *without* touching code that calls out to
scripts that happen to be implemented in Perl.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 16:25 ` Junio C Hamano
@ 2009-04-03 17:15   ` Jeff King
  2009-04-03 20:20     ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-03 17:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

On Fri, Apr 03, 2009 at 09:25:21AM -0700, Junio C Hamano wrote:

> While I do not mind omitting whole programs in fringes like you did for
> archimport and relink, I *really* do not like this particular change (and
> any change to the C code).
> 
> I'd rather see something along the lines of:
> 
> 	if NO_PERL
>         git-add--interactive: unimplemented.sh
>         	rm -f $@+ $@
>                 cat $? >$@+
>                 chmod +x $@+
>                 mv $@+ $@
> 	else
>         git-add--interactive: git-add--interactive.perl
>         	... usual .perl to script rule applies
> 	endif
> 
> and ship:
> 
> 	#!/bin/sh
>         echo >&2 "Sorry $0 not available here"
>         exit 1
> 
> in unimplemented.sh, *without* touching code that calls out to
> scripts that happen to be implemented in Perl.

OK, that is more or less what I proposed elsewhere in the thread. Below
is a cleaned up version of the Makefile bits.

The test script changes should come in a different patch, but they are a
bit more complex. Tests that rely on missing scripts should obviously
just be skipped outright.  But there are some test scripts that use perl
for testing tasks (e.g., t4103), and those are not covered by Robin's
patch. Do we want to also skip those? Do we want to place the burden on
authors of the test suite to always check for NO_PERL whenever they use
perl?

The other option would be saying "we support building with NO_PERL, but
not running the test suite".

Thoughts?

-- >8 --
Subject: [PATCH] Makefile: allow building without perl

For systems with a missing or broken perl, it is nicer to
explicitly say "we don't want perl" because:

  1. The Makefile knows not to bother with Perl-ish things
     like Git.pm.

  2. We can print a more user-friendly error message
     than "foo is not a git command" or whatever the broken
     perl might barf

  3. Test scripts that require perl can mark themselves and
     such and be skipped

This patch implements parts (1) and (2). The perl/
subdirectory is skipped entirely, gitweb is not built, and
any git commands which rely on perl will print a
human-readable message and exit with an error code.

This patch is based on one from Robin H. Johnson.

Signed-off-by: Jeff King <peff@peff.net>
---
 Makefile         |   23 +++++++++++++++++++++--
 unimplemented.sh |    4 ++++
 2 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 unimplemented.sh

diff --git a/Makefile b/Makefile
index 0675c43..44c856d 100644
--- a/Makefile
+++ b/Makefile
@@ -139,6 +139,8 @@ all::
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
 # MakeMaker (e.g. using ActiveState under Cygwin).
 #
+# Define NO_PERL if you do not want Perl scripts or libraries at all.
+#
 # Define NO_TCLTK if you do not want Tcl/Tk GUI.
 #
 # The TCL_PATH variable governs the location of the Tcl interpreter
@@ -335,7 +337,10 @@ BUILT_INS += git-whatchanged$X
 ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
 
 # what 'all' will build but not install in gitexecdir
-OTHER_PROGRAMS = git$X gitweb/gitweb.cgi
+OTHER_PROGRAMS = git$X
+ifndef NO_PERL
+OTHER_PROGRAMS += gitweb/gitweb.cgi
+endif
 
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
@@ -1142,7 +1147,9 @@ ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
 	$(QUIET_SUBDIR0)gitk-git $(QUIET_SUBDIR1) all
 endif
+ifndef NO_PERL
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
+endif
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
 please_set_SHELL_PATH_to_a_more_modern_shell:
@@ -1189,6 +1196,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	chmod +x $@+ && \
 	mv $@+ $@
 
+ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
 perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL
@@ -1248,6 +1256,15 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
+else # NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
+	    unimplemented.sh >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+endif # NO_PERL
 
 configure: configure.ac
 	$(QUIET_GEN)$(RM) $@ $<+ && \
@@ -1565,9 +1582,11 @@ clean:
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
-	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
+ifndef NO_PERL
+	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C perl clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 ifndef NO_TCLTK
diff --git a/unimplemented.sh b/unimplemented.sh
new file mode 100644
index 0000000..5252de4
--- /dev/null
+++ b/unimplemented.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+echo >&2 "fatal: git was built without support for `basename $0` (@@REASON@@)."
+exit 128
-- 
1.6.2.2.569.g2d4b2



> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 13:20 ` Jeff King
  2009-04-03 14:51   ` Johannes Schindelin
@ 2009-04-03 17:54   ` Johannes Sixt
  2009-04-03 18:37     ` Jeff King
  1 sibling, 1 reply; 30+ messages in thread
From: Johannes Sixt @ 2009-04-03 17:54 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Jeff King, Git Mailing List

On Freitag, 3. April 2009, Jeff King wrote:
> On Fri, Apr 03, 2009 at 12:03:50AM -0700, Robin H. Johnson wrote:
> > +if test -n "$NO_PERL"
> > +then
> > +	test_expect_success 'skipping git-svn tests, NO_PERL defined' :
> > +	test_done
> > +	exit
> > +fi
>
> This probably got copied from an older example, but I think the
> recommended way to skip tests these days is to use 'say' instead of
> test_expect_success (since we have statistics on passing/failing tests
> now).
>
> Also, it may make sense to integrate this with Johannes Sixt's
> test_have_prereq work (which is still in next), but I haven't looked too
> closely at that.

If you base the patch on 'master', you can add this line to test-lib.sh

test -z "$NO_PERL" && test_set_prereq PERL

[But actually, if I read you patch correctly, NO_PERL will be set in 
t/Makefile only if one runs 'make test' from the main directory. You should 
invent some method to detect missing perl (or that NO_PERL was set) if 'make' 
is run directly from t/.]

Now you write the above as

if ! test_have_prereq PERL
then
	say 'perl not available - skipping git-svn tests'
	test_done
	exit
fi

Furthermore, you can skip single tests like this:

> > -test_expect_success \
> > +[ -z "$NO_PERL" ] && test_expect_success \

test_expect_success PERL \

-- Hannes

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 17:54   ` Johannes Sixt
@ 2009-04-03 18:37     ` Jeff King
  0 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 18:37 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Robin H. Johnson, Git Mailing List

On Fri, Apr 03, 2009 at 07:54:57PM +0200, Johannes Sixt wrote:

> If you base the patch on 'master', you can add this line to test-lib.sh
> 
> test -z "$NO_PERL" && test_set_prereq PERL

Thanks, I'm working up a patch using this now.

> [But actually, if I read you patch correctly, NO_PERL will be set in 
> t/Makefile only if one runs 'make test' from the main directory. You should 
> invent some method to detect missing perl (or that NO_PERL was set) if 'make' 
> is run directly from t/.]

Yes, it should go into GIT-BUILD-OPTIONS which gets sourced by
test-lib.sh (and then it not only works with "make test" from "t/", but
also with "sh t????-whatever.sh".

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/4] NO_PERL support
  2009-04-03  7:03 [PATCH] NO_PERL support Robin H. Johnson
                   ` (2 preceding siblings ...)
  2009-04-03 16:25 ` Junio C Hamano
@ 2009-04-03 19:27 ` Jeff King
  2009-04-03 19:28   ` [PATCH 1/4] commit: abort commit if interactive add failed Jeff King
                     ` (4 more replies)
  3 siblings, 5 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 19:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

OK, here is a series based on Robin's patch that I think is suitable for
inclusion in mainstream git. The first two are related cleanups, the
third is a rebase of what I sent earlier today, and the fourth covers
the matching tests.

  1/4: commit: abort commit if interactive add failed
  2/4: tests: remove exit after test_done call
  3/4: Makefile: allow building without perl
  4/4: tests: skip perl tests if NO_PERL is defined

With these applied, you can build and pass the tests with NO_PERL
defined. _But_ you still can't pass the tests without perl installed at
all, as several of the other tests rely on perl to do text munging in
the tests. I'm not sure it is possible to rewrite them not to use perl;
we ended up with perl in the first place because many versions of
standard tools like sed don't handle NULs very well. So I think the only
option would be to skip those tests, too.

Breaking my perl installation yields these failing tests:

  $ cat <<'EOF' >$HOME/local/bin
  #!/bin/sh
  echo >&2 I am a broken perl.
  exit 1
  EOF
  $ make test NO_PERL=NoThanks
  ...
  $ cd t && grep 'failed [^0]' test-results/*
  test-results/t0020-crlf-25091:failed 16
  test-results/t1300-repo-config-30265:failed 2
  test-results/t3300-funny-names-4931:failed 3
  test-results/t4012-diff-binary-22876:failed 1
  test-results/t4014-format-patch-23170:failed 14
  test-results/t4020-diff-external-25409:failed 2
  test-results/t4029-diff-trailing-space-26781:failed 1
  test-results/t4030-diff-textconv-26899:failed 5
  test-results/t4031-diff-rewrite-binary-26911:failed 1
  test-results/t4103-apply-binary-28344:failed 8
  test-results/t4116-apply-reverse-29276:failed 7
  test-results/t4200-rerere-30898:failed 1
  test-results/t5300-pack-object-983:failed 24
  test-results/t5303-pack-corruption-resilience-2980:failed 3
  test-results/t6002-rev-list-bisect-17175:failed 12
  test-results/t6003-rev-list-topo-order-17683:failed 31
  test-results/t6011-rev-list-with-bad-commit-19971:failed 3
  test-results/t6013-rev-list-reverse-parents-20684:failed 2
  test-results/t8001-annotate-11125:failed 10
  test-results/t8002-blame-11523:failed 10

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] commit: abort commit if interactive add failed
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
@ 2009-04-03 19:28   ` Jeff King
  2009-04-03 19:31   ` [PATCH 2/4] tests: remove exit after test_done call Jeff King
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 19:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

Previously we ignored the result of calling add_interactive,
which meant that if an error occurred we simply committed
whatever happened to be in the index.

Signed-off-by: Jeff King <peff@peff.net>
---
Just a fix I noticed while looking at t7501.

 builtin-commit.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 46e649c..81371b1 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -224,7 +224,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 	const char **pathspec = NULL;
 
 	if (interactive) {
-		interactive_add(argc, argv, prefix);
+		if (interactive_add(argc, argv, prefix) != 0)
+			die("interactive add failed");
 		if (read_cache_preload(NULL) < 0)
 			die("index file corrupt");
 		commit_style = COMMIT_AS_IS;
-- 
1.6.2.2.569.g2d4b2

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] tests: remove exit after test_done call
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
  2009-04-03 19:28   ` [PATCH 1/4] commit: abort commit if interactive add failed Jeff King
@ 2009-04-03 19:31   ` Jeff King
  2009-04-03 19:32   ` [PATCH 3/4] Makefile: allow building without perl Jeff King
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 19:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

test_done always exits, so this line is never executed.

Signed-off-by: Jeff King <peff@peff.net>
---
A cleanup I noticed while adding more test skipping. Technically these
lines were not hurting anything, but I think it prevents a reader from
wondering why some test_done instances are followed by exit and some are
not.

And just look at that diffstat line count. It _must_ be an improvement!

 t/lib-git-svn.sh                       |    4 ----
 t/lib-httpd.sh                         |    3 ---
 t/t4004-diff-rename-symlink.sh         |    1 -
 t/t4011-diff-symlink.sh                |    1 -
 t/t4023-diff-rename-typechange.sh      |    1 -
 t/t4114-apply-typechange.sh            |    1 -
 t/t4115-apply-symlink.sh               |    1 -
 t/t4122-apply-symlink-inside.sh        |    1 -
 t/t5503-tagfollow.sh                   |    1 -
 t/t5522-pull-symlink.sh                |    1 -
 t/t5540-http-push.sh                   |    1 -
 t/t7005-editor.sh                      |    1 -
 t/t9200-git-cvsexportcommit.sh         |    1 -
 t/t9400-git-cvsserver-server.sh        |    2 --
 t/t9401-git-cvsserver-crlf.sh          |    2 --
 t/t9500-gitweb-standalone-no-errors.sh |    1 -
 t/t9600-cvsimport.sh                   |    3 ---
 17 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index de384e6..cdd7ccd 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -7,7 +7,6 @@ if test -n "$NO_SVN_TESTS"
 then
 	say 'skipping git svn tests, NO_SVN_TESTS defined'
 	test_done
-	exit
 fi
 
 GIT_DIR=$PWD/.git
@@ -19,7 +18,6 @@ if test $? -ne 1
 then
     say 'skipping git svn tests, svn not found'
     test_done
-    exit
 fi
 
 svnrepo=$PWD/svnrepo
@@ -43,7 +41,6 @@ then
 	fi
 	say "$err"
 	test_done
-	exit
 fi
 
 rawsvnrepo="$svnrepo"
@@ -144,7 +141,6 @@ require_svnserve () {
     then
         say 'skipping svnserve test. (set $SVNSERVE_PORT to enable)'
         test_done
-        exit
     fi
 }
 
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 589aaf8..cde659d 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -8,7 +8,6 @@ then
 	say "skipping test, network testing disabled by default"
 	say "(define GIT_TEST_HTTPD to enable)"
 	test_done
-	exit
 fi
 
 HTTPD_PARA=""
@@ -36,7 +35,6 @@ if ! test -x "$LIB_HTTPD_PATH"
 then
 	say "skipping test, no web server found at '$LIB_HTTPD_PATH'"
 	test_done
-	exit
 fi
 
 HTTPD_VERSION=`$LIB_HTTPD_PATH -v | \
@@ -50,7 +48,6 @@ then
 		then
 			say "skipping test, at least Apache version 2 is required"
 			test_done
-			exit
 		fi
 
 		LIB_HTTPD_MODULE_PATH="$DEFAULT_HTTPD_MODULE_PATH"
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
index 3db7444..a4da119 100755
--- a/t/t4004-diff-rename-symlink.sh
+++ b/t/t4004-diff-rename-symlink.sh
@@ -16,7 +16,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 test_expect_success \
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 3a81309..d7e327c 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -13,7 +13,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 cat > expected << EOF
diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh
index 5099862..9bdf659 100755
--- a/t/t4023-diff-rename-typechange.sh
+++ b/t/t4023-diff-rename-typechange.sh
@@ -8,7 +8,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 test_expect_success setup '
diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh
index 7dc35de..99ec13d 100755
--- a/t/t4114-apply-typechange.sh
+++ b/t/t4114-apply-typechange.sh
@@ -13,7 +13,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 test_expect_success 'setup repository and commits' '
diff --git a/t/t4115-apply-symlink.sh b/t/t4115-apply-symlink.sh
index 1a3aea3..b852e58 100755
--- a/t/t4115-apply-symlink.sh
+++ b/t/t4115-apply-symlink.sh
@@ -13,7 +13,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 test_expect_success setup '
diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh
index 8aad20b..0d3c1d5 100755
--- a/t/t4122-apply-symlink-inside.sh
+++ b/t/t4122-apply-symlink-inside.sh
@@ -7,7 +7,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 lecho () {
diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
index e75ccbc..d5db75d 100755
--- a/t/t5503-tagfollow.sh
+++ b/t/t5503-tagfollow.sh
@@ -8,7 +8,6 @@ case $(uname -s) in
 *MINGW*)
 	say "GIT_DEBUG_SEND_PACK not supported - skipping tests"
 	test_done
-	exit
 esac
 
 # End state of the repository:
diff --git a/t/t5522-pull-symlink.sh b/t/t5522-pull-symlink.sh
index d887eb6..86bbd7d 100755
--- a/t/t5522-pull-symlink.sh
+++ b/t/t5522-pull-symlink.sh
@@ -8,7 +8,6 @@ if ! test_have_prereq SYMLINKS
 then
 	say 'Symbolic links not supported, skipping tests.'
 	test_done
-	exit
 fi
 
 # The scenario we are building:
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index c46592f..5fe479e 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -17,7 +17,6 @@ if git http-push > /dev/null 2>&1 || [ $? -eq 128 ]
 then
 	say "skipping test, USE_CURL_MULTI is not defined"
 	test_done
-	exit
 fi
 
 . "$TEST_DIRECTORY"/lib-httpd.sh
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index e83bc8f..b647957 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -92,7 +92,6 @@ if ! echo 'echo space > "$1"' > "e space.sh"
 then
 	say "Skipping; FS does not support spaces in filenames"
 	test_done
-	exit
 fi
 
 test_expect_success 'editor with a space' '
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 995f607..3665692 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -11,7 +11,6 @@ if test $? -ne 1
 then
     say 'skipping git cvsexportcommit tests, cvs not found'
     test_done
-    exit
 fi
 
 CVSROOT=$(pwd)/cvsroot
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 466240c..39185db 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -15,12 +15,10 @@ if test $? -ne 1
 then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
-    exit
 fi
 perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
-    exit
 }
 
 unset GIT_DIR GIT_CONFIG
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index 8882230..12e0e50 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -51,12 +51,10 @@ if test $? -ne 1
 then
     say 'skipping git-cvsserver tests, cvs not found'
     test_done
-    exit
 fi
 perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {
     say 'skipping git-cvsserver tests, Perl SQLite interface unavailable'
     test_done
-    exit
 }
 
 unset GIT_DIR GIT_CONFIG
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 9ec5030..0bd332c 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -68,7 +68,6 @@ gitweb_run () {
 perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
     say 'skipping gitweb tests, perl version is too old'
     test_done
-    exit
 }
 
 gitweb_init
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index d2379e7..33eb519 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -14,7 +14,6 @@ if ! type cvs >/dev/null 2>&1
 then
 	say 'skipping cvsimport tests, cvs not found'
 	test_done
-	exit
 fi
 
 cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
@@ -24,12 +23,10 @@ case "$cvsps_version" in
 '')
 	say 'skipping cvsimport tests, cvsps not found'
 	test_done
-	exit
 	;;
 *)
 	say 'skipping cvsimport tests, unsupported cvsps version'
 	test_done
-	exit
 	;;
 esac
 
-- 
1.6.2.2.569.g2d4b2

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] Makefile: allow building without perl
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
  2009-04-03 19:28   ` [PATCH 1/4] commit: abort commit if interactive add failed Jeff King
  2009-04-03 19:31   ` [PATCH 2/4] tests: remove exit after test_done call Jeff King
@ 2009-04-03 19:32   ` Jeff King
  2009-04-04 22:47     ` Robin H. Johnson
  2009-04-03 19:33   ` [PATCH 4/4] tests: skip perl tests if NO_PERL is defined Jeff King
  2009-04-07  7:31   ` [PATCH 0/4] NO_PERL support Jeff King
  4 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-03 19:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

For systems with a missing or broken perl, it is nicer to
explicitly say "we don't want perl" because:

  1. The Makefile knows not to bother with Perl-ish things
     like Git.pm.

  2. We can print a more user-friendly error message
     than "foo is not a git command" or whatever the broken
     perl might barf

  3. Test scripts that require perl can mark themselves and
     such and be skipped

This patch implements parts (1) and (2). The perl/
subdirectory is skipped entirely, gitweb is not built, and
any git commands which rely on perl will print a
human-readable message and exit with an error code.

This patch is based on one from Robin H. Johnson.

Signed-off-by: Jeff King <peff@peff.net>
---
This is a rebase of what I sent earlier (the original was based on
1.6.2.1).

 Makefile         |   23 +++++++++++++++++++++--
 unimplemented.sh |    4 ++++
 2 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 unimplemented.sh

diff --git a/Makefile b/Makefile
index 7867eac..d7b0837 100644
--- a/Makefile
+++ b/Makefile
@@ -145,6 +145,8 @@ all::
 # Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's
 # MakeMaker (e.g. using ActiveState under Cygwin).
 #
+# Define NO_PERL if you do not want Perl scripts or libraries at all.
+#
 # Define NO_TCLTK if you do not want Tcl/Tk GUI.
 #
 # The TCL_PATH variable governs the location of the Tcl interpreter
@@ -353,7 +355,10 @@ BUILT_INS += git-whatchanged$X
 ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
 
 # what 'all' will build but not install in gitexecdir
-OTHER_PROGRAMS = git$X gitweb/gitweb.cgi
+OTHER_PROGRAMS = git$X
+ifndef NO_PERL
+OTHER_PROGRAMS += gitweb/gitweb.cgi
+endif
 
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
@@ -1178,7 +1183,9 @@ ifndef NO_TCLTK
 	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
 	$(QUIET_SUBDIR0)gitk-git $(QUIET_SUBDIR1) all
 endif
+ifndef NO_PERL
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
+endif
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
 please_set_SHELL_PATH_to_a_more_modern_shell:
@@ -1225,6 +1232,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	chmod +x $@+ && \
 	mv $@+ $@
 
+ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
 perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL
@@ -1284,6 +1292,15 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
+else # NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
+	    unimplemented.sh >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+endif # NO_PERL
 
 configure: configure.ac
 	$(QUIET_GEN)$(RM) $@ $<+ && \
@@ -1602,9 +1619,11 @@ clean:
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
-	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C Documentation/ clean
+ifndef NO_PERL
+	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C perl clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 ifndef NO_TCLTK
diff --git a/unimplemented.sh b/unimplemented.sh
new file mode 100644
index 0000000..5252de4
--- /dev/null
+++ b/unimplemented.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+echo >&2 "fatal: git was built without support for `basename $0` (@@REASON@@)."
+exit 128
-- 
1.6.2.2.569.g2d4b2

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] tests: skip perl tests if NO_PERL is defined
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
                     ` (2 preceding siblings ...)
  2009-04-03 19:32   ` [PATCH 3/4] Makefile: allow building without perl Jeff King
@ 2009-04-03 19:33   ` Jeff King
  2009-04-04 23:30     ` Robin H. Johnson
  2009-04-07  7:31   ` [PATCH 0/4] NO_PERL support Jeff King
  4 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-03 19:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

These scripts all test git programs that are written in
perl, and thus obviously won't work if NO_PERL is defined.
We pass NO_PERL to the scripts from the building Makefile
via the GIT-BUILD-OPTIONS file.

Signed-off-by: Jeff King <peff@peff.net>
---
This touches every test that Robin's patch touched except for
t5505-remote. There is no need to skip those tests, as these days
"git remote" is a builtin.

 Makefile                               |    1 +
 t/lib-git-svn.sh                       |    4 ++++
 t/t3701-add-interactive.sh             |    5 +++++
 t/t7501-commit.sh                      |    4 ++--
 t/t9001-send-email.sh                  |    5 +++++
 t/t9200-git-cvsexportcommit.sh         |    5 +++++
 t/t9400-git-cvsserver-server.sh        |    4 ++++
 t/t9500-gitweb-standalone-no-errors.sh |    5 +++++
 t/t9600-cvsimport.sh                   |    5 +++++
 t/t9700-perl-git.sh                    |    5 +++++
 t/test-lib.sh                          |    2 ++
 11 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d7b0837..584a757 100644
--- a/Makefile
+++ b/Makefile
@@ -1417,6 +1417,7 @@ GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
 	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
+	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index cdd7ccd..773d47c 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -8,6 +8,10 @@ then
 	say 'skipping git svn tests, NO_SVN_TESTS defined'
 	test_done
 fi
+if ! test_have_prereq PERL; then
+	say 'skipping git svn tests, perl not available'
+	test_done
+fi
 
 GIT_DIR=$PWD/.git
 GIT_SVN_DIR=$GIT_DIR/svn/git-svn
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index fe01783..dfc6560 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -3,6 +3,11 @@
 test_description='add -i basic tests'
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping git add -i tests, perl not available'
+	test_done
+fi
+
 test_expect_success 'setup (initial)' '
 	echo content >file &&
 	git add file &&
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b4e2b4d..e2ef532 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -38,7 +38,7 @@ test_expect_success \
 	"echo King of the bongo >file &&
 	test_must_fail git commit -m foo -a file"
 
-test_expect_success \
+test_expect_success PERL \
 	"using paths with --interactive" \
 	"echo bong-o-bong >file &&
 	! (echo 7 | git commit -m foo --interactive file)"
@@ -119,7 +119,7 @@ test_expect_success \
 	"echo 'gak' >file && \
 	 git commit -m 'author' --author 'Rubber Duck <rduck@convoy.org>' -a"
 
-test_expect_success \
+test_expect_success PERL \
 	"interactive add" \
 	"echo 7 | git commit --interactive | grep 'What now'"
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 192b97b..4281ce2 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -3,6 +3,11 @@
 test_description='git send-email'
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping git send-email tests, perl not available'
+	test_done
+fi
+
 PROG='git send-email'
 test_expect_success \
     'prepare reference tree' \
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 3665692..56b7c06 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -6,6 +6,11 @@ test_description='Test export of commits to CVS'
 
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping git cvsexportcommit tests, perl not available'
+	test_done
+fi
+
 cvs >/dev/null 2>&1
 if test $? -ne 1
 then
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 39185db..64f947d 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -10,6 +10,10 @@ cvs CLI client via git-cvsserver server'
 
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping git cvsserver tests, perl not available'
+	test_done
+fi
 cvs >/dev/null 2>&1
 if test $? -ne 1
 then
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 0bd332c..f4210fb 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -65,6 +65,11 @@ gitweb_run () {
 
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping gitweb tests, perl not available'
+	test_done
+fi
+
 perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
     say 'skipping gitweb tests, perl version is too old'
     test_done
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 33eb519..4322a0c 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -3,6 +3,11 @@
 test_description='git cvsimport basic tests'
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping git cvsimport tests, perl not available'
+	test_done
+fi
+
 CVSROOT=$(pwd)/cvsroot
 export CVSROOT
 unset CVS_SERVER
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 4a501c6..b4ca244 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -6,6 +6,11 @@
 test_description='perl interface (Git.pm)'
 . ./test-lib.sh
 
+if ! test_have_prereq PERL; then
+	say 'skipping perl interface tests, perl not available'
+	test_done
+fi
+
 perl -MTest::More -e 0 2>/dev/null || {
 	say "Perl Test::More unavailable, skipping test"
 	test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b050196..4bd986f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -698,6 +698,8 @@ case $(uname -s) in
 	;;
 esac
 
+test -z "$NO_PERL" && test_set_prereq PERL
+
 # test whether the filesystem supports symbolic links
 ln -s x y 2>/dev/null && test -h y 2>/dev/null && test_set_prereq SYMLINKS
 rm -f y
-- 
1.6.2.2.569.g2d4b2

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 17:15   ` Jeff King
@ 2009-04-03 20:20     ` Junio C Hamano
  2009-04-03 20:56       ` Jeff King
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2009-04-03 20:20 UTC (permalink / raw)
  To: Jeff King; +Cc: Robin H. Johnson, Git Mailing List

Jeff King <peff@peff.net> writes:

> Do we want to also skip those? Do we want to place the burden on
> authors of the test suite to always check for NO_PERL whenever they use
> perl?
>
> The other option would be saying "we support building with NO_PERL, but
> not running the test suite".
>
> Thoughts?

Yes, no, and probably not.

Let me clarify the last "probably not" first, because it will be the
reason behind the first "Yes".

Saying "We support building but not testing" is like saying "we don't
support it", and honestly, we'd be better off leaving this patch out of
tree if that is what we are going to do.  Even though I am not personally
very enthused about NO_PERL, the Makefile patch itself does not look too
bad, and if we can finish this with very limited injury to the overall
codebase, I wouldn't mind carrying the option in-tree.

        Side note: by the way, what did you or Robin's patch do to
        Documentation/cmd-list.perl and other bits of build infrastructure
        that rely on Perl?

To solve the second "no" cleanly, I am wondering if we can do something
clever by defining $PERL to be used in t/t*.sh scripts.  They should be
using configured PERL_PATH for running the tests _anyway_, even though I
see many hits from "git grep -e perl t/" right now.

But even if there isn't a room for doing something clever there, I think
the test prerequisite framework J6t did recently should be usable without
cluttering the test suite too much.  That forces test authors to be aware
of NO_PERL, which is slightly yucky, but if it cannot be helped, I think
we can survive.  We do the same for UTF8 and SYMLINKS already.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] NO_PERL support
  2009-04-03 20:20     ` Junio C Hamano
@ 2009-04-03 20:56       ` Jeff King
  0 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-03 20:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

On Fri, Apr 03, 2009 at 01:20:11PM -0700, Junio C Hamano wrote:

> Saying "We support building but not testing" is like saying "we don't
> support it", and honestly, we'd be better off leaving this patch out of
> tree if that is what we are going to do.  Even though I am not personally

Well, there are actually multiple levels that are worth considering. The
tasks you might want to accomplish are:

  1. Build the programs
  2. Run the test suite
  3. Run the programs (excepting the perl ones, of course); note
     that being able to build is not necessarily a prerequisite,
     as you might receive git as a binary package.
  4. Build the documentation
  5. View the documentation

Right now, (1), (2), and (4) are broken without perl. (5) works fine
because it doesn't involve perl at all. (3) works the same, except you
get error messages like "not a git command".

I can think of three situations which are helped by my patch series:

  - You are on a system with no perl. You don't care about running the
    test suite, but you do want to use basic git. You can now build and
    run, with the except of the perl scripts.

  - You are on a system with an old or inferior perl. For example, my
    Solaris test box has perl 5.005, and I know Alex has complained
    about Activestate perl on more than one occasion. The inferior perl
    is generally enough to run the little snippets in the test suite and
    the Documentation Makefile. So you can now build and run git (minus
    the perl bits) and the documentation (in theory -- you don't have
    perl but you _do_ have working asciidoc and xmlto?), and you can run
    the test suite without having to manually skip a bunch of tests
    (which is what I do now).

  - You are a package builder distributing binary packages. You have
    perl, but you want to build a noperl variant. Perl works for
    building the package, but you want the _result_ not to use perl. The
    test scripts, of course, must also respect NO_PERL since you have
    built placeholder scripts instead of the real thing.

> very enthused about NO_PERL, the Makefile patch itself does not look too
> bad, and if we can finish this with very limited injury to the overall
> codebase, I wouldn't mind carrying the option in-tree.

I don't think it is. See my 3/4.

>         Side note: by the way, what did you or Robin's patch do to
>         Documentation/cmd-list.perl and other bits of build infrastructure
>         that rely on Perl?

Neither patch does anything. AFAIK, Documentation/Makefile uses perl,
but nothing else does (aside from the test scripts previously
mentioned).

> To solve the second "no" cleanly, I am wondering if we can do something
> clever by defining $PERL to be used in t/t*.sh scripts.  They should be
> using configured PERL_PATH for running the tests _anyway_, even though I
> see many hits from "git grep -e perl t/" right now.

We can redefine them to use $PERL_PATH (which I agree should be done
anyway), but intercepting there is probably too late. You will be in the
middle of a test, and want to say "Oh wait, I was supposed to skip this
test." Certainly possible, but I think it might make the test code
pretty ugly.

  Aside: I think the reason that the lack of PERL_PATH hasn't been a
  problem is that people are generally not picking a _different_ perl,
  but rather need the whole path to go on the #! line. So the full path
  and "whatever is in my PATH" tend to be the same thing.

> But even if there isn't a room for doing something clever there, I think
> the test prerequisite framework J6t did recently should be usable without
> cluttering the test suite too much.  That forces test authors to be aware
> of NO_PERL, which is slightly yucky, but if it cannot be helped, I think
> we can survive.  We do the same for UTF8 and SYMLINKS already.

See my 4/4, which uses J6t's framework to do the first part (disabling
tests for scripts which we didn't actually build).

A 5/4 might be "disable uses of perl in tests when NO_PERL is set". And
that would be much more invasive. In my scenarios above, it buys the "I
don't have perl at all" people the ability to run a subset of the test
suite (but unlike 4/4, it is skipping tests that are actually useful and
important, but just happen to rely on perl for their infrastructure --
4/4 is by definition skipping tests that have no meaning).

I don't have any Gentoo boxen, but my understanding is that pretty much
everything is built from source. So the "you can make binary packages"
fix doesn't help them that much. OTOH, Robin's current patch doesn't
help that either, so I can only assume they're not really running the
test suite on perl-less systems. And I don't see a reason why upstream
git can't do part of the work (my series), and the Gentoo build can't do
the rest (setting GIT_SKIP_TESTS as appropriate).

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/4] Makefile: allow building without perl
  2009-04-03 19:32   ` [PATCH 3/4] Makefile: allow building without perl Jeff King
@ 2009-04-04 22:47     ` Robin H. Johnson
  2009-04-04 23:39       ` Jeff King
  0 siblings, 1 reply; 30+ messages in thread
From: Robin H. Johnson @ 2009-04-04 22:47 UTC (permalink / raw)
  To: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

On Fri, Apr 03, 2009 at 03:32:20PM -0400, Jeff King wrote:
> For systems with a missing or broken perl, it is nicer to
> explicitly say "we don't want perl" because:
Part of the patch got missed. In the case of missing perl, we can detect it,
originally we had a compare of PERL_PATH for emptiness.

diff Makefile.orig Makefile
--- Makefile.orig
+++ Makefile
@@ -354,12 +354,6 @@ BUILT_INS += git-whatchanged$X
 # what 'all' will build and 'install' will install, in gitexecdir
 ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
 
-# what 'all' will build but not install in gitexecdir
-OTHER_PROGRAMS = git$X
-ifndef NO_PERL
-OTHER_PROGRAMS += gitweb/gitweb.cgi
-endif
-
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
        SHELL_PATH = /bin/sh
@@ -367,6 +361,18 @@ endif
 ifndef PERL_PATH
        PERL_PATH = /usr/bin/perl
 endif
+ifeq ($(wildcard $(PERL_PATH)),)
+PERL_PATH =
+NO_PERL = NoThanks
+NO_PERL_MAKEMAKER = NoThanks
+export NO_PERL NO_PERL_MAKEMAKER
+endif
+
+# what 'all' will build but not install in gitexecdir
+OTHER_PROGRAMS = git$X
+ifndef NO_PERL
+OTHER_PROGRAMS += gitweb/gitweb.cgi
+endif

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/4] tests: skip perl tests if NO_PERL is defined
  2009-04-03 19:33   ` [PATCH 4/4] tests: skip perl tests if NO_PERL is defined Jeff King
@ 2009-04-04 23:30     ` Robin H. Johnson
  2009-04-04 23:42       ` Jeff King
  0 siblings, 1 reply; 30+ messages in thread
From: Robin H. Johnson @ 2009-04-04 23:30 UTC (permalink / raw)
  To: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 9031 bytes --]

On Fri, Apr 03, 2009 at 03:33:59PM -0400, Jeff King wrote:
> These scripts all test git programs that are written in
> perl, and thus obviously won't work if NO_PERL is defined.
> We pass NO_PERL to the scripts from the building Makefile
> via the GIT-BUILD-OPTIONS file.
Something is missing in the porting of the patch (possibly from an earlier
version where I did check for perl usage in the testcases and just avoided
those tests).

Testing on a box without Perl, I get the following failures.
You can reproduce by doing mv /usr/bin/perl{,.tmp}, 
gmake NO_PERL=1 clean all test

This is the only one that doesn't appear to use perl.
*** t6030-bisect-porcelain.sh ***
* FAIL 2: bisect starts with only one bad
* FAIL 3: bisect does not start with only one good
* FAIL 4: bisect start with one bad and good
* FAIL 5: bisect fails if given any junk instead of revs
* FAIL 7: bisect reset: back in another branch
* FAIL 8: bisect reset when not bisecting
* FAIL 9: bisect reset removes packed refs
* FAIL 10: bisect start: back in good branch
* FAIL 11: bisect start: no ".git/BISECT_START" if junk rev
* FAIL 12: bisect start: no ".git/BISECT_START" if mistaken rev
* FAIL 14: bisect skip: successfull result
* FAIL 15: bisect skip: cannot tell between 3 commits
* FAIL 16: bisect skip: cannot tell between 2 commits
* FAIL 17: bisect skip: with commit both bad and skipped
* FAIL 18: "git bisect run" simple case
* FAIL 19: "git bisect run" with more complex "git bisect start"
* FAIL 20: bisect skip: add line and then a new test
* FAIL 21: bisect skip and bisect replay
* FAIL 22: bisect run & skip: cannot tell between 2
* FAIL 23: bisect run & skip: find first bad
* FAIL 24: bisect skip only one range
* FAIL 25: bisect skip many ranges
* FAIL 26: bisect starting with a detached HEAD
* FAIL 27: bisect errors out if bad and good are mistaken
* FAIL 28: bisect does not create a "bisect" branch
* FAIL 30: good merge base when good and bad are siblings
* FAIL 31: skipped merge base when good and bad are siblings
* FAIL 32: bad merge base when good and bad are siblings
* FAIL 34: good merge bases when good and bad are siblings
* FAIL 35: optimized merge base checks

All the rest of these are all due to perl in test infrastructure:
*** t0020-crlf.sh ***
* FAIL 1: setup
* FAIL 7: switch off autocrlf, safecrlf, reset HEAD
* FAIL 10: checkout with autocrlf=true
* FAIL 11: checkout with autocrlf=input
* FAIL 12: apply patch (autocrlf=input)
* FAIL 13: apply patch --cached (autocrlf=input)
* FAIL 14: apply patch --index (autocrlf=input)
* FAIL 15: apply patch (autocrlf=true)
* FAIL 16: apply patch --cached (autocrlf=true)
* FAIL 17: apply patch --index (autocrlf=true)
* FAIL 18: .gitattributes says two is binary
* FAIL 20: .gitattributes says two and three are text
* FAIL 21: in-tree .gitattributes (1)
* FAIL 22: in-tree .gitattributes (2)
* FAIL 23: in-tree .gitattributes (3)
* FAIL 24: in-tree .gitattributes (4)

*** t1300-repo-config.sh ***
* FAIL 71: --null --list
* FAIL 72: --null --get-regexp

*** t3300-funny-names.sh ***
* FAIL 3: git ls-files -z with-funny
* FAIL 7: git diff-index -z with-funny
* FAIL 8: git diff-tree -z with-funny

*** t4012-diff-binary.sh ***
* FAIL 9: diff --no-index with binary creation

*** t4014-format-patch.sh ***
* FAIL 16: no threading
* FAIL 17: thread
* FAIL 18: thread in-reply-to
* FAIL 19: thread cover-letter
* FAIL 20: thread cover-letter in-reply-to
* FAIL 21: thread explicit shallow
* FAIL 22: thread deep
* FAIL 23: thread deep in-reply-to
* FAIL 24: thread deep cover-letter
* FAIL 25: thread deep cover-letter in-reply-to
* FAIL 26: thread via config
* FAIL 27: thread deep via config
* FAIL 28: thread config + override
* FAIL 29: thread config + --no-thread

*** t4020-diff-external.sh ***
* FAIL 12: force diff with "diff"
* FAIL 15: diff --cached

*** t4029-diff-trailing-space.sh ***
* FAIL 1: diff honors config option, diff.suppressBlankEmpty

*** t4030-diff-textconv.sh ***
* FAIL 5: diff produces text
* FAIL 7: log produces text
* FAIL 9: status -v produces text
* FAIL 10: diffstat does not run textconv
* FAIL 11: textconv does not act on symlinks

*** t4031-diff-rewrite-binary.sh ***
* FAIL 6: rewrite diff respects textconv

*** t4103-apply-binary.sh ***
* FAIL 3: check binary diff -- should fail.
* FAIL 4: check binary diff (copy) -- should fail.
* FAIL 5: check incomplete binary diff with replacement -- should fail.
* FAIL 6: check incomplete binary diff with replacement (copy) -- should fail.
* FAIL 9: apply binary diff -- should fail.
* FAIL 10: apply binary diff -- should fail.
* FAIL 11: apply binary diff (copy) -- should fail.
* FAIL 12: apply binary diff (copy) -- should fail.

*** t4116-apply-reverse.sh ***
* FAIL 1: setup
* FAIL 2: apply in forward
* FAIL 3: apply in reverse
* FAIL 4: setup separate repository lacking postimage
* FAIL 5: apply in forward without postimage
* FAIL 6: apply in reverse without postimage
* FAIL 7: reversing a whitespace introduction

*** t4200-rerere.sh ***
* FAIL 15: clear removed the directory
	test ! -d .git/rr-cache/b5a3939d25d0a649294d5d94f2d338ef8da51812
mkdir: cannot create directory `.git/rr-cache/b5a3939d25d0a649294d5d94f2d338ef8da51812': File exists

*** t5300-pack-object.sh ***
* FAIL 1: setup
* FAIL 2: pack without delta
* FAIL 3: unpack without delta
* FAIL 5: pack with REF_DELTA
* FAIL 6: unpack with REF_DELTA
* FAIL 8: pack with OFS_DELTA
* FAIL 9: unpack with OFS_DELTA
* FAIL 11: compare delta flavors
* FAIL 12: use packed objects
* FAIL 13: use packed deltified (REF_DELTA) objects
* FAIL 14: use packed deltified (OFS_DELTA) objects
* FAIL 15: survive missing objects/pack directory
* FAIL 16: verify pack
* FAIL 17: verify pack -v
* FAIL 18: verify-pack catches mismatched .idx and .pack files
* FAIL 19: verify-pack catches a corrupted pack signature
* FAIL 20: verify-pack catches a corrupted pack version
* FAIL 21: verify-pack catches a corrupted type/size of the 1st packed object data
* FAIL 22: verify-pack catches a corrupted sum of the index file itself
* FAIL 23: build pack index for an existing pack
* FAIL 24: fake a SHA1 hash collision
* FAIL 25: make sure index-pack detects the SHA1 collision
* FAIL 26: honor pack.packSizeLimit
* FAIL 29: tolerate absurdly small packsizelimit

*** t5303-pack-corruption-resilience.sh ***
* FAIL 5: create corruption in data of first object
* FAIL 7: ... and loose copy of second object allows for partial recovery
* FAIL 11: create corruption in data of first delta

*** t6002-rev-list-bisect.sh ***
* FAIL 34: --bisect l5 ^root
* FAIL 35: --bisect l5 ^root ^c3
* FAIL 36: --bisect l5 ^root ^c3 ^b4
* FAIL 37: --bisect l3 ^root ^c3 ^b4
* FAIL 38: --bisect l5 ^b3 ^a3 ^b4 ^a4
* FAIL 39: --bisect l4 ^a2 ^a3 ^b ^a4
* FAIL 40: --bisect l3 ^a2 ^a3 ^b ^a4
* FAIL 41: --bisect a4 ^a2 ^a3 ^b4
* FAIL 42: --bisect a4 ^a2 ^a3 ^b4 ^c2
* FAIL 43: --bisect a4 ^a2 ^a3 ^b4 ^c2 ^c3
* FAIL 44: --bisect a4 ^a2 ^a3 ^b4
* FAIL 45: --bisect c3 ^a2 ^a3 ^b4 ^c2

*** t6003-rev-list-topo-order.sh ***
* FAIL 1: rev-list has correct number of entries
* FAIL 2: simple topo order
* FAIL 3: two diamonds topo order (g6)
* FAIL 4: multiple heads
* FAIL 5: multiple heads, prune at a1
* FAIL 6: multiple heads, prune at l1
* FAIL 7: cross-epoch, head at l5, prune at l1
* FAIL 8: duplicated head arguments
* FAIL 9: prune near topo
* FAIL 10: head has no parent
* FAIL 11: two nodes - one head, one base
* FAIL 12: three nodes one head, one internal, one base
* FAIL 13: linear prune l2 ^root
* FAIL 14: linear prune l2 ^l0
* FAIL 15: linear prune l2 ^l1
* FAIL 16: linear prune l5 ^a4
* FAIL 17: linear prune l5 ^l3
* FAIL 18: linear prune l5 ^l4
* FAIL 19: max-count 10 - topo order
* FAIL 20: max-count 10 - non topo order
* FAIL 21: --max-age=c3, no --topo-order
* FAIL 22: one specified head reachable from another a4, c3, --topo-order
* FAIL 23: one specified head reachable from another c3, a4, --topo-order
* FAIL 24: one specified head reachable from another a4, c3, no --topo-order
* FAIL 25: one specified head reachable from another c3, a4, no --topo-order
* FAIL 26: graph with c3 and a4 parents of head
* FAIL 27: graph with a4 and c3 parents of head
* FAIL 30: simple topo order (l5r1)
* FAIL 31: simple topo order (r1l5)
* FAIL 32: don't print things unreachable from one branch
* FAIL 33: --topo-order a4 l3

*** t6011-rev-list-with-bad-commit.sh ***
* FAIL 3: corrupt second commit object
* FAIL 4: rev-list should fail
* FAIL 5: git repack _MUST_ fail

*** t6013-rev-list-reverse-parents.sh ***
* FAIL 2: --reverse --parents --full-history combines correctly
* FAIL 3: --boundary does too

annotate-tests.sh
(lots of the t8* tests).

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/4] Makefile: allow building without perl
  2009-04-04 22:47     ` Robin H. Johnson
@ 2009-04-04 23:39       ` Jeff King
  2009-04-04 23:51         ` Robin H. Johnson
  0 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-04 23:39 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

On Sat, Apr 04, 2009 at 03:47:29PM -0700, Robin H. Johnson wrote:

> On Fri, Apr 03, 2009 at 03:32:20PM -0400, Jeff King wrote:
> > For systems with a missing or broken perl, it is nicer to
> > explicitly say "we don't want perl" because:
> Part of the patch got missed. In the case of missing perl, we can detect it,
> originally we had a compare of PERL_PATH for emptiness.

Thanks. I was hesitant on this at first because I think the behavior
should be the same as with TCLTK_PATH, but I didn't realize that
TCLTK_PATH already automagically sets NO_TCLTK in the same way.

> +ifeq ($(wildcard $(PERL_PATH)),)
> +PERL_PATH =
> +NO_PERL = NoThanks
> +NO_PERL_MAKEMAKER = NoThanks
> +export NO_PERL NO_PERL_MAKEMAKER
> +endif

The TCLTK code is just:

  ifeq ($(TCLTK_PATH),)
  NO_TCLTK=NoThanks
  endif

I'm not sure what you're trying to accomplish with the wildcard, unless
it is "PERL_PATH = /usr/*/perl" or similar, but that seems a bit crazy
to me. It should probably behave the same as TCLTK_PATH, though (so if
there is a good use case, TCLTK_PATH should be enhanced).

I don't think there is a point in setting NO_PERL_MAKEMAKER if NO_PERL
is set, and I believe the export is pointless, as I described in an
earlier email.

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4/4] tests: skip perl tests if NO_PERL is defined
  2009-04-04 23:30     ` Robin H. Johnson
@ 2009-04-04 23:42       ` Jeff King
  0 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-04 23:42 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

On Sat, Apr 04, 2009 at 04:30:10PM -0700, Robin H. Johnson wrote:

> Testing on a box without Perl, I get the following failures.
> You can reproduce by doing mv /usr/bin/perl{,.tmp}, 
> gmake NO_PERL=1 clean all test
> 
> This is the only one that doesn't appear to use perl.
> *** t6030-bisect-porcelain.sh ***

Probably this:

  $ grep -B1 -A7 PERL git-bisect.sh
  sq() {
          @@PERL@@ -e '
                  for (@ARGV) {
                          s/'\''/'\'\\\\\'\''/g;
                          print " '\''$_'\''";
                  }
                  print "\n";
          ' "$@"
  }

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/4] Makefile: allow building without perl
  2009-04-04 23:39       ` Jeff King
@ 2009-04-04 23:51         ` Robin H. Johnson
  2009-04-04 23:56           ` Jeff King
  0 siblings, 1 reply; 30+ messages in thread
From: Robin H. Johnson @ 2009-04-04 23:51 UTC (permalink / raw)
  To: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On Sat, Apr 04, 2009 at 07:39:36PM -0400, Jeff King wrote:
> > +ifeq ($(wildcard $(PERL_PATH)),)
> I'm not sure what you're trying to accomplish with the wildcard, unless
> it is "PERL_PATH = /usr/*/perl" or similar, but that seems a bit crazy
> to me. It should probably behave the same as TCLTK_PATH, though (so if
> there is a good use case, TCLTK_PATH should be enhanced).
No. The ifeq ($(wildcard $(PERL_PATH)),) is entirely correct.
It's one of the few ways to detect the existence of a file from within
Make, without any exec calls. If you give it a non-expandable path,
/usr/bin/perl in this case, it checks only that path, and either returns
it or an empty string. This enables us to check that /usr/bin/perl
exists, and take suitable action if it does not.

It's not suitable for use with the TCLTK_PATH, as that isn't an absolute
path, so the wildcard trick doesn't work.

> I don't think there is a point in setting NO_PERL_MAKEMAKER if NO_PERL
> is set, and I believe the export is pointless, as I described in an
> earlier email.
From further down the Makefile:
ifdef NO_PERL_MAKEMAKER
    export NO_PERL_MAKEMAKER
endif

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/4] Makefile: allow building without perl
  2009-04-04 23:51         ` Robin H. Johnson
@ 2009-04-04 23:56           ` Jeff King
  2009-04-05  0:06             ` Robin H. Johnson
  0 siblings, 1 reply; 30+ messages in thread
From: Jeff King @ 2009-04-04 23:56 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

On Sat, Apr 04, 2009 at 04:51:48PM -0700, Robin H. Johnson wrote:

> On Sat, Apr 04, 2009 at 07:39:36PM -0400, Jeff King wrote:
> > > +ifeq ($(wildcard $(PERL_PATH)),)
> > I'm not sure what you're trying to accomplish with the wildcard, unless
> > it is "PERL_PATH = /usr/*/perl" or similar, but that seems a bit crazy
> > to me. It should probably behave the same as TCLTK_PATH, though (so if
> > there is a good use case, TCLTK_PATH should be enhanced).
> No. The ifeq ($(wildcard $(PERL_PATH)),) is entirely correct.
> It's one of the few ways to detect the existence of a file from within
> Make, without any exec calls. If you give it a non-expandable path,
> /usr/bin/perl in this case, it checks only that path, and either returns
> it or an empty string. This enables us to check that /usr/bin/perl
> exists, and take suitable action if it does not.

Ah, I see. You are not asking "was PERL_PATH blank" but rather "does
PERL_PATH exist". And I think that is not the right thing for the
Makefile, as it is unlike any other part of the git Makefile, which
generally does what it is told with the minimum of magic. That sort of
magic generally goes into configure.ac.

> > I don't think there is a point in setting NO_PERL_MAKEMAKER if NO_PERL
> > is set, and I believe the export is pointless, as I described in an
> > earlier email.
> From further down the Makefile:
> ifdef NO_PERL_MAKEMAKER
>     export NO_PERL_MAKEMAKER
> endif

So NO_PERL_MAKEMAKER is _already_ exported, and I don't think there is
any reason to export NO_PERL in the environment (see patch 4/4, which
exports it via GIT-BUILD-OPTIONS).

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/4] Makefile: allow building without perl
  2009-04-04 23:56           ` Jeff King
@ 2009-04-05  0:06             ` Robin H. Johnson
  2009-04-07  7:27               ` Jeff King
  0 siblings, 1 reply; 30+ messages in thread
From: Robin H. Johnson @ 2009-04-05  0:06 UTC (permalink / raw)
  To: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On Sat, Apr 04, 2009 at 07:56:36PM -0400, Jeff King wrote:
> Ah, I see. You are not asking "was PERL_PATH blank" but rather "does
> PERL_PATH exist". And I think that is not the right thing for the
> Makefile, as it is unlike any other part of the git Makefile, which
> generally does what it is told with the minimum of magic. That sort of
> magic generally goes into configure.ac.
At a glance, PERL_PATH in configure.ac isn't actually used at the
moment, the definition in the Makefile overrides it.

> So NO_PERL_MAKEMAKER is _already_ exported, and I don't think there is
> any reason to export NO_PERL in the environment (see patch 4/4, which
> exports it via GIT-BUILD-OPTIONS).
Ok, drop the export then, but do keep at least the if(is empty
PERL_PATH) then { set NO_PERL }, even if you won't keep the existence
check.

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Infra Guy
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/4] Makefile: allow building without perl
  2009-04-05  0:06             ` Robin H. Johnson
@ 2009-04-07  7:27               ` Jeff King
  0 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-07  7:27 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List

On Sat, Apr 04, 2009 at 05:06:09PM -0700, Robin H. Johnson wrote:

> At a glance, PERL_PATH in configure.ac isn't actually used at the
> moment, the definition in the Makefile overrides it.

It is. The configure variables go into config.mak.autogen which is
included after PERL_PATH is set to a default value.

> Ok, drop the export then, but do keep at least the if(is empty
> PERL_PATH) then { set NO_PERL }, even if you won't keep the existence
> check.

Will do.

-Peff

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/4] NO_PERL support
  2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
                     ` (3 preceding siblings ...)
  2009-04-03 19:33   ` [PATCH 4/4] tests: skip perl tests if NO_PERL is defined Jeff King
@ 2009-04-07  7:31   ` Jeff King
  4 siblings, 0 replies; 30+ messages in thread
From: Jeff King @ 2009-04-07  7:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List

On Fri, Apr 03, 2009 at 03:27:01PM -0400, Jeff King wrote:

> OK, here is a series based on Robin's patch that I think is suitable for
> inclusion in mainstream git. The first two are related cleanups, the
> third is a rebase of what I sent earlier today, and the fourth covers
> the matching tests.
> 
>   1/4: commit: abort commit if interactive add failed
>   2/4: tests: remove exit after test_done call
>   3/4: Makefile: allow building without perl
>   4/4: tests: skip perl tests if NO_PERL is defined

And here is a 5/4 that I missed when looking at Robin's patch.

-- >8 --
Subject: [PATCH] Makefile: disable perl entirely for empty PERL_PATH

This matches the TCLTK_PATH behavior.

Signed-off-by: Jeff King <peff@peff.net>
---
 Makefile |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 584a757..6e0fe24 100644
--- a/Makefile
+++ b/Makefile
@@ -1109,6 +1109,10 @@ ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
 endif
 
+ifeq ($(PERL_PATH),)
+NO_PERL=NoThanks
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2009-04-07  7:33 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-03  7:03 [PATCH] NO_PERL support Robin H. Johnson
2009-04-03 13:20 ` Jeff King
2009-04-03 14:51   ` Johannes Schindelin
2009-04-03 14:53     ` Jeff King
2009-04-03 15:02       ` Johannes Schindelin
2009-04-03 15:11         ` Miklos Vajna
2009-04-03 15:16         ` Jeff King
2009-04-03 17:54   ` Johannes Sixt
2009-04-03 18:37     ` Jeff King
2009-04-03 15:46 ` Jeff King
2009-04-03 15:59   ` Robin H. Johnson
2009-04-03 16:18     ` Jeff King
2009-04-03 16:25 ` Junio C Hamano
2009-04-03 17:15   ` Jeff King
2009-04-03 20:20     ` Junio C Hamano
2009-04-03 20:56       ` Jeff King
2009-04-03 19:27 ` [PATCH 0/4] " Jeff King
2009-04-03 19:28   ` [PATCH 1/4] commit: abort commit if interactive add failed Jeff King
2009-04-03 19:31   ` [PATCH 2/4] tests: remove exit after test_done call Jeff King
2009-04-03 19:32   ` [PATCH 3/4] Makefile: allow building without perl Jeff King
2009-04-04 22:47     ` Robin H. Johnson
2009-04-04 23:39       ` Jeff King
2009-04-04 23:51         ` Robin H. Johnson
2009-04-04 23:56           ` Jeff King
2009-04-05  0:06             ` Robin H. Johnson
2009-04-07  7:27               ` Jeff King
2009-04-03 19:33   ` [PATCH 4/4] tests: skip perl tests if NO_PERL is defined Jeff King
2009-04-04 23:30     ` Robin H. Johnson
2009-04-04 23:42       ` Jeff King
2009-04-07  7:31   ` [PATCH 0/4] NO_PERL support Jeff King

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).