git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Robin H. Johnson" <robbat2@gentoo.org>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] NO_PERL support
Date: Fri, 3 Apr 2009 13:15:14 -0400	[thread overview]
Message-ID: <20090403171514.GA11112@coredump.intra.peff.net> (raw)
In-Reply-To: <7vljqhaemm.fsf@gitster.siamese.dyndns.org>

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

  reply	other threads:[~2009-04-03 17:17 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090403171514.GA11112@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=robbat2@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).