patchMakefile: Remove excess backslashes from sed

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.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---

 I'm not sure if this is a result of OS X's python or sed, but t5800
 won't work for me without this change.

 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Brian Gernhardt <brian@gernhardtsoftware.com> writes:

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.

On Thu, Apr 08, 2010 at 10:57:56PM -0700, Junio C Hamano wrote:

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?

Instead of requiring scripts to start with "import sys", install the sys.path initialization code immediately after each script's new shebang line.

This works around portability issues with multi-line sed and simplifies the work for contributors. There is no longer the requirement that every script must "import sys" before importing git-provided modules.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 Makefile |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

On Apr 9, 2010, at 4:02 AM, David Aguilar wrote:

No.  OS X's sed renders it as 

  #!/usr/bin/pythonnimport os, sys; sys.path.insert(0, os.getenv("GITPYTHONLIB", "/usr/local/lib/python2.6/site-packages"))

So NAK for your patch.

~~ Brian

Heya,

On Fri, Apr 9, 2010 at 07:57, Junio C Hamano <gitster@pobox.com> wrote:

Thank you, I like this a lot better than our current approach. I think it's very ugly that git (needs to) mangle(s) the python files when installing, since that means that line-numbers don't match in stacktraces. I'd definitely prefer using this.

The sed script intended to add a standard opening to python scripts was non-compatible and overly complex. Simplifying it down to a set of one-liners removes the compatibility issues of newlines. Moving the environment alterations from the Makefile to the python scripts makes also makes the scripts easier to run in-place.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
 Makefile              |    9 ++-------
 git-remote-testgit.py |    2 ++
 2 files changed, 4 insertions(+), 7 deletions(-)

On Thu, Apr 08, 2010 at 11:22:10PM -0400, Brian Gernhardt wrote:

On linux sed I get (or I was dreaming when I tried it):

import sys; \
	import os; \
	sys.path.insert(0, os.getenv(foo,
	                             bar))

We should instead smash it all into one line instead to avoid differences in sed behavior:

import sys; import os; sys.path.insert(0, os.getenv(foo,
                                                    bar))

If that's what it is, then your patch would produce this on linux:

import sys;
	import os;
	sys.path.insert(foo,
	                bar)

...which is invalid syntax.

So let's smash it all onto one line instead. That way I don't have to think about or test whether there's any difference in how sed handles newlines. It'll be the Obviously Correct(tm) sol'n. Or maybe I was dreaming?

If you don't beat me to it by the tomorrow night I'll try and throw together a patch. Thanks for catching this.