aboutsummaryrefslogtreecommitdiff
path: root/libbuild2
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2023-04-19 10:27:14 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2023-04-19 10:27:14 +0200
commitbc582eb32448bf7eb61ab3b1c76597885c8ec02a (patch)
tree64ed4501fac82b25e03e9f59a2d5a91a6450bcbc /libbuild2
parent7a995b778ab86c8ae63ba8f86950fd25e3ab5162 (diff)
Fix several issues in build system module importation logicmodule-import
Diffstat (limited to 'libbuild2')
-rw-r--r--libbuild2/context.cxx2
-rw-r--r--libbuild2/context.hxx12
-rw-r--r--libbuild2/file.cxx11
-rw-r--r--libbuild2/module.cxx282
-rw-r--r--libbuild2/module.hxx51
-rw-r--r--libbuild2/operation.cxx6
-rw-r--r--libbuild2/scope.hxx11
7 files changed, 275 insertions, 100 deletions
diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index 1bc294d..afbb59a 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -298,7 +298,7 @@ namespace build2
const strings& cmd_vars,
reserves res,
optional<context*> mc,
- const loaded_modules_lock* ml,
+ const module_libraries_lock* ml,
const function<var_override_function>& var_ovr_func)
: data_ (new data (*this)),
sched (&s),
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 5c426ad..b0fac02 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -21,7 +21,7 @@
namespace build2
{
class file_cache;
- class loaded_modules_lock;
+ class module_libraries_lock;
class LIBBUILD2_SYMEXPORT run_phase_mutex
{
@@ -142,9 +142,9 @@ namespace build2
// instead go the multiple communicating schedulers route, a la the job
// server).
//
- // The loaded_modules state (module.hxx) is shared among all the contexts
+ // The module_libraries state (module.hxx) is shared among all the contexts
// (there is no way to have multiple shared library loading "contexts") and
- // is protected by loaded_modules_lock. A nested context should normally
+ // is protected by module_libraries_lock. A nested context should normally
// inherit this lock value from its outer context.
//
// Note also that any given thread should not participate in multiple
@@ -648,9 +648,9 @@ namespace build2
dir_path old_src_root;
dir_path new_src_root;
- // NULL if this context hasn't already locked the loaded_modules state.
+ // NULL if this context hasn't already locked the module_libraries state.
//
- const loaded_modules_lock* modules_lock;
+ const module_libraries_lock* modules_lock;
// Nested context for updating build system modules and ad hoc recipes.
//
@@ -697,7 +697,7 @@ namespace build2
const strings& cmd_vars = {},
reserves = {0, 160},
optional<context*> module_context = nullptr,
- const loaded_modules_lock* inherited_mudules_lock = nullptr,
+ const module_libraries_lock* inherited_modules_lock = nullptr,
const function<var_override_function>& = nullptr);
// Special context with bare minimum of initializations. It is only
diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx
index acfb5d7..99eab96 100644
--- a/libbuild2/file.cxx
+++ b/libbuild2/file.cxx
@@ -1337,9 +1337,9 @@ namespace build2
// Call module's post-boot functions.
//
- for (size_t i (0); i != root.root_extra->modules.size (); ++i)
+ for (size_t i (0); i != root.root_extra->loaded_modules.size (); ++i)
{
- module_state& s (root.root_extra->modules[i]);
+ module_state& s (root.root_extra->loaded_modules[i]);
if (s.boot_post != nullptr)
boot_post_module (root, s);
@@ -1556,11 +1556,11 @@ namespace build2
// Note that init() can load additional modules invalidating iterators.
//
auto init_modules =
- [&root, n = root.root_extra->modules.size ()] (module_boot_init v)
+ [&root, n = root.root_extra->loaded_modules.size ()] (module_boot_init v)
{
for (size_t i (0); i != n; ++i)
{
- module_state& s (root.root_extra->modules[i]);
+ module_state& s (root.root_extra->loaded_modules[i]);
if (s.boot_init && *s.boot_init == v)
init_module (root, root, s.name, s.loc);
@@ -2085,6 +2085,9 @@ namespace build2
// Return empty name if an ad hoc import resulted in a NULL target (only
// allowed if optional is true).
//
+ // Note that this function has a side effect of potentially marking some
+ // config.import.* variables as used.
+ //
pair<name, optional<dir_path>>
import_search (bool& new_value,
scope& ibase,
diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx
index 4c1271c..dd83225 100644
--- a/libbuild2/module.cxx
+++ b/libbuild2/module.cxx
@@ -30,21 +30,20 @@ using namespace butl;
namespace build2
{
- mutex loaded_modules_lock::mutex_;
+ mutex module_libraries_lock::mutex_;
- loaded_module_map loaded_modules;
+ module_libraries_map module_libraries;
void
load_builtin_module (module_load_function* lf)
{
for (const module_functions* i (lf ()); i->name != nullptr; ++i)
- loaded_modules[i->name] = i;
+ module_libraries.emplace (i->name, module_library {*i, dir_path ()});
}
// Sorted array of bundled modules (excluding core modules bundled with
// libbuild2; see below).
//
-#if !defined(BUILD2_BOOTSTRAP) && !defined(LIBBUILD2_STATIC_BUILD)
static const char* bundled_modules[] = {
"bash",
"bin",
@@ -64,7 +63,6 @@ namespace build2
bundled_modules + sizeof (bundled_modules) / sizeof (*bundled_modules),
mod);
}
-#endif
// Note: also used by ad hoc recipes thus not static.
//
@@ -244,11 +242,20 @@ namespace build2
}
#endif
- static module_load_function*
+ // Return the module functions as well as the module project directory or
+ // empty if not imported from project. Return {nullptr, nullopt} if not
+ // found.
+ //
+ // The dry-run mode only calls import_search() and always returns NULL for
+ // module functions (see below for background).
+ //
+ static pair<module_load_function*, optional<dir_path>>
import_module (
#if defined(BUILD2_BOOTSTRAP) || defined(LIBBUILD2_STATIC_BUILD)
+ bool,
scope&,
#else
+ bool dry_run,
scope& bs,
#endif
const string& mod,
@@ -262,15 +269,21 @@ namespace build2
{
tracer trace ("import_module");
+ pair<module_load_function*, optional<dir_path>> r (nullptr, nullopt);
+
// Take care of core modules that are bundled with libbuild2 in case they
// are not pre-loaded by the driver.
//
- if (mod == "config") return &config::build2_config_load;
- else if (mod == "dist") return &dist::build2_dist_load;
- else if (mod == "install") return &install::build2_install_load;
- else if (mod == "test") return &test::build2_test_load;
+ if (mod == "config") r.first = &config::build2_config_load;
+ else if (mod == "dist") r.first = &dist::build2_dist_load;
+ else if (mod == "install") r.first = &install::build2_install_load;
+ else if (mod == "test") r.first = &test::build2_test_load;
- module_load_function* r (nullptr);
+ if (r.first != nullptr)
+ {
+ r.second = dir_path ();
+ return r;
+ }
// No dynamic loading of build system modules during bootstrap or if
// statically-linked..
@@ -339,7 +352,7 @@ namespace build2
// and undefined if the module was not mentioned.
//
if (boot && !bundled && ctx.no_external_modules)
- return nullptr;
+ return r; // NULL
// See if we can import a target for this module.
//
@@ -394,7 +407,7 @@ namespace build2
if (ir.first.empty ())
{
assert (opt);
- return nullptr;
+ return r; // NULL
}
if (ir.second)
@@ -402,6 +415,8 @@ namespace build2
// What if a module is specified with config.import.<mod>.<lib>.libs?
// Note that this could still be a project-qualified target.
//
+ // Note: we now return an empty directory to mean something else.
+ //
if (ir.second->empty ())
fail (loc) << "direct module target importation not yet supported";
@@ -409,6 +424,17 @@ namespace build2
// the target (which will also give us the shared library path).
//
l5 ([&]{trace << "found " << ir.first << " in " << *ir.second;});
+ }
+
+ if (dry_run)
+ {
+ r.second = ir.second ? move (*ir.second) : dir_path ();
+ return r;
+ }
+
+ if (ir.second)
+ {
+ r.second = *ir.second;
// Create the build context if necessary.
//
@@ -421,7 +447,7 @@ namespace build2
create_module_context (ctx, loc);
}
- // Inherit loaded_modules lock from the outer context.
+ // Inherit module_libraries lock from the outer context.
//
ctx.module_context->modules_lock = ctx.modules_lock;
@@ -481,6 +507,8 @@ namespace build2
}
else
{
+ r.second = dir_path ();
+
// No module project found. Form the shared library name (incorporating
// build system core version) and try using system-default search
// (installed, rpath, etc).
@@ -523,7 +551,7 @@ namespace build2
fail (loc) << "unable to lookup " << sym << " in build system module "
<< mod << " (" << lib << "): " << err;
- r = function_cast<module_load_function*> (hs.second);
+ r.first = function_cast<module_load_function*> (hs.second);
}
else if (!opt)
{
@@ -535,7 +563,10 @@ namespace build2
<< "line variable to specify its project out_root";
}
else
+ {
+ r.second = nullopt;
l5 ([&]{trace << "unable to load " << lib << ": " << err;});
+ }
#endif // BUILD2_BOOTSTRAP || LIBBUILD2_STATIC_BUILD
@@ -551,89 +582,200 @@ namespace build2
{
tracer trace ("find_module");
- // Note that we hold the lock for the entire time it takes to build a
- // module.
+ // If this is a submodule, get the main module name.
//
- loaded_modules_lock lock (bs.ctx);
+ string mmod (smod, 0, smod.find ('.'));
- // Optional modules and submodules sure make this logic convoluted. So we
- // divide it into two parts: (1) find or insert an entry (for submodule
- // or, failed that, for the main module, the latter potentially NULL) and
- // (2) analyze the entry and issue diagnostics.
+ // We have a somewhat strange two-level caching in imported_modules
+ // and module_libraries in order to achieve the following:
+ //
+ // 1. Correctly handle cases where a module can be imported from one
+ // project but not the other.
+ //
+ // 2. Make sure that for each project that imports the module we actually
+ // call import_search() in order to mark any config.import.* as used.
//
- auto i (loaded_modules.find (smod)), e (loaded_modules.end ());
+ // 3. Make sure that all the projects import the same module.
+ //
+ scope& rs (*bs.root_scope ());
+
+ const string* mod;
+ const module_functions* fun;
- if (i == e)
+ // First check the project's imported_modules in case this (main) module
+ // is known to be not found.
+ //
+ auto j (rs.root_extra->imported_modules.find (mmod));
+ auto je (rs.root_extra->imported_modules.end ());
+
+ if (j != je && !j->found)
+ {
+ mod = &mmod;
+ fun = nullptr;
+ }
+ else
{
- // If this is a submodule, get the main module name.
+ // Note that we hold the lock for the entire time it takes to build a
+ // module.
//
- string mmod (smod, 0, smod.find ('.'));
+ module_libraries_lock lock (bs.ctx);
- if (mmod != smod)
- i = loaded_modules.find (mmod);
+ // Optional modules and submodules sure make this logic convoluted. So
+ // we divide it into two parts: (1) find or insert an entry (for
+ // submodule or, failed that, for the main module) and (2) analyze the
+ // entry and issue diagnostics.
+ //
+ auto i (module_libraries.find (smod));
+ auto ie (module_libraries.end ());
- if (i == e)
+ bool imported (false);
+ if (i == ie)
{
- module_load_function* f (import_module (bs, mmod, loc, boot, opt));
+ if (mmod != smod)
+ i = module_libraries.find (mmod);
- if (f != nullptr)
+ if (i == ie)
{
- // Enter all the entries noticing which one is our submodule. If
- // none are, then we notice the main module.
- //
- for (const module_functions* j (f ()); j->name != nullptr; ++j)
+ pair<module_load_function*, optional<dir_path>> ir (
+ import_module (false /* dry_run */, bs, mmod, loc, boot, opt));
+
+ if (module_load_function* f = ir.first)
{
- const string& n (j->name);
+ // Enter all the entries noticing which one is our submodule. If
+ // none are, then we notice the main module.
+ //
+ for (const module_functions* j (f ()); j->name != nullptr; ++j)
+ {
+ const string& n (j->name);
- l5 ([&]{trace << "registering " << n;});
+ l5 ([&]{trace << "registering " << n;});
- auto p (loaded_modules.emplace (n, j));
+ bool main (n == mmod);
- if (!p.second)
- fail (loc) << "build system submodule name " << n << " of main "
- << "module " << mmod << " is already in use";
+ auto p (module_libraries.emplace (
+ n,
+ module_library {
+ *j,
+ main ? move (*ir.second) : dir_path ()}));
- if (n == smod || (i == e && n == mmod))
- i = p.first;
+ if (!p.second)
+ fail (loc) << "build system submodule name " << n << " of main "
+ << "module " << mmod << " is already in use";
+
+ // Note: this assumes the main module is last.
+ //
+ if (n == smod || (main && i == ie))
+ i = p.first;
+ }
+
+ // We should at least have the main module.
+ //
+ if (i == ie)
+ fail (loc) << "invalid function list in build system module "
+ << mmod;
}
- // We should at least have the main module.
- //
- if (i == e)
- fail (loc) << "invalid function list in build system module "
- << mmod;
+ imported = true;
}
- else
- i = loaded_modules.emplace (move (mmod), nullptr).first;
+ }
+
+ // Now the iterator points to a submodule or to the main module, or to
+ // end if neither is found.
+ //
+ assert (j == je || i != ie); // Cache state consistecy sanity check.
+
+ if (i != ie)
+ {
+ // Note: these should remain stable after we release the lock.
+ //
+ mod = &i->first;
+ fun = &i->second.functions.get ();
+
+ // If this project hasn't imported this main module and we found the
+ // entry in the cache, then we have to perform the import_search()
+ // part of import_module() in order to cover items (2) and (3) above.
+ //
+ // There is one nuance: omit this for bundled modules since it's
+ // possible to first import them ad hoc and then, if we call
+ // import_search() again, to find them differently (e.g., as a
+ // subproject).
+ //
+ if (j == je && !imported && !bundled_module (mmod))
+ {
+ pair<module_load_function*, optional<dir_path>> ir (
+ import_module (true /* dry_run */, bs, mmod, loc, boot, opt));
+
+ if (ir.second)
+ {
+ if (i->first != mmod)
+ {
+ i = module_libraries.find (mmod);
+ assert (i != ie); // Has to be there.
+ }
+
+ const dir_path& cd (*ir.second);
+ const dir_path& pd (i->second.import_path);
+
+ if (cd != pd)
+ {
+ fail (loc) << "inconsistent build system module " << mmod
+ << " importation" <<
+ info << rs << " imports it as "
+ << (cd.empty () ? "ad hoc" : cd.representation ().c_str ()) <<
+ info << "previously imported as "
+ << (pd.empty () ? "ad hoc" : pd.representation ().c_str ());
+ }
+ }
+ else
+ {
+ // This module is not found from this project.
+ //
+ mod = &mmod;
+ fun = nullptr;
+ }
+ }
+ }
+ else
+ {
+ mod = &mmod;
+ fun = nullptr;
}
}
+ // Cache the result in imported_modules if necessary.
+ //
+ if (j == je)
+ rs.root_extra->imported_modules.push_back (
+ module_import {mmod, fun != nullptr});
+
// Reduce skipped external module to optional.
//
- if (boot && i->second == nullptr)
+ if (boot && fun == nullptr)
opt = true;
- // Now the iterator points to a submodule or to the main module, the
- // latter potentially NULL.
+ // Handle optional.
//
- if (!opt)
+ if (fun == nullptr)
{
- if (i->second == nullptr)
- {
- fail (loc) << "unable to load build system module " << i->first;
- }
- else if (i->first != smod)
- {
- fail (loc) << "build system module " << i->first << " has no "
+ if (!opt)
+ fail (loc) << "unable to load build system module " << *mod;
+ }
+ else if (*mod != smod)
+ {
+ if (!opt)
+ fail (loc) << "build system module " << *mod << " has no "
<< "submodule " << smod;
+ else
+ {
+ // Note that if the main module exists but has no such submodule, we
+ // return NULL rather than fail (think of an older version of a module
+ // that doesn't implement some extra functionality).
+ //
+ fun = nullptr;
}
}
- // Note that if the main module exists but has no such submodule, we
- // return NULL rather than fail (think of an older version of a module
- // that doesn't implement some extra functionality).
- //
- return i->second;
+ return fun;
}
void
@@ -641,7 +783,7 @@ namespace build2
{
// First see if this modules has already been booted for this project.
//
- module_map& lm (rs.root_extra->modules);
+ module_state_map& lm (rs.root_extra->loaded_modules);
auto i (lm.find (mod));
if (i != lm.end ())
@@ -717,7 +859,7 @@ namespace build2
{
// First see if this modules has already been inited for this project.
//
- module_map& lm (rs.root_extra->modules);
+ module_state_map& lm (rs.root_extra->loaded_modules);
auto i (lm.find (mod));
bool f (i == lm.end ());
@@ -837,7 +979,7 @@ namespace build2
if (cast_false<bool> (bs[name + ".loaded"]))
{
if (cast_false<bool> (bs[name + ".configured"]))
- return rs.root_extra->modules.find (name)->module;
+ return rs.root_extra->loaded_modules.find (name)->module;
}
else
{
@@ -859,7 +1001,7 @@ namespace build2
// attempt to load it was optional?
return cast_false<bool> (bs[name + ".loaded"])
- ? rs.root_extra->modules.find (name)->module
+ ? rs.root_extra->loaded_modules.find (name)->module
: init_module (rs, bs, name, loc, false /* optional */, hints)->module;
}
}
diff --git a/libbuild2/module.hxx b/libbuild2/module.hxx
index 2f2d8a7..c638c70 100644
--- a/libbuild2/module.hxx
+++ b/libbuild2/module.hxx
@@ -161,7 +161,31 @@ namespace build2
extern "C"
using module_load_function = const module_functions* ();
- // Module state.
+ // Imported module state.
+ //
+ // The module name is the main module (corresponding to the library). If
+ // found is false then this module could not be imported from this project.
+ //
+ struct module_import
+ {
+ const string name;
+ bool found;
+ };
+
+ struct module_import_map: vector<module_import>
+ {
+ iterator
+ find (const string& name)
+ {
+ return find_if (
+ begin (), end (),
+ [&name] (const module_import& i) {return i.name == name;});
+ }
+ };
+
+ // Loaded module state.
+ //
+ // Note that unlike import_state, the module name here could be a submodule.
//
struct module_state
{
@@ -173,7 +197,7 @@ namespace build2
optional<module_boot_init> boot_init;
};
- struct module_map: vector<module_state>
+ struct module_state_map: vector<module_state>
{
iterator
find (const string& name)
@@ -274,23 +298,28 @@ namespace build2
return static_cast<T&> (*load_module (root, base, name, l, config_hints));
}
- // Loaded modules (as in libraries).
+ // Loaded module libraries.
//
- // A NULL entry for the main module indicates that a module library was not
- // found.
+ // Note that this map contains entries for all the submodules.
//
- using loaded_module_map = map<string, const module_functions*>;
+ struct module_library
+ {
+ reference_wrapper<const module_functions> functions;
+ dir_path import_path; // Only for main module.
+ };
+
+ using module_libraries_map = map<string, module_library>;
- // The loaded_modules map is locked per top-level (as opposed to nested)
+ // The module_libraries map is locked per top-level (as opposed to nested)
// context (see context.hxx for details).
//
// Note: should only be constructed during contexts-wide serial execution.
//
- class LIBBUILD2_SYMEXPORT loaded_modules_lock
+ class LIBBUILD2_SYMEXPORT module_libraries_lock
{
public:
explicit
- loaded_modules_lock (context& c)
+ module_libraries_lock (context& c)
: ctx_ (c), lock_ (mutex_, defer_lock)
{
if (ctx_.modules_lock == nullptr)
@@ -300,7 +329,7 @@ namespace build2
}
}
- ~loaded_modules_lock ()
+ ~module_libraries_lock ()
{
if (ctx_.modules_lock == this)
ctx_.modules_lock = nullptr;
@@ -312,7 +341,7 @@ namespace build2
mlock lock_;
};
- LIBBUILD2_SYMEXPORT extern loaded_module_map loaded_modules;
+ LIBBUILD2_SYMEXPORT extern module_libraries_map modules_libraries;
// Load a builtin module (i.e., a module linked as a static/shared library
// or that is part of the build system driver).
diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx
index d84db4f..e57caa5 100644
--- a/libbuild2/operation.cxx
+++ b/libbuild2/operation.cxx
@@ -952,7 +952,7 @@ namespace build2
//
auto print_mods = [&rs] ()
{
- for (const module_state& ms: rs.root_extra->modules)
+ for (const module_state& ms: rs.root_extra->loaded_modules)
cout << ' ' << ms.name;
};
@@ -1123,12 +1123,12 @@ namespace build2
// Print modules.
//
- if (!rs.root_extra->modules.empty ())
+ if (!rs.root_extra->loaded_modules.empty ())
{
s.member_name ("modules", false /* check */);
s.begin_array ();
- for (const module_state& ms: rs.root_extra->modules)
+ for (const module_state& ms: rs.root_extra->loaded_modules)
s.value (ms.name, false /* check */);
s.end_array ();
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 0eabd17..5578d0c 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -510,9 +510,10 @@ namespace build2
build2::meta_operations meta_operations;
build2::operations operations;
- // Modules loaded by this project.
+ // Modules imported/loaded by this project.
//
- module_map modules;
+ module_import_map imported_modules;
+ module_state_map loaded_modules;
// Buildfiles already loaded for this project.
//
@@ -591,21 +592,21 @@ namespace build2
bool
find_module (const string& name) const
{
- return root_extra->modules.find_module<module> (name) != nullptr;
+ return root_extra->loaded_modules.find_module<module> (name) != nullptr;
}
template <typename T>
T*
find_module (const string& name)
{
- return root_extra->modules.find_module<T> (name);
+ return root_extra->loaded_modules.find_module<T> (name);
}
template <typename T>
const T*
find_module (const string& name) const
{
- return root_extra->modules.find_module<T> (name);
+ return root_extra->loaded_modules.find_module<T> (name);
}
public: