From 9810f0fd0cefdc5bd1541db4446318dfe24c027a Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 30 Jul 2021 16:12:07 +0200 Subject: Fix issue in amalgamation discovery --- libbuild2/file.cxx | 94 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 53 insertions(+), 41 deletions(-) diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 7c20152..4c88ed9 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -1022,6 +1022,11 @@ namespace build2 // which we convert to NULL below). When calculated, the NULL value // indicates that we are not amalgamated. // + // Before we used to assume that if there is an outer root scope, then + // that got to be our amalgamation. But it turns our this is not always + // the case (for example, a private host configuration in bpkg) and there + // could be an unbootstrapped project between us and an outer root scope. + // // Note: the amalgamation variable value is always a relative directory. // if (!simple) @@ -1032,66 +1037,73 @@ namespace build2 if (v && v.empty ()) // Convert empty to NULL. v = nullptr; - if (scope* ars = rs.parent_scope ()->root_scope ()) + scope* ars (rs.parent_scope ()->root_scope ()); + + if (rp.second) { - // We must not be amalgamated by a simple project. + // If the amalgamation variable hasn't been set, then we need to check + // if any of the outer directories is a project's out_root. If so, + // then that's (likely) our amalgamation. // - if (!ars->root_extra->project || *ars->root_extra->project != nullptr) - { - const dir_path& ad (ars->out_path ()); - dir_path rd (ad.relative (out_root)); + optional altn; + const dir_path& d (find_out_root (out_root.directory (), altn).first); - // If we already have the amalgamation variable set, verify that - // aroot matches its value. + if (!d.empty ()) + { + // Note that the sub() test is important: during configuration we + // may find a project that is outside the outer root scope in which + // case we should use the latter instead. // - if (!rp.second) + if (ars == nullptr || + (d != ars->out_path () && d.sub (ars->out_path ()))) { - if (v) - { - const dir_path& vd (cast (v)); - - if (vd != rd) - { - fail << "inconsistent amalgamation of " << out_root << - info << "specified: " << vd << - info << "actual: " << rd << " by " << ad; - } - } - } - else - { - // Otherwise, use the outer root as our amalgamation. - // + dir_path rd (d.relative (out_root)); l5 ([&]{trace << out_root << " amalgamated as " << rd;}); v = move (rd); + ars = nullptr; // Skip the checks blow. } + // Else fall through. } + else + assert (ars == nullptr); // Shouldn't find_out_root() have found it? } - else if (rp.second) + + // Do additional checks if the outer root could be our amalgamation. + // + if (ars != nullptr) { - // If there is no outer root and the amalgamation variable hasn't been - // set, then we need to check if any of the outer directories is a - // project's out_root. If so, then that's our amalgamation. + // We must not be amalgamated by a simple project. // - optional altn; - const dir_path& ad (find_out_root (out_root.directory (), altn).first); + bool simple (ars->root_extra->project && + *ars->root_extra->project == nullptr); + + const dir_path& ad (ars->out_path ()); + + // If we have the amalgamation variable set by the user, verify that + // it's a subdirectory of the outer root scope. + // + if (!rp.second) + { + if (v) + { + const dir_path& vd (cast (v)); + dir_path d (out_root / vd); + d.normalize (); - if (!ad.empty ()) + if (!d.sub (ad) || (simple && d == ad)) + fail << "incorrect amalgamation " << vd << " of " << out_root; + } + } + else if (!simple) { + // Otherwise, use the outer root as our amalgamation. + // dir_path rd (ad.relative (out_root)); + l5 ([&]{trace << out_root << " amalgamated as " << rd;}); v = move (rd); } - //@@ else: the value will be NULL and amalgamation will be disabled. - // We could omit setting it in root_extra... But maybe this is - // correct: we don't want to load half of the project as - // amalgamated and the other half as not, would we now? - } - // @@ else if (v): shouldn't we try to bootstrap a project in the - // user-specified directory? Though this case is not - // used outside of some controlled cases (like module - // sidebuilds). rs.root_extra->amalgamation = cast_null (v); } -- cgit v1.1