{"thread":{"id":"23386","subject":"[PATCH] Makefile: Remove excess backslashes from sed","startedAt":"2010-04-09T03:22:10Z","lastAt":"2010-04-09T15:35:15Z","messageCount":8,"participants":["Brian Gernhardt","David Aguilar","Junio C Hamano","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"138998","messageId":"1270783330-35215-1-git-send-email-brian@gernhardtsoftware.com","threadId":"23386","inReplyTo":null,"subject":"[PATCH] Makefile: Remove excess backslashes from sed","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T03:22:10Z","receivedAt":"2010-04-09T03:22:10Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"The sed script that was intended to add lines altering the sys.path\nhad extra backslashes in them. Instead resulting in\n\n import sys; import os; sys.path.insert( ... )\n\nIt output\n\n import sys; \\ import os; \\ sys.path.insert( ... )\n\nUnfortunately this caused python (2.6.1 on OS X 10.6.3) to error\n\n SyntaxError: unexpected character after line continuation character\n\nRemoving two of the backslashes solves this problem.\n\nSigned-off-by: Brian Gernhardt \n---\n\n I'm not sure if this is a result of OS X's python or sed, but t5800\n won't work for me without this change.\n\n Makefile | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f0fe351..b9eb1ca 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1632,8 +1632,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \tsed -e '1{' \\\n \t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n \t -e '}' \\\n-\t -e 's|^import sys.*|&; \\\\\\\n-\t import os; \\\\\\\n+\t -e 's|^import sys.*|&; \\\n+\t import os; \\\n \t sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\\\n \t \"@@INSTLIBDIR@@\"));|' \\\n \t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n-- \n1.7.1.rc0.210.ge6da\n"},{"id":"139015","messageId":"7vvdc12mmz.fsf@alter.siamese.dyndns.org","threadId":"23386","inReplyTo":"1270783330-35215-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH] Makefile: Remove excess backslashes from sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-09T05:57:56Z","receivedAt":"2010-04-09T05:57:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt writes:\n\n> The sed script that was intended to add lines altering the sys.path\n> had extra backslashes in them. Instead resulting in\n>\n> import sys; import os; sys.path.insert( ... )\n>\n> It output\n>\n> import sys; \\ import os; \\ sys.path.insert( ... )\n>\n> Unfortunately this caused python (2.6.1 on OS X 10.6.3) to error\n>\n> SyntaxError: unexpected character after line continuation character\n>\n> Removing two of the backslashes solves this problem.\n\nTraditionally, multi-line sed statements written in the Makefile were\nportability nightmare, as the line folding rules (especially how the\nbackslash is treated in the output) in make implementations were subtly\ndifferent, and implementations of sed also got multi-line statement often\nwrong. These days, things might have gotten much better, but in olden\ndays (back when BSD vs SysV war was still raging), the trick to write\nthings like this portably was to invoke a shell script that has multi-line\nsed statement from the Makefile. It was painful.\n\nI wonder if we can make this a lot simpler to avoid multi-line sed script.\nFor example, we could write the source Python script to always begin with:\n\n\t#!/usr/bin/python\n import sys;\n import os;\n sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"));\n\nand then do something like this in the Makefile:\n\n\tINSTLIBDIR=... && \\\n\tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n -e 's|\\(os\\.getenv(\"GITPYTHONLIB\"\\)[^)]*)|\\1,\"'\"$$INSTLIBDIR\"'\")|' \\\n <$@.py >$@+\n\tmv $@+ $@\n\nContributors can then run things in-place while developing the scripts,\nperhaps setting GITPYTHONLIB to the source area.\n"},{"id":"139013","messageId":"20100409060427.GA6012@gmail.com","threadId":"23386","inReplyTo":"1270783330-35215-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH] Makefile: Remove excess backslashes from sed","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-09T06:04:28Z","receivedAt":"2010-04-09T06:04:28Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Apr 08, 2010 at 11:22:10PM -0400, Brian Gernhardt wrote:\n> The sed script that was intended to add lines altering the sys.path\n> had extra backslashes in them. Instead resulting [snip]\n> import sys; \\ import os; \\ sys.path.insert( ... )\n\nOn linux sed I get (or I was dreaming when I tried it):\n\nimport sys; \\\n\timport os; \\\n\tsys.path.insert(0, os.getenv(foo,\n\t bar))\n\nWe should instead smash it all into one line instead to\navoid differences in sed behavior:\n\nimport sys; import os; sys.path.insert(0, os.getenv(foo,\n bar))\n\nIf that's what it is, then your patch would produce this on\nlinux:\n\nimport sys;\n\timport os;\n\tsys.path.insert(foo,\n\t bar)\n\n...which is invalid syntax.\n\nSo let's smash it all onto one line instead. That way I don't\nhave to think about or test whether there's any difference in\nhow sed handles newlines. It'll be the Obviously Correct(tm)\nsol'n. Or maybe I was dreaming?\n\nIf you don't beat me to it by the tomorrow night I'll try and\nthrow together a patch. Thanks for catching this.\n\n> diff --git a/Makefile b/Makefile\n> index f0fe351..b9eb1ca 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1632,8 +1632,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n> \tsed -e '1{' \\\n> \t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n> \t -e '}' \\\n> -\t -e 's|^import sys.*|&; \\\\\\\n> -\t import os; \\\\\\\n> +\t -e 's|^import sys.*|&; \\\n> +\t import os; \\\n> \t sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\\\n> \t \"@@INSTLIBDIR@@\"));|' \\\n-- \n\n\tDavid\n"},{"id":"139021","messageId":"20100409080256.GA12527@gmail.com","threadId":"23386","inReplyTo":"7vvdc12mmz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: Remove excess backslashes from sed","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-09T08:02:57Z","receivedAt":"2010-04-09T08:02:57Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Apr 08, 2010 at 10:57:56PM -0700, Junio C Hamano wrote:\n> Brian Gernhardt writes:\n> \n> > The sed script that was intended to add lines altering the sys.path\n> > had extra backslashes in them. Instead resulting in\n> >\n> > import sys; import os; sys.path.insert( ... )\n> >\n> > It output\n> >\n> > import sys; \\ import os; \\ sys.path.insert( ... )\n> >\n> > Unfortunately this caused python (2.6.1 on OS X 10.6.3) to error\n> >\n> > SyntaxError: unexpected character after line continuation character\n> >\n> > Removing two of the backslashes solves this problem.\n> \n> Traditionally, multi-line sed statements written in the Makefile were\n> portability nightmare, as the line folding rules (especially how the\n> backslash is treated in the output) in make implementations were subtly\n> different, and implementations of sed also got multi-line statement often\n> wrong. These days, things might have gotten much better, but in olden\n> days (back when BSD vs SysV war was still raging), the trick to write\n> things like this portably was to invoke a shell script that has multi-line\n> sed statement from the Makefile. It was painful.\n> \n> I wonder if we can make this a lot simpler to avoid multi-line sed script.\n> For example, we could write the source Python script to always begin with:\n> \n> \t#!/usr/bin/python\n> import sys;\n> import os;\n> sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"));\n> \n> and then do something like this in the Makefile:\n> \n> \tINSTLIBDIR=... && \\\n> \tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n> -e 's|\\(os\\.getenv(\"GITPYTHONLIB\"\\)[^)]*)|\\1,\"'\"$$INSTLIBDIR\"'\")|' \\\n> <$@.py >$@+\n> \tmv $@+ $@\n> \n> Contributors can then run things in-place while developing the scripts,\n> perhaps setting GITPYTHONLIB to the source area.\n\nHere's what I cooked up so far...:\n\n$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS\n$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n\tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n\t\t--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \\\n\t\tinstlibdir` && \\\n\tsed -e '1{' \\\n\t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)\\n@@STARTUP@@\\n|' \\\n\t -e '}' \\\n\t -e 's|@@STARTUP@@|import os, sys; sys.path.insert(0, @@ENV@@)|' \\\n\t -e 's|@@ENV@@|os.getenv(\"GITPYTHONLIB\", \"@@INSTLIBDIR@@\")|' \\\n\t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n\t $@.py >$@+ && \\\n\tchmod +x $@+ && \\\n\tmv $@+ $@\n\nIt avoids the multi-line portability concerns by substituting variables\nalong each step.\n\nThis is also nicer in that we don't need to start scripts with\n\"import sys\" or os.getenv() anything... all we require is a\nshebang line. We inject @@STARTUP@@ when we rewrite the\nshebang.\n\nThat seems nicer then the old expression which searched for\n\"import sys.*\" when doing the replacements.\n\nIs the \\n@@STARTUP\\n thing portable?\n\n-- \n\t\tDavid\n"},{"id":"139023","messageId":"1270802559-14791-1-git-send-email-davvid@gmail.com","threadId":"23386","inReplyTo":"20100409080256.GA12527@gmail.com","subject":"[PATCH] Makefile: Avoid multi-line sed when building python scripts","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-09T08:42:39Z","receivedAt":"2010-04-09T08:42:39Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Instead of requiring scripts to start with \"import sys\",\ninstall the sys.path initialization code immediately\nafter each script's new shebang line.\n\nThis works around portability issues with multi-line sed and\nsimplifies the work for contributors. There is no longer\nthe requirement that every script must \"import sys\" before\nimporting git-provided modules.\n\nSigned-off-by: David Aguilar \n---\n Makefile | 8 +++-----\n 1 files changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 956e781..b763947 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1608,12 +1608,10 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \t\t--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \\\n \t\tinstlibdir` && \\\n \tsed -e '1{' \\\n-\t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n+\t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)\\n@@STARTUP@@|' \\\n \t -e '}' \\\n-\t -e 's|^import sys.*|&; \\\\\\\n-\t import os; \\\\\\\n-\t sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\\\n-\t \"@@INSTLIBDIR@@\"));|' \\\n+\t -e 's|@@STARTUP@@|import os, sys; sys.path.insert(0, @@ENV@@)|' \\\n+\t -e 's|@@ENV@@|os.getenv(\"GITPYTHONLIB\", \"@@INSTLIBDIR@@\")|' \\\n \t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n \t $@.py >$@+ && \\\n \tchmod +x $@+ && \\\n-- \n1.7.0.3.313.g87b3c\n"},{"id":"139039","messageId":"1C4F213E-9DF1-4184-B953-303DEDD69AC1@gernhardtsoftware.com","threadId":"23386","inReplyTo":"20100409080256.GA12527@gmail.com","subject":"Re: [PATCH] Makefile: Remove excess backslashes from sed","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T14:33:57Z","receivedAt":"2010-04-09T14:33:57Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Apr 9, 2010, at 4:02 AM, David Aguilar wrote:\n\n> Is the \\n@@STARTUP\\n thing portable?\n\nNo. OS X's sed renders it as \n\n #!/usr/bin/pythonnimport os, sys; sys.path.insert(0, os.getenv(\"GITPYTHONLIB\", \"/usr/local/lib/python2.6/site-packages\"))\n\nSo NAK for your patch.\n\n~~ Brian"},{"id":"139046","messageId":"g2xfabb9a1e1004090834y7d9c05f0xf09ef7f5c34c3ab7@mail.gmail.com","threadId":"23386","inReplyTo":"7vvdc12mmz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: Remove excess backslashes from sed","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-04-09T15:34:15Z","receivedAt":"2010-04-09T15:34:15Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Apr 9, 2010 at 07:57, Junio C Hamano wrote:\n> I wonder if we can make this a lot simpler to avoid multi-line sed script.\n> For example, we could write the source Python script to always begin with:\n>\n>        #!/usr/bin/python\n>        import sys;\n>        import os;\n>        sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"));\n\nThank you, I like this a lot better than our current approach. I think\nit's very ugly that git (needs to) mangle(s) the python files when\ninstalling, since that means that line-numbers don't match in\nstacktraces. I'd definitely prefer using this.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"139048","messageId":"1270827315-29024-1-git-send-email-brian@gernhardtsoftware.com","threadId":"23386","inReplyTo":"7vvdc12mmz.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Makefile: Simplify handling of python scripts","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-04-09T15:35:15Z","receivedAt":"2010-04-09T15:35:15Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"The sed script intended to add a standard opening to python scripts\nwas non-compatible and overly complex. Simplifying it down to a set\nof one-liners removes the compatibility issues of newlines. Moving\nthe environment alterations from the Makefile to the python scripts\nmakes also makes the scripts easier to run in-place.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Brian Gernhardt \n---\n Makefile | 9 ++-------\n git-remote-testgit.py | 2 ++\n 2 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f0fe351..87c90d6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1629,13 +1629,8 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py\n \tINSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \\\n \t\t--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \\\n \t\tinstlibdir` && \\\n-\tsed -e '1{' \\\n-\t -e '\ts|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n-\t -e '}' \\\n-\t -e 's|^import sys.*|&; \\\\\\\n-\t import os; \\\\\\\n-\t sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\\\n-\t \"@@INSTLIBDIR@@\"));|' \\\n+\tsed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \\\n+\t -e 's|\\(os\\.getenv(\"GITPYTHONLIB\"\\)[^)]*)|\\1,\"@@INSTLIBDIR@@\")|' \\\n \t -e 's|@@INSTLIBDIR@@|'\"$$INSTLIBDIR\"'|g' \\\n \t $@.py >$@+ && \\\n \tchmod +x $@+ && \\\ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex f61624e..9253922 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -2,6 +2,8 @@\n \n import hashlib\n import sys\n+import os\n+sys.path.insert(0, os.getenv(\"GITPYTHONLIB\",\".\"))\n \n from git_remote_helpers.util import die, debug, warn\n from git_remote_helpers.git.repo import GitRepo\n-- \n1.7.1.rc0.243.g2ce66\n"}]}