From 6f6ea12ebbd9251cc885a0720b0ec09fbc2d9331 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 19 Jul 2022 11:11:44 +0200 Subject: Warn about conditional dependency declarations during distribution --- libbuild2/parser.cxx | 42 ++++++++++++++++++++++++++++++++++++++++++ libbuild2/parser.hxx | 9 +++++++++ 2 files changed, 51 insertions(+) diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index cc8fd9e..84b6366 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -247,6 +247,8 @@ namespace build2 { pre_parse_ = false; attributes_.clear (); + imported_ = false; + condition_ = nullopt; default_target_ = nullptr; peeked_ = false; replay_ = replay::stop; @@ -2368,6 +2370,34 @@ namespace build2 // tracer trace ("parser::parse_dependency", &path_); + // Diagnose conditional prerequisites. Note that we want to diagnose this + // even if pns is empty (think empty variable expansion; the literal "no + // prerequisites" case is handled elsewhere). We also want to omit this + // check for imported buildfiles (export stub can reasonably wrap loading + // of a buildfile in a condition). + // + // @@ TMP For now we only do it during the dist meta-operation. In the + // future we should tighten this to any meta-operation provided + // the dist module is loaded. + // + // @@ TMP For now it's a warning because we have dependencies like + // cli.cxx{foo}: cli{foo} which are not currently possible to + // rewrite (cli.cxx{} is not always registered). + // + if (condition_ && + !imported_ && + ctx->current_mif != nullptr && + ctx->current_mif->id == dist_id) + { + warn (tloc) << "conditional dependency declaration may result in " + << "incomplete distribution" << + info (ploc) << "prerequisite declared here" << + info (*condition_) << "conditional buildfile fragment starts here" << + info << "instead use `include` prerequisite-specific variable to " + << "conditionally include prerequisites" << + info << "for example: : : include = ()"; + } + // First enter all the targets. // small_vector, 1> tgs ( @@ -3699,6 +3729,12 @@ namespace build2 void parser:: parse_if_else (token& t, type& tt) { + auto g = make_guard ([this, old = condition_] () mutable + { + condition_ = old; + }); + condition_ = get_location (t); + parse_if_else (t, tt, false /* multi */, [this] (token& t, type& tt, bool s, const string& k) @@ -3840,6 +3876,12 @@ namespace build2 void parser:: parse_switch (token& t, type& tt) { + auto g = make_guard ([this, old = condition_] () mutable + { + condition_ = old; + }); + condition_ = get_location (t); + parse_switch (t, tt, false /* multi */, [this] (token& t, type& tt, bool s, const string& k) diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx index f806568..61ecd5b 100644 --- a/libbuild2/parser.hxx +++ b/libbuild2/parser.hxx @@ -91,6 +91,12 @@ namespace build2 parse_export_stub (istream& is, const path_name& name, scope& rs, scope& bs) { + auto g = make_guard ([this, old = imported_] () mutable + { + imported_ = old; + }); + imported_ = true; + parse_buildfile (is, name, &rs, bs); return move (export_value); } @@ -906,6 +912,9 @@ namespace build2 small_vector attributes_; + bool imported_ = false; // True if loaded via export stub. + optional condition_; // Innermost if/switch (but not in recipe). + target* default_target_ = nullptr; replay_token peek_; -- cgit v1.1