aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2020-07-06 10:22:12 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2020-07-06 10:22:12 +0200
commit9a9276e1d151910ba31db4d91521b41e5ea1435f (patch)
treea4bf5d320c6c1c6f5d98e00a2620c2bb8a33fb3e
parent736812cb586e0f80742337dce802ad24adf331ad (diff)
Adjust variable block applicability in dependency chains
Before the block used to apply to the set of prerequisites before the last `:`. This turned out to be counterintuitive and not very useful since prerequisite-specific variables are a lot less common than target specific. And it doesn't fit with ad hoc recipes. The new rule is if the chain ends with `:`, then the block applies to the last set of prerequisites. Otherwise, it applies to the last set of targets. For example: ./: exe{test}: cxx{main} { test = true # Applies to the exe{test} target. } ./: exe{test}: libue{test}: { bin.whole = false # Applies to the libue{test} prerequisite. } This is actually consistent with both non-chain and non-block cases. Consider: exe{test}: cxx{main} { test = true } exe{test}: libue{test}: { bin.whole = false } exe{test}: libue{test}: bin.whole = false The only exception we now have in this overall approach of "if the dependency declaration ends with a colon, then what follows is for a prerequisite" is for the first semicolon: exe{test}: { test = true } exe{test}: test = true But that's probably intuitive enough since there cannot be a prerequisite without a target.
-rw-r--r--doc/manual.cli12
-rw-r--r--libbuild2/parser.cxx228
-rw-r--r--libbuild2/parser.hxx5
-rw-r--r--tests/dependency/chain/testscript49
-rw-r--r--tests/dependency/recipe/testscript46
5 files changed, 228 insertions, 112 deletions
diff --git a/doc/manual.cli b/doc/manual.cli
index cf2ec3a..ba0aa63 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -3259,11 +3259,17 @@ h{config}: in{config}
}
\
-In case of a dependency chain, the block applies to the set of prerequisites
-(note: \i{not targets}) before last \c{:}. For example:
+In case of a dependency chain, if the chain ends with a colon (\c{:}), then
+the block applies to the last set of prerequisites. Otherwise, it applies to
+the last set of targets. For example:
\
-./: exe{test}: libue{test}: cxx{test}
+./: exe{test}: cxx{main}
+{
+ test = true # Applies to the exe{test} target.
+}
+
+./: exe{test}: libue{test}:
{
bin.whole = false # Applies to the libue{test} prerequisite.
}
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index 21b5794..b6004de 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -706,8 +706,18 @@ namespace build2
// Note that we cannot just let parse_dependency() handle this case
// because we can have (a mixture of) target type/patterns.
//
- // @@ This might change once we support ad hoc rules (where we may
- // have prerequisites for a pattern; but perhaps this should be
+ // Also, it handles the exception to the rule that if a dependency
+ // declaration ends with a colon, then the variable assignment/block
+ // that follows is for the prerequisite. Compare:
+ //
+ // foo: x = y foo: fox: x = y
+ // bar: bar: baz:
+ // { {
+ // x = y x = y
+ // } }
+ //
+ // @@ This might change a bit once we support ad hoc rules (where we
+ // may have prerequisites for a pattern; but perhaps this should be
// handled separately since the parse_dependency() is already too
// complex and there will be no chains in this case).
//
@@ -808,6 +818,9 @@ namespace build2
const variable& var (parse_variable_name (move (pns), ploc));
apply_variable_attributes (var);
+ // If variable visibility ends before, then it doesn't make sense
+ // to assign it in this context.
+ //
if (var.visibility > variable_visibility::target)
{
fail (nloc) << "variable " << var << " has " << var.visibility
@@ -841,11 +854,10 @@ namespace build2
else
attributes_pop ();
- bool r (parse_dependency (t, tt,
- move (ns), nloc,
- move (ans),
- move (pns), ploc));
- assert (r); // Block must have been claimed.
+ parse_dependency (t, tt,
+ move (ns), nloc,
+ move (ans),
+ move (pns), ploc);
}
continue;
@@ -899,7 +911,7 @@ namespace build2
const variable& var (parse_variable_name (move (ns), nloc));
apply_variable_attributes (var);
- if (var.visibility >= variable_visibility::target)
+ if (var.visibility > variable_visibility::scope)
{
diag_record dr (fail (nloc));
@@ -1021,7 +1033,7 @@ namespace build2
const variable& var (parse_variable_name (move (ns), nloc));
apply_variable_attributes (var);
- if (prerequisite_ != nullptr &&
+ if (prerequisite_ == nullptr &&
var.visibility > variable_visibility::target)
{
fail (t) << "variable " << var << " has " << var.visibility
@@ -1565,30 +1577,17 @@ namespace build2
return tgs;
}
- bool parser::
+ void parser::
parse_dependency (token& t, token_type& tt,
names&& tns, const location& tloc, // Target names.
adhoc_names&& ans, // Ad hoc target names.
- names&& pns, const location& ploc, // Prereq names.
- bool chain)
+ names&& pns, const location& ploc) // Prereq names.
{
// Parse a dependency chain and/or a target/prerequisite-specific variable
- // assignment/block and/or recipe block(s). Return true if the following
- // block(s) (if any) have been "claimed", meaning they "belong" to
- // targets/prerequisites before the last colon.
- //
- // enter: colon (anything else is not handled)
- // leave: - first token on the next line if returning true
- // - newline (presumably, must be verified) if returning false
- //
- // Note that top-level call (with chain == false) is expected to always
- // return true.
- //
- // This dual-return "complication" is necessary to handle non-block cases
- // like this:
+ // assignment/block and/or recipe block(s).
//
- // foo: bar
- // {hxx ixx}: install = true
+ // enter: colon or newline (latter only in recursive calls)
+ // leave: - first token on the next line
//
tracer trace ("parser::parse_dependency", &path_);
@@ -1700,17 +1699,18 @@ namespace build2
};
// Do we have a dependency chain and/or prerequisite-specific variable
- // assignment? If not, check for the target-specific variable block and/or
- // recipe block(s) unless this is a chained call (in which case the block,
- // if any, "belongs" to prerequisites).
+ // assignment/block? If not, check for the target-specific variable block
+ // and/or recipe block(s).
//
if (tt != type::colon)
{
- if (chain)
- return false;
-
next_after_newline (t, tt); // Must be a newline then.
+ // Note: watch out for non-block cases like this:
+ //
+ // foo: bar
+ // {hxx ixx}: install = true
+ //
if (tt == type::percent ||
tt == type::multi_lcbrace ||
(tt == type::lcbrace && peek () == type::newline))
@@ -1755,7 +1755,7 @@ namespace build2
for_each_t (parse);
}
- return true; // Claimed or isn't any.
+ return;
}
// What should we do if there are no prerequisites (for example, because
@@ -1773,97 +1773,113 @@ namespace build2
auto at (attributes_push (t, tt));
- // @@ PAT: currently we pattern-expand prerequisite-specific vars.
+ // If we are here, then this can be one of three things:
//
- const location loc (get_location (t));
- names ns (tt != type::newline && tt != type::eos
- ? parse_names (t, tt, pattern_mode::expand)
- : names ());
-
- // Prerequisite-specific variable assignment.
+ // 1. A prerequisite-specific variable bloc:
//
- if (tt == type::assign || tt == type::prepend || tt == type::append)
+ // foo: bar:
+ // {
+ // x = y
+ // }
+ //
+ // 2. A prerequisite-specific variable asignment:
+ //
+ // foo: bar: x = y
+ //
+ // 3. A further dependency chain :
+ //
+ // foo: bar: baz ...
+ //
+ if (tt == type::newline || tt == type::eos)
{
- type at (tt);
-
- const variable& var (parse_variable_name (move (ns), loc));
- apply_variable_attributes (var);
+ attributes_pop (); // Must be none since can't be standalone.
- // Parse the assignment for each prerequisites of each target.
+ // There must be a block.
//
- for_each_p ([this, &var, at] (token& t, token_type& tt)
- {
- parse_variable (t, tt, var, at);
- });
+ if (next_after_newline (t, tt) != type::lcbrace)
+ fail (t) << "expected '{' instead of " << t;
- // Pretend that we have claimed the block to cause an error if there is
- // one. Failed that, the following would result in a valid (target-
- // specific) block:
- //
- // foo: bar: x = y
- // {
- // ...
- // }
+ if (next (t, tt) != type::newline)
+ fail (t) << "expected newline after '{'";
+
+ // Parse the block for each prerequisites of each target.
//
- next_after_newline (t, tt);
- return true;
+ for_each_p ([this] (token& t, token_type& tt)
+ {
+ next (t, tt); // First token inside the block.
+
+ parse_variable_block (t, tt);
+
+ if (tt != type::rcbrace)
+ fail (t) << "expected '}' instead of " << t;
+ });
+
+ next (t, tt); // Presumably newline after '}'.
+ next_after_newline (t, tt, '}'); // Should be on its own line.
}
- //
- // Dependency chain.
- //
else
{
- if (at.first)
- fail (at.second) << "attributes before prerequisites";
- else
- attributes_pop ();
-
- // Note that we could have "pre-resolved" these prerequisites to actual
- // targets or, at least, made their directories absolute. We don't do it
- // for ease of documentation: with the current semantics we can just say
- // that the dependency chain is equivalent to specifying each dependency
- // separately.
- //
- // Also note that supporting ad hoc target group specification in chains
- // will be complicated. For example, what if prerequisites that have ad
- // hoc targets don't end up being chained? Do we just silently drop
- // them? Also, these are prerequsites first that happened to be reused
- // as target names so perhaps it is the right thing not to support,
- // conceptually.
+ // @@ PAT: currently we pattern-expand prerequisite-specific vars.
//
- if (parse_dependency (t, tt,
- names (pns), ploc, // Note: can't move.
- {} /* ad hoc target name */,
- move (ns), loc,
- true /* chain */))
- return true;
+ const location loc (get_location (t));
+ names ns (parse_names (t, tt, pattern_mode::expand));
- // Claim the block (if any) for these prerequisites if it hasn't been
- // claimed by the inner ones.
+ // Prerequisite-specific variable assignment.
//
- next_after_newline (t, tt); // Must be a newline.
-
- if (tt == type::lcbrace && peek () == type::newline)
+ if (tt == type::assign || tt == type::prepend || tt == type::append)
{
- next (t, tt); // Newline.
+ type at (tt);
+
+ const variable& var (parse_variable_name (move (ns), loc));
+ apply_variable_attributes (var);
- // Parse the block for each prerequisites of each target.
+ // Parse the assignment for each prerequisites of each target.
//
- for_each_p ([this] (token& t, token_type& tt)
+ for_each_p ([this, &var, at] (token& t, token_type& tt)
{
- next (t, tt); // First token inside the block.
-
- parse_variable_block (t, tt);
-
- if (tt != type::rcbrace)
- fail (t) << "expected '}' instead of " << t;
+ parse_variable (t, tt, var, at);
});
- next (t, tt); // Presumably newline after '}'.
- next_after_newline (t, tt, '}'); // Should be on its own line.
+ next_after_newline (t, tt);
+
+ // Check we don't also have a variable block:
+ //
+ // foo: bar: x = y
+ // {
+ // ...
+ // }
+ //
+ if (tt == type::lcbrace && peek () == type::newline)
+ fail (t) << "variable assignment block after variable assignment";
}
+ //
+ // Dependency chain.
+ //
+ else
+ {
+ if (at.first)
+ fail (at.second) << "attributes before prerequisites";
+ else
+ attributes_pop ();
- return true; // Claimed or isn't any.
+ // Note that we could have "pre-resolved" these prerequisites to
+ // actual targets or, at least, made their directories absolute. We
+ // don't do it for ease of documentation: with the current semantics
+ // we just say that the dependency chain is equivalent to specifying
+ // each dependency separately.
+ //
+ // Also note that supporting ad hoc target group specification in
+ // chains will be complicated. For example, what if prerequisites that
+ // have ad hoc targets don't end up being chained? Do we just silently
+ // drop them? Also, these are prerequsites first that happened to be
+ // reused as target names so perhaps it is the right thing not to
+ // support, conceptually.
+ //
+ parse_dependency (t, tt,
+ move (pns), ploc,
+ {} /* ad hoc target name */,
+ move (ns), loc);
+ }
}
}
@@ -2590,7 +2606,7 @@ namespace build2
{
apply_variable_attributes (*var);
- if (var->visibility >= variable_visibility::target)
+ if (var->visibility > variable_visibility::scope)
{
fail (vloc) << "variable " << *var << " has " << var->visibility
<< " visibility but is assigned in import";
@@ -3323,7 +3339,7 @@ namespace build2
const variable& var (parse_variable_name (move (vns), vloc));
apply_variable_attributes (var);
- if (var.visibility >= variable_visibility::target)
+ if (var.visibility > variable_visibility::scope)
{
fail (vloc) << "variable " << var << " has " << var.visibility
<< " visibility but is assigned in for-loop";
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index 2db7ade..24a1ac1 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -152,12 +152,11 @@ namespace build2
small_vector<reference_wrapper<target>, 1>
enter_targets (names&&, const location&, adhoc_names&&, size_t);
- bool
+ void
parse_dependency (token&, token_type&,
names&&, const location&,
adhoc_names&&,
- names&&, const location&,
- bool = false);
+ names&&, const location&);
void
parse_assert (token&, token_type&);
diff --git a/tests/dependency/chain/testscript b/tests/dependency/chain/testscript
index ac4a946..a24ace3 100644
--- a/tests/dependency/chain/testscript
+++ b/tests/dependency/chain/testscript
@@ -36,3 +36,52 @@ EOI
% .+/dir\{y/\}: .+:dir\{a/\} .+:dir\{b/\}%
EOE
+
+: var-prereq
+:
+$* <<EOI 2>>/~%EOE%
+./: dir{x}: dir{a}: x = y
+dump dir{x}
+EOI
+<stdin>:2:1: dump:
+% .+/dir\{x/\}: .+:dir\{a/\}:%
+ {
+ x = y
+ }
+EOE
+
+: var-prereq-block
+:
+$* <<EOI 2>>/~%EOE%
+./: dir{x}: dir{a}:
+{
+ x = y
+ z = x
+}
+dump dir{x}
+EOI
+<stdin>:6:1: dump:
+% .+/dir\{x/\}: .+:dir\{a/\}:%
+ {
+ x = y
+ z = x
+ }
+EOE
+
+: var-target-block
+:
+$* <<EOI 2>>/~%EOE%
+./: dir{x}: dir{a}
+{
+ x = y
+ z = x
+}
+dump dir{x}
+EOI
+<stdin>:6:1: dump:
+% .+/dir\{x/\}: .+:dir\{a/\}%
+ {
+ x = y
+ z = x
+ }
+EOE
diff --git a/tests/dependency/recipe/testscript b/tests/dependency/recipe/testscript
index bd33bcc..43ec514 100644
--- a/tests/dependency/recipe/testscript
+++ b/tests/dependency/recipe/testscript
@@ -139,6 +139,52 @@ EOI
}}
EOE
+: with-vars-depchain
+:
+$* <<EOI 2>>/~%EOE%
+./: alias{x}: alias{y}
+{
+ var = x
+}
+{{
+ echo
+}}
+dump alias{x}
+EOI
+<stdin>:8:1: dump:
+% .+/alias\{x\}: .+/:alias\{y\}%
+ {
+ var = x
+ }
+ % [diag=echo] perform(update)
+ {{
+ echo
+ }}
+EOE
+
+: with-vars-replay-depchain
+:
+$* <<EOI 2>>/~%EOE%
+./: alias{x y}: alias{z}
+{
+ var = x
+}
+{{
+ echo
+}}
+dump alias{y}
+EOI
+<stdin>:8:1: dump:
+% .+/alias\{y\}: .+/:alias\{z\}%
+ {
+ var = x
+ }
+ % [diag=echo] perform(update)
+ {{
+ echo
+ }}
+EOE
+
: with-vars-header
:
$* <<EOI 2>>/~%EOE%