From 7368566619bc990b69f90a4828be2966854fa785 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 2 Jun 2020 09:22:54 +0200 Subject: Hash checksum metadata of exe prerequisites in ad hoc script rule --- libbuild2/rule.cxx | 72 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 16 deletions(-) (limited to 'libbuild2/rule.cxx') diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx index 2e5fe5d..5f532ef 100644 --- a/libbuild2/rule.cxx +++ b/libbuild2/rule.cxx @@ -440,16 +440,6 @@ namespace build2 if (t.data ()) return &perform_clean_depdb; - // For update inject dependency on the tool target(s). - // - // @@ We could see that it's a target and do it but not sure if we should - // bother. We dropped this idea of implicit targets in tests. Maybe we - // should verify path assigned, like we do there? I think we will have - // to. - // - // if (a == perform_update_id) - // inject (a, t, tgt); - if (a == perform_update_id && t.is_a ()) { return [this] (action a, const target& t) @@ -476,6 +466,59 @@ namespace build2 const file& t (xt.as ()); const path& tp (t.path ()); + // The script can reference a program in one of four ways: + // + // 1. As an (imported) target (e.g., $cli) + // + // 2. As a process_path_ex (e.g., $cxx.path). + // + // 3. As a builtin (e.g., sed) + // + // 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. + // + // This leaves the (1) case, which itself splits into two sub-cases: the + // target comes with the dependency information (e.g., imported from a + // project via an export stub) or it does not (e.g., imported as + // installed). We don't need to do anything extra for the first sub-case + // since the target's state/mtime can be relied upon like any other + // prerequisite. Which cannot be said about the second sub-case, where we + // reply on checksum that may be included as part of the target metadata. + // + // So what we are going to do here is hash checksum metadata of every + // executable prerequisite target that has it. We do it before executing + // in order to include ad hoc prerequisites (which feels like the right + // thing to do; the user may mark tools as ad hoc in order to omit them + // from $<). + // + sha256 prog_cs; + for (const target* pt: t.prerequisite_targets[a]) + { + if (pt != nullptr) + { + if (auto* e = pt->is_a ()) + { + if (auto* ns = cast_null (e->vars[ctx.var_export_metadata])) + { + // Metadata variable prefix is in the second name. + // + assert (ns->size () == 2 && (*ns)[1].simple ()); + + if (auto* c = cast_null ( + e->vars[(*ns)[1].value + ".checksum"])) + { + prog_cs.append (*c); + } + } + } + } + } + // Update prerequisites and determine if any of them render this target // out-of-date. // @@ -593,13 +636,10 @@ namespace build2 l4 ([&]{trace << "prerequisite set change forcing update of " << t;}); } - // Then the tools checksums. - // - // @@ TODO: obtain checksums of all the targets used as commands in - // the script. + // Finally the programs checksum. // - //if (dd.expect (csum) != nullptr) - // l4 ([&]{trace << "compiler mismatch forcing update of " << t;}); + if (dd.expect (prog_cs.string ()) != nullptr) + l4 ([&]{trace << "program checksum change forcing update of " << t;}); } // Update if depdb mismatch. -- cgit v1.1