[reprotest] 01/04: shell_ast: Remove most of it because it's not used, move the rest to shell_syn

Ximin Luo infinity0 at debian.org
Fri Oct 13 14:33:39 UTC 2017


This is an automated email from the git hooks/post-receive script.

infinity0 pushed a commit to branch master
in repository reprotest.

commit 3ddb72e8ca64dba9b69f7bf761296bc999af3f3c
Author: Ximin Luo <infinity0 at debian.org>
Date:   Fri Oct 13 15:43:32 2017 +0200

    shell_ast: Remove most of it because it's not used, move the rest to shell_syn
---
 debian/TODO             |  10 --
 debian/changelog        |   6 +
 reprotest/__init__.py   |  50 +++---
 reprotest/_shell_ast.py | 430 ------------------------------------------------
 reprotest/build.py      | 113 +++++++------
 reprotest/shell_syn.py  |  79 +++++++++
 6 files changed, 170 insertions(+), 518 deletions(-)

diff --git a/debian/TODO b/debian/TODO
index 3d12054..e004833 100644
--- a/debian/TODO
+++ b/debian/TODO
@@ -4,17 +4,7 @@ Make the CLI nicer
 Add a "auto-sbuild" pseudo-command that selects sbuild to build .dsc files (and
 guess the command artifact output), this should help with #847805.
 
-Refactoring
-===========
-
-To allow to run N builds instead of just 2.
-
-Afterwards we can then implement Bernhard's algorithm #850284.
-
 Misc
 ====
 
---no-clean-on-error doesn't work since we did the shell-script based
-refactoring, fix it.
-
 add a default Debian-reprotest user in the Debian packaging and make this cross-distro
diff --git a/debian/changelog b/debian/changelog
index 64c9c45..dd9eb29 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+reprotest (0.7.3) UNRELEASED; urgency=medium
+
+  * Fix --no-clean-on-error, it should work again.
+
+ -- Ximin Luo <infinity0 at debian.org>  Fri, 13 Oct 2017 15:42:51 +0200
+
 reprotest (0.7.2) unstable; urgency=medium
 
   * Various bug fixes to get the basic dsc+schroot example working.
diff --git a/reprotest/__init__.py b/reprotest/__init__.py
index 6e73ed5..19c43a6 100644
--- a/reprotest/__init__.py
+++ b/reprotest/__init__.py
@@ -51,6 +51,7 @@ def get_all_servers():
 # approaches.
 
 class Testbed(adt_testbed.Testbed):
+
     def check_exec2(self, argv, stdout=False, kind='short', xenv=[]):
         """Like check_exec but does not bomb on stderr, and can pass xenv."""
         (code, out, err) = self.execute(argv,
@@ -61,6 +62,11 @@ class Testbed(adt_testbed.Testbed):
                       adtlog.AutopkgtestError)
         return out
 
+    def bomb(self, m, _type=adtlog.TestbedFailure):
+        adtlog.debug('%s %s' % (_type.__name__, m))
+        #self.stop() # don't stop when bombing, so we can control it via no_clean_on_error
+        raise _type(m)
+
 @contextlib.contextmanager
 def start_testbed(args, temp_dir, no_clean_on_error=False, host_distro='debian'):
     '''This is a simple wrapper around adt_testbed that automates the
@@ -76,8 +82,11 @@ def start_testbed(args, temp_dir, no_clean_on_error=False, host_distro='debian')
     should_clean = True
     try:
         yield testbed
-    except:
+    except GeneratorExit:
+        pass
+    except BaseException as e:
         if no_clean_on_error:
+            logging.warn("preserving temporary stuff: %s", testbed.scratch)
             should_clean = False
         raise
     finally:
@@ -143,11 +152,17 @@ class BuildContext(collections.namedtuple('_BuildContext',
         return os.path.join(self.local_dist_root, self.build_name)
 
     def make_build_commands(self, script, env):
-        return Build.from_command(
-            build_command = script,
+        _ = self.plan_variations(Build.from_command(
+            build_command =
+                'cd "$REPROTEST_BUILD_PATH"; unset REPROTEST_BUILD_PATH; ' +
+                'umask "$REPROTEST_UMASK"; unset REPROTEST_UMASK; ' +
+                script,
             env = types.MappingProxyType(env),
             tree = self.testbed_src
-        )
+        ))
+        _ = _.append_setup_exec_raw('export', 'REPROTEST_BUILD_PATH=%s' % _.tree)
+        _ = _.append_setup_exec_raw('export', 'REPROTEST_UMASK=$(umask)')
+        return _
 
     def plan_variations(self, build):
         actions = self.variations.spec.actions()
@@ -166,7 +181,7 @@ class BuildContext(collections.namedtuple('_BuildContext',
         logging.info("copying %s back from virtual server's %s", self.testbed_dist, self.local_dist)
         testbed.command('copyup', (self.testbed_dist, os.path.join(self.local_dist, '')))
 
-    def run_build(self, testbed, build, artifact_pattern, testbed_build_pre):
+    def run_build(self, testbed, build, artifact_pattern, testbed_build_pre, no_clean_on_error):
         logging.info("starting build with source directory: %s, artifact pattern: %s",
             self.testbed_src, artifact_pattern)
         # we remove existing artifacts in case the build doesn't overwrite it
@@ -176,11 +191,9 @@ class BuildContext(collections.namedtuple('_BuildContext',
             (self.testbed_src, artifact_pattern, testbed_build_pre or "true")])
         # this dance is necessary because the cwd can't be cd'd into during the
         # setup phase under some variations like user_group
-        _ = build
-        _ = _.append_setup_exec_raw('export', 'REPROTEST_BUILD_PATH=%s' % build.tree)
-        _ = _.append_setup_exec_raw('export', 'REPROTEST_UMASK=$(umask)')
-        new_script = _.to_script()
-        logging.info("executing: %s", new_script)
+        new_script = build.to_script(no_clean_on_error)
+        logging.info("executing build...")
+        logging.debug(new_script)
         testbed.check_exec2(['sh', '-ec', new_script],
             xenv=['%s=%s' % (k, v) for k, v in build.env.items()],
             kind='build')
@@ -286,16 +299,9 @@ class TestArgs(collections.namedtuple('_Test',
                     var = var.replace.spec.apply_dynamic_defaults(source_root)
                     bctx = BuildContext(testbed.scratch, result_dir, source_root, name, var)
 
-                    build = bctx.make_build_commands(
-                        'cd "$REPROTEST_BUILD_PATH"; unset REPROTEST_BUILD_PATH; ' +
-                        'umask "$REPROTEST_UMASK"; unset REPROTEST_UMASK; ' +
-                        build_command, os.environ)
-                    logging.log(5, "build %s: %r", name, build)
-                    build = bctx.plan_variations(build)
-                    logging.log(5, "build %s: %r", name, build)
-
+                    build = bctx.make_build_commands(build_command, os.environ)
                     bctx.copydown(testbed)
-                    bctx.run_build(testbed, build, artifact_pattern, testbed_build_pre)
+                    bctx.run_build(testbed, build, artifact_pattern, testbed_build_pre, no_clean_on_error)
                     bctx.copyup(testbed)
 
                     name_variation = yield bctx.local_dist
@@ -597,7 +603,7 @@ def run(argv, dry_run=None):
     parsed_args = command_line(parser, config_args + argv)
 
     verbosity = parsed_args.verbosity
-    adtlog.verbosity = verbosity
+    adtlog.verbosity = verbosity - 1
     logging.basicConfig(
         format='%(message)s', level=30-10*verbosity, stream=sys.stdout)
     logging.debug('%r', parsed_args)
@@ -648,8 +654,10 @@ def run(argv, dry_run=None):
     testbed_build_pre = parsed_args.testbed_build_pre
     diffoscope_args = parsed_args.diffoscope_arg
     source_pattern = parsed_args.source_pattern
-    if verbosity >= 2:
+    if verbosity >= 3:
         diffoscope_args += ["--debug"]
+    elif not verbosity:
+        diffoscope_args += ["--no-progress"]
 
     # Do presets
     if build_command == 'auto':
diff --git a/reprotest/_shell_ast.py b/reprotest/_shell_ast.py
deleted file mode 100644
index 3c6683b..0000000
--- a/reprotest/_shell_ast.py
+++ /dev/null
@@ -1,430 +0,0 @@
-# Licensed under the GPL: https://www.gnu.org/licenses/gpl-3.0.en.html
-# For details: reprotest/debian/copyright
-'''This is partial implementation of an abstract syntax tree (AST) for
-the POSIX shell command language from the grammar at
-http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10
-.  It's used exclusively for generating shell scripts from ASTs, which
-means that it doesn't include a parser, and only implements the
-functionality needed for reprotest's shell scripts, which means that
-it doesn't include any superfluous features or alternatives that
-construct the same semantics.
-
-The nodes are classes.  Nodes that correspond directly to rules in
-POSIX standard's grammar are named using camel-case conversions of the
-names in the grammar (e.g. simple_command becomes SimpleCommand).
-Each docstring should contain the right-hand side of the grammar
-definition for the corresponding node, if any.  Otherwise, the
-docstring should contain describe which grammar rules the node
-represents and what other types of nodes it's allowed to contain.  All
-nodes should contain only other nodes and strings.  Empty fields are
-denoted by the empty string.  All nodes must have __slots__ set to the
-empty tuple.  Each class has only one overloaded method, __str__,
-which should transform the AST into valid shell code.
-'''
-
-import collections
-import itertools
-import shlex
-
-
-class _SequenceNode(tuple):
-    '''Tuple subclass that returns appropriate types from methods.
-
-    This overloads tuple methods so they return the subclass's type
-    rather than tuple and provides a nicer __repr__.
-
-    '''
-
-    def __add__(self, other):
-        if self.__class__ is other.__class__:
-            return self.__class__(itertools.chain(self, other))
-        else:
-            raise TypeError('Cannot add two shell AST nodes of different types.')
-    __iadd__ = __add__
-
-    def __radd__(self, other):
-        if self.__class__ is other.__class__:
-            return self.__class__(itertools.chain(other, self))
-        else:
-            raise TypeError('Cannot add two shell AST nodes of different types: %s, %s' % (repr(self), repr(other)))
-
-    def __getitem__(self, index):
-        if isinstance(index, slice):
-            return self.__class__(super().__getitem__(index))
-        else:
-            return super().__getitem__(index)
-
-    def __repr__(self):
-        return self.__class__.__name__ + super().__repr__()
-
-
-class BaseNode:
-    '''Abstract base class for all nodes.  This class should never be
-    instantiated.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        '''A generic implementation of __str__ that returns the node's fields
-        separated by spaces.'''
-        return ' '.join(str(field) for field in self)
-
-
-class Command(BaseNode):
-    '''Abstract base class for command nodes.  This class exists to define
-    a type that other classes can refer to to show where any command
-    is allowed, and should never be instantiated.
-
-    Grammar rules:
-    
-    command: simple_command | compound_command | compound_command
-    redirect_list | function_definition;
-
-    compound_command: brace_group | subshell | for_clause | case_clause
-    | if_clause | while_clause | until_clause;
-
-    '''
-    __slots__ = ()
-
-
-class List(BaseNode, _SequenceNode):
-    '''The recursion in this rule is flatted into a sequence.
-    separator_op is a & or ;.
-
-    Grammar rules:
-
-    list: list separator_op and_or | and_or;
-
-    compound_list: term | newline_list term | term separator |
-    newline_list term separator;
-
-    newline_list: NEWLINE | newline_list NEWLINE;
-
-    separator: separator_op linebreak | newline_list;
-
-    linebreak: newline_list | /* empty */
-
-    Attributes:
-        *args (Sequence[Term]): A sequence of commands terminated by & or ;.
-
-    '''
-    __slots__ = ()
-
-
-class Term(BaseNode, collections.namedtuple('_Term', 'command separator')):
-    '''This rule is recursive in the grammar, but its direct recursion is
-    handled in List in this AST.
-
-    Grammar rule:
-
-    term separator and_or | and_or
-
-    Attributes:
-        command (AndList, OrList, Command): A command or sequence of commands.
-        separator (str): & or ;.
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return str(self.command) + self.separator
-
-
-class AndList(BaseNode, _SequenceNode):
-    '''While the && and || operators are not associative with each other,
-    each is associative with itself, so this recursion can also be
-    flattened into a sequence.
-
-    Grammar rules:
-    
-    list: list separator_op and_or | and_or;
-
-    and_or: pipeline | and_or AND_IF linebreak pipeline
-    | and_or OR_IF linebreak pipeline;
-
-    Attributes:
-        *args (Sequence[Pipelines, Commands]): A sequence of commands and/or
-            pipelines.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return ' && '.join(str(field) for field in self)
-
-
-class OrList(BaseNode, _SequenceNode):
-    '''While the && and || operators are not associative with each other,
-    each is associative with itself, so this recursion can also be
-    flattened into a sequence.
-
-    Grammar rules:
-    
-    list: list separator_op and_or | and_or;
-
-    and_or: pipeline | and_or AND_IF linebreak pipeline
-    | and_or OR_IF linebreak pipeline;
-
-    Attributes:
-        *args (Sequence[Pipelines, Commands]): A sequence of commands and/or
-            pipelines.
-
-    '''
-    def __str__(self):
-        return ' || '.join(str(field) for field in self)
-
-
-class Pipeline(BaseNode, _SequenceNode):
-    '''The recursion in this rule is flatted into a sequence.  The option
-    to prepend the bang (!) to a pipeline is deliberately omitted.  It
-    would require another class because the __str__ method would be
-    different.
-
-    Grammar rules:
-
-    pipeline: pipe_sequence | Bang pipe_sequence;
-
-    pipe_sequence: command | pipe_sequence '|' linebreak command;
-
-    Attributes:
-        *args (Sequence[Command]): commands to be piped together.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return ' | '.join(str(field) for field in self)
-
-
-class SimpleCommand(Command,
-                    collections.namedtuple('_SimpleCommand',
-                                           'cmd_prefix cmd_name cmd_suffix')):
-    '''The rule distinguishes between command names prefixed with
-    redirection or environment variables by using cmd_word instead of
-    cmd_name, but this difference is immaterial when generating shell
-    code from an AST.  cmd_name and cmd_word are just WORDs.
-
-    Grammar rule:
-
-    cmd_prefix cmd_word cmd_suffix | cmd_prefix cmd_word | cmd_prefix |
-    cmd_name cmd_suffix | cmd_name;
-
-    Attributes:
-        cmd_prefix (CmdPrefix, ''): Environment variables and IO redirection.
-        cmd_name (str): A valid shell command name.
-        cmd_suffix (CmdSuffix, ''): Command arguments and IO redirection.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return ((str(self.cmd_prefix) + ' ' if self.cmd_prefix else '') +
-                str(self.cmd_name) +
-                (' ' + str(self.cmd_suffix) if self.cmd_suffix else ''))
-
-    @classmethod
-    def make(cls, *args):
-        '''Convenience constructor for generating SimpleCommand nodes.'''
-        return cls('', args[0], CmdSuffix(args[1:]))
-
-
-class CmdPrefix(BaseNode, _SequenceNode):
-    '''The recursion in this rule is flatted into a sequence.
-
-    Grammar rule:
-
-    io_redirect | cmd_prefix io_redirect | ASSIGNMENT_WORD | cmd_prefix
-    ASSIGNMENT_WORD;
-
-    Attributes:
-        *args (AssignmentWord, IORedirect): IO redirection and
-            environment variables.
-    '''
-    __slots__ = ()
-
-
-class AssignmentWord(BaseNode,
-                     collections.namedtuple('_AssignmentWord', 'target value')):
-    '''Corresponds to environment variable assignments of the form
-    target=value.
-
-    Attributes:
-        target (str): Environment variable name.
-        value (str): Environment variable value.
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return str(self.target) + '=' + str(self.value)
-
-
-class IORedirect(BaseNode,
-                 collections.namedtuple('_IORedirect',
-                                        'io_number operator filename')):
-    '''This represents a redirection and combines three rules.  here_end
-    is just a WORD.
-
-    Grammar rules:
-
-    io_redirect: io_file | IO_NUMBER io_file | io_here | IO_NUMBER io_here;
-    
-    io_file: '<' filename | LESSAND filename | '>' filename | GREATAND filename
-    | DGREAT filename | LESSGREAT filename | CLOBBER filename;
-
-    io_here: DLESS here_end | DLESSDASH here_end;
-
-    Attributes:
-        io_number (int, ''): A file descriptor.  This should hold the empty
-            string if omitted.
-        operator (str): One of >, <, <<, >>, <&, >&, <>, <<-, or >|.
-        filename (str): Valid file name to redirect to.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return (str(self.io_number) + str(self.operator) + ' ' +
-                str(self.filename))
-
-
-class CmdSuffix(BaseNode, _SequenceNode):
-    ''''The recursion in this rule is flatted into a sequence.  This node
-    represents the arguments passed to a simple command.
-
-    Grammar rule:
-
-    io_redirect | cmd_suffix io_redirect | WORD | cmd_suffix WORD
-
-    Attributes:
-        *args (str, IORedirect): command arguments and IO redirection.
-
-    '''
-    __slots__ = ()
-
-
-class IfClause(Command,
-               collections.namedtuple('_IfClause', 'condition then else_part')):
-    '''The start of an if-then conditional clause.
-
-    Grammar rule:
-
-    If compound_list Then compound_list else_part Fi
-
-    Attributes:
-        condition (List, Command): The command whose exit status determines
-            which branch is taken.
-        then (List, Command): The command to execute if condition exits with
-            zero.
-        else_part (ElsePart, ''): The optional command to execute if
-            condition exits with a nonzero value.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return ('if ' + str(self.condition) + '\nthen ' + str(self.then) +
-                '\n' + str(self.else_part) + '\nfi')
-
-
-class ElsePart(BaseNode, collections.namedtuple('_IfClause', 'elifs then')):
-    '''The elif and else parts of an if-then conditional clause.  The rule
-    corresponding to this node is recursive, but the recursion is
-    handled in Elifs.
-
-    Grammar rule:
-
-    Elif compound_list Then compound_list | Elif compound_list Then
-    compound_list else_part | Else compound_list
-
-    Attributes:
-        elifs (Elifs, ''): The optional conditions and commands to execute if
-            the preceding command exits with nonzero status.
-        then (List, Command, ''): The optional command to execute if
-            all other conditionals exit with nonzero statuses.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return (str(self.elifs) +
-                ('\nelse ' + str(self.then)) if self.then else '')
-
-class Elifs(BaseNode, _SequenceNode):
-    '''This node doesn't directly correspond to a grammar rule.  It
-    flattens the recursion for elif statements in else_part.
-
-    Grammar rule:
-
-    Elif compound_list Then compound_list | Elif compound_list Then
-    compound_list else_part | Else compound_list
-
-    Attributes:
-        *args (Sequence[Elif]): A sequence of elif statements.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return '\n'.join(str(field) for field in self)
-
-
-class Elif(BaseNode, collections.namedtuple('_Elif', 'condition then')):
-    '''This node also doesn't directly correspond to a grammar rule
-
-    Grammar rule:
-
-    Elif compound_list Then compound_list | Elif compound_list Then
-    compound_list else_part | Else compound_list
-
-    Attributes:
-        condition (List, Command): The command whose exit status determines
-            which branch is taken.
-        then (List, Command): The command to execute if condition exits with
-            zero.
-
-    '''
-    __slots__ = ()
-    
-    def __str__(self):
-        return 'elif ' + str(self.condition) + '\nthen ' + str(self.then)
-
-
-class BraceGroup(Command, collections.namedtuple('_BraceGroup', 'list')):
-    '''Grammar rule:
-
-    Lbrace compound_list Rbrace'''
-    __slots__ = ()
-
-    def __str__(self):
-        return '{ ' + str(self.list) + ' }'
-
-
-class Subshell(Command, collections.namedtuple('_Subshell', 'list')):
-    '''Grammar rule:
-
-    '(' compound_list ')'
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return '( ' + str(self.list) + ' )'
-
-
-class Quote(Command, collections.namedtuple('_Quote', 'command')):
-    '''This is a special node that allows nesting of commands using shell
-    quoting.  For example, to pass a script to a specific shell:
-
-    SimpleCommand('', 'bash', CmdSuffix([Quote(<script>)]))
-
-    This can also be used to insert shell code from other sources into
-    an AST in a proper way.
-
-    Attributes:
-        command (List, Command, str): AST or string to quote.
-
-    '''
-    __slots__ = ()
-
-    def __str__(self):
-        return shlex.quote(str(self.command))
diff --git a/reprotest/build.py b/reprotest/build.py
index b1487fa..cb0dcfd 100644
--- a/reprotest/build.py
+++ b/reprotest/build.py
@@ -13,8 +13,8 @@ import random
 import time
 import types
 
-from reprotest import _shell_ast
 from reprotest import mdiffconf
+from reprotest import shell_syn
 from reprotest.utils import AttributeReplacer
 
 
@@ -48,21 +48,19 @@ class Build(collections.namedtuple('_Build', 'build_command setup cleanup env tr
     '''Holds the shell ASTs and various other data, used to execute each build.
 
     Fields:
-        build_command (_shell_ast.Command): The build command itself, including
-            all commands that accept other commands as arguments.  Examples:
-            setarch.
-        setup (_shell_ast.AndList): These are shell commands that change the
+        build_command (shell_syn.Command): The build command itself, including
+            wrapper commands like setarch and sudo that never need cleanup.
+        setup (shell_syn.AndList): These are shell commands that change the
             shell environment and need to be run as part of the same script as
             the main build command but don't take other commands as arguments.
             These execute conditionally because if one command fails,
             the whole script should fail.  Examples: cd, umask.
-        cleanup (_shell_ast.List): All commands that have to be run to return
+        cleanup (shell_syn.List): All commands that have to be run to return
             the testbed to its initial state, before the testbed does its own
-            cleanup.  These are executed only if the build command fails,
-            because otherwise the cleanup has to occur after the build artifact
-            is copied out.  These execution unconditionally, one after another,
+            cleanup.  These execute one after another regardless of failure,
             because all cleanup commands should be attempted irrespective of
-            whether others succeed.  Examples: fileordering.
+            whether others succeed.  Examples: fileordering.  This is *not* run
+            if no_clean_on_error is given and setup or build_command failed.
         env (types.MappingProxyType): Immutable mapping of the environment.
         tree (str): Path to the source root where the build should take place.
     '''
@@ -70,10 +68,10 @@ class Build(collections.namedtuple('_Build', 'build_command setup cleanup env tr
     @classmethod
     def from_command(cls, build_command, env, tree):
         return cls(
-            build_command = _shell_ast.SimpleCommand(
-                "sh", "-ec", _shell_ast.Quote(build_command)),
-            setup = _shell_ast.AndList(),
-            cleanup = _shell_ast.List(),
+            build_command = shell_syn.Command.make(
+                "sh", "-ec", shlex.quote(str(build_command))),
+            setup = shell_syn.AndList(),
+            cleanup = shell_syn.List(),
             env = env,
             tree = tree,
         )
@@ -84,47 +82,36 @@ class Build(collections.namedtuple('_Build', 'build_command setup cleanup env tr
         new_mapping[key] = value
         return self._replace(env=types.MappingProxyType(new_mapping))
 
-    def append_to_build_command(self, command):
-        '''Passes the current build command as the last argument to a given
-        _shell_ast.SimpleCommand.
-
-        '''
-        new_suffix = (command.cmd_suffix +
-                      _shell_ast.CmdSuffix([self.build_command]))
-        new_command = _shell_ast.SimpleCommand(command.cmd_prefix,
-                                               command.cmd_name,
-                                               new_suffix)
+    def prepend_to_build_command(self, *prefix):
+        '''Prepend a wrapper command onto the build_command.'''
+        new_command = shell_syn.Command(
+            cmd_prefix=shell_syn.CmdPrefix(prefix),
+            cmd_suffix=self.build_command)
         return self._replace(build_command=new_command)
 
     def append_setup(self, command):
-        '''Adds a command to the setup phase.
-
-        '''
-        new_setup = self.setup + _shell_ast.AndList([command])
+        '''Adds a command to the setup phase.'''
+        new_setup = self.setup + shell_syn.AndList([command])
         return self._replace(setup=new_setup)
 
     def append_setup_exec(self, *args):
-        return self.append_setup_exec_raw(*map(_shell_ast.Quote, args))
+        return self.append_setup_exec_raw(*map(shlex.quote, args))
 
     def append_setup_exec_raw(self, *args):
-        return self.append_setup(_shell_ast.SimpleCommand.make(*args))
+        return self.append_setup(shell_syn.Command.make(*args))
 
     def prepend_cleanup(self, command):
-        '''Adds a command to the cleanup phase.
-
-        '''
+        '''Adds a command to the cleanup phase.'''
         # if this command fails, save the exit code but keep executing
         # we run with -e, so it would fail otherwise
-        new_cleanup = (_shell_ast.List([_shell_ast.Term(
-                            "{0} || __c=$?".format(command), ';')])
-                       + self.cleanup)
-        return self._replace(cleanup=new_cleanup)
+        new_cleanup = shell_syn.List.make("{0} || __c=$?".format(command))
+        return self._replace(cleanup=new_cleanup + self.cleanup)
 
     def prepend_cleanup_exec(self, *args):
-        return self.prepend_cleanup_exec_raw(*map(_shell_ast.Quote, args))
+        return self.prepend_cleanup_exec_raw(*map(shlex.quote, args))
 
     def prepend_cleanup_exec_raw(self, *args):
-        return self.prepend_cleanup(_shell_ast.SimpleCommand.make(*args))
+        return self.prepend_cleanup(shell_syn.Command.make(*args))
 
     def move_tree(self, source, target, set_tree):
         new_build = self.append_setup_exec(
@@ -135,7 +122,7 @@ class Build(collections.namedtuple('_Build', 'build_command setup cleanup env tr
         else:
             return new_build
 
-    def to_script(self):
+    def to_script(self, no_clean_on_error):
         '''Generates the shell code for the script.
 
         The build command is only executed if all the setup commands
@@ -146,21 +133,35 @@ class Build(collections.namedtuple('_Build', 'build_command setup cleanup env tr
         directory when the cleanup tries to unmount it.)
 
         '''
-        subshell = _shell_ast.Subshell(self.setup +
-                                       _shell_ast.AndList([self.build_command]))
+        subshell = self.setup + shell_syn.AndList([self.build_command])
 
         if self.cleanup:
-            cleanup = """( __c=0; {0} exit $__c; )""".format(str(self.cleanup))
-            return """\
-if {0}; then
-    {1};
+            cleanup = shell_syn.List.make("__c=0") + self.cleanup + \
+                      shell_syn.List.make("exit $__c")
+            main_script = "if ( run_build ); then ( cleanup ); "
+            if no_clean_on_error:
+                main_script += "fi"
+            else:
+                main_script += """\
 else
-    __x=$?;
-    if {1}; then exit $__x; else
-        echo >&2; "cleanup failed with exit code $?"; exit $__x;
-    fi;
+    __x=$?; # save the exit code of run_build
+    if ( cleanup ); then :; else echo >&2 "cleanup failed with exit code $?"; fi;
+    exit $__x
 fi
-""".format(str(subshell), str(cleanup))
+"""
+            return """
+#### BEGIN REPROTEST BUILD SCRIPT ##############################################
+run_build() {{
+    {0}
+}}
+
+cleanup() {{
+    {1}
+}}
+
+{2}
+#### END REPROTEST BUILD SCRIPT ################################################
+""".format(subshell.__str__(4), cleanup.__str__(4), main_script.rstrip()).rstrip()
         else:
             return str(subshell)
 
@@ -231,9 +232,9 @@ def kernel(ctx, build, vary):
     # its two child reprotests will see the same value for "uname" but the
     # tests expect different values.
     if not vary:
-        return build.append_to_build_command(_shell_ast.SimpleCommand.make('linux64', '--uname-2.6'))
+        return build.prepend_to_build_command('linux64', '--uname-2.6')
     else:
-        return build.append_to_build_command(_shell_ast.SimpleCommand.make('linux32'))
+        return build.prepend_to_build_command('linux32')
 
 # TODO: if this locale doesn't exist on the system, Python's
 # locales.getlocale() will return (None, None) rather than this
@@ -298,11 +299,10 @@ def faketime(ctx, build, vary):
     else:
         # otherwise use a date far in the future
         faket = '+373days+7hours+13minutes'
-    settime = _shell_ast.SimpleCommand.make('faketime', faket)
     # faketime's manpages are stupidly misleading; it also modifies file timestamps.
     # this is only mentioned in the README. we do not want this, it really really
     # messes with GNU make and other buildsystems that look at timestamps.
-    return build.add_env('NO_FAKE_STAT', '1').append_to_build_command(settime)
+    return build.add_env('NO_FAKE_STAT', '1').prepend_to_build_command('faketime', faket)
 
 def umask(ctx, build, vary):
     if not vary:
@@ -336,10 +336,9 @@ def user_group(ctx, build, vary):
     else:
         user = user_group # "user" is used below
         sudo_command = ('sudo', '-E', '-u', user)
-    sudobuild = _shell_ast.SimpleCommand.make(*sudo_command)
     binpath = os.path.join(dirname(build.tree), 'bin')
 
-    _ = build.append_to_build_command(sudobuild)
+    _ = build.prepend_to_build_command(*sudo_command)
     # disorderfs needs to run as a different user.
     # we prefer that to running it as root, principle of least-privilege.
     _ = _.append_setup_exec('sh', '-ec', r'''
diff --git a/reprotest/shell_syn.py b/reprotest/shell_syn.py
index 98a6ab5..8728181 100644
--- a/reprotest/shell_syn.py
+++ b/reprotest/shell_syn.py
@@ -1,6 +1,85 @@
 # Licensed under the GPL: https://www.gnu.org/licenses/gpl-3.0.en.html
 # For details: reprotest/debian/copyright
 
+import collections
+import itertools
+
+
+### Formatting
+
+class _Tuple(tuple):
+    '''Tuple subclass that returns appropriate types from methods.
+
+    This overloads tuple methods so they return the subclass's type
+    rather than tuple and provides a nicer __repr__.
+    '''
+    __slots__ = ()
+
+    def __add__(self, other):
+        if self.__class__ is other.__class__:
+            return self.__class__(itertools.chain(self, other))
+        else:
+            raise TypeError('Cannot add two shell AST nodes of different types.')
+    __iadd__ = __add__
+
+    def __radd__(self, other):
+        if self.__class__ is other.__class__:
+            return self.__class__(itertools.chain(other, self))
+        else:
+            raise TypeError('Cannot add two shell AST nodes of different types: %s, %s' % (repr(self), repr(other)))
+
+    def __getitem__(self, index):
+        if isinstance(index, slice):
+            return self.__class__(super().__getitem__(index))
+        else:
+            return super().__getitem__(index)
+
+    def __repr__(self):
+        return self.__class__.__name__ + super().__repr__()
+
+
+    def __str__(self, indent=None):
+        return ' '.join(str(field) for field in self)
+
+
+class CmdPrefix(_Tuple):
+    pass
+
+class CmdSuffix(_Tuple):
+    pass
+
+class Command(collections.namedtuple('_Command', 'cmd_prefix cmd_suffix')):
+    '''A command arbitrarily divided as (prefix, suffix) for formatting.'''
+    def __str__(self, indent=None):
+        sep = ' ' if indent is None else ' \\\n%s' % (" " * indent)
+        return ((self.cmd_prefix.__str__(indent) + sep if self.cmd_prefix else '') +
+                (self.cmd_suffix.__str__(indent) if self.cmd_suffix else ''))
+
+    @classmethod
+    def make(cls, *args):
+        return cls(CmdPrefix(), CmdSuffix(args))
+
+
+class List(_Tuple):
+    '''List of commands separated by semicolon, for formatting.'''
+    def __str__(self, indent=None):
+        sep = '; ' if indent is None else '; \\\n%s' % (" " * indent)
+        return sep.join(field.__str__(indent) for field in self)
+
+    @classmethod
+    def make(cls, *args):
+        return cls(list(map(Command.make, args)))
+
+
+class AndList(_Tuple):
+    '''List of commands separated by &&, for formatting.'''
+    def __str__(self, indent=None):
+        sep = ' && ' if indent is None else ' && \\\n%s' % (" " * indent)
+        return sep.join(field.__str__(indent) for field in self)
+
+
+### Parsing
+
 # http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
 # Special chars except allowed ones <space> " ' \
 special_except_quotes=r"""

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/reprotest.git



More information about the Reproducible-commits mailing list