From 088537d6a4f220836daf155e507255ca1e3eea01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D0=BE=D0=BC=D0=B0=D0=BD=20=D0=94=D0=BE=D0=BD=D1=87?= =?UTF-8?q?=D0=B5=D0=BD=D0=BA=D0=BE?= Date: Fri, 26 Sep 2014 00:33:14 +0400 Subject: [PATCH] Remove the "lone pipe" exception in ShellCommand argument handling As discussed in . This makes it possible to pass a pipe as an argument, plus it makes argument handling consistent between Windows and Unix-likes. --- master/docs/manual/cfg-buildsteps.rst | 3 --- master/docs/relnotes/index.rst | 6 ++++++ slave/buildslave/runprocess.py | 5 ----- slave/buildslave/test/unit/test_runprocess.py | 17 ----------------- 4 files changed, 6 insertions(+), 25 deletions(-) diff --git a/master/docs/manual/cfg-buildsteps.rst b/master/docs/manual/cfg-buildsteps.rst index 0b6fc81f32b..140526188fb 100644 --- a/master/docs/manual/cfg-buildsteps.rst +++ b/master/docs/manual/cfg-buildsteps.rst @@ -1002,9 +1002,6 @@ The :bb:step:`ShellCommand` arguments are: If ``command`` contains nested lists (for example, from a properties substitution), then that list will be flattened before it is executed. - On the topic of shell metacharacters, note that in DOS the pipe character (``|``) is conditionally escaped (to ``^|``) when it occurs inside a more complex string in a list of strings. - It remains unescaped when it occurs as part of a single string or as a lone pipe in a list of strings. - ``workdir`` All ShellCommands are run by default in the ``workdir``, which defaults to the :file:`build` subdirectory of the slave builder's base directory. The absolute path of the workdir will thus be the slave's basedir (set as an option to ``buildslave create-slave``, :ref:`Creating-a-buildslave`) plus the builder's basedir (set in the builder's ``builddir`` key in :file:`master.cfg`) plus the workdir itself (a class-level attribute of the BuildFactory, defaults to :file:`build`). diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst index 888de5ea2ee..ebc17ab1bd3 100644 --- a/master/docs/relnotes/index.rst +++ b/master/docs/relnotes/index.rst @@ -295,6 +295,12 @@ Deprecations, Removals, and Non-Compatible Changes Buildbot-slave-0.9.0 will still run successfully against Buildbot-0.8.9 or earlier, configured to use old-style steps. However, the support is stlil deprecated and will be removed as soon as it is inconvenient for developers. +* On Windows, if a :bb:step:`ShellCommand` step in which ``command`` was specified as a list is executed, and a + list element is a string consisting of a single pipe character, it no longer creates a pipeline. + Instead, the pipe character is passed verbatim as an argument to the program, like any other string. + This makes command handling consistent between Windows and Unix-like systems. + To have a pipeline, specify ``command`` as a string. + Details ------- diff --git a/slave/buildslave/runprocess.py b/slave/buildslave/runprocess.py index 40b160e38cb..a6a81ee4696 100644 --- a/slave/buildslave/runprocess.py +++ b/slave/buildslave/runprocess.py @@ -49,12 +49,7 @@ def win32_batch_quote(cmd_list): # Quote cmd_list to a string that is suitable for inclusion in a # Windows batch file. This is not quite the same as quoting it for the # shell, as cmd.exe doesn't support the %% escape in interactive mode. - # As an exception, a lone pipe as an argument is not escaped, and - # becomes a shell pipe. def escape_arg(arg): - if arg == '|': - return arg - arg = quoteArguments([arg]) # escape shell special characters arg = re.sub(r'[@()^"<>&|]', r'^\g<0>', arg) diff --git a/slave/buildslave/test/unit/test_runprocess.py b/slave/buildslave/test/unit/test_runprocess.py index 3e8bf80fc56..b3ca2cce8cb 100644 --- a/slave/buildslave/test/unit/test_runprocess.py +++ b/slave/buildslave/test/unit/test_runprocess.py @@ -282,23 +282,6 @@ def check(ign): d.addCallback(check) return d - @compat.skipUnlessPlatformIs("win32") - def testPipeAlone(self): - b = FakeSlaveBuilder(False, self.basedir) - # this is highly contrived, but it proves the point. - cmd = stdoutCommand("b\\na") - cmd[0] = cmd[0].replace(".exe", "") - cmd.extend(['|', 'sort']) - s = runprocess.RunProcess(b, cmd, self.basedir) - - d = s.start() - - def check(ign): - self.failUnless({'stdout': nl('a\nb\n')} in b.updates, b.show()) - self.failUnless({'rc': 0} in b.updates, b.show()) - d.addCallback(check) - return d - @compat.skipUnlessPlatformIs("win32") def testPipeString(self): b = FakeSlaveBuilder(False, self.basedir)