Merge branch '1.0.x' into 'master'

CHANGELOG.md

@@ -11,6 +11,16 @@ Features:
 * Configuration file contains the walltime taken by each run
 * It is now possible to upload or download any file via its full path
 
+1.0.8 (???)
+-----------
+
+Behavior changes:
+* No longer default to overwriting trace directories. ReproZip will ask what to do or exit with an error if one of --continue/--overwrite is not provided
+
+Bugfixes:
+* Fix an issue identifying Debian packages when a file's in two packages
+* Fix Python error `Mixing iteration and read methods would lose data`
+
 1.0.7 (2016-08-22)
 ------------------
 

docs/packing.rst

@@ -18,7 +18,7 @@ The following command is used to trace a command line, or a `run`, used by the e
 
 where `<command-line>` is the command line. By running this command, *reprozip* executes `<command-line>` and uses ``ptrace`` to trace all the system calls issued, storing them in an SQLite database.
 
-Multiple runs can also be traced and combined in a single package by using the flag ``--continue``, or ``-c``, for all the runs except the first one::
+If you run the command multiple times, *reprozip* might ask you if you want to continue with your current trace (append the new command-line to it) or replace it (throw away the previous command-line you traced). You can skip this prompt by using either the ``--continue`` or ``--overwrite`` flag, like this::
 
     $ reprozip trace --continue <command-line>
 

reprounzip-vistrails/reprounzip/plugins/vistrails.py

@@ -305,7 +305,8 @@ def run_from_vistrails():
     config = load_config(Path(args.directory) / 'config.yml', canonical=True)
 
     python = sys.executable
-    rpuz = [python, '-m', 'reprounzip.main', args.unpacker]
+    rpuz = [python, '-c', 'from reprounzip.main import main; main()',
+            args.unpacker]
 
     os.environ['REPROUNZIP_NON_INTERACTIVE'] = 'y'
 

reprounzip/reprounzip/unpackers/common/packages.py

@@ -74,7 +74,7 @@ def get_packages_info(packages):
                     pkg, _ = pkgs_dict[name]
                     pkgs_dict[name] = pkg, status
         finally:
-            p.communicate()
+            p.wait()
 
         return pkgs_dict
 
@@ -130,7 +130,7 @@ def get_packages_info(packages):
                         pkg, _ = pkgs_dict[name]
                         pkgs_dict[name] = pkg, status
         finally:
-            p.communicate()
+            p.wait()
 
         return pkgs_dict
 

reprounzip/reprounzip/unpackers/default.py

@@ -209,7 +209,7 @@ def directory_run(args):
             if l.endswith(b':\n'):
                 lib_dirs.append(Path(l[:-2]))
     finally:
-        p.communicate()
+        p.wait()
     lib_dirs = ('export LD_LIBRARY_PATH=%s' % ':'.join(
                 shell_escape(unicode_(join_root(root, d)))
                 for d in lib_dirs))

reprozip/reprozip/main.py

@@ -212,10 +212,19 @@ def trace(args):
         argv = [args.arg0] + args.cmdline[1:]
     else:
         argv = args.cmdline
+    if args.append and args.overwrite:
+        logging.critical("You can't use both --continue and --overwrite")
+        sys.exit(2)
+    elif args.append:
+        append = True
+    elif args.overwrite:
+        append = False
+    else:
+        append = None
     reprozip.tracer.trace.trace(args.cmdline[0],
                                 argv,
                                 Path(args.dir),
-                                args.append,
+                                append,
                                 args.verbosity)
     reprozip.tracer.trace.write_configuration(Path(args.dir),
                                               args.identify_packages,
@@ -344,7 +353,10 @@ def add_options(opt):
         help="argument 0 to program, if different from program path")
     parser_trace.add_argument(
         '-c', '--continue', action='store_true', dest='append',
-        help="add to the previous run instead of replacing it")
+        help="add to the previous trace, don't replace it")
+    parser_trace.add_argument(
+        '-w', '--overwrite', action='store_true', dest='overwrite',
+        help="overwrite the previous trace, don't add to it")
     parser_trace.add_argument('cmdline', nargs=argparse.REMAINDER,
                               help="command-line to run under trace")
     parser_trace.set_defaults(func=trace)

reprozip/reprozip/tracer/linux_pkgs.py

@@ -49,6 +49,8 @@ def filter_files(self, files):
                 seen_files.add(f.path)
 
     def search_for_files(self, files):
+        nb_pkg_files = 0
+
         for f in self.filter_files(files):
             pkgnames = self._get_packages_for_file(f.path)
 
@@ -67,6 +69,7 @@ def search_for_files(self, files):
                             pkgs.append(self.packages[pkgname])
                 if len(pkgs) == 1:
                     pkgs[0].add_file(f)
+                    nb_pkg_files += 1
                 else:
                     self.unknown_files.add(f)
 
@@ -75,6 +78,11 @@ def search_for_files(self, files):
                          for pkgname, pkg in iteritems(self.packages)
                          if pkg.files}
 
+        logging.info("%d packages with %d files, and %d other files",
+                     len(self.packages),
+                     nb_pkg_files,
+                     len(self.unknown_files))
+
     def _filter(self, f):
         # Special files
         if any(f.path.lies_under(c) for c in magic_dirs):
@@ -131,15 +139,25 @@ def search_for_files(self, files):
         # Remaining files are not from packages
         self.unknown_files.update(
             f for f in files
-            if f.path in requested and f.path not in found)
+            if f.path in requested and found.get(f.path) is None)
+
+        nb_pkg_files = 0
 
         for path, pkgname in iteritems(found):
+            if pkgname is None:
+                continue
             if pkgname in self.packages:
                 package = self.packages[pkgname]
             else:
                 package = self._create_package(pkgname)
                 self.packages[pkgname] = package
             package.add_file(requested.pop(path))
+            nb_pkg_files += 1
+
+        logging.info("%d packages with %d files, and %d other files",
+                     len(self.packages),
+                     nb_pkg_files,
+                     len(self.unknown_files))
 
     def _get_packages_for_file(self, filename):
         # This method is no longer used for dpkg: instead of querying each file
@@ -165,10 +183,14 @@ def _create_package(self, pkgname):
                     version = fields[1].decode('ascii')
                     size = int(fields[2].decode('ascii')) * 1024    # kbytes
                     break
+            for l in p.stdout:  # finish draining stdout
+                pass
         finally:
-            p.communicate()
+            p.wait()
         if p.returncode == 0:
-            return Package(pkgname, version, size=size)
+            pkg = Package(pkgname, version, size=size)
+            logging.debug("Found package %s", pkg)
+            return pkg
         else:
             return None
 
@@ -197,7 +219,9 @@ def _create_package(self, pkgname):
         if p.returncode == 0:
             version, size = out.strip().decode('iso-8859-1').rsplit(' ', 1)
             size = int(size)
-            return Package(pkgname, version, size=size)
+            pkg = Package(pkgname, version, size=size)
+            logging.debug("Found package %s", pkg)
+            return pkg
         else:
             return None
 
@@ -207,14 +231,15 @@ def identify_packages(files):
     """
     distribution = platform.linux_distribution()[0].lower()
     if distribution in ('debian', 'ubuntu'):
-        logging.info("Identifying Debian packages...")
+        logging.info("Identifying Debian packages for %d files...", len(files))
         manager = DpkgManager()
     elif (distribution in ('centos', 'centos linux',
                            'fedora', 'scientific linux') or
             distribution.startswith('red hat')):
-        logging.info("Identifying RPM packages...")
+        logging.info("Identifying RPM packages for %d files...", len(files))
         manager = RpmManager()
     else:
+        logging.info("Unknown distribution, can't identify packages")
         return files, []
 
     begin = time.time()

reprozip/reprozip/tracer/trace.py

@@ -18,6 +18,7 @@
 import platform
 from rpaths import Path
 import sqlite3
+import sys
 
 from reprozip import __version__ as reprozip_version
 from reprozip import _pytracer
@@ -225,6 +226,52 @@ def get_files(conn):
     return files, inputs, outputs
 
 
+def tty_prompt(prompt, chars):
+    """Get input from the terminal.
+
+    On Linux, this will find the controlling terminal and ask there.
+
+    :param prompt: String to be displayed on the terminal before reading the
+        input.
+    :param chars: Accepted character responses.
+    """
+    try:
+        import termios
+
+        fd = os.open('/dev/tty', os.O_RDWR | os.O_NOCTTY)
+        stream = os.fdopen(fd, 'w+', 1)
+        old = termios.tcgetattr(fd)
+    except (ImportError, AttributeError, IOError, OSError):
+        ostream = sys.stdout
+        istream = sys.stdin
+        if not os.isatty(sys.stdin.fileno()):
+            return None
+
+        while True:
+            ostream.write(prompt)
+            ostream.flush()
+            line = istream.readline()
+            if not line:
+                return None
+            elif line[0] in chars:
+                return line[0]
+    else:
+        new = old[:]
+        new[3] &= ~termios.ICANON  # 3 == 'lflags'
+        tcsetattr_flags = termios.TCSAFLUSH | getattr(termios, 'TCSASOFT', 0)
+        try:
+            termios.tcsetattr(fd, tcsetattr_flags, new)
+            stream.write(prompt)
+            stream.flush()
+            while True:
+                char = stream.read(1)
+                if char in chars:
+                    return char
+        finally:
+            termios.tcsetattr(fd, tcsetattr_flags, old)
+            stream.flush()
+
+
 def trace(binary, argv, directory, append, verbosity=1):
     """Main function for the trace subcommand.
     """
@@ -237,16 +284,35 @@ def trace(binary, argv, directory, append, verbosity=1):
             "intended")
 
     # Trace directory
-    if not append:
-        if directory.exists():
-            logging.warning("Removing existing directory %s", directory)
+    if directory.exists():
+        if append is None:
+            r = tty_prompt(
+                "Trace directory %s exists\n"
+                "(a)ppend run to the trace, (d)elete it or (s)top? [a/d/s] " %
+                directory,
+                'aAdDsS')
+            if r is None:
+                logging.critical(
+                    "Trace directory %s exists\n"
+                    "Please use either --continue or --overwrite\n",
+                    directory)
+                sys.exit(1)
+            elif r in 'sS':
+                sys.exit(1)
+            elif r in 'dD':
+                directory.rmtree()
+                directory.mkdir()
+            logging.warning(
+                "You can use --overwrite to replace the existing trace "
+                "(or --continue to append\nwithout prompt)")
+        elif append is False:
+            logging.info("Removing existing trace directory %s", directory)
             directory.rmtree()
-        directory.mkdir(parents=True)
-    else:
-        if not directory.exists():
-            logging.warning("--continue was specified but %s does not exist "
-                            "-- creating", directory)
             directory.mkdir(parents=True)
+    else:
+        if append is True:
+            logging.warning("--continue was set but trace doesn't exist yet")
+        directory.mkdir()
 
     # Runs the trace
     database = directory / 'trace.sqlite3'

tests/functional.py

@@ -151,7 +151,7 @@ def functional_tests(raise_warnings, interactive, run_vagrant, run_docker):
                       'cat ../../../../../etc/passwd;'
                       'cd /var/lib;'
                       'cat ../../etc/group'])
-    check_call(rpz + ['trace',
+    check_call(rpz + ['trace', '--overwrite',
                       'bash', '-c', 'cat /etc/passwd;echo'])
     check_call(rpz + ['trace', '--continue',
                       'sh', '-c', 'cat /etc/group;/usr/bin/id'])
@@ -182,7 +182,7 @@ def check_simple(args, stream, infile=1):
     # Build
     build('simple', ['simple.c'])
     # Trace
-    check_call(rpz + ['trace', '-d', 'rpz-simple',
+    check_call(rpz + ['trace', '--overwrite', '-d', 'rpz-simple',
                       './simple',
                       (tests / 'simple_input.txt').path,
                       'simple_output.txt'])
@@ -541,7 +541,8 @@ def check_simple(args, stream, infile=1):
     # Build
     build('exec_echo', ['exec_echo.c'])
     # Trace
-    check_call(rpz + ['trace', './exec_echo', 'originalexecechooutput'])
+    check_call(rpz + ['trace', '--overwrite',
+                      './exec_echo', 'originalexecechooutput'])
     # Pack
     check_call(rpz + ['pack', 'exec_echo.rpz'])
     # Unpack chroot
@@ -616,7 +617,8 @@ def check_simple(args, stream, infile=1):
     # Build
     build('rename', ['rename.c'])
     # Trace
-    check_call(rpz + ['trace', '-d', 'rename-trace', './rename'])
+    check_call(rpz + ['trace', '--overwrite', '-d', 'rename-trace',
+                      './rename'])
     with Path('rename-trace/config.yml').open(encoding='utf-8') as fp:
         config = yaml.safe_load(fp)
     # Check that written files were logged
@@ -659,8 +661,8 @@ def check_simple(args, stream, infile=1):
     Path('e').chmod(0o744)
 
     # Trace
-    out = check_output(rpz + ['trace', '--dont-identify-packages',
-                              '-d', 'shebang-trace', './a', '1', '2'])
+    out = check_output(rpz + ['trace', '--overwrite', '-d', 'shebang-trace',
+                              '--dont-identify-packages', './a', '1', '2'])
     out = out.splitlines()[0]
     assert out == ('e %s 0 ./a 1 2' % (Path.cwd() / 'c')).encode('ascii')