From 90d37f3fe126fa7b3d97fb071f537f910bd4a7fa Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 3 Feb 2021 09:45:46 +0200 Subject: Propagate relevant options/prerequisites to header unit sidebuilds --- libbuild2/cc/common.cxx | 17 +++- libbuild2/cc/compile-rule.cxx | 211 +++++++++++++++++++++++++++++++++++++----- libbuild2/cc/compile-rule.hxx | 5 +- libbuild2/cc/link-rule.cxx | 2 + libbuild2/cc/pkgconfig.cxx | 14 ++- libbuild2/scope.hxx | 14 ++- 6 files changed, 227 insertions(+), 36 deletions(-) diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx index 1bbdf14..7492181 100644 --- a/libbuild2/cc/common.cxx +++ b/libbuild2/cc/common.cxx @@ -335,9 +335,22 @@ namespace build2 // on Windows import-installed DLLs may legally have empty // paths. // - if (t.mtime () == timestamp_unknown) + const char* w (nullptr); + if (t.ctx.phase == run_phase::match) + { + size_t o (t.state[a].task_count.load (memory_order_consume) - + t.ctx.count_base ()); + + if (o != target::offset_applied && + o != target::offset_executed) + w = "not matched"; + } + else if (t.mtime () == timestamp_unknown) + w = "out of date"; + + if (w != nullptr) fail << (impl ? "implementation" : "interface") - << " dependency " << t << " is out of date" << + << " dependency " << t << " is " << w << info << "mentioned in *.export." << (impl ? "imp_" : "") << "libs of target " << l << info << "is it a prerequisite of " << l << "?"; diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 531f2a4..8354bca 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -450,7 +450,7 @@ namespace build2 }; process_libraries (a, bs, li, sys_lib_dirs, - l, la, 0, // Hack: lflags unused. + l, la, 0, // lflags unused. imp, nullptr, opt); } @@ -544,7 +544,7 @@ namespace build2 continue; process_libraries (a, bs, li, sys_lib_dirs, - pt->as (), la, 0, // Hack: lflags unused. + pt->as (), la, 0, // lflags unused. impf, nullptr, optf); } } @@ -759,8 +759,8 @@ namespace build2 continue; // A dependency on a library is there so that we can get its - // *.export.poptions, modules, etc. This is the library metadata - // protocol. See also append_library_options(). + // *.export.poptions, modules, importable headers, etc. This is the + // library metadata protocol. See also append_library_options(). // if (pi == include_type::normal && (p.is_a () || @@ -773,8 +773,8 @@ namespace build2 // Handle (phase two) imported libraries. We know that for such // libraries we don't need to do match() in order to get options // (if any, they would be set by search_library()). But we do need - // to match it if we may need its modules (see search_modules() - // for details). + // to match it if we may need its modules or importable headers + // (see search_modules(), make_header_sidebuild() for details). // if (p.proj ()) { @@ -2207,7 +2207,7 @@ namespace build2 // Synthesize the BMI dependency then update and add the BMI // target as a prerequisite. // - const file& bt (make_header_sidebuild (a, bs, li, *ht)); + const file& bt (make_header_sidebuild (a, bs, t, li, *ht)); if (!skip) { @@ -2240,7 +2240,8 @@ namespace build2 } catch (const failed&) { - r = "ERROR 'unable to update header unit "; r += hs; r += '\''; + r = "ERROR 'unable to update header unit for "; + r += hs; r += '\''; continue; } } @@ -2700,7 +2701,7 @@ namespace build2 // Synthesize the BMI dependency then update and add the BMI // target as a prerequisite. // - const file& bt (make_header_sidebuild (a, bs, li, *ht)); + const file& bt (make_header_sidebuild (a, bs, t, li, *ht)); if (!skip) { @@ -3946,7 +3947,7 @@ namespace build2 // if (inject_header (a, t, *ht, mt, false /* fail */)) { - const file& bt (make_header_sidebuild (a, bs, li, *ht)); + const file& bt (make_header_sidebuild (a, bs, t, li, *ht)); // It doesn't look like we need the cache semantics here since given // the header, we should be able to build its BMI. In other words, a @@ -5878,7 +5879,7 @@ namespace build2 // Find or create a modules sidebuild subproject returning its root // directory. // - dir_path compile_rule:: + pair compile_rule:: find_modules_sidebuild (const scope& rs) const { context& ctx (rs.ctx); @@ -5976,7 +5977,7 @@ namespace build2 assert (m != nullptr && m->modules); #endif - return pd; + return pair (move (pd), *as); } // Synthesize a dependency for building a module binary interface on @@ -5993,7 +5994,7 @@ namespace build2 // Note: see also make_header_sidebuild() below. - dir_path pd (find_modules_sidebuild (*bs.root_scope ())); + dir_path pd (find_modules_sidebuild (*bs.root_scope ()).first); // We need to come up with a file/target name that will be unique enough // not to conflict with other modules. If we assume that within an @@ -6042,14 +6043,6 @@ namespace build2 if (include (a, lt, p) != include_type::normal) // Excluded/ad hoc. continue; - // @@ TODO: will probably need revision if using sidebuild for - // non-installed libraries (e.g., direct BMI dependencies - // will probably have to be translated to mxx{} or some such). - // Hm, don't think we want it this way: we want BMIs of binless - // library to be built in the library rather than on the side - // (so they can be potentially re-used by multiple independent - // importers). - // if (p.is_a () || p.is_a () || p.is_a () || p.is_a ()) { @@ -6087,8 +6080,9 @@ namespace build2 // the side. // const file& compile_rule:: - make_header_sidebuild (action, + make_header_sidebuild (action a, const scope& bs, + const file& t, linfo li, const file& ht) const { @@ -6096,7 +6090,105 @@ namespace build2 // Note: similar to make_module_sidebuild() above. - dir_path pd (find_modules_sidebuild (*bs.root_scope ())); + auto sb (find_modules_sidebuild (*bs.root_scope ())); + dir_path pd (move (sb.first)); + const scope& as (sb.second); + + // Determine if this header belongs to one of the libraries we depend + // on. + // + // Note that because libraries are not in prerequisite_targets, we have + // to go through prerequisites, similar to append_library_options(). + // + const file* lt (nullptr); + { + // Note that any such library would necessarily be an interface + // dependency so we never need to go into implementations. + // + auto imp = [] (const file&, bool) + { + return false; + }; + + // The same logic as in append_libraries(). + // + struct data + { + action a; + const file& ht; + const file*& lt; + } d {a, ht, lt}; + + auto lib = [&d] (const file* const* lc, + const string&, + lflags, + bool) + { + // It's unfortunate we have no way to bail out. + // + if (d.lt != nullptr) + return; + + const file* l (lc != nullptr ? *lc : nullptr); + + if (l == nullptr) + return; + + // Feels like we should only consider non-utility libraries with + // utilities being treated as "direct" use. + // + if (l->is_a ()) + return; + + // Since the library is searched and matched, all the headers should + // be in prerequisite_targets. + // + const auto& pts (l->prerequisite_targets[d.a]); + if (find (pts.begin (), pts.end (), &d.ht) != pts.end ()) + d.lt = l; + + // This is a bit of a hack: for the installed case the library + // prerequisites are matched by file_rule which won't pick the + // liba/libs{} member (naturally) but will just match the lib{} + // group. So we also check the group's prerequisite targets. This + // should be harmless since for now all the importable headers are + // specified on the group. + // + if (d.lt == nullptr && l->group != nullptr) + { + const auto& pts (l->group->prerequisite_targets[d.a]); + if (find (pts.begin (), pts.end (), &d.ht) != pts.end ()) + d.lt = l; + } + }; + + for (prerequisite_member p: group_prerequisite_members (a, t)) + { + if (include (a, t, p) != include_type::normal) // Excluded/ad hoc. + continue; + + // Should be already searched and matched for libraries. + // + if (const target* pt = p.load ()) + { + if (const libx* l = pt->is_a ()) + pt = link_member (*l, a, li); + + bool la; + const file* f; + if ((la = (f = pt->is_a ())) || + (la = (f = pt->is_a ())) || + ( (f = pt->is_a ()))) + { + process_libraries ( + a, bs, li, sys_lib_dirs, + *f, la, 0, // lflags unused. + imp, lib, {}, + true); + } + } + } + } // What should we use as a file/target name? On one hand we want it // unique enough so that and don't end up @@ -6121,7 +6213,12 @@ namespace build2 mf += sha256 (hp.string ()).abbreviated_string (12); } - const target_type& tt (compile_types (li.type).hbmi); + // If the header comes from the library, use its hbmi?{} type to + // maximize reuse. + // + const target_type& tt ( + compile_types ( + lt != nullptr ? link_type (*lt).type : li.type).hbmi); if (const file* bt = bs.ctx.targets.find ( tt, @@ -6135,6 +6232,48 @@ namespace build2 prerequisites ps; ps.push_back (prerequisite (ht)); + // Similar story as for modules: the header may need poptions from its + // library (e.g., -I to find other headers that it includes). + // + if (lt != nullptr) + ps.push_back (prerequisite (*lt)); + else + { + // If the header does not belong to a library then this is a "direct" + // use, for example, by an exe{} target. In this case we need to add + // all the prerequisite libraries as well as scope p/coptions (in a + // sense, we are trying to approximate how all the sources that would + // typically include such a header are build). + // + // Note that this is also the case when we build the library's own + // sources (in a way it would have been cleaner to always build + // library's headers with only its "interface" options/prerequisites + // but that won't be easy to achieve). + // + // Note also that at first it might seem like a good idea to + // incorporate this information into the hash we use to form the BMI + // name. But that would reduce sharing of the BMI. For example, that + // would mean we will build the library header twice, once with the + // implementation options/prerequisites and once -- with interface. + // On the other hand, importable headers are expected to be "modular" + // and should probably not depend on any of the implementation + // options/prerequisites (though one could conceivably build a + // "richer" BMI if it is also to be used to build the library + // implementation -- interesting idea). + // + for (prerequisite_member p: group_prerequisite_members (a, t)) + { + if (include (a, t, p) != include_type::normal) // Excluded/ad hoc. + continue; + + if (p.is_a () || + p.is_a () || p.is_a () || p.is_a ()) + { + ps.push_back (p.as_prerequisite ()); + } + } + } + auto p (bs.ctx.targets.insert_locked ( tt, move (pd), @@ -6149,8 +6288,32 @@ namespace build2 // while we were preparing the prerequisite list. // if (p.second) + { bt.prerequisites (move (ps)); + // Add the p/coptions from our scope in case of a "direct" use. Take + // into account hbmi{} target-type/pattern values to allow specifying + // hbmi-specific options. + // + if (lt == nullptr) + { + auto set = [this, &bs, &as, &tt, &bt] (const variable& var) + { + // Avoid duplicating the options if they are from the same + // amalgamation as the sidebuild. + // + lookup l (bs.lookup (var, tt, bt.name, hbmi::static_type, bt.name)); + if (l.defined () && !l.belongs (as)) + bt.assign (var) = *l; + }; + + set (c_poptions); + set (x_poptions); + set (c_coptions); + set (x_coptions); + } + } + return bt; } diff --git a/libbuild2/cc/compile-rule.hxx b/libbuild2/cc/compile-rule.hxx index a5eb8e8..edff1d8 100644 --- a/libbuild2/cc/compile-rule.hxx +++ b/libbuild2/cc/compile-rule.hxx @@ -162,7 +162,7 @@ namespace build2 const target_type&, const file&, module_imports&, sha256&) const; - dir_path + pair find_modules_sidebuild (const scope&) const; const file& @@ -170,7 +170,8 @@ namespace build2 const target&, const string&) const; const file& - make_header_sidebuild (action, const scope&, linfo, const file&) const; + make_header_sidebuild (action, const scope&, const file&, + linfo, const file&) const; void append_header_options (environment&, cstrings&, small_vector&, diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index 5b12876..8b81536 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -1563,6 +1563,8 @@ namespace build2 lflags f, bool) { + // Note: see also make_header_sidebuild(). + const file* l (lc != nullptr ? *lc : nullptr); // Suppress duplicates. diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx index be7674a..25df3df 100644 --- a/libbuild2/cc/pkgconfig.cxx +++ b/libbuild2/cc/pkgconfig.cxx @@ -744,7 +744,7 @@ namespace build2 }; // Parse --libs into loptions/libs (interface and implementation). If - // ps is not NULL, add each resolves library target as a prerequisite. + // ps is not NULL, add each resolved library target as a prerequisite. // auto parse_libs = [a, &s, top_sysd, this] (target& t, bool binless, @@ -1239,12 +1239,14 @@ namespace build2 } }; - // Parse importable headers and enter them as targets. + // Parse importable headers, enter them as targets, and add them to + // the prerequisites. // auto parse_headers = [&trace, this, &next, &s, <] (const pkgconf& pc, const target_type& tt, - const char* lang) + const char* lang, + prerequisites& ps) { string var (string (lang) + "_importable_headers"); string val (pc.variable (var)); @@ -1285,6 +1287,8 @@ namespace build2 info << "make sure this header is used via " << lt << " prerequisite"; } + + ps.push_back (prerequisite (ht)); } }; @@ -1361,8 +1365,8 @@ namespace build2 // We treat headers outside of any project as C headers (see // enter_header() for details). // - parse_headers (ipc, h::static_type /* **x_hdr */, x); - parse_headers (ipc, h::static_type, "c"); + parse_headers (ipc, h::static_type /* **x_hdr */, x, prs); + parse_headers (ipc, h::static_type, "c", prs); } assert (!lt.has_prerequisites ()); diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx index bd82853..bc04bae 100644 --- a/libbuild2/scope.hxx +++ b/libbuild2/scope.hxx @@ -139,12 +139,20 @@ namespace build2 return lookup (var, &tt, &tn).first; } + lookup_type + lookup (const variable& var, + const target_type& tt, const string& tn, + const target_type& gt, const string& gn) const + { + return lookup (var, &tt, &tn, >, &gn).first; + } + pair lookup (const variable& var, - const target_type* tt = nullptr, - const string* tn = nullptr) const + const target_type* tt = nullptr, const string* tn = nullptr, + const target_type* gt = nullptr, const string* gn = nullptr) const { - auto p (lookup_original (var, tt, tn)); + auto p (lookup_original (var, tt, tn, gt, gn)); return var.overrides == nullptr ? p : lookup_override (var, move (p)); } -- cgit v1.1