# [PATCH] Makefile: Remove excess backslashes from sed 8 messages from 2010-04-09 to 2010-04-09. Participants: Brian Gernhardt, David Aguilar, Junio C Hamano, Sverre Rabbelier. Thread: https://gitlist.dev/t/23386 ## Brian Gernhardt, 2010-04-09 03:22 Subject: [PATCH] Makefile: Remove excess backslashes from sed Message-ID: <1270783330-35215-1-git-send-email-brian@gernhardtsoftware.com> URL: https://gitlist.dev/e/1270783330-35215-1-git-send-email-brian%40gernhardtsoftware.com ``` 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 --- 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(-) diff --git a/Makefile b/Makefile index f0fe351..b9eb1ca 100644 --- a/Makefile +++ b/Makefile @@ -1632,8 +1632,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py sed -e '1{' \ -e ' s|#!.*python|#!$(PYTHON_PATH_SQ)|' \ -e '}' \ - -e 's|^import sys.*|&; \\\ - import os; \\\ + -e 's|^import sys.*|&; \ + import os; \ sys.path.insert(0, os.getenv("GITPYTHONLIB",\ "@@INSTLIBDIR@@"));|' \ -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \ -- 1.7.1.rc0.210.ge6da ``` ## Junio C Hamano, 2010-04-09 05:57 Subject: Re: [PATCH] Makefile: Remove excess backslashes from sed Message-ID: <7vvdc12mmz.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvdc12mmz.fsf%40alter.siamese.dyndns.org In-Reply-To: <1270783330-35215-1-git-send-email-brian@gernhardtsoftware.com> ``` Brian Gernhardt 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. ``` ## David Aguilar, 2010-04-09 06:04 Subject: Re: [PATCH] Makefile: Remove excess backslashes from sed Message-ID: <20100409060427.GA6012@gmail.com> URL: https://gitlist.dev/e/20100409060427.GA6012%40gmail.com In-Reply-To: <1270783330-35215-1-git-send-email-brian@gernhardtsoftware.com> ``` On Thu, Apr 08, 2010 at 11:22:10PM -0400, Brian Gernhardt wrote: > The sed script that was intended to add lines altering the sys.path > had extra backslashes in them. Instead resulting [snip] > import sys; \ import os; \ sys.path.insert( ... ) 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. > diff --git a/Makefile b/Makefile > index f0fe351..b9eb1ca 100644 > --- a/Makefile > +++ b/Makefile > @@ -1632,8 +1632,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py > sed -e '1{' \ > -e ' s|#!.*python|#!$(PYTHON_PATH_SQ)|' \ > -e '}' \ > - -e 's|^import sys.*|&; \\\ > - import os; \\\ > + -e 's|^import sys.*|&; \ > + import os; \ > sys.path.insert(0, os.getenv("GITPYTHONLIB",\ > "@@INSTLIBDIR@@"));|' \ -- David ``` ## David Aguilar, 2010-04-09 08:02 Subject: Re: [PATCH] Makefile: Remove excess backslashes from sed Message-ID: <20100409080256.GA12527@gmail.com> URL: https://gitlist.dev/e/20100409080256.GA12527%40gmail.com 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 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 ``` ## David Aguilar, 2010-04-09 08:42 Subject: [PATCH] Makefile: Avoid multi-line sed when building python scripts Message-ID: <1270802559-14791-1-git-send-email-davvid@gmail.com> URL: https://gitlist.dev/e/1270802559-14791-1-git-send-email-davvid%40gmail.com In-Reply-To: <20100409080256.GA12527@gmail.com> ``` 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 --- Makefile | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 956e781..b763947 100644 --- a/Makefile +++ b/Makefile @@ -1608,12 +1608,10 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py --no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \ instlibdir` && \ sed -e '1{' \ - -e ' s|#!.*python|#!$(PYTHON_PATH_SQ)|' \ + -e ' s|#!.*python|#!$(PYTHON_PATH_SQ)\n@@STARTUP@@|' \ -e '}' \ - -e 's|^import sys.*|&; \\\ - import os; \\\ - sys.path.insert(0, os.getenv("GITPYTHONLIB",\ - "@@INSTLIBDIR@@"));|' \ + -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 $@+ && \ -- 1.7.0.3.313.g87b3c ``` ## Brian Gernhardt, 2010-04-09 14:33 Subject: Re: [PATCH] Makefile: Remove excess backslashes from sed Message-ID: <1C4F213E-9DF1-4184-B953-303DEDD69AC1@gernhardtsoftware.com> URL: https://gitlist.dev/e/1C4F213E-9DF1-4184-B953-303DEDD69AC1%40gernhardtsoftware.com In-Reply-To: <20100409080256.GA12527@gmail.com> ``` On Apr 9, 2010, at 4:02 AM, David Aguilar wrote: > Is the \n@@STARTUP\n thing portable? 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 ``` ## Sverre Rabbelier, 2010-04-09 15:34 Subject: Re: [PATCH] Makefile: Remove excess backslashes from sed Message-ID: URL: https://gitlist.dev/e/g2xfabb9a1e1004090834y7d9c05f0xf09ef7f5c34c3ab7%40mail.gmail.com In-Reply-To: <7vvdc12mmz.fsf@alter.siamese.dyndns.org> ``` Heya, On Fri, Apr 9, 2010 at 07:57, Junio C Hamano wrote: > 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",".")); 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. -- Cheers, Sverre Rabbelier ``` ## Brian Gernhardt, 2010-04-09 15:35 Subject: [PATCH] Makefile: Simplify handling of python scripts Message-ID: <1270827315-29024-1-git-send-email-brian@gernhardtsoftware.com> URL: https://gitlist.dev/e/1270827315-29024-1-git-send-email-brian%40gernhardtsoftware.com In-Reply-To: <7vvdc12mmz.fsf@alter.siamese.dyndns.org> ``` 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 Signed-off-by: Brian Gernhardt --- Makefile | 9 ++------- git-remote-testgit.py | 2 ++ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index f0fe351..87c90d6 100644 --- a/Makefile +++ b/Makefile @@ -1629,13 +1629,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py 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)|' \ - -e '}' \ - -e 's|^import sys.*|&; \\\ - import os; \\\ - sys.path.insert(0, os.getenv("GITPYTHONLIB",\ - "@@INSTLIBDIR@@"));|' \ + sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \ + -e 's|\(os\.getenv("GITPYTHONLIB"\)[^)]*)|\1,"@@INSTLIBDIR@@")|' \ -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \ $@.py >$@+ && \ chmod +x $@+ && \ diff --git a/git-remote-testgit.py b/git-remote-testgit.py index f61624e..9253922 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -2,6 +2,8 @@ import hashlib import sys +import os +sys.path.insert(0, os.getenv("GITPYTHONLIB",".")) from git_remote_helpers.util import die, debug, warn from git_remote_helpers.git.repo import GitRepo -- 1.7.1.rc0.243.g2ce66 ```