git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brian Gernhardt <brian@gernhardtsoftware.com>,
	Git List <git@vger.kernel.org>
Subject: Re: [PATCH] Makefile: Remove excess backslashes from sed
Date: Fri, 9 Apr 2010 01:02:57 -0700	[thread overview]
Message-ID: <20100409080256.GA12527@gmail.com> (raw)
In-Reply-To: <7vvdc12mmz.fsf@alter.siamese.dyndns.org>

On Thu, Apr 08, 2010 at 10:57:56PM -0700, Junio C Hamano wrote:
> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
> 
> > The sed script that was intended to add lines altering the sys.path
> > had extra backslashes in them.  Instead resulting in
> >
> >   import sys;  import os;  sys.path.insert( ... )
> >
> > It output
> >
> >   import sys; \ import os; \ sys.path.insert( ... )
> >
> > Unfortunately this caused python (2.6.1 on OS X 10.6.3) to error
> >
> >   SyntaxError: unexpected character after line continuation character
> >
> > Removing two of the backslashes solves this problem.
> 
> Traditionally, multi-line sed statements written in the Makefile were
> portability nightmare, as the line folding rules (especially how the
> backslash is treated in the output) in make implementations were subtly
> different, and implementations of sed also got multi-line statement often
> wrong.  These days, things might have gotten much better, but in olden
> days (back when BSD vs SysV war was still raging), the trick to write
> things like this portably was to invoke a shell script that has multi-line
> sed statement from the Makefile.  It was painful.
> 
> I wonder if we can make this a lot simpler to avoid multi-line sed script.
> For example, we could write the source Python script to always begin with:
> 
> 	#!/usr/bin/python
>         import sys;
>         import os;
>         sys.path.insert(0, os.getenv("GITPYTHONLIB","."));
> 
> and then do something like this in the Makefile:
> 
> 	INSTLIBDIR=... && \
> 	sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
>         -e 's|\(os\.getenv("GITPYTHONLIB"\)[^)]*)|\1,"'"$$INSTLIBDIR"'")|' \
>         <$@.py >$@+
> 	mv $@+ $@
> 
> Contributors can then run things in-place while developing the scripts,
> perhaps setting GITPYTHONLIB to the source area.

Here's what I cooked up so far...:

$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS
$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
	$(QUIET_GEN)$(RM) $@ $@+ && \
	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
		--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \
		instlibdir` && \
	sed -e '1{' \
	    -e '	s|#!.*python|#!$(PYTHON_PATH_SQ)\n@@STARTUP@@\n|' \
	    -e '}' \
	    -e 's|@@STARTUP@@|import os, sys; sys.path.insert(0, @@ENV@@)|' \
	    -e 's|@@ENV@@|os.getenv("GITPYTHONLIB", "@@INSTLIBDIR@@")|' \
	    -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
	    $@.py >$@+ && \
	chmod +x $@+ && \
	mv $@+ $@

It avoids the multi-line portability concerns by substituting variables
along each step.

This is also nicer in that we don't need to start scripts with
"import sys" or os.getenv() anything...  all we require is a
shebang line.  We inject @@STARTUP@@ when we rewrite the
shebang.

That seems nicer then the old expression which searched for
"import sys.*" when doing the replacements.

Is the \n@@STARTUP\n thing portable?

-- 
		David

  reply	other threads:[~2010-04-09  8:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-09  3:22 [PATCH] Makefile: Remove excess backslashes from sed Brian Gernhardt
2010-04-09  5:57 ` Junio C Hamano
2010-04-09  8:02   ` David Aguilar [this message]
2010-04-09  8:42     ` [PATCH] Makefile: Avoid multi-line sed when building python scripts David Aguilar
2010-04-09 14:33     ` [PATCH] Makefile: Remove excess backslashes from sed Brian Gernhardt
2010-04-09 15:34   ` Sverre Rabbelier
2010-04-09 15:35   ` [PATCH] Makefile: Simplify handling of python scripts Brian Gernhardt
2010-04-09  6:04 ` [PATCH] Makefile: Remove excess backslashes from sed David Aguilar
  -- strict thread matches above, loose matches on Subject: below --
2010-04-09 14:41 Brian Gernhardt
2010-04-09 14:44 ` Brian Gernhardt

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20100409080256.GA12527@gmail.com \
    --to=davvid@gmail.com \
    --cc=brian@gernhardtsoftware.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).