From ba8ddd80a4323eb85ee5a569295eb4e80986003f Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 28 Jun 2022 10:51:17 +0200 Subject: Complete support for saving/loading library metadata to/from pkg-config files --- libbuild2/cc/common.cxx | 236 +++++++++++++++++++++++++++++---------------- libbuild2/cc/common.hxx | 10 +- libbuild2/cc/link-rule.cxx | 26 ++++- libbuild2/cc/link-rule.hxx | 5 + libbuild2/cc/pkgconfig.cxx | 198 ++++++++++++++++++++++--------------- 5 files changed, 308 insertions(+), 167 deletions(-) diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx index a320626..3b49d58 100644 --- a/libbuild2/cc/common.cxx +++ b/libbuild2/cc/common.cxx @@ -877,12 +877,13 @@ namespace build2 return pair {t, g}; } - // Note that pk's scope should not be NULL (even if dir is absolute). + // Action should be absent if called during the load phase. Note that pk's + // scope should not be NULL (even if dir is absolute). // // Note: see similar logic in find_system_library(). // target* common:: - search_library (action act, + search_library (optional act, const dir_paths& sysd, optional& usrd, const prerequisite_key& p, @@ -890,7 +891,7 @@ namespace build2 { tracer trace (x, "search_library"); - assert (p.scope != nullptr); + assert (p.scope != nullptr && (!exist || act)); context& ctx (p.scope->ctx); const scope& rs (*p.scope->root_scope ()); @@ -1232,20 +1233,87 @@ namespace build2 if (exist) return r; - // If we cannot acquire the lock then this mean the target has already - // been matched and we assume all of this has already been done. + // Try to extract library information from pkg-config. We only add the + // default macro if we could not extract more precise information. The + // idea is that in .pc files that we generate, we copy those macros (or + // custom ones) from *.export.poptions. // - auto lock = [act] (const target* t) -> target_lock + // @@ Should we add .pc files as ad hoc members so pkconfig_save() can + // use their names when deriving -l-names (this would be especially + // helpful for binless libraries to get hold of prefix/suffix, etc). + // + auto load_pc = [this, &trace, + act, &p, &name, + &sysd, &usrd, + pd, &pc, lt, a, s] (pair metaonly) { - auto l (t != nullptr ? build2::lock (act, *t, true) : target_lock ()); + l5 ([&]{trace << "loading pkg-config information during " + << (act ? "match" : "load") << " for " + << (a != nullptr ? "static " : "") + << (s != nullptr ? "shared " : "") + << "member(s) of " << *lt << "; metadata only: " + << metaonly.first << " " << metaonly.second;}); + + // Add the "using static/shared library" macro (used, for example, to + // handle DLL export). The absence of either of these macros would + // mean some other build system that cannot distinguish between the + // two (and no pkg-config information). + // + auto add_macro = [this] (target& t, const char* suffix) + { + // If there is already a value (either in cc.export or x.export), + // don't add anything: we don't want to be accumulating defines nor + // messing with custom values. And if we are adding, then use the + // generic cc.export. + // + // The only way we could already have this value is if this same + // library was also imported as a project (as opposed to installed). + // Unlikely but possible. In this case the values were set by the + // export stub and we shouldn't touch them. + // + if (!t.vars[x_export_poptions]) + { + auto p (t.vars.insert (c_export_poptions)); - if (l && l.offset == target::offset_matched) + if (p.second) + { + // The "standard" macro name will be LIB_{STATIC,SHARED}, + // where is the target name. Here we want to strike a + // balance between being unique and not too noisy. + // + string d ("-DLIB"); + + d += sanitize_identifier ( + ucase (const_cast (t.name))); + + d += '_'; + d += suffix; + + strings o; + o.push_back (move (d)); + p.first = move (o); + } + } + }; + + if (pc.first.empty () && pc.second.empty ()) { - assert ((*t)[act].rule == &file_rule::rule_match); - l.unlock (); + if (!pkgconfig_load (act, *p.scope, + *lt, a, s, + p.proj, name, + *pd, sysd, *usrd, + metaonly)) + { + if (a != nullptr && !metaonly.first) add_macro (*a, "STATIC"); + if (s != nullptr && !metaonly.second) add_macro (*s, "SHARED"); + } } - - return l; + else + pkgconfig_load (act, *p.scope, + *lt, a, s, + pc, + *pd, sysd, *usrd, + metaonly); }; // Mark as a "cc" library (unless already marked) and set the system @@ -1266,6 +1334,75 @@ namespace build2 return p.second; }; + // Deal with the load phase case. The rest is already hairy enough so + // let's not try to weave this logic into that. + // + if (!act) + { + assert (ctx.phase == run_phase::load); + + // The overall idea here is to set everything up so that the default + // file_rule matches the returned targets, the same way as it would if + // multiple operations were executed for the normal case (see below). + // + timestamp mt (timestamp_nonexistent); + if (a != nullptr) {lt->a = a; a->group = lt; mt = a->mtime ();} + if (s != nullptr) {lt->s = s; s->group = lt; mt = s->mtime ();} + + // @@ TODO: we currently always reload pkgconfig for lt (and below). + // + mark_cc (*lt); + lt->mtime (mt); + + // We can only load metadata from here since we can only do this + // during the load phase. But it's also possible that a racing match + // phase already found and loaded this library without metadata. So + // looks like the only way is to load the metadata incrementally. We + // can base this decision on the presense/absense of cc.type and + // export.metadata. + // + pair metaonly {false, false}; + + if (a != nullptr && !mark_cc (*a)) + { + if (a->vars[ctx.var_export_metadata]) + a = nullptr; + else + metaonly.first = true; + } + + if (s != nullptr && !mark_cc (*s)) + { + if (s->vars[ctx.var_export_metadata]) + s = nullptr; + else + metaonly.second = true; + } + + // Try to extract library information from pkg-config. + // + if (a != nullptr || s != nullptr) + load_pc (metaonly); + + return r; + } + + // If we cannot acquire the lock then this mean the target has already + // been matched and we assume all of this has already been done. + // + auto lock = [a = *act] (const target* t) -> target_lock + { + auto l (t != nullptr ? build2::lock (a, *t, true) : target_lock ()); + + if (l && l.offset == target::offset_matched) + { + assert ((*t)[a].rule == &file_rule::rule_match); + l.unlock (); + } + + return l; + }; + target_lock ll (lock (lt)); // Set lib{} group members to indicate what's available. Note that we @@ -1275,12 +1412,15 @@ namespace build2 timestamp mt (timestamp_nonexistent); if (ll) { - if (s != nullptr) {lt->s = s; mt = s->mtime ();} if (a != nullptr) {lt->a = a; mt = a->mtime ();} + if (s != nullptr) {lt->s = s; mt = s->mtime ();} // Mark the group since sometimes we use it itself instead of one of // the liba/libs{} members (see process_libraries_impl() for details). // + // @@ TODO: we currently always reload pkgconfig for lt (and above). + // Maybe pass NULL lt to pkgconfig_load() in this case? + // mark_cc (*lt); } @@ -1299,77 +1439,11 @@ namespace build2 if (a != nullptr && !mark_cc (*a)) a = nullptr; if (s != nullptr && !mark_cc (*s)) s = nullptr; - // Add the "using static/shared library" macro (used, for example, to - // handle DLL export). The absence of either of these macros would - // mean some other build system that cannot distinguish between the - // two (and no pkg-config information). - // - auto add_macro = [this] (target& t, const char* suffix) - { - // If there is already a value (either in cc.export or x.export), - // don't add anything: we don't want to be accumulating defines nor - // messing with custom values. And if we are adding, then use the - // generic cc.export. - // - // The only way we could already have this value is if this same - // library was also imported as a project (as opposed to installed). - // Unlikely but possible. In this case the values were set by the - // export stub and we shouldn't touch them. - // - if (!t.vars[x_export_poptions]) - { - auto p (t.vars.insert (c_export_poptions)); - - if (p.second) - { - // The "standard" macro name will be LIB_{STATIC,SHARED}, - // where is the target name. Here we want to strike a - // balance between being unique and not too noisy. - // - string d ("-DLIB"); - - d += sanitize_identifier ( - ucase (const_cast (t.name))); - - d += '_'; - d += suffix; - - strings o; - o.push_back (move (d)); - p.first = move (o); - } - } - }; - if (ll && (a != nullptr || s != nullptr)) { - // Try to extract library information from pkg-config. We only add the - // default macro if we could not extract more precise information. The - // idea is that in .pc files that we generate, we copy those macros - // (or custom ones) from *.export.poptions. + // Try to extract library information from pkg-config. // - // @@ Should we add .pc files as ad hoc members so pkconfig_save() can - // use their names when deriving -l-names (this would be especially - // helpful for binless libraries to get hold of prefix/suffix, etc). - // - if (pc.first.empty () && pc.second.empty ()) - { - if (!pkgconfig_load (act, *p.scope, - *lt, a, s, - p.proj, name, - *pd, sysd, *usrd, - false /* metadata */)) - { - if (a != nullptr) add_macro (*a, "STATIC"); - if (s != nullptr) add_macro (*s, "SHARED"); - } - } - else - pkgconfig_load (act, *p.scope, - *lt, a, s, - pc, - *pd, sysd, *usrd, - false /* metadata */); + load_pc ({false, false} /* metaonly */); } // If we have the lock (meaning this is the first time), set the matched diff --git a/libbuild2/cc/common.hxx b/libbuild2/cc/common.hxx index 2aaa0d0..9c276f7 100644 --- a/libbuild2/cc/common.hxx +++ b/libbuild2/cc/common.hxx @@ -396,7 +396,7 @@ namespace build2 tracer&); target* - search_library (action, + search_library (optional, const dir_paths&, optional&, const prerequisite_key&, @@ -442,23 +442,23 @@ namespace build2 bool) const; void - pkgconfig_load (action, const scope&, + pkgconfig_load (optional, const scope&, bin::lib&, bin::liba*, bin::libs*, const pair&, const dir_path&, const dir_paths&, const dir_paths&, - bool) const; + pair) const; bool - pkgconfig_load (action, const scope&, + pkgconfig_load (optional, const scope&, bin::lib&, bin::liba*, bin::libs*, const optional&, const string&, const dir_path&, const dir_paths&, const dir_paths&, - bool) const; + pair) const; }; } } diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index d0a1f4a..71d2055 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -1122,12 +1122,8 @@ namespace build2 { // Handle imported libraries. // - // Note that since the search is rule-specific, we don't cache the - // target in the prerequisite. - // if (p.proj ()) - pt = search_library ( - a, sys_lib_dirs, usr_lib_dirs, p.prerequisite); + pt = search_library (a, sys_lib_dirs, usr_lib_dirs, p.prerequisite); // The rest is the same basic logic as in search_and_match(). // @@ -4254,5 +4250,25 @@ namespace build2 return perform_clean_extra (a, t, extras, adhoc_extras); } + + const target* link_rule:: + import (const prerequisite_key& pk, + const optional&, + const location&) const + { + tracer trace (x, "link_rule::import"); + + // @@ TODO: do we want to make metadata loading optional? + // + optional usr_lib_dirs; + const target* r (search_library (nullopt /* action */, + sys_lib_dirs, usr_lib_dirs, + pk)); + + if (r == nullptr) + l4 ([&]{trace << "unable to find installed library " << pk;}); + + return r; + } } } diff --git a/libbuild2/cc/link-rule.hxx b/libbuild2/cc/link-rule.hxx index 14a70d2..9b491c2 100644 --- a/libbuild2/cc/link-rule.hxx +++ b/libbuild2/cc/link-rule.hxx @@ -59,6 +59,11 @@ namespace build2 target_state perform_clean (action, const target&, match_data&) const; + virtual const target* + import (const prerequisite_key&, + const optional&, + const location&) const override; + public: // Library handling. // diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx index 1fd96da..0ae4341 100644 --- a/libbuild2/cc/pkgconfig.cxx +++ b/libbuild2/cc/pkgconfig.cxx @@ -670,7 +670,7 @@ namespace build2 }; bool common:: - pkgconfig_load (action a, + pkgconfig_load (optional act, const scope& s, lib& lt, liba* at, @@ -680,7 +680,7 @@ namespace build2 const dir_path& libd, const dir_paths& top_sysd, const dir_paths& top_usrd, - bool meta) const + pair metaonly) const { assert (at != nullptr || st != nullptr); @@ -690,12 +690,16 @@ namespace build2 if (p.first.empty () && p.second.empty ()) return false; - pkgconfig_load (a, s, lt, at, st, p, libd, top_sysd, top_usrd, meta); + pkgconfig_load ( + act, s, lt, at, st, p, libd, top_sysd, top_usrd, metaonly); return true; } + // Action should be absent if called during the load phase. If metaonly is + // true then only load the metadata. + // void common:: - pkgconfig_load (action a, + pkgconfig_load (optional act, const scope& s, lib& lt, liba* at, @@ -704,7 +708,7 @@ namespace build2 const dir_path& libd, const dir_paths& top_sysd, const dir_paths& top_usrd, - bool meta) const + pair metaonly) const { tracer trace (x, "pkgconfig_load"); @@ -773,11 +777,11 @@ namespace build2 // Parse --libs into loptions/libs (interface and implementation). If // ps is not NULL, add each resolved library target as a prerequisite. // - auto parse_libs = [a, &s, top_sysd, this] (target& t, - bool binless, - const pkgconf& pc, - bool la, - prerequisites* ps) + auto parse_libs = [this, act, &s, top_sysd] (target& t, + bool binless, + const pkgconf& pc, + bool la, + prerequisites* ps) { strings lops; vector libs; @@ -1072,7 +1076,7 @@ namespace build2 dr << info (f) << "while resolving pkg-config dependency " << l; }); - lt = search_library (a, top_sysd, usrd, pk); + lt = search_library (act, top_sysd, usrd, pk); } if (lt != nullptr) @@ -1204,13 +1208,17 @@ namespace build2 // Parse user metadata and set extracted variables on the specified // target. // - auto parse_metadata = [&next] (const target&, + auto parse_metadata = [&next] (target& t, pkgconf& pc, const string& md) { const location loc (pc.path); + context& ctx (t.ctx); + auto& vp (ctx.var_pool.rw ()); // Load phase. + optional ver; + optional pfx; string s; for (size_t b (0), e (0); !(s = next (md, b, e)).empty (); ) @@ -1223,7 +1231,8 @@ namespace build2 } catch (const invalid_argument& e) { - fail (loc) << "invalid version in metadata variable; " << e; + fail (loc) << "invalid version in build2.metadata variable: " + << e; } if (*ver != 1) @@ -1232,6 +1241,15 @@ namespace build2 continue; } + if (!pfx) + { + if (s.empty ()) + fail (loc) << "empty variable prefix in build2.metadata varible"; + + pfx = s; + continue; + } + // The rest is variable name/type pairs. // size_t p (s.find ('/')); @@ -1239,17 +1257,13 @@ namespace build2 if (p == string::npos) fail (loc) << "expected name/type pair instead of '" << s << "'"; - //@@ We cannot insert a variable since we are not in the load phase - // (but if we were to somehow support immediate importation of - // libraries, we would be in the load phase). - // - string var (s, 0, p); + string vn (s, 0, p); string tn (s, p + 1); - optional val (pc.variable (var)); + optional val (pc.variable (vn)); if (!val) - fail (loc) << "metadata variable " << var << " not set"; + fail (loc) << "metadata variable " << vn << " not set"; pair vt (metadata_type (tn)); if (vt.first == nullptr) @@ -1263,17 +1277,23 @@ namespace build2 : name (move (s))); } - value v (vt.first); - v.assign (move (ns), nullptr /* @@ TODO: var */); + const variable& var (vp.insert (move (vn))); - //names storage; - //text << var << " = " << reverse (v, storage); + value& v (t.assign (var)); + v.assign (move (ns), &var); + typify (v, *vt.first, &var); } if (!ver) - fail (loc) << "version expected in metadata variable"; + fail (loc) << "version expected in build2.metadata variable"; + + if (!pfx) + fail (loc) << "variable prefix expected in build2.metadata variable"; - // @@ TODO: we should probably also set export.metadata? + // Set export.metadata to indicate the presence of user metadata. + // + t.assign (ctx.var_export_metadata) = names { + name (std::to_string (*ver)), name (move (*pfx))}; }; // Parse modules, enter them as targets, and add them to the @@ -1433,17 +1453,8 @@ namespace build2 } }; - // For now we only populate prerequisites for lib{}. To do it for - // liba{} would require weeding out duplicates that are already in - // lib{}. - // - // Currently, this information is only used by the modules machinery to - // resolve module names to module files (but we cannot only do this if - // modules are enabled since the same installed library can be used by - // multiple builds). + // Load the information from the pkg-config files. // - prerequisites prs; - pkgconf apc; pkgconf spc; @@ -1471,6 +1482,58 @@ namespace build2 if (ps || ap.empty ()) spc = pkgconf (sp, pc_dirs, sys_lib_dirs, sys_hdr_dirs); + // Load user metadata if we are in the load phase. + // + // Note also that we are not failing here if the metadata was requested + // but not present (potentially only partially) letting the caller + // (i.e., the import machinery) verify that the export.metadata was set + // on the target being imported. This would also allow supporting + // optional metadata. + // + if (!act) + { + // We can only do it during the load phase. + // + assert (lt.ctx.phase == run_phase::load); + + pkgconf& ipc (ps ? spc : apc); // As below. + + // Since it's not easy to say if things are the same, we load a copy + // into the group and each member, if any. + // + // @@ TODO: check if already loaded? Don't we have the same problem + // below with reloading the rest for lt? What if we passed NULL + // in this case (and I suppose another bool in metaonly)? + // + if (optional md = ipc.variable ("build2.metadata")) + parse_metadata (lt, ipc, *md); + + if (pa) + if (optional md = apc.variable ("build2.metadata")) + parse_metadata (*at, apc, *md); + + if (ps) + if (optional md = spc.variable ("build2.metadata")) + parse_metadata (*st, spc, *md); + + // If we only need metadata, then we are done. + // + if (at != nullptr && metaonly.first) + { + pa = false; + at = nullptr; + } + + if (st != nullptr && metaonly.second) + { + ps = false; + st = nullptr; + } + + if (at == nullptr && st == nullptr) + return; + } + // Sort out the interface dependencies (which we are setting on lib{}). // If we have the shared .pc variant, then we use that. Otherwise -- // static but extract without the --static option (see also the saving @@ -1478,6 +1541,17 @@ namespace build2 // pkgconf& ipc (ps ? spc : apc); // Interface package info. + // For now we only populate prerequisites for lib{}. To do it for + // liba{} would require weeding out duplicates that are already in + // lib{}. + // + // Currently, this information is only used by the modules machinery to + // resolve module names to module files (but we cannot only do this if + // modules are enabled since the same installed library can be used by + // multiple builds). + // + prerequisites prs; + parse_libs ( lt, (ps ? st->mtime () : at->mtime ()) == timestamp_unreal /* binless */, @@ -1508,39 +1582,6 @@ namespace build2 } } - // Load user metadata, if requested. - // - // Note that we cannot just load if there is the metadata variable since - // this could be some third-party .pc file which happens to set the same - // variable. - // - // Note also that we are not failing here if the metadata was requested - // but not present (potentially only partially) letting the caller - // (i.e., the import machinery) verify that the export.metadata was set - // on the target being imported. This would also allow supporting - // optional metadata. - // - if (meta) - { - // We can only do it during the load phase. - // - assert (lt.ctx.phase == run_phase::load); - - // Since it's not easy to say if things are the same, we load a copy - // into the group and each member, if any. - // - if (optional md = ipc.variable ("metadata")) - parse_metadata (lt, ipc, *md); - - if (pa) - if (optional md = apc.variable ("metadata")) - parse_metadata (*at, apc, *md); - - if (ps) - if (optional md = spc.variable ("metadata")) - parse_metadata (*st, spc, *md); - } - // For now we assume static and shared variants export the same set of // modules/importable headers. While technically possible, having // different sets will most likely lead to all sorts of complications @@ -1574,7 +1615,7 @@ namespace build2 } bool common:: - pkgconfig_load (action, + pkgconfig_load (optional, const scope&, lib&, liba*, @@ -1584,13 +1625,13 @@ namespace build2 const dir_path&, const dir_paths&, const dir_paths&, - bool) const + pair) const { return false; } void common:: - pkgconfig_load (action, + pkgconfig_load (optional, const scope&, lib&, liba*, @@ -1599,7 +1640,7 @@ namespace build2 const dir_path&, const dir_paths&, const dir_paths&, - bool) const + pair) const { assert (false); // Should never be called. } @@ -2017,6 +2058,10 @@ namespace build2 // Save user metadata. // + // @@ Maybe we should use build2.metadata as a general indication + // of the metadata being present. Having only version would + // indicate absense of user metadata. + // { // The export.metadata value should start with the version followed // by the metadata variable prefix. @@ -2100,11 +2145,12 @@ namespace build2 append (*l.group, true); } - // First write the `metadata` variable with the version and all - // the variable names/types (which don't require any escaping). + // First write the build2.metadata variable with the version, + // prefix, and all the variable names/types (which should not + // require any escaping). // os << endl - << "metadata = " << ver; + << "build2.metadata = " << ver << ' ' << pfx; for (const binding& b: vars) { -- cgit v1.1