Issue 2099: have Execute() print an error message if an action

fails.  Better document the behavior of returning the exit status,
and that exit-on-failure is the SConscript writer's responsibility.

git-svn-id: http://scons.tigris.org/svn/scons/trunk@3310 fdb21ef1-2011-0410-befe-b5e4ea1792b1

diff --git a/doc/man/scons.1 b/doc/man/scons.1
index 21e475e186e74edd430cd5463dcf716ffd03f9ac..baaa2349f7e9f06abfc103bcaf58d34c2bada636 100644 (file)
--- a/doc/man/scons.1
+++ b/doc/man/scons.1
@@ -3545,6 +3545,33 @@ The exit value of the command
 or return value of the Python function
 will be returned.
 
+Note that
+.B scons
+will print an error message if the executed
+.I action
+fails--that is,
+exits with or returns a non-zero value.
+.B scons
+will
+.I not ,
+however,
+automatically terminate the build
+if the specified
+.I action
+fails.
+If you want the build to stop in response to a failed
+.BR Execute ()
+call,
+you must explicitly check for a non-zero return value:
+
+.ES
+Execute(Copy('file.out', 'file.in'))
+
+if Execute("mkdir sub/dir/ectory"):
+    # The mkdir failed, don't try to build.
+    Exit(1)
+.EE
+
 '\"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
 .TP
 .RI Exit([ value ])

diff --git a/doc/user/factories.in b/doc/user/factories.in
index 94af6a39547d7c693855ca12e646314993c16106..34973f170ba6cc4804cd5f9c89c6122b1305ff78 100644 (file)
--- a/doc/user/factories.in
+++ b/doc/user/factories.in
@@ -440,11 +440,10 @@
     You can also execute an &Action; returned by a factory
     (or actually, any &Action;)
     at the time the &SConscript; file is read
-    by wrapping it up in the &Execute; function.
+    by using the &Execute; function.
     For example, if we need to make sure that
     a directory exists before we build any targets,
 
-
     </para>
 
     <scons_example name="Execute">
@@ -482,4 +481,27 @@
 
     </para>
 
+    <para>
+
+    The &Execute; function returns the exit status
+    or return value of the underlying action being executed.
+    It will also print an error message if the action
+    fails and returns a non-zero value.
+    &SCons; will <emphasis>not</emphasis>, however,
+    actually stop the build if the action fails.
+    If you want the build to stop
+    in response to a failure in an action called by &Execute;,
+    you must do so by explicitly
+    checking the return value
+    and calling the &Exit; function
+    (or a Python equivalent):
+
+    </para>
+
+    <sconstruct>
+    if Execute(Mkdir('__ROOT__/tmp/my_temp_directory')):
+        # A problem occurred while making the temp directory.
+        Exit(1)
+    </sconstruct>
+
   </section>

diff --git a/doc/user/factories.xml b/doc/user/factories.xml
index ae6e9d03f614ffadab23566d849538627e5e3e21..9599930404c9225aaa52bed2eeb1178c5ba9710e 100644 (file)
--- a/doc/user/factories.xml
+++ b/doc/user/factories.xml
@@ -395,11 +395,10 @@
     You can also execute an &Action; returned by a factory
     (or actually, any &Action;)
     at the time the &SConscript; file is read
-    by wrapping it up in the &Execute; function.
+    by using the &Execute; function.
     For example, if we need to make sure that
     a directory exists before we build any targets,
 
-
     </para>
 
     <programlisting>
@@ -441,4 +440,27 @@
 
     </para>
 
+    <para>
+
+    The &Execute; function returns the exit status
+    or return value of the underlying action being executed.
+    It will also print an error message if the action
+    fails and returns a non-zero value.
+    &SCons; will <emphasis>not</emphasis>, however,
+    actually stop the build if the action fails.
+    If you want the build to stop
+    in response to a failure in an action called by &Execute;,
+    you must do so by explicitly
+    checking the return value
+    and calling the &Exit; function
+    (or a Python equivalent):
+
+    </para>
+
+    <programlisting>
+    if Execute(Mkdir('/tmp/my_temp_directory')):
+        # A problem occurred while making the temp directory.
+        Exit(1)
+    </programlisting>
+
   </section>

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 292c7411adbf478af5bc49b64de221d48e30aee7..05c7f9679dc5a1c6e4844119afbd54b6cfb7eab9 100644 (file)
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -24,6 +24,9 @@ RELEASE 1.0.0 - XXX
 
     - Document the GetLaunchDir() function in the User's Guide.
 
+    - Have the env.Execute() method print an error message if the
+      executed command fails.
+
   From Greg Noel:
 
     - Handle yacc/bison on newer Mac OS X versions creating file.hpp,

diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index f972550e1ab437d7c5702d5b338141b627ebdcf6..db69d623895acc07f57a28899906ddc55421c0da 100644 (file)
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -1813,6 +1813,11 @@ class Base(SubstitutionEnvironment):
         action = apply(self.Action, (action,) + args, kw)
         result = action([], [], self)
         if isinstance(result, SCons.Errors.BuildError):
+            errstr = result.errstr
+            if result.filename:
+                errstr = result.filename + ': ' + errstr
+            import sys
+            sys.stderr.write("scons: *** %s\n" % errstr)
             return result.status
         else:
             return result

diff --git a/test/Execute.py b/test/Execute.py
index 44abc735a83ba7e5d8b25ed08865d22b2011e909..35ee949fe849a890d5703f4153c9c9059d5c527a 100644 (file)
--- a/test/Execute.py
+++ b/test/Execute.py
@@ -63,6 +63,9 @@ Execute(lambda target, source, env: shutil.copy('i.in', 'i.out'))
 Execute(Action(lambda target, source, env: shutil.copy('j.in', 'j.out')))
 env.Execute(lambda target, source, env: shutil.copy('k.in', 'k.out'))
 env.Execute(Action(lambda target, source, env: shutil.copy('l.in', 'l.out')))
+
+Execute(Copy('m.out', 'm.in'))
+Execute(Copy('nonexistent.out', 'nonexistent.in'))
 """ % locals())
 
 test.write('a.in', "a.in\n")
@@ -77,20 +80,28 @@ test.write('i.in', "i.in\n")
 test.write('j.in', "j.in\n")
 test.write('k.in', "k.in\n")
 test.write('l.in', "l.in\n")
+test.write('m.in', "m.in\n")
+
+expect = """\
+scons: *** Error 1
+scons: *** Error 2
+scons: *** nonexistent.in: No such file or directory
+"""
 
-test.run(arguments = '.')
+test.run(arguments = '.', stderr=expect)
 
-test.fail_test(test.read('a.out') != "a.in\n")
-test.fail_test(test.read('b.out') != "b.in\n")
-test.fail_test(test.read('c.out') != "c.in\n")
-test.fail_test(test.read('d.out') != "d.in\n")
-test.fail_test(test.read('e.out') != "e.in\n")
-test.fail_test(test.read('f.out') != "f.in\n")
-test.fail_test(test.read('g.out') != "g.in\n")
-test.fail_test(test.read('h.out') != "h.in\n")
-test.fail_test(test.read('i.out') != "i.in\n")
-test.fail_test(test.read('j.out') != "j.in\n")
-test.fail_test(test.read('k.out') != "k.in\n")
-test.fail_test(test.read('l.out') != "l.in\n")
+test.must_match('a.out', "a.in\n")
+test.must_match('b.out', "b.in\n")
+test.must_match('c.out', "c.in\n")
+test.must_match('d.out', "d.in\n")
+test.must_match('e.out', "e.in\n")
+test.must_match('f.out', "f.in\n")
+test.must_match('g.out', "g.in\n")
+test.must_match('h.out', "h.in\n")
+test.must_match('i.out', "i.in\n")
+test.must_match('j.out', "j.in\n")
+test.must_match('k.out', "k.in\n")
+test.must_match('l.out', "l.in\n")
+test.must_match('m.out', "m.in\n")
 
 test.pass_test()