From a9663e16d5765a42175ce6131b8cae5ecc622b17 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 6 Apr 2016 07:46:43 +0200 Subject: Test and fix override logic --- build2/scope.cxx | 129 ++++++++----- tests/variable/override/build/bootstrap.build | 2 + tests/variable/override/buildfile | 58 ++++++ tests/variable/override/cache | 13 ++ tests/variable/override/p/build/bootstrap.build | 1 + tests/variable/override/p/buildfile | 49 +++++ tests/variable/override/simple | 3 + tests/variable/override/test.sh | 240 ++++++++++++++++++++++++ 8 files changed, 451 insertions(+), 44 deletions(-) create mode 100644 tests/variable/override/build/bootstrap.build create mode 100644 tests/variable/override/buildfile create mode 100644 tests/variable/override/cache create mode 100644 tests/variable/override/p/build/bootstrap.build create mode 100644 tests/variable/override/p/buildfile create mode 100644 tests/variable/override/simple create mode 100755 tests/variable/override/test.sh diff --git a/build2/scope.cxx b/build2/scope.cxx index ad251b5..3fd3c95 100644 --- a/build2/scope.cxx +++ b/build2/scope.cxx @@ -78,6 +78,10 @@ namespace build2 size_t depth (0); const variable_map* vars (nullptr); + // Root scope of a project from which our initial value comes. See below. + // + const scope* proj (nullptr); + // One special case is if the original is target-specific, which is the // most innermost. Or is it innermostest? // @@ -88,8 +92,9 @@ namespace build2 if (targetspec) { - vars = origl.vars; depth = origd; + vars = origl.vars; + proj = root_scope (); } else depth = 2; // For implied target-specific lookup. @@ -97,51 +102,53 @@ namespace build2 const scope* s; - // Return override value if it is present, applies, and ends with suffix. + // Return true if the override applies. Note that it expects vars and proj + // to be not NULL; if there is nothing "inner", then any override will + // still be "visible". // - auto find = [&vars, &s, this] (const variable* o, const char* sf = nullptr) - -> lookup + auto applies = [&vars, &s] (const variable* o, const scope* proj) -> bool { - if (sf != nullptr && o->name.rfind (sf) == string::npos) - return lookup (); - - // Next see if it would apply. If there is nothing "inner", then any - // override will still be "visible". - // - if (vars != nullptr) + switch (o->visibility) { - switch (o->visibility) - { - case variable_visibility::scope: - { - // Does not apply if the innermost value is not in this scope. - // - if (vars != &s->vars) - return lookup (); + case variable_visibility::scope: + { + // Does not apply if the innermost value is not in this scope. + // + if (vars != &s->vars) + return false; - break; - } - case variable_visibility::project: - { - // Does not apply if in a different project. - // - if (root_scope () != s->root_scope ()) - return lookup (); + break; + } + case variable_visibility::project: + { + // Does not apply if in a different project. + // + if (proj != s->root_scope ()) + return false; - break; - } - case variable_visibility::normal: - break; - } + break; + } + case variable_visibility::normal: + break; } + return true; + }; + + // Return override value if it is present and optionally ends with suffix. + // + auto find = [&s] (const variable* o, const char* sf = nullptr) -> lookup + { + if (sf != nullptr && o->name.rfind (sf) == string::npos) + return lookup (); + return lookup (s->vars.find (*o), &s->vars); }; // Return true if a value is from this scope (either target type/pattern- // specific or ordinary). // - auto test = [&s] (const lookup& l) -> bool + auto belongs = [&s] (const lookup& l) -> bool { for (auto& p1: s->target_vars) for (auto& p2: p1.second) @@ -163,10 +170,11 @@ namespace build2 // the target type/patter-specific variables, which is "more inner" than // normal scope variables (see find_original()). // - if (vars == nullptr && origl.defined () && test (origl)) + if (vars == nullptr && origl.defined () && belongs (origl)) { - vars = origl.vars; depth = origd; + vars = origl.vars; + proj = s->root_scope (); } if (vars == nullptr) @@ -178,6 +186,9 @@ namespace build2 o != nullptr; o = o->override.get ()) { + if (vars != 0 && !applies (o, proj)) + continue; + auto l (find (o)); if (l.defined ()) @@ -215,19 +226,21 @@ namespace build2 // Now find our "stem", that is the value to which we will be appending // suffixes and prepending prefixes. This is either the original or the - // __override, depending on which one is the innermost. We may also not - // have one at all. + // __override provided it applies. We may also not have either. // lookup stem (targetspec ? origl : lookup ()); for (s = this; s != nullptr; s = s->parent_scope ()) { + bool done (false); + // First check if the original is from this scope. // - if (origl.defined () && test (origl)) + if (origl.defined () && belongs (origl)) { stem = origl; - break; + proj = s->root_scope (); + // Keep searching. } // Then look for an __override that applies. @@ -236,13 +249,24 @@ namespace build2 o != nullptr; o = o->override.get ()) { - stem = find (o, ".__override"); + // If we haven't yet found anything, then any override will still be + // "visible" even if it doesn't apply. + // + if (stem.defined () && !applies (o, root_scope ())) + continue; - if (stem.defined ()) + auto l (find (o, ".__override")); + + if (l.defined ()) + { + stem = move (l); + proj = s->root_scope (); + done = true; break; + } } - if (stem.defined ()) + if (done) break; } @@ -283,18 +307,35 @@ namespace build2 o != nullptr; o = o->override.get ()) { + // First see if this override applies. This is actually tricky: what + // if the stem is a "visible" override from an outer project? + // Shouldn't its overrides apply? Sure sounds logical. So it seems + // we should use the project of the stem's scope and not the project + // of this scope. + // + if (proj != nullptr && !applies (o, proj)) + continue; + // Note that we keep override values as untyped names even if the // variable itself is typed. We also pass the original variable for // diagnostics. // - if (auto l = find (o, ".__prefix")) // No sense if NULL. + auto l (find (o, ".__prefix")); + + if (l) // No sense to prepend/append if NULL. { cache.value.prepend (names (cast (l)), var); } - else if (auto l = find (o, ".__suffix")) // No sense if NULL. + else if ((l = find (o, ".__suffix"))) { cache.value.append (names (cast (l)), var); } + + // If we had no stem, use the scope of the first override that applies + // as the project. + // + if (proj == nullptr && l.defined ()) + proj = s; } } diff --git a/tests/variable/override/build/bootstrap.build b/tests/variable/override/build/bootstrap.build new file mode 100644 index 0000000..1c2e239 --- /dev/null +++ b/tests/variable/override/build/bootstrap.build @@ -0,0 +1,2 @@ +project = override +amalgamation = # Disabled. diff --git a/tests/variable/override/buildfile b/tests/variable/override/buildfile new file mode 100644 index 0000000..18c9831 --- /dev/null +++ b/tests/variable/override/buildfile @@ -0,0 +1,58 @@ +if ("$t" != "") +{ + [$t] v = [null] +} + +print "/ :" $(/: v) + +if ($a == as) +{ + v = x +} +elif ($a == ap) +{ + v += s +} +elif ($a == pr) +{ + v =+ p +} + +print ". :" $v + +d/: +{ + if ($d_a == as) + { + v = x + } + elif ($d_a == ap) + { + v += s + } + elif ($d_a == pr) + { + v =+ p + } + + print "d :" $v + + + if ($d_t_a == as) + { + file{t}: v = x + } + elif ($d_t_a == ap) + { + file{t}: v += s + } + elif ($d_t_a == pr) + { + file{t}: v =+ p + } + + print "d/t :" $(file{t}: v) +} + +include p/ +./: diff --git a/tests/variable/override/cache b/tests/variable/override/cache new file mode 100644 index 0000000..8378688 --- /dev/null +++ b/tests/variable/override/cache @@ -0,0 +1,13 @@ +x = [string] 0 +print $x + +x = [uint64] 1 +print $x + +y = 0 +print $y + +[uint64] y = [null] +print $y + +./: diff --git a/tests/variable/override/p/build/bootstrap.build b/tests/variable/override/p/build/bootstrap.build new file mode 100644 index 0000000..723e2a3 --- /dev/null +++ b/tests/variable/override/p/build/bootstrap.build @@ -0,0 +1 @@ +project = override-p diff --git a/tests/variable/override/p/buildfile b/tests/variable/override/p/buildfile new file mode 100644 index 0000000..527b9ae --- /dev/null +++ b/tests/variable/override/p/buildfile @@ -0,0 +1,49 @@ +if ($p_a == as) +{ + v = x +} +elif ($p_a == ap) +{ + v += s +} +elif ($p_a == pr) +{ + v =+ p +} + +print "p :" $v + +d/: +{ + if ($p_d_a == as) + { + v = x + } + elif ($p_d_a == ap) + { + v += s + } + elif ($p_d_a == pr) + { + v =+ p + } + + print "p/d :" $v + + if ($p_d_t_a == as) + { + file{t}: v = x + } + elif ($p_d_t_a == ap) + { + file{t}: v += s + } + elif ($p_d_t_a == pr) + { + file{t}: v =+ p + } + + print "p/d/t :" $(file{t}: v) +} + +./: diff --git a/tests/variable/override/simple b/tests/variable/override/simple new file mode 100644 index 0000000..899daa2 --- /dev/null +++ b/tests/variable/override/simple @@ -0,0 +1,3 @@ +print $foo + +./: diff --git a/tests/variable/override/test.sh b/tests/variable/override/test.sh new file mode 100755 index 0000000..a195766 --- /dev/null +++ b/tests/variable/override/test.sh @@ -0,0 +1,240 @@ +#! /usr/bin/env bash + +verbose=n + +function error () { echo "$*" 1>&2; exit 1; } + +function fail () +{ + if [ "$verbose" = "y" ]; then + b $* + else + b -q $* 2>/dev/null + fi + + if [ $? -eq 0 ]; then + error "succeeded: b $*" + fi + + return 0 +} + +function test () +{ + # There is no way to get the exit code in process substitution + # so ruin the output. + # + diff -u - <(b -q $* || echo "") + + if [ $? -ne 0 ]; then + error "failed: b $*" + fi +} + +fail foo=bar[] # error: unexpected [ in variable assignment 'foo=bar[]' +fail foo=[string]bar # error: typed override of variable foo +fail "!foo=bar" "!foo=BAR" # error: multiple global overrides of variable foo +fail "foo=bar" "foo=BAR" # error: multiple project overrides of variable foo +fail "%foo=bar" "%foo=BAR" # error: multiple project overrides of variable foo + +test --buildfile simple foo=bar ./ ./ <<< "bar" # Multiple bootstraps of the same project. + +# Visibility. +# +test !v=X <