In update ad hoc recipe buildscripts allow non-pure function calls only in depdeb preamble

5 files changed, 115 insertions, 2 deletions

diff --git a/libbuild2/build/script/parser+depdb.test.testscript b/libbuild2/build/script/parser+depdb.test.testscript
index 38c4236..573ba6d 100644
--- a/libbuild2/build/script/parser+depdb.test.testscript
+++ b/libbuild2/build/script/parser+depdb.test.testscript
@@ -88,3 +88,52 @@ test.options += -d
       EOI
   }
 }
+
+: impure-function
+:
+{
+  : var-assignment
+  :
+  $* <<EOI 2>>~%EOE% != 0
+    v = $getenv('VAR')
+    EOI
+    buildfile:11:6: error: call to impure function getenv is only allowed in depdb preamble
+    %  info: .+%
+    EOE
+
+  : quoted
+  :
+  $* <<EOI 2>>~%EOE% != 0
+    v = "$getenv('VAR')"
+    EOI
+    buildfile:11:7: error: call to impure function getenv is only allowed in depdb preamble
+    %  info: .*%
+    EOE
+
+  : if-cond
+  :
+  $* <<EOI 2>>~%EOE% != 0
+    if ($getenv('VAR') == 'yes')
+      foo
+    end
+    EOI
+    buildfile:11:6: error: call to impure function getenv is only allowed in depdb preamble
+    %  info: .+%
+    EOE
+
+  : command
+  :
+  $* <<EOI 2>>~%EOE% != 0
+    $getenv('VAR')
+    EOI
+    buildfile:11:2: error: call to impure function getenv is only allowed in depdb preamble
+    %  info: .+%
+    EOE
+
+  : non-functions
+  :
+  {
+    $* <'$getenv'             : var
+    $* <'$getenv (v ? a : b)' : eval
+  }
+}
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 04a257f..9b9f324 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -5,6 +5,7 @@
 
 #include <libbutl/builtin.mxx>
 
+#include <libbuild2/function.hxx>
 #include <libbuild2/algorithm.hxx>
 
 #include <libbuild2/build/script/lexer.hxx>
@@ -26,7 +27,7 @@ namespace build2
       //
 
       script parser::
-      pre_parse (const target& tg, const adhoc_actions& acts,
+      pre_parse (const target& tg, const adhoc_actions& as,
                  istream& is, const path_name& pn, uint64_t line,
                  optional<string> diag, const location& diag_loc)
       {
@@ -40,12 +41,15 @@ namespace build2
         // The script shouldn't be able to modify the target/scopes.
         //
         target_  = const_cast<target*> (&tg);
-        actions_ = &acts;
+        actions_ = &as;
         scope_   = const_cast<scope*> (&tg.base_scope ());
         root_    = scope_->root_scope ();
 
         pbase_  = scope_->src_path_;
 
+        perform_update_ = find (as.begin (), as.end (), perform_update_id) !=
+                          as.end ();
+
         script s;
         script_ = &s;
         runner_ = nullptr;
@@ -65,6 +69,15 @@ namespace build2
 
         s.end_loc = get_location (t);
 
+        // Diagnose impure function calls.
+        //
+        if (impure_func_)
+          fail (impure_func_->second)
+            << "call to impure function " << impure_func_->first << " is "
+            << "only allowed in depdb preamble" <<
+            info << "consider using 'depdb' builtin to track its result "
+                 << "changes";
+
         // Diagnose absent/ambigous script name.
         //
         {
@@ -538,6 +551,11 @@ namespace build2
                 script_->body_temp_dir = false;
               }
 
+              // Reset the impure function call info since it's valid for the
+              // depdb preamble.
+              //
+              impure_func_ = nullopt;
+
               // Instruct the parser to save the depdb builtin line separately
               // from the script lines, when it is fully parsed. Note that the
               // builtin command arguments will be validated during execution,
@@ -1164,6 +1182,18 @@ namespace build2
 
         return r;
       }
+
+      void parser::
+      lookup_function (string&& name, const location& loc)
+      {
+        if (perform_update_ && !impure_func_)
+        {
+          const function_overloads* f (ctx.functions.find (name));
+
+          if (f != nullptr && !f->pure)
+            impure_func_ = make_pair (move (name), loc);
+        }
+      }
     }
   }
 }
diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx
index e96a806..b5a45ca 100644
--- a/libbuild2/build/script/parser.hxx
+++ b/libbuild2/build/script/parser.hxx
@@ -121,6 +121,9 @@ namespace build2
         virtual lookup
         lookup_variable (name&&, string&&, const location&) override;
 
+        virtual void
+        lookup_function (string&&, const location&) override;
+
         // During execution translate the process path and executable targets
         // leaving the rest for the base parser to handle.
         //
@@ -142,6 +145,11 @@ namespace build2
         script* script_;
         const adhoc_actions* actions_; // Non-NULL during pre-parsing.
 
+        // True if performing update is one of the actions. Only set for the
+        // pre-parse mode.
+        //
+        bool perform_update_;
+
         // Current low-verbosity script diagnostics and its weight.
         //
         // During pre-parsing each command leading names are translated into a
@@ -197,6 +205,17 @@ namespace build2
         optional<location> depdb_clear_;    // 'depdb clear' location if any.
         lines              depdb_preamble_; // Note: excludes 'depdb clear'.
 
+        // If present, the first impure function called in the body of the
+        // script that performs update.
+        //
+        // Note that during the line pre-parsing we cannot tell if this is a
+        // body or depdb preamble line. Thus, if we encounter an impure
+        // function call we just save its name/location and postpone the
+        // potential failure till the end of the script pre-parsing, if it
+        // turns out to be a body line.
+        //
+        optional<pair<string, location>> impure_func_;
+
         // True during pre-parsing when the pre-parse mode is temporarily
         // suspended to perform expansion.
         //
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 92e0090..8118e0e 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -6013,6 +6013,8 @@ namespace build2
               result_data = ctx.functions.call (scope_, name, args, loc);
               what = "function call";
             }
+            else
+              lookup_function (move (name), loc);
           }
           else
           {
@@ -6850,6 +6852,12 @@ namespace build2
   }
 
   void parser::
+  lookup_function (string&&, const location&)
+  {
+    assert (pre_parse_);
+  }
+
+  void parser::
   switch_scope (const dir_path& d)
   {
     tracer trace ("parser::switch_scope", &path_);
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index 24a1ac1..1e924f0 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -512,6 +512,13 @@ namespace build2
     virtual lookup
     lookup_variable (name&& qual, string&& name, const location&);
 
+    // This function is only called during pre-parse and is the continuation
+    // of the similar logic in lookup_variable() above (including the fact
+    // that the name is empty for computed function names).
+    //
+    virtual void
+    lookup_function (string&& name, const location&);
+
     // Utilities.
     //
   protected: