Detect environment changes in ad hoc recipes

9 files changed, 101 insertions, 37 deletions

diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index dad638e..168856c 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -229,7 +229,7 @@ namespace build2
     timestamp mt (t.load_mtime ());
     optional<target_state> ps;
 
-    sha256 pcs, ecs;
+    sha256 prq_cs, exe_cs, env_cs;
     {
       // This is essentially ps=execute_prerequisites(a, t, mt) which we
       // cannot use because we need to see ad hoc prerequisites.
@@ -302,7 +302,7 @@ namespace build2
         if (script.depdb_clear)
           continue;
 
-        hash_target (pcs, pt);
+        hash_target (prq_cs, pt);
 
         // The script can reference a program in one of four ways:
         //
@@ -315,11 +315,12 @@ namespace build2
         // 4. As a program path/name.
         //
         // When it comes to change tracking, there is nothing we can do for
-        // (4) and there is nothing to do for (3) (assuming builtin semantics
-        // is stable/backwards-compatible). The (2) case is handled
-        // automatically by hashing all the variable values referenced by the
-        // script (see below), which in case of process_path_ex includes the
-        // checksum, if available.
+        // (4) (the user can track its environment manually with depdb-env)
+        // and there is nothing to do for (3) (assuming builtin semantics is
+        // stable/backwards-compatible). The (2) case is handled automatically
+        // by hashing all the variable values referenced by the script (see
+        // below), which in case of process_path_ex includes the checksums
+        // (both executable and environment), if available.
         //
         // This leaves the (1) case, which itself splits into two sub-cases:
         // the target comes with the dependency information (e.g., imported
@@ -336,11 +337,16 @@ namespace build2
         // to do; the user may mark tools as ad hoc in order to omit them from
         // $<).
         //
-        if (auto* e = pt.is_a<exe> ())
+        if (auto* et = pt.is_a<exe> ())
         {
-          if (auto* c = e->lookup_metadata<string> ("checksum"))
+          if (auto* c = et->lookup_metadata<string> ("checksum"))
           {
-            ecs.append (*c);
+            exe_cs.append (*c);
+          }
+
+          if (auto* e = et->lookup_metadata<strings> ("environment"))
+          {
+            hash_environment (env_cs, *e);
           }
         }
       }
@@ -395,6 +401,9 @@ namespace build2
     // Note that this excludes the special $< and $> variables which we
     // handle below.
     //
+    // @@ TODO: maybe detect and decompose process_path_ex in order to
+    //    properly attribute checksum and environment changes?
+    //
     if (!script.depdb_clear)
     {
       sha256 cs;
@@ -436,16 +445,19 @@ namespace build2
       if (dd.expect (tcs.string ()) != nullptr)
         l4 ([&]{trace << "target set change forcing update of " << t;});
 
-      if (dd.expect (pcs.string ()) != nullptr)
+      if (dd.expect (prq_cs.string ()) != nullptr)
         l4 ([&]{trace << "prerequisite set change forcing update of " << t;});
     }
 
-    // Finally the programs checksum.
+    // Finally the programs and environment checksums.
     //
     if (!script.depdb_clear)
     {
-      if (dd.expect (ecs.string ()) != nullptr)
+      if (dd.expect (exe_cs.string ()) != nullptr)
         l4 ([&]{trace << "program checksum change forcing update of " << t;});
+
+      if (dd.expect (env_cs.string ()) != nullptr)
+        l4 ([&]{trace << "environment change forcing update of " << t;});
     }
 
     const scope* bs (nullptr);
diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx
index 62c7bcf..7e022fd8 100644
--- a/libbuild2/bin/init.cxx
+++ b/libbuild2/bin/init.cxx
@@ -724,8 +724,11 @@ namespace build2
           }
         }
 
-        rs.assign<process_path_ex> ("bin.ar.path")   =
-          process_path_ex (ari.ar_path, "ar", ari.ar_checksum);
+        rs.assign<process_path_ex> ("bin.ar.path")   = process_path_ex (
+          ari.ar_path,
+          "ar",
+          ari.ar_checksum,
+          hash_environment (ari.ar_environment));
         rs.assign<string>       ("bin.ar.id")        = ari.ar_id;
         rs.assign<string>       ("bin.ar.signature") = ari.ar_signature;
         rs.assign<string>       ("bin.ar.checksum")  = ari.ar_checksum;
@@ -744,8 +747,11 @@ namespace build2
 
         if (ranlib != nullptr)
         {
-          rs.assign<process_path_ex> ("bin.ranlib.path")   =
-            process_path_ex (ari.ranlib_path, "ranlib", ari.ranlib_checksum);
+          rs.assign<process_path_ex> ("bin.ranlib.path")   = process_path_ex (
+            ari.ranlib_path,
+            "ranlib",
+            ari.ranlib_checksum,
+            hash_environment (ari.ranlib_environment));
           rs.assign<string>       ("bin.ranlib.id")        = ari.ranlib_id;
           rs.assign<string>       ("bin.ranlib.signature") = ari.ranlib_signature;
           rs.assign<string>       ("bin.ranlib.checksum")  = ari.ranlib_checksum;
@@ -859,8 +865,11 @@ namespace build2
              << "  checksum   " << ldi.checksum;
         }
 
-        rs.assign<process_path_ex> ("bin.ld.path")   =
-          process_path_ex (ldi.path, "ld", ldi.checksum);
+        rs.assign<process_path_ex> ("bin.ld.path")   = process_path_ex (
+          ldi.path,
+          "ld",
+          ldi.checksum,
+          hash_environment (ldi.environment));
         rs.assign<string>       ("bin.ld.id")        = ldi.id;
         rs.assign<string>       ("bin.ld.signature") = ldi.signature;
         rs.assign<string>       ("bin.ld.checksum")  = ldi.checksum;
@@ -996,8 +1005,11 @@ namespace build2
                << "  checksum   " << rci.checksum;
         }
 
-        rs.assign<process_path_ex> ("bin.rc.path")   =
-          process_path_ex (rci.path, "rc", rci.checksum);
+        rs.assign<process_path_ex> ("bin.rc.path")   = process_path_ex (
+          rci.path,
+          "rc",
+          rci.checksum,
+          hash_environment (rci.environment));
         rs.assign<string>       ("bin.rc.id")        = rci.id;
         rs.assign<string>       ("bin.rc.signature") = rci.signature;
         rs.assign<string>       ("bin.rc.checksum")  = rci.checksum;
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 9b9f324..4c11688 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -678,7 +678,7 @@ namespace build2
         // if it's process_path_ex, then we may end up with something along
         // these lines:
         //
-        // /usr/bin/g++ name@c++ checksum@deadbeef --version
+        // /usr/bin/g++ name@c++ checksum@... env-checksum@... --version
         //
         // Which is a bit harder to recognize syntactically. So what we are
         // going to do is have a separate first pass which reduces the
diff --git a/libbuild2/cc/module.cxx b/libbuild2/cc/module.cxx
index 7ef8cd5..98e0451 100644
--- a/libbuild2/cc/module.cxx
+++ b/libbuild2/cc/module.cxx
@@ -216,9 +216,8 @@ namespace build2
 
       // Assign values to variables that describe the compiler.
       //
-      // @@ TODO: env_checksum.
-      //
-      rs.assign (x_path) = process_path_ex (xi.path, x_name, xi.checksum);
+      rs.assign (x_path) = process_path_ex (
+        xi.path, x_name, xi.checksum, env_checksum);
       const strings& xm (cast<strings> (rs.assign (x_mode) = move (mode)));
 
       rs.assign (x_id) = xi.id.string ();
diff --git a/libbuild2/functions-process-path.cxx b/libbuild2/functions-process-path.cxx
index 199ad0d..486a806 100644
--- a/libbuild2/functions-process-path.cxx
+++ b/libbuild2/functions-process-path.cxx
@@ -28,6 +28,7 @@ namespace build2
 
       f["name"] += &process_path_ex::name;
       f["checksum"] += &process_path_ex::checksum;
+      f["env_checksum"] += &process_path_ex::env_checksum;
     }
   }
 }
diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx
index 4be5149..c18d949 100644
--- a/libbuild2/functions-process.cxx
+++ b/libbuild2/functions-process.cxx
@@ -217,7 +217,8 @@ namespace build2
       size_t erase (0);
 
       // This can be a process_path (pair), process_path_ex (process_path
-      // followed by the name@ and checksum@ pairs), or just a path.
+      // optionally followed by the name@, checksum@, and env-checksum@
+      // pairs), or just a path.
       //
       // First, check if the arguments begin with a process_path[_ex] and, if
       // that's the case, only use the leading name/pair to create the process
diff --git a/libbuild2/types.hxx b/libbuild2/types.hxx
index 6877f92..dd82ef1 100644
--- a/libbuild2/types.hxx
+++ b/libbuild2/types.hxx
@@ -293,22 +293,40 @@ namespace build2
   //
   // See also {import,export}.metadata.
   //
+  // Note that the environment checksum is calculated in the (potentially
+  // hermetic) project environment which makes instances of process_path_ex
+  // project-specific. (We could potentially store the original list of
+  // environment variables and calculate the checksum on the fly in the
+  // current context thus making it project-independent. But this will
+  // complicate things without, currently, much real benefit since all our
+  // use-cases fit well with the project-specific restriction. We could
+  // probably even support both variants if desirable.)
+  //
   struct process_path_ex: process_path
   {
-    optional<string> name;     // Stable name for diagnostics.
-    optional<string> checksum; // Checksum for change tracking.
+    optional<string> name;         // Stable name for diagnostics.
+    optional<string> checksum;     // Executable checksum for change tracking.
+    optional<string> env_checksum; // Environment checksum for change tracking.
 
     using process_path::process_path;
 
-    process_path_ex (const process_path& p, string n, optional<string> c = {})
+    process_path_ex (const process_path& p,
+                     string n,
+                     optional<string> c = {},
+                     optional<string> ec = {})
         : process_path (p, false /* init */),
           name (std::move (n)),
-          checksum (std::move (c)) {}
+          checksum (std::move (c)),
+          env_checksum (std::move (ec)) {}
 
-    process_path_ex (process_path&& p, string n, optional<string> c = {})
+    process_path_ex (process_path&& p,
+                     string n,
+                     optional<string> c = {},
+                     optional<string> ec = {})
         : process_path (std::move (p)),
           name (std::move (n)),
-          checksum (std::move (c)) {}
+          checksum (std::move (c)),
+          env_checksum (std::move (ec)) {}
 
     process_path_ex () = default;
   };
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index efda63d..f9e60c1 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -1158,10 +1158,19 @@ namespace build2
       else if (k == "checksum")
       {
         if (!i->simple ())
-          throw_invalid_argument (*i, nullptr, "process_path_ex checksum");
+          throw_invalid_argument (
+            *i, nullptr, "process_path_ex executable checksum");
 
         pp.checksum = move (i->value);
       }
+      else if (k == "env-checksum")
+      {
+        if (!i->simple ())
+          throw_invalid_argument (
+            *i, nullptr, "process_path_ex environment checksum");
+
+        pp.env_checksum = move (i->value);
+      }
       else
         throw invalid_argument (
           "unknown key '" + k + "' in process_path_ex value");
@@ -1176,7 +1185,9 @@ namespace build2
     auto b (ns.begin ()), i (b), e (ns.end ());
     for (i += i->pair ? 2 : 1; i != e && i->pair; i += 2)
     {
-      if (!i->simple () || (i->value != "name" && i->value != "checksum"))
+      if (!i->simple () || (i->value != "name" &&
+                            i->value != "checksum" &&
+                            i->value != "env-checksum"))
         break;
     }
 
@@ -1217,6 +1228,7 @@ namespace build2
 
       lhs.name = move (rhs.name);
       lhs.checksum = move (rhs.checksum);
+      lhs.env_checksum = move (rhs.env_checksum);
     }
     else
     {
@@ -1224,6 +1236,7 @@ namespace build2
 
       lhs.name = rhs.name;
       lhs.checksum = rhs.checksum;
+      lhs.env_checksum = rhs.env_checksum;
     }
   }
 
@@ -1252,7 +1265,8 @@ namespace build2
     {
       s.reserve ((x.effect.empty () ? 1 : 2) +
                  (x.name ? 2 : 0)            +
-                 (x.checksum ? 2 : 0));
+                 (x.checksum ? 2 : 0)        +
+                 (x.env_checksum ? 2 : 0));
 
       process_path_reverse_impl (x, s);
 
@@ -1269,6 +1283,13 @@ namespace build2
         s.back ().pair = '@';
         s.push_back (name (*x.checksum));
       }
+
+      if (x.env_checksum)
+      {
+        s.push_back (name ("env-checksum"));
+        s.back ().pair = '@';
+        s.push_back (name (*x.env_checksum));
+      }
     }
 
     return s;
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index d59a891..805b93d 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -957,8 +957,8 @@ namespace build2
                    "insufficient space");
 
     // Represented as a potential @-pair of name(s) corresponding to
-    // process_path followed by the name@ and checksum@ pairs. So it's a
-    // container-like.
+    // process_path optionally followed by the name@, checksum@, and
+    // env-checksum@ pairs. So it's a container-like.
     //
     static process_path_ex convert (names&&);
     static void assign (value&, process_path_ex&&);