From 9d860f46631c1567c62336fcb25cf7b8090855a4 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 6 Dec 2021 11:16:22 +0200 Subject: Recognize absolute Windows paths in make parser --- libbuild2/adhoc-rule-buildscript.cxx | 3 +- libbuild2/build/script/parser.cxx | 2 +- libbuild2/cc/compile-rule.cxx | 29 +++++++++++----- libbuild2/make-parser.cxx | 63 ++++++++++++++++++++++++----------- libbuild2/make-parser.hxx | 20 +++++------ libbuild2/make-parser.test.cxx | 4 +-- libbuild2/make-parser.test.testscript | 39 +++++++++++++++++++--- 7 files changed, 111 insertions(+), 49 deletions(-) diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index 14df51a..dce82dc 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -911,8 +911,7 @@ namespace build2 // Note that we don't really need a diag frame that prints the // line being parsed since we are always parsing the file. // - pair r ( - make.next (l, pos, il, false /* strict */)); + pair r (make.next (l, pos, il)); if (r.second.empty ()) continue; diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index 042eabb..41a040b 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -1876,7 +1876,7 @@ namespace build2 << "declaration line '" << l << "'"; }); - r = make.next (l, pos, il, false /* strict */); + r = make.next (l, pos, il); } if (r.second.empty ()) diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 44d1fd5..84151d4 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -4242,18 +4242,29 @@ namespace build2 // Fall through to the 'second' block. } - if (second) - { - // Skip the source file. - // - make_parser::next (l, pos, true /* prereq */); - second = false; - } - while (pos != l.size ()) { string f ( - make_parser::next (l, pos, true /* prereq */).first); + make_parser::next ( + l, pos, make_parser::type::prereq).first); + + if (pos != l.size () && l[pos] == ':') + { + // @@ Hm, the same as above. + // + text << l; + + bad_error = true; + break; + } + + // Skip the source file. + // + if (second) + { + second = false; + continue; + } // Skip until where we left off. // diff --git a/libbuild2/make-parser.cxx b/libbuild2/make-parser.cxx index cab81d0..c6c077f 100644 --- a/libbuild2/make-parser.cxx +++ b/libbuild2/make-parser.cxx @@ -3,23 +3,21 @@ #include +#include // strchr() + #include namespace build2 { auto make_parser:: - next (const string& l, - size_t& p, - const location& ll, - bool strict) -> pair + next (const string& l, size_t& p, const location& ll) -> pair { assert (state != end); - pair r ( - next (l, p, !strict ? state == prereqs : optional ())); - type t (state == prereqs ? type::prereq : type::target); + pair r (next (l, p, t)); + // Deal with the end. // if (r.second) @@ -67,8 +65,15 @@ namespace build2 } } + // Note: backslash must be first. + // + // Note also that, at least in GNU make 4.1, `%` seems to be unescapable + // if appears in a target and literal if in a prerequisite. + // + static const char escapable[] = "\\ :#"; + pair make_parser:: - next (const string& l, size_t& p, optional prereq) + next (const string& l, size_t& p, type) { size_t n (l.size ()); @@ -87,9 +92,36 @@ namespace build2 // // @@ Can't we do better for the (common) case where nothing is escaped? // - for (char c, q (prereq && *prereq ? '\0' : ':'); - p != n && (c = l[p]) != ' ' && c != q; ) +#ifdef _WIN32 + size_t b (p); +#endif + + for (char c; p != n && (c = l[p]) != ' '; r += c) { + if (c == ':') + { +#ifdef _WIN32 + // See if this colon is part of the driver letter component in an + // absolute Windows path. + // + // Note that here we assume we are not dealing with directories (in + // which case c: would be a valid path) and thus an absolute path is + // at least 4 characters long (e.g., c:\x). + // + if (p == b + 1 && // Colon is second character. + alpha (l[b]) && // First is drive letter. + p + 2 < n && // At least two more characters after colon. + ((l[p + 1] == '/') || // Next is directory separator. + (l[p + 1] == '\\' && // But not part of a non-\\ escape sequence. + strchr (escapable + 1, l[p + 2]) == nullptr))) + { + ++p; + continue; + } +#endif + break; + } + // If we have another character, then handle the escapes. // if (++p != n) @@ -99,13 +131,8 @@ namespace build2 // This may or may not be an escape sequence depending on whether // what follows is "escapable". // - switch (c = l[p]) - { - case '\\': - case ' ': - case ':': ++p; break; - default: c = '\\'; // Restore. - } + if (strchr (escapable, l[p]) != nullptr) + c = l[p++]; } else if (c == '$') { @@ -122,8 +149,6 @@ namespace build2 --p; break; } - - r += c; } // Skip trailing spaces. diff --git a/libbuild2/make-parser.hxx b/libbuild2/make-parser.hxx index 2ff7be2..f6da7a1 100644 --- a/libbuild2/make-parser.hxx +++ b/libbuild2/make-parser.hxx @@ -17,6 +17,11 @@ namespace build2 // parse one line at a time. This allows the caller to bail out early (for // example, on encountering a non-existent generated file). // + // Note that most tools (MinGW GCC, Qt moc, etc) do not escape `:` in + // absolute Windows paths. To handle such cases the parser recognizes `:` + // that is a part of the driver letter component and does not treat it as + // the target/prerequisite separator. + // class LIBBUILD2_SYMEXPORT make_parser { public: @@ -30,8 +35,6 @@ namespace build2 // which should normally be just skipped). Issue diagnostics and throw // failed if the declaration or path is invalid. // - // If strict is false, then allow unescaped `:` in prerequisites. - // // Note that the (pos != line.size) should be in the do-while rather than // in a while loop. In other words, except for the leading blank lines, // the parser needs to see the blank line to correctly identify the end of @@ -43,7 +46,7 @@ namespace build2 enum class type {target, prereq}; pair - next (const string& line, size_t& pos, const location&, bool strict); + next (const string& line, size_t& pos, const location&); // Lower-level stateless API. // @@ -54,11 +57,6 @@ namespace build2 // position to point to the start of the following target/prerequisite, // `:`, or line.size() if there is nothing left on this line. // - // Note that some broken tools (notably MinGW GCC) do not escape `:` - // properly. To tolerate such cases the caller may specify that what's - // being parsed is the prerequisite list in which case unescaped `:` will - // be treated literally. - // // Note also that this function may return an empty string (with // end=false) for a valid if unlikely dependency declaration, for example // (using | to represent backslash): @@ -67,8 +65,8 @@ namespace build2 // | // bar // - // It would also return an empty string (with end=true) if passed and - // empty or whitespace-only line. + // It would also return an empty string (with end=true) if passed an empty + // or whitespace-only line. // // Note also that in the make language line continuations introduce a // whitespace rather than just being remove. For example, the following @@ -78,7 +76,7 @@ namespace build2 // baz // static pair - next (const string& line, size_t& pos, optional prereq = nullopt); + next (const string& line, size_t& pos, type); }; } diff --git a/libbuild2/make-parser.test.cxx b/libbuild2/make-parser.test.cxx index 9f60bad..5c57978 100644 --- a/libbuild2/make-parser.test.cxx +++ b/libbuild2/make-parser.test.cxx @@ -19,8 +19,6 @@ namespace build2 int main (int, char* argv[]) { - bool strict (false); - // Fake build system driver, default verbosity. // init_diag (1); @@ -54,7 +52,7 @@ namespace build2 size_t pos (0); do { - pair r (make.next (l, pos, ll, strict)); + pair r (make.next (l, pos, ll)); cout << (r.first == make_type::target ? 'T' : 'P'); diff --git a/libbuild2/make-parser.test.testscript b/libbuild2/make-parser.test.testscript index 6db00eb..9108ba3 100644 --- a/libbuild2/make-parser.test.testscript +++ b/libbuild2/make-parser.test.testscript @@ -1,6 +1,8 @@ # file : libbuild2/make-parser.test.testscript # license : MIT; see accompanying LICENSE file +windows = ($cxx.target.class == 'windows') + : valid : $* <>EOO @@ -88,11 +90,40 @@ $* <>EOO P bar EOO -: lax +: escape : $* <>EOO - foo: c:\tmp\bar + \#foo\:\ bar: fox$$\\baz EOI - T foo - P c:\tmp\bar + T #foo: bar + P fox$\baz EOO + +: windows +: +if $windows +{ + $* <>EOO + foo: c:\tmp\bar + + c:\tmp\foo: c:\tmp\bar \ + c:\tmp\baz + + c:\\tmp\\foo: c:\\tmp\\bar + + c:\x:c:\y + EOI + T foo + P c:\tmp\bar + + T c:\tmp\foo + P c:\tmp\bar + P c:\tmp\baz + + T c:\tmp\foo + P c:\tmp\bar + + T c:\x + P c:\y + EOO +} -- cgit v1.1