From de2daaa41ec6064181e6b9e73a34c32cd0008242 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 30 May 2023 05:51:23 +0200 Subject: Store dynamic group member types in depdb, use in clean --- libbuild2/adhoc-rule-buildscript.cxx | 80 ++++++++++++++++----------------- libbuild2/build/script/parser.cxx | 85 +++++++++++++++++++++++++++++++----- libbuild2/build/script/parser.hxx | 20 ++++++--- libbuild2/dyndep.cxx | 71 ++++++++++++++++++++---------- libbuild2/dyndep.hxx | 49 ++++++++++++--------- 5 files changed, 203 insertions(+), 102 deletions(-) diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index 06c8867..4277573 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -220,6 +220,9 @@ namespace build2 perform_update_id) != actions.end ()); } + using dynamic_target = build::script::parser::dynamic_target; + using dynamic_targets = build::script::parser::dynamic_targets; + struct adhoc_buildscript_rule::match_data { match_data (action a, const target& t, const scope& bs, bool temp_dir) @@ -229,7 +232,7 @@ namespace build2 build::script::default_runner run; path dd; - paths dyn_targets; + dynamic_targets dyn_targets; const scope* bs; timestamp mt; @@ -551,11 +554,11 @@ namespace build2 // Read the list of dynamic targets from depdb, if exists (used in a few // depdb-dyndep --dyn-target handling places below). // - auto read_dyn_targets = [] (path ddp) -> paths + auto read_dyn_targets = [] (path ddp) -> dynamic_targets { depdb dd (move (ddp), true /* read_only */); - paths r; + dynamic_targets r; while (dd.reading ()) // Breakout loop. { string* l; @@ -597,7 +600,15 @@ namespace build2 if (!read () || l->empty ()) break; - r.push_back (path (*l)); + // Split into type and path. + // + size_t p (l->find (' ')); + if (p == string::npos || // Invalid format. + p == 0 || // Empty type. + p + 1 == l->size ()) // Empty path. + break; + + r.emplace_back (string (*l, 0, p), path (*l, p + 1, string::npos)); } break; @@ -640,12 +651,6 @@ namespace build2 // using dyndep = dyndep_rule; - function map_ext ( - [] (const scope& bs, const string& n, const string& e) - { - return dyndep::map_extension (bs, n, e, nullptr); - }); - function filter; if (g != nullptr) { @@ -656,33 +661,20 @@ namespace build2 }; } - // @@ We don't have --target-what, --target-default-type here. Could - // we do the same thing as byproduct to get them? That would - // require us running the first half of the depdb preamble but - // ignoring all the depdb builtins (we still want all the variable - // assignments -- maybe we could automatically skip them if we see - // depdb is not open). Wonder if there would be any other - // complications... - // - // BTW, this sort of works for --target-default-type since we just - // clean them as file{} targets (but diagnostics is off). It does - // break, however, if there is s batch, since then we end up - // detecting different targets sharing a path. This will also not - // work at all if/when we support specifying custom extension to - // type mapping in order to resolve ambiguities. - // - const char* what ("file"); - const target_type& def_tt (file::static_type); - - for (path& f: read_dyn_targets (target_path () + ".d")) + for (dynamic_target& dt: read_dyn_targets (target_path () + ".d")) { + path& f (dt.path); + + // Resolve target type. Clean it as file if unable to. + // + const target_type* tt (bs.find_target_type (dt.type)); + if (tt == nullptr) + tt = &file::static_type; + if (g != nullptr) { pair r ( - dyndep::inject_group_member (what, - a, bs, *g, - move (f), - map_ext, def_tt, filter)); + dyndep::inject_group_member (a, bs, *g, move (f), *tt, filter)); if (r.second) g->members.push_back (&r.first); @@ -692,10 +684,7 @@ namespace build2 // Note that here we don't bother cleaning any old dynamic targets // -- the more we can clean, the merrier. // - dyndep::inject_adhoc_group_member (what, - a, bs, t, - move (f), - map_ext, def_tt); + dyndep::inject_adhoc_group_member (a, bs, t, move (f), *tt); } } } @@ -824,7 +813,7 @@ namespace build2 unique_ptr md; unique_ptr mdb; - paths old_dyn_targets; + dynamic_targets old_dyn_targets; if (script.depdb_dyndep_byproduct) { @@ -976,7 +965,9 @@ namespace build2 const path* p ( g->members_static != 0 ? &tp /* first static member path */ - : (!old_dyn_targets.empty () ? &old_dyn_targets.front () : nullptr)); + : (!old_dyn_targets.empty () + ? &old_dyn_targets.front ().path + : nullptr)); if (p != nullptr) mt = g->load_mtime (*p); @@ -1181,10 +1172,15 @@ namespace build2 // unconditionally, update or not, since if everything is up to date, // then old and new sets should be the same. // - for (const path& f: old_dyn_targets) + for (const dynamic_target& dt: old_dyn_targets) { - if (find (md->dyn_targets.begin (), md->dyn_targets.end (), f) == - md->dyn_targets.end ()) + const path& f (dt.path); + + if (find_if (md->dyn_targets.begin (), md->dyn_targets.end (), + [&f] (const dynamic_target& dt) + { + return dt.path == f; + }) == md->dyn_targets.end ()) { // This is an optimization so best effort. // diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index c6eb9cd..3814305 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -1291,7 +1291,7 @@ namespace build2 environment& e, const script& s, runner& r, lines_iterator begin, lines_iterator end, depdb& dd, - paths* dyn_targets, + dynamic_targets* dyn_targets, bool* update, optional mt, bool* deferred_failure, @@ -1320,7 +1320,7 @@ namespace build2 const script& scr; depdb& dd; - paths* dyn_targets; + dynamic_targets* dyn_targets; bool* update; bool* deferred_failure; optional mt; @@ -1647,7 +1647,7 @@ namespace build2 size_t li, const location& ll, action a, const scope& bs, target& t, depdb& dd, - paths& dyn_targets, + dynamic_targets& dyn_targets, bool& update, timestamp mt, bool& deferred_failure, @@ -2647,10 +2647,25 @@ namespace build2 break; } - if (l->empty ()) // Done with target. + if (l->empty ()) // Done with targets. break; - dyn_targets.push_back (path (move (*l))); + // Split into type and path (see below for background). + // + size_t p (l->find (' ')); + if (p == string::npos || // Invalid format. + p == 0 || // Empty type. + p + 1 == l->size ()) // Empty path. + { + dd.write (); // Invalidate this line. + restart = true; + break; + } + + string t (*l, 0, p); + l->erase (0, p + 1); + + dyn_targets.emplace_back (move (t), path (move (*l))); } } @@ -2861,7 +2876,9 @@ namespace build2 << "directory " << rs.out_path (); } - dyn_targets.push_back (move (f)); + // Note: type is resolved later. + // + dyn_targets.emplace_back (string (), move (f)); } continue; @@ -2969,7 +2986,9 @@ namespace build2 << rs.out_path (); } - dyn_targets.push_back (move (f)); + // Note: type is resolved later. + // + dyn_targets.emplace_back (string (), move (f)); } else { @@ -3099,11 +3118,23 @@ namespace build2 }; } - for (const path& f: dyn_targets) + // Unlike for prerequisites, for targets we store in depdb both the + // resolved target type and path. The target type is used in clean + // (see adhoc_rule_buildscript::apply()) where we cannot easily get + // hold of all the dyndep options to map the path to target type. + // So the format of the target line is: + // + // + // + string l; // Reuse the buffer. + for (dynamic_target& dt: dyn_targets) { + const path& f (dt.path); + // Note that this logic should be consistent with what we have in // adhoc_buildscript_rule::apply() for perform_clean. // + const build2::file* ft (nullptr); if (g != nullptr) { pair r ( @@ -3117,12 +3148,17 @@ namespace build2 // each update. // if (!r.second) + { + dt.type.clear (); // Static indicator. continue; + } + + ft = &r.first; // Note: we only currently support dynamic file members so it // will be file if first. // - g->members.push_back (&r.first); + g->members.push_back (ft); } else { @@ -3137,14 +3173,41 @@ namespace build2 // already a member (think `b update && b clean update`). // if (!r.second && r.first.decl == target_decl::real) + { + dt.type.clear (); // Static indicator. continue; + } + + ft = &r.first; if (dts) - dts->push_back (&r.first); + dts->push_back (ft); + } + + const char* tn (ft->type ().name); + + if (dt.type.empty ()) + dt.type = tn; + else if (dt.type != tn) + { + // This can, for example, happen if the user changed the + // extension to target type mapping. Say swapped extension + // variable values of two target types. + // + fail << "mapping of " << what_tgt << " target path " << f + << " to target type has changed" << + info << "previously mapped to " << dt.type << "{}" << + info << "now mapped to " << tn << "{}" << + info << "perform from scratch rebuild of " << t; } if (!cache) - dd.expect (f); + { + l = dt.type; + l += ' '; + l += f.string (); + dd.expect (l); + } } // Add the dynamic targets terminating blank line. diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx index f975194..ce550fc 100644 --- a/libbuild2/build/script/parser.hxx +++ b/libbuild2/build/script/parser.hxx @@ -121,12 +121,20 @@ namespace build2 dd); } + struct dynamic_target + { + string type; // Target type name (absent if static member). + build2::path path; + }; + + using dynamic_targets = vector; + void execute_depdb_preamble_dyndep ( action a, const scope& base, target& t, environment& e, const script& s, runner& r, depdb& dd, - paths& dyn_targets, + dynamic_targets& dyn_targets, bool& update, timestamp mt, bool& deferred_failure) { exec_depdb_preamble ( @@ -162,11 +170,13 @@ namespace build2 environment& e, const script& s, runner& r, depdb& dd, bool& update, timestamp mt) { + // Dummies. + // // This is getting a bit ugly (we also don't really need to pass // depdb here). One day we will find a better way... // - paths dyn_targets; - bool deferred_failure; // Dymmy. + dynamic_targets dyn_targets; + bool deferred_failure; dyndep_byproduct v; exec_depdb_preamble ( @@ -228,7 +238,7 @@ namespace build2 environment&, const script&, runner&, lines_iterator begin, lines_iterator end, depdb&, - paths* dyn_targets = nullptr, + dynamic_targets* dyn_targets = nullptr, bool* update = nullptr, optional mt = nullopt, bool* deferred_failure = nullptr, @@ -241,7 +251,7 @@ namespace build2 size_t line_index, const location&, action, const scope& base, target&, depdb&, - paths& dyn_targets, + dynamic_targets& dyn_targets, bool& update, timestamp, bool& deferred_failure, diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index 9c0f8a8..6d1a32c 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -910,9 +910,11 @@ namespace build2 return pair (t, true); } - const file& dyndep_rule:: + pair dyndep_rule:: inject_group_member (action a, const scope& bs, mtime_target& g, - path f, const target_type& tt) + path f, + const target_type& tt, + const function& filter) { path n (f.leaf ()); string e (n.extension ()); @@ -921,7 +923,7 @@ namespace build2 return inject_group_member_impl (a, bs, g, move (f), move (n).string (), move (e), tt, - nullptr /* filter */).first; + filter); } static const target_type& @@ -943,7 +945,7 @@ namespace build2 { diag_record dr (fail); - dr << "mapping of " << what << " target file " << f + dr << "mapping of " << what << " target path " << f << " to target type is ambiguous"; for (const target_type* tt: tts) @@ -954,7 +956,7 @@ namespace build2 if (!tt.is_a ()) { - fail << what << " target file " << f << " mapped to non-file-based " + fail << what << " target path " << f << " mapped to non-file-based " << "target type " << tt.name << "{}"; } @@ -984,22 +986,11 @@ namespace build2 filter); } - pair dyndep_rule:: - inject_adhoc_group_member (const char* what, - action, const scope& bs, target& t, - path f, - const function& map_ext, - const target_type& fallback) + pair + inject_adhoc_group_member_impl (action, const scope& bs, target& t, + path f, string n, string e, + const target_type& tt) { - path n (f.leaf ()); - string e (n.extension ()); - n.make_base (); - - // Map extension to the target type, falling back to the fallback type. - // - const target_type& tt ( - map_target_type (what, bs, f, n.string (), e, map_ext, fallback)); - // Assume nobody else can insert these members (seems reasonable seeing // that their names are dynamically discovered). // @@ -1008,7 +999,7 @@ namespace build2 tt, f.directory (), dir_path (), // Always in out. - move (n).string (), + move (n), &e, &bs)); @@ -1040,8 +1031,8 @@ namespace build2 return pair (*ft, false); if (!l.second) - fail << "dynamic " << what << " target " << *ft << " already exists " - << "and cannot be made ad hoc member of group " << t; + fail << "dynamic target " << *ft << " already exists and cannot be " + << "made ad hoc member of group " << t; ft->group = &t; l.second.unlock (); @@ -1056,4 +1047,38 @@ namespace build2 return pair (*ft, true); } + + pair dyndep_rule:: + inject_adhoc_group_member (action a, const scope& bs, target& t, + path f, + const target_type& tt) + { + path n (f.leaf ()); + string e (n.extension ()); + n.make_base (); + + return inject_adhoc_group_member_impl ( + a, bs, t, move (f), move (n).string (), move (e), tt); + } + + pair dyndep_rule:: + inject_adhoc_group_member (const char* what, + action a, const scope& bs, target& t, + path f, + const function& map_ext, + const target_type& fallback) + { + path n (f.leaf ()); + string e (n.extension ()); + n.make_base (); + + // Map extension to the target type, falling back to the fallback type. + // + const target_type& tt ( + map_target_type (what, bs, f, n.string (), e, map_ext, fallback)); + + + return inject_adhoc_group_member_impl ( + a, bs, t, move (f), move (n).string (), move (e), tt); + } } diff --git a/libbuild2/dyndep.hxx b/libbuild2/dyndep.hxx index ee9c1dd..1de2858 100644 --- a/libbuild2/dyndep.hxx +++ b/libbuild2/dyndep.hxx @@ -228,40 +228,41 @@ namespace build2 const srcout_map& = {}); // Find or insert a target file path as a target of the specified type, - // make it a member of the specified (non-ad hoc) mtime target group, - // set its path, and match it with group_recipe. + // make it a member of the specified (non-ad hoc) mtime target group and + // set its path. Return the target and an indication of whether it was + // made a member (can only be false if a filter is provided; see below). // // The file path must be absolute and normalized. Note that this function // assumes that this member can only be matched via this group. The group // type must have the target_type::flag::dyn_members flag. // - // Note: we can split this function into {enter,match}_group_member() - // if necessary. + // If specified, the group_filter function is called on the target before + // making it a group member, skipping it if this function returns false. + // Note that the filter is skipped if the target is newly inserted (the + // filter is meant to be used to skip duplicates). // - static const file& + using group_filter_func = bool (mtime_target& g, const file&); + + static pair inject_group_member (action, const scope& base, mtime_target&, - path, const target_type&); + path, + const target_type&, + const function& = nullptr); template - static const T& - inject_group_member (action a, const scope& bs, mtime_target& g, path f) + static pair + inject_group_member (action a, const scope& bs, mtime_target& g, + path f, + const function& filter = nullptr) { return inject_group_member ( - a, bs, g, move (f), T::static_type).template as (); + a, bs, g, move (f), T::static_type, filter).template as (); } // As above but the target type is determined using the map_extension // function if specified, falling back to the fallback type if unable to - // (the what argument is used for diagnostics during this process). Return - // the target and an indication of whether it was made a member. + // (the what argument is used for diagnostics during this process). // - // If specified, the group_filter function is called on the target before - // making it a group member, skipping it if this function returns false. - // Note that the filter is skipped if the target is newly inserted (the - // filter is meant to be used to skip duplicates). - // - using group_filter_func = bool (mtime_target& g, const file&); - static pair inject_group_member (const char* what, action, const scope& base, mtime_target& g, @@ -275,13 +276,19 @@ namespace build2 // specified ad hoc group unless it already is, and set its path. Return // the target and an indication of whether it was added as a member. // - // The target type is determined using the map_extension function if - // specified, falling back to the fallback type if unable to. - // // The file path must be absolute and normalized. Note that this function // assumes that this target can only be known as a member of this group. // static pair + inject_adhoc_group_member (action, const scope& base, target& g, + path, + const target_type&); + + // As above but the target type is determined using the map_extension + // function if specified, falling back to the fallback type if unable to + // (the what argument is used for diagnostics during this process). + // + static pair inject_adhoc_group_member (const char* what, action, const scope& base, target& g, path, -- cgit v1.1