From 1bb564a690e2661094e9071d4003638390a5a6fe Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 5 Feb 2018 12:02:32 +0200 Subject: Fix test and install rules to handle see-through groups correctly --- build2/cc/install-rule.cxx | 162 ++++++++++++++++++++--------- build2/cc/install-rule.hxx | 4 +- build2/install/init.cxx | 20 ++-- build2/install/rule.cxx | 61 ++++++++--- build2/install/rule.hxx | 39 ++++++- build2/target.hxx | 20 +++- build2/target.ixx | 14 ++- build2/test/rule.cxx | 71 ++++++------- build2/test/rule.hxx | 25 +++-- old-tests/cli/simple/build/bootstrap.build | 3 + old-tests/cli/simple/buildfile | 10 +- 11 files changed, 297 insertions(+), 132 deletions(-) diff --git a/build2/cc/install-rule.cxx b/build2/cc/install-rule.cxx index 4e232ff..0fdb98b 100644 --- a/build2/cc/install-rule.cxx +++ b/build2/cc/install-rule.cxx @@ -26,36 +26,11 @@ namespace build2 : common (move (d)), link_ (l) {} const target* install_rule:: - filter (action a, const target& t, prerequisite_member p) const + filter (action a, const target& t, prerequisite_iterator& i) const { // NOTE: see libux_install_rule::filter() if changing anything here. - otype ot (link_type (t).type); - - // Don't install executable's prerequisite headers. - // - if (t.is_a () && x_header (p)) - return nullptr; - - // Here is a problem: if the user spells the obj*/bmi*{} targets - // explicitly, then the source files, including headers/modules may be - // specified as preprequisites of those targets and not of this target. - // While this can be worked around for headers by also listing them as - // prerequisites of this target, this won't work for modules (since they - // are compiled). So what we are going to do here is detect bmi*{} and - // translate them to their mxx{} (this doesn't quite work for headers - // since there would normally be several of them). - // - if (p.is_a () || p.is_a (compile_types (ot).bmi)) - { - const target& mt (p.search (t)); - - for (prerequisite_member mp: group_prerequisite_members (a, mt)) - { - if (mp.is_a (*x_mod)) - return t.is_a () ? nullptr : file_rule::filter (a, mt, mp); - } - } + const prerequisite& p (i->prerequisite); // If this is a shared library prerequisite, install it as long as it // is in the same amalgamation as we are. @@ -63,13 +38,18 @@ namespace build2 // Less obvious: we also want to install a static library prerequisite // of a library (since it could be referenced from its .pc file, etc). // + // Note: for now we assume these prerequisites never come from see- + // through groups. + // + otype ot (link_type (t).type); + bool st (t.is_a () || t.is_a ()); // Target needs shared. bool at (t.is_a () || t.is_a ()); // Target needs static. if ((st && (p.is_a () || p.is_a ())) || (at && (p.is_a () || p.is_a ()))) { - const target* pt (&p.search (t)); + const target* pt (&search (t, p)); // If this is the lib{}/libu{} group, pick a member which we would // link. For libu{} we want the "see through" logic. @@ -87,7 +67,68 @@ namespace build2 return pt; } - return file_rule::filter (a, t, p); + // The rest of the tests only succeed if the base filter() succeeds. + // + const target* pt (file_rule::filter (a, t, p)); + if (pt == nullptr) + return pt; + + // Don't install executable's prerequisite headers and module + // interfaces. + // + // Note that if they come from a group, then we assume the entire + // group is not to be installed. + // + if (t.is_a ()) + { + if (x_header (p)) + pt = nullptr; + else if (p.type.see_through) + { + for (i.enter_group (); i.group (); ) + { + if (x_header (*++i)) + pt = nullptr; + } + } + + if (pt == nullptr) + return pt; + } + + // Here is a problem: if the user spells the obj*/bmi*{} targets + // explicitly, then the source files, including headers/modules may be + // specified as preprequisites of those targets and not of this target. + // While this can be worked around for headers by also listing them as + // prerequisites of this target, this won't work for modules (since they + // are compiled). So what we are going to do here is detect bmi*{} and + // translate them to their mxx{} (this doesn't quite work for headers + // since there would normally be many of them). + // + // Note: for now we assume bmi*{} never come from see-through groups. + // + if (p.is_a () || p.is_a (compile_types (ot).bmi)) + { + // This is tricky: we need to "look" inside groups for mxx{} but if + // found, remap to the group, not member. + // + for (prerequisite_member pm: + group_prerequisite_members (a, *pt, members_mode::maybe)) + { + if (pm.is_a (*x_mod)) + { + pt = t.is_a () + ? nullptr + : file_rule::filter (a, *pt, pm.prerequisite); + break; + } + } + + if (pt == nullptr) + return pt; + } + + return pt; } bool install_rule:: @@ -214,35 +255,23 @@ namespace build2 : common (move (d)), link_ (l) {} const target* libux_install_rule:: - filter (action a, const target& t, prerequisite_member p) const + filter (action a, const target& t, prerequisite_iterator& i) const { + const prerequisite& p (i->prerequisite); + // The "see through" semantics that should be parallel to install_rule // above. In particular, here we use libue/libua/libus{} as proxies for // exe/liba/libs{} there. - + // otype ot (link_type (t).type); - if (t.is_a () && x_header (p)) - return nullptr; - - if (p.is_a () || p.is_a (compile_types (ot).bmi)) - { - const target& mt (p.search (t)); - - for (prerequisite_member mp: group_prerequisite_members (a, mt)) - { - if (mp.is_a (*x_mod)) - return t.is_a () ? nullptr : alias_rule::filter (a, mt, mp); - } - } - bool st (t.is_a () || t.is_a ()); // Target needs shared. bool at (t.is_a () || t.is_a ()); // Target needs static. if ((st && (p.is_a () || p.is_a ())) || (at && (p.is_a () || p.is_a ()))) { - const target* pt (&p.search (t)); + const target* pt (&search (t, p)); if (const libx* l = pt->is_a ()) pt = &link_member (*l, a, link_info (t.base_scope (), ot)); @@ -254,7 +283,46 @@ namespace build2 return pt; } - return alias_rule::filter (a, t, p); + const target* pt (install::file_rule::instance.filter (a, t, p)); + if (pt == nullptr) + return pt; + + if (t.is_a ()) + { + if (x_header (p)) + pt = nullptr; + else if (p.type.see_through) + { + for (i.enter_group (); i.group (); ) + { + if (x_header (*++i)) + pt = nullptr; + } + } + + if (pt == nullptr) + return pt; + } + + if (p.is_a () || p.is_a (compile_types (ot).bmi)) + { + for (prerequisite_member pm: + group_prerequisite_members (a, *pt, members_mode::maybe)) + { + if (pm.is_a (*x_mod)) + { + pt = t.is_a () + ? nullptr + : install::file_rule::instance.filter (a, *pt, pm.prerequisite); + break; + } + } + + if (pt == nullptr) + return pt; + } + + return pt; } bool libux_install_rule:: diff --git a/build2/cc/install-rule.hxx b/build2/cc/install-rule.hxx index ac2f93a..dbbe4e5 100644 --- a/build2/cc/install-rule.hxx +++ b/build2/cc/install-rule.hxx @@ -33,7 +33,7 @@ namespace build2 install_rule (data&&, const link_rule&); virtual const target* - filter (action, const target&, prerequisite_member) const override; + filter (action, const target&, prerequisite_iterator&) const override; virtual bool match (action, target&, const string&) const override; @@ -63,7 +63,7 @@ namespace build2 libux_install_rule (data&&, const link_rule&); virtual const target* - filter (action, const target&, prerequisite_member) const override; + filter (action, const target&, prerequisite_iterator&) const override; virtual bool match (action, target&, const string&) const override; diff --git a/build2/install/init.cxx b/build2/install/init.cxx index a11f3e5..259fb07 100644 --- a/build2/install/init.cxx +++ b/build2/install/init.cxx @@ -165,6 +165,8 @@ namespace build2 static const dir_path dir_man (dir_path (dir_data) /= "man"); static const dir_path dir_man1 (dir_path ("man") /= "man1"); + static const group_rule group_rule_ (true /* see_through_only */); + bool init (scope& rs, scope& bs, @@ -207,18 +209,24 @@ namespace build2 vp.insert ("install.subdirs", variable_visibility::project); } - // Register our alias and file rules. + // Register our rules. // { + auto& r (bs.rules); + const auto& ar (alias_rule::instance); const auto& fr (file_rule::instance); + const auto& gr (group_rule_); - bs.rules.insert (perform_install_id, "install.alias", ar); - bs.rules.insert (perform_uninstall_id, "uninstall.alias", ar); + r.insert (perform_install_id, "install.alias", ar); + r.insert (perform_uninstall_id, "uninstall.alias", ar); - bs.rules.insert (perform_install_id, "install.file", fr); - bs.rules.insert (perform_uninstall_id, "uninstall.file", fr); - } + r.insert (perform_install_id, "install.file", fr); + r.insert (perform_uninstall_id, "uninstall.file", fr); + + r.insert (perform_install_id, "install.file", gr); + r.insert (perform_uninstall_id, "uninstall.file", gr); + } // Configuration. // diff --git a/build2/install/rule.cxx b/build2/install/rule.cxx index 4d4cb51..2b5d048 100644 --- a/build2/install/rule.cxx +++ b/build2/install/rule.cxx @@ -53,9 +53,16 @@ namespace build2 } const target* alias_rule:: - filter (action, const target& t, prerequisite_member p) const + filter (action a, const target& t, prerequisite_iterator& i) const { - return &p.search (t); + assert (i->target == nullptr); + return filter (a, t, i->prerequisite); + } + + const target* alias_rule:: + filter (action, const target& t, const prerequisite& p) const + { + return &search (t, p); } recipe alias_rule:: @@ -68,17 +75,24 @@ namespace build2 // @@ Shouldn't we do match in parallel (here and below)? // auto& pts (t.prerequisite_targets[a]); - for (prerequisite_member p: group_prerequisite_members (a, t)) + + auto pms (group_prerequisite_members (a, t, members_mode::never)); + for (auto i (pms.begin ()), e (pms.end ()); i != e; ++i) { + const prerequisite& p (i->prerequisite); + // Ignore unresolved targets that are imported from other projects. // We are definitely not installing those. // - if (p.proj ()) + if (p.proj) continue; // Let a customized rule have its say. // - const target* pt (filter (a, t, p)); + // Note: we assume that if the filter enters the group, then it + // iterates over all its members. + // + const target* pt (filter (a, t, i)); if (pt == nullptr) { l5 ([&]{trace << "ignoring " << p << " (filtered out)";}); @@ -112,7 +126,14 @@ namespace build2 // group_rule // - const group_rule group_rule::instance; + const group_rule group_rule::instance (false /* see_through_only */); + + bool group_rule:: + match (action a, target& t, const string& h) const + { + return (!see_through || t.type ().see_through) && + alias_rule::match (a, t, h); + } const target* group_rule:: filter (action, const target&, const target& m) const @@ -142,6 +163,7 @@ namespace build2 if (gv.members != nullptr) { auto& pts (t.prerequisite_targets[a]); + for (size_t i (0); i != gv.count; ++i) { const target* m (gv.members[i]); @@ -194,9 +216,16 @@ namespace build2 } const target* file_rule:: - filter (action, const target& t, prerequisite_member p) const + filter (action a, const target& t, prerequisite_iterator& i) const + { + assert (i->target == nullptr); + return filter (a, t, i->prerequisite); + } + + const target* file_rule:: + filter (action, const target& t, const prerequisite& p) const { - const target& pt (p.search (t)); + const target& pt (search (t, p)); return pt.in (t.root_scope ()) ? &pt : nullptr; } @@ -220,21 +249,25 @@ namespace build2 // In both cases, the next step is to search, match, and collect all the // installable prerequisites. // - // @@ Unconditional group? How does it work for cli? Change to maybe - // same like test? If so, also in alias_rule. - // auto& pts (t.prerequisite_targets[a]); - for (prerequisite_member p: group_prerequisite_members (a, t)) + + auto pms (group_prerequisite_members (a, t, members_mode::never)); + for (auto i (pms.begin ()), e (pms.end ()); i != e; ++i) { + const prerequisite& p (i->prerequisite); + // Ignore unresolved targets that are imported from other projects. // We are definitely not installing those. // - if (p.proj ()) + if (p.proj) continue; // Let a customized rule have its say. // - const target* pt (filter (a, t, p)); + // Note: we assume that if the filter enters the group, then it + // iterates over all its members. + // + const target* pt (filter (a, t, i)); if (pt == nullptr) { l5 ([&]{trace << "ignoring " << p << " (filtered out)";}); diff --git a/build2/install/rule.hxx b/build2/install/rule.hxx index b9699e3..2ad3d4b 100644 --- a/build2/install/rule.hxx +++ b/build2/install/rule.hxx @@ -25,8 +25,17 @@ namespace build2 // Return NULL if this prerequisite should be ignored and pointer to its // target otherwise. The default implementation accepts all prerequsites. // + // The prerequisite it passed as an iterator allowing the filter to + // "see" inside groups. + // + using prerequisite_iterator = + prerequisite_members_range::iterator; + + virtual const target* + filter (action, const target&, prerequisite_iterator&) const; + virtual const target* - filter (action, const target&, prerequisite_member) const; + filter (action, const target&, const prerequisite&) const; virtual recipe apply (action, target&) const override; @@ -38,10 +47,19 @@ namespace build2 // In addition to the alias rule's semantics, this rule sees through to // the group's members. // + // The default group_rule::instance matches any target for which it was + // registered. It is to be used for non-see-through groups that should + // exhibit the see-through behavior for install (see lib{} in the bin + // module for an example). + // + // We also register (for all targets) another instance of this rule that + // only matches see-through groups. + // class group_rule: public alias_rule { public: - using alias_rule::filter; // "Unhide" to make Clang happy. + virtual bool + match (action, target&, const string&) const override; // Return NULL if this group member should be ignored and pointer to its // target otherwise. The default implementation accepts all members. @@ -49,11 +67,15 @@ namespace build2 virtual const target* filter (action, const target&, const target& group_member) const; + using alias_rule::filter; // "Unhide" to make Clang happy. + virtual recipe apply (action, target&) const override; - group_rule () {} + group_rule (bool see_through_only): see_through (see_through_only) {} static const group_rule instance; + + bool see_through; }; struct install_dir; @@ -68,8 +90,17 @@ namespace build2 // target otherwise. The default implementation ignores prerequsites // that are outside of this target's project. // + // The prerequisite it passed as an iterator allowing the filter to + // "see" inside groups. + // + using prerequisite_iterator = + prerequisite_members_range::iterator; + + virtual const target* + filter (action, const target&, prerequisite_iterator&) const; + virtual const target* - filter (action, const target&, prerequisite_member) const; + filter (action, const target&, const prerequisite&) const; virtual recipe apply (action, target&) const override; diff --git a/build2/target.hxx b/build2/target.hxx index 105cc4b..5c8e7bd 100644 --- a/build2/target.hxx +++ b/build2/target.hxx @@ -697,8 +697,6 @@ namespace build2 // also be traversed in reverse, but that's what you usually want, // anyway. // - // For constant iteration use const_group_prerequisites(). - // class group_prerequisites { public: @@ -1015,6 +1013,24 @@ namespace build2 bool enter_group (); + // Return true if the next element is this group's members. Normally + // used to iterate over group members only, for example: + // + // for (...; ++i) + // { + // if (i->prerequisite.type.see_through) + // { + // for (i.enter_group (); i.group (); ) + // { + // ++i; + // ... + // } + // } + // } + // + bool + group () const; + value_type operator* () const { const target* t (k_ != nullptr ? k_: diff --git a/build2/target.ixx b/build2/target.ixx index 16dbf61..242e81d 100644 --- a/build2/target.ixx +++ b/build2/target.ixx @@ -212,7 +212,7 @@ namespace build2 if (k_ == nullptr && g_.count != 0) // Iterating over a normal group. { - if (g_.members == nullptr || // leave_group() + if (g_.members == nullptr || // Special case, see leave_group(). ++j_ > g_.count) g_.count = 0; } @@ -248,7 +248,7 @@ namespace build2 { // Otherwise assume it is a normal group. // - g_ = resolve_group_members (r_->a_, search (*i_)); + g_ = resolve_group_members (r_->a_, search (r_->t_, *i_)); if (g_.members == nullptr) // Members are not know. { @@ -283,6 +283,16 @@ namespace build2 } } + template + inline bool prerequisite_members_range::iterator:: + group () const + { + return + k_ != nullptr ? k_->member != nullptr : /* ad hoc */ + g_.count != 0 ? g_.members != nullptr && j_ < g_.count : /* normal */ + false; + } + // mtime_target // inline void mtime_target:: diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx index bc4060d..7c55445 100644 --- a/build2/test/rule.cxx +++ b/build2/test/rule.cxx @@ -56,11 +56,37 @@ namespace build2 // ---------------------------------------+-------------+--------- // test test (& pass) | pass | noop // + auto& pts (t.prerequisite_targets[a]); - // If we are passing-through, then match our prerequisites. + // Resolve group members. // - // Note that we may already have stuff in prerequisite_targets (see - // group_rule). + if (!see_through || t.type ().see_through) + { + // Remember that we are called twice: first during update for test + // (pre-operation) and then during test. During the former, we rely on + // the normall update rule to resolve the group members. During the + // latter, there will be no rule to do this but the group will already + // have been resolved by the pre-operation. + // + // If the rule could not resolve the group, then we ignore it. + // + group_view gv (a.outer () + ? resolve_group_members (a, t) + : t.group_members (a)); + + if (gv.members != nullptr) + { + for (size_t i (0); i != gv.count; ++i) + { + if (const target* m = gv.members[i]) + pts.push_back (m); + } + + match_members (a, t, pts); + } + } + + // If we are passing-through, then match our prerequisites. // if (t.is_a () && pass (t)) { @@ -74,7 +100,6 @@ namespace build2 match_prerequisites (a, t, t.root_scope ()); } - auto& pts (t.prerequisite_targets[a]); size_t pass_n (pts.size ()); // Number of pass-through prerequisites. // See if it's testable and if so, what kind. @@ -153,8 +178,8 @@ namespace build2 test = false; else { - // Look for test input/stdin/stdout prerequisites. The same - // group logic as in the testscript case above. + // Look for test input/stdin/stdout prerequisites. The same group + // reasoning as in the testscript case above. // for (prerequisite_member p: group_prerequisite_members (a, t, members_mode::maybe)) @@ -274,40 +299,6 @@ namespace build2 } } - recipe group_rule:: - apply (action a, target& t) const - { - // Resolve group members. - // - // Remember that we are called twice: first during update for test - // (pre-operation) and then during test. During the former, we rely on - // the normall update rule to resolve the group members. During the - // latter, there will be no rule to do this but the group will already - // have been resolved by the pre-operation. - // - // If the rule could not resolve the group, then we ignore it. - // - group_view gv (a.outer () - ? resolve_group_members (a, t) - : t.group_members (a)); - - if (gv.members != nullptr) - { - auto& pts (t.prerequisite_targets[a]); - for (size_t i (0); i != gv.count; ++i) - { - if (const target* m = gv.members[i]) - pts.push_back (m); - } - - match_members (a, t, pts); - } - - // Delegate to the base rule. - // - return rule::apply (a, t); - } - target_state rule:: perform_update (action a, const target& t) { diff --git a/build2/test/rule.hxx b/build2/test/rule.hxx index b331263..4f02390 100644 --- a/build2/test/rule.hxx +++ b/build2/test/rule.hxx @@ -20,9 +20,6 @@ namespace build2 class rule: public build2::rule, protected virtual common { public: - explicit - rule (common_data&& d): common (move (d)) {} - virtual bool match (action, target&, const string&) const override; @@ -37,26 +34,32 @@ namespace build2 target_state perform_script (action, const target&, size_t) const; + + rule (common_data&& d, bool see_through_only) + : common (move (d)), see_through (see_through_only) {} + + bool see_through; }; - class default_rule: public rule // For disambiguation in module. + class default_rule: public rule { public: explicit - default_rule (common_data&& d): common (move (d)), rule (move (d)) {} + default_rule (common_data&& d) + : common (move (d)), + rule (move (d), true /* see_through_only */) {} }; - // In addition to the above rule's semantics, this rule sees through to - // the group's members. + // To be used for non-see-through groups that should exhibit the see- + // through behavior for install (see lib{} in the bin module for an + // example). // class group_rule: public rule { public: explicit - group_rule (common_data&& d): common (move (d)), rule (move (d)) {} - - virtual recipe - apply (action, target&) const override; + group_rule (common_data&& d) + : common (move (d)), rule (move (d), false /* see_through_only */) {} }; } } diff --git a/old-tests/cli/simple/build/bootstrap.build b/old-tests/cli/simple/build/bootstrap.build index 2c116c9..40e30ac 100644 --- a/old-tests/cli/simple/build/bootstrap.build +++ b/old-tests/cli/simple/build/bootstrap.build @@ -1,3 +1,6 @@ project = cli-simple +version = 1.0.0 amalgamation = # Disabled. using config +using install +using test diff --git a/old-tests/cli/simple/buildfile b/old-tests/cli/simple/buildfile index ddbf27c..5aa86a9 100644 --- a/old-tests/cli/simple/buildfile +++ b/old-tests/cli/simple/buildfile @@ -8,8 +8,10 @@ cxx.poptions += "-I$out_root" using cli -exe{driver}: cxx{driver} cxx{test} -cxx{test} hxx{test}: cli{test} +#exe{driver}: cxx{driver} cxx{test} +#cxx{test} hxx{test}: cli{test} -#exe{driver}: cxx{driver} cli.cxx{test} -#cli.cxx{test}: cli{test} +exe{driver}: cxx{driver} cli.cxx{test} +#exe{driver}: cxx{driver} lib{test} +#lib{test}: cli.cxx{test} +cli.cxx{test}: cli{test} -- cgit v1.1