[reprotest] 01/01: main: Improve error messages in some common scenarios

[Ximin Luo]

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

infinity0 pushed a commit to branch master
in repository reprotest.

commit 3e00d338bf171d59b62402a9de400e9bd0001fef
Author: Ximin Luo <infinity0 at debian.org>
Date:   Tue Sep 26 11:28:36 2017 +0200

    main: Improve error messages in some common scenarios
---
 README.rst            |  4 ----
 debian/changelog      |  3 +++
 reprotest/__init__.py | 25 ++++++++++++++++++++++++-
 reprotest/build.py    | 21 +++++++++++++++++++++
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/README.rst b/README.rst
index 3bf7ab0..bc6603e 100644
--- a/README.rst
+++ b/README.rst
@@ -309,10 +309,6 @@ mechanism to vary the system time.
 Known bugs
 ==========
 
-If the first argument is a file but it doesn't exist, then the heuristic will
-fail and reprotest might give a confusing error message about "No <artifact>
-provided".
-
 If the first argument is a file then reprotest will copy its whole directory.
 Sometimes this is unsuitable, e.g. when it contains 3GB of other stuff.
 Also if this copying fails (e.g. because irrelevant files are not copyable due
diff --git a/debian/changelog b/debian/changelog
index 7735988..252a158 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,9 @@ reprotest (0.7.1) UNRELEASED; urgency=medium
   * Add a --auto-build option to try to determine which specific variations
     cause unreproducibility.
   * Fix varying both umask and user_group at the same time.
+  * Improve error messages in some common scenarios:
+    - giving a source_root or build_command that doesn't exist
+    - using reprotest with default settings after not installing Recommends
 
  -- Ximin Luo <infinity0 at debian.org>  Fri, 22 Sep 2017 17:57:31 +0200
 
diff --git a/reprotest/__init__.py b/reprotest/__init__.py
index 88e721f..27d31e2 100644
--- a/reprotest/__init__.py
+++ b/reprotest/__init__.py
@@ -10,6 +10,7 @@ import os
 import pathlib
 import random
 import re
+import shlex
 import shutil
 import subprocess
 import sys
@@ -21,7 +22,7 @@ import pkg_resources
 
 from reprotest.lib import adtlog
 from reprotest.lib import adt_testbed
-from reprotest.build import Build, VariationSpec, Variations
+from reprotest.build import Build, VariationSpec, Variations, tool_missing
 from reprotest import presets, shell_syn
 
 
@@ -583,6 +584,13 @@ def run(argv, dry_run=None):
         elif os.path.exists(first_arg):
             source_root = first_arg
         else:
+            parts = shlex.split(first_arg)
+            if len(parts) == 1:
+                if shutil.which(parts[0]) is None:
+                    logging.warn("XXX")
+                    raise RuntimeError("Not found, neither as a file nor as a command: %s" % first_arg)
+            # if len(parts) > 1 then it could be something like '( command )'
+            # which is valid despite '(' not existing.
             build_command = first_arg
     build_command = build_command or parsed_args.build_command or "auto"
     source_root = source_root or parsed_args.source_root or '.'
@@ -622,6 +630,21 @@ def run(argv, dry_run=None):
         check_func = check
     build_variations = Variations.of(*specs, verbosity=verbosity)
 
+    # Warn about missing programs
+    if virtual_server_args[0] == "null":
+        missing = [(var, tool_missing(action))
+            for spec in specs
+            for var, vary, action in spec.actions()
+            if vary]
+        missing = [(var, tools) for var, tools in missing if tools]
+        for var, tools in missing:
+            if tools:
+                logging.warn("Varying '%s' requires these program(s): %s", var, ", ".join(tools))
+        if missing:
+            logging.warn("Your build will probably fail, either install them or disable the variations.")
+            logging.warn("(From a system package manager, simply install the 'optional' or 'recommended' "
+                         "dependencies of reprotest.)")
+
     # Remaining args
     host_distro = parsed_args.host_distro
     store_dir = parsed_args.store_dir
diff --git a/reprotest/build.py b/reprotest/build.py
index 03c1d03..937cf09 100644
--- a/reprotest/build.py
+++ b/reprotest/build.py
@@ -2,11 +2,13 @@
 # For details: reprotest/debian/copyright
 
 import collections
+import functools
 import getpass
 import grp
 import logging
 import os
 import shlex
+import shutil
 import random
 import time
 import types
@@ -15,6 +17,22 @@ from reprotest import _shell_ast
 from reprotest import mdiffconf
 
 
+def tool_required(*tools):
+    def wrap(f):
+        @functools.wraps(f)
+        def wf(*args, **kwargs):
+            return f(*args, **kwargs)
+        wf.tool_required = tools
+        return wf
+    return wrap
+
+
+def tool_missing(f):
+    if not hasattr(f, "tool_required"):
+        return []
+    return [t for t in f.tool_required if shutil.which(t) is None]
+
+
 def dirname(p):
     # works more intuitively for paths with a trailing /
     return os.path.normpath(os.path.dirname(os.path.normpath(p)))
@@ -174,6 +192,7 @@ def build_path(ctx, build, vary):
     const_path = os.path.join(dirname(build.tree), 'const_build_path')
     return build.move_tree(build.tree, const_path, True)
 
+ at tool_required("disorderfs")
 def fileordering(ctx, build, vary):
     if not vary:
         return build
@@ -260,6 +279,7 @@ def timezone(ctx, build, vary):
     else:
         return build.add_env('TZ', 'GMT-14')
 
+ at tool_required("faketime")
 def faketime(ctx, build, vary):
     if not vary:
         # FIXME: this does not actually fix the time, it just lets the system clock run normally
@@ -287,6 +307,7 @@ def umask(ctx, build, vary):
 
 # Note: this needs to go before anything that might need to run setup commands
 # as the other user (e.g. due to permissions).
+ at tool_required("sudo")
 def user_group(ctx, build, vary):
     if not vary:
         return build

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