Make notion of name pattern explicit, fix various related loose ends

13 files changed, 285 insertions, 145 deletions

diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 372c622..8b8aa38 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -445,7 +445,7 @@ namespace build2
             // be printed, thus we parse it in the value lexer mode.
             //
             mode (lexer_mode::value);
-            parse_names (t, tt, pattern_mode::ignore);
+            parse_names (t, tt, pattern_mode::preserve);
             return nullopt;
           }
           else if (v == "depdb")
@@ -567,7 +567,7 @@ namespace build2
 
             // Parse the rest of the line and bail out.
             //
-            parse_names (t, tt, pattern_mode::ignore);
+            parse_names (t, tt, pattern_mode::preserve);
             return nullopt;
           }
         }
@@ -627,7 +627,7 @@ namespace build2
 
           pr = parse_names (t, tt,
                             ns,
-                            pattern_mode::ignore,
+                            pattern_mode::preserve,
                             true /* chunk */,
                             "command line",
                             nullptr);
diff --git a/libbuild2/name.cxx b/libbuild2/name.cxx
index d956a99..06bf9b9 100644
--- a/libbuild2/name.cxx
+++ b/libbuild2/name.cxx
@@ -12,6 +12,25 @@ namespace build2
   const name empty_name;
   const names empty_names;
 
+  void name::
+  canonicalize ()
+  {
+    // We cannot assume the name part is a valid filesystem name so we will
+    // have to do the splitting manually.
+    //
+    size_t p (path_traits::rfind_separator (value));
+
+    if (p != string::npos)
+    {
+      if (p + 1 == value.size ())
+        throw invalid_argument ("empty value");
+
+      dir /= dir_path (value, p != 0 ? p : 1); // Special case: "/".
+
+      value.erase (0, p + 1);
+    }
+  }
+
   string
   to_string (const name& n)
   {
@@ -61,17 +80,20 @@ namespace build2
   ostream&
   to_stream (ostream& os, const name& n, bool quote, char pair, bool escape)
   {
-    auto write_string = [quote, pair, escape, &os](const string& v)
+    auto write_string = [quote, pair, escape, &os](const string& v, bool pat)
     {
       char sc[] = {
         '{', '}', '[', ']', '$', '(', ')', // Token endings.
         ' ', '\t', '\n', '#',              // Spaces.
         '\\', '"',                         // Escaping and quoting.
         '%',                               // Project name separator.
-        '*', '?',                          // Wildcard characters.
         pair,                              // Pair separator, if any.
         '\0'};
 
+      char pc[] = {
+        '*', '?',                          // Wildcard characters.
+        '\0'};
+
       if (quote && v.find ('\'') != string::npos)
       {
         // Quote the string with the double quotes rather than with the single
@@ -91,7 +113,8 @@ namespace build2
         if (escape) os << '\\';
         os << '"';
       }
-      else if (quote && v.find_first_of (sc) != string::npos)
+      else if (quote && (v.find_first_of (sc) != string::npos ||
+                         (!pat && v.find_first_of (pc) != string::npos)))
       {
         if (escape) os << '\\';
         os << '\'';
@@ -107,15 +130,16 @@ namespace build2
 
     uint16_t dv (stream_verb (os).path); // Directory verbosity.
 
-    auto write_dir = [dv, quote, &os, &write_string] (const dir_path& d)
+    auto write_dir = [dv, quote, &os, &write_string] (const dir_path& d,
+                                                      bool pat)
     {
       if (quote)
-        write_string (dv < 1 ? diag_relative (d) : d.representation ());
+        write_string (dv < 1 ? diag_relative (d) : d.representation (), pat);
       else
         os << d;
     };
 
-    // Note: similar to to_string() below.
+    // Note: similar to to_string() above.
     //
 
     // If quoted then print empty name as '' rather than {}.
@@ -125,7 +149,7 @@ namespace build2
 
     if (n.proj)
     {
-      write_string (n.proj->string ());
+      write_string (n.proj->string (), false);
       os << '%';
     }
 
@@ -145,26 +169,26 @@ namespace build2
                         dir_path ());
 
     if (!pd.empty ())
-      write_dir (pd);
+      write_dir (pd, false);
 
     if (t || (!d && !v))
     {
       if (t)
-        write_string (n.type);
+        write_string (n.type, false);
 
       os << '{';
     }
 
     if (v)
-      write_string (n.value);
+      write_string (n.value, n.pattern);
     else if (d)
     {
       if (rd.empty ())
-        write_string (dir_path (".").representation ());
+        write_string (dir_path (".").representation (), false);
       else if (!pd.empty ())
-        write_string (rd.leaf ().representation ());
+        write_string (rd.leaf ().representation (), n.pattern);
       else
-        write_dir (rd);
+        write_dir (rd, n.pattern);
     }
 
     if (t || (!d && !v))
diff --git a/libbuild2/name.hxx b/libbuild2/name.hxx
index 8385e7c..3d14976 100644
--- a/libbuild2/name.hxx
+++ b/libbuild2/name.hxx
@@ -33,6 +33,8 @@ namespace build2
   // If pair is not '\0', then this name and the next in the list form a
   // pair. Can be used as a bool flag.
   //
+  // If pattern is true then this is a name pattern (e.g., file{*.txt}).
+  //
   struct name
   {
     optional<project_name> proj;
@@ -40,6 +42,7 @@ namespace build2
     string type;
     string value;
     char pair = '\0';
+    bool pattern = false;
 
     name () {} // = default; Clang needs this to initialize const object.
     name (string v): value (move (v)) {}
@@ -54,8 +57,13 @@ namespace build2
         : proj (project_name (move (p))), dir (move (d)), type (move (t)),
           value (move (v)) {}
 
-    name (optional<project_name> p, dir_path d, string t, string v)
-        : proj (move (p)), dir (move (d)), type (move (t)), value (move (v)) {}
+      name (optional<project_name> p,
+            dir_path d,
+            string t,
+            string v,
+            bool pat = false)
+        : proj (move (p)), dir (move (d)), type (move (t)), value (move (v)),
+          pattern (pat) {}
 
     bool
     qualified () const {return proj.has_value ();}
@@ -115,6 +123,13 @@ namespace build2
 
     int
     compare (const name&) const;
+
+    // Canonicalize the name by moving the directory component (if any) from
+    // value to dir. Throw invalid_argument if value would become empty. May
+    // also throw invalid_path.
+    //
+    void
+    canonicalize ();
   };
 
   LIBBUILD2_SYMEXPORT extern const name empty_name;
@@ -130,6 +145,9 @@ namespace build2
 
   // Return string representation of a name.
   //
+  // Note that this function does not quote special characters and you should
+  // use the to_stream() function below if this is necessary.
+  //
   LIBBUILD2_SYMEXPORT string
   to_string (const name&);
 
@@ -143,11 +161,12 @@ namespace build2
     cs.append (n.type);
     cs.append (n.value);
     cs.append (n.pair);
+    cs.append (n.pattern);
   }
 
   // Store a string in a name in a reversible way. If the string ends with a
   // trailing directory separator then it is stored as a directory, otherwise
-  // as a simple name.
+  // as a simple name. Note that the returned name is never a pattern.
   //
   name
   to_name (string);
@@ -157,6 +176,10 @@ namespace build2
   //
   // {}[]$() \t\n#\"'%
   //
+  // And additionally, if name is not a pattern:
+  //
+  // *?
+  //
   // If the pair argument is not '\0', then it is added to the above special
   // characters set. If the quote character is present in the component then
   // it is double quoted rather than single quoted. In this case the following
diff --git a/libbuild2/name.ixx b/libbuild2/name.ixx
index 837915f..80a097e 100644
--- a/libbuild2/name.ixx
+++ b/libbuild2/name.ixx
@@ -20,6 +20,9 @@ namespace build2
     if (r == 0)
       r = pair < x.pair ? -1 : (pair > x.pair ? 1 : 0);
 
+    if (r == 0)
+      r = pattern == x.pattern ? 0 : (!pattern && x.pattern ? -1 : 1);
+
     return r;
   }
 
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index b6d348e..244dbfc 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -516,7 +516,7 @@ namespace build2
 
       if (tt != type::labrace)
       {
-        ns = parse_names (t, tt, pattern_mode::ignore);
+        ns = parse_names (t, tt, pattern_mode::preserve);
 
         // Allow things like function calls that don't result in anything.
         //
@@ -570,7 +570,7 @@ namespace build2
             // should move to ans.
             //
             size_t m (ns.size ());
-            parse_names (t, tt, ns, pattern_mode::ignore);
+            parse_names (t, tt, ns, pattern_mode::preserve);
             size_t n (ns.size ());
 
             // Another empty case (<$empty>).
@@ -604,7 +604,7 @@ namespace build2
           //
           next (t, tt);
           if (start_names (tt))
-            parse_names (t, tt, ns, pattern_mode::ignore);
+            parse_names (t, tt, ns, pattern_mode::preserve);
         }
 
         if (!ans.empty ())
@@ -661,7 +661,7 @@ namespace build2
             // Figure out if this is a target or a target type/pattern (yeah,
             // it can be a mixture).
             //
-            if (path_pattern (n.value))
+            if (n.pattern)
             {
               if (n.pair)
                 fail (nloc) << "out-qualified target type/pattern";
@@ -669,14 +669,84 @@ namespace build2
               if (!ans.empty () && !ans[i].ns.empty ())
                 fail (ans[i].loc) << "ad hoc member in target type/pattern";
 
+              // Reduce the various directory/value combinations to the scope
+              // directory (if any) and the pattern. Here are more interesting
+              // examples of patterns:
+              //
+              // */           --  */{}
+              // dir{*}       --  dir{*}
+              // dir{*/}      --  */dir{}
+              //
+              // foo/*/       --  foo/*/{}
+              // foo/dir{*/}  --  foo/*/dir{}
+              //
+              // Note that these are not patterns:
+              //
+              // foo*/file{bar}
+              // foo*/dir{bar/}
+              //
+              // While these are:
+              //
+              // file{foo*/bar}
+              // dir{foo*/bar/}
+              //
+              // And this is a half-pattern (foo* should no be treated as a
+              // pattern but that's unfortunately indistinguishable):
+              //
+              // foo*/dir{*/}  --  foo*/*/dir{}
+              //
+              if (n.value.empty () && !n.dir.empty ())
+              {
+                // Note that we use string and not the representation: in a
+                // sense the trailing slash in the pattern is subsumed by the
+                // target type.
+                //
+                if (n.dir.simple ())
+                  n.value = move (n.dir).string ();
+                else
+                {
+                  n.value = n.dir.leaf ().string ();
+                  n.dir.make_directory ();
+                }
+
+                // Treat directory as type dir{} similar to other places.
+                //
+                if (n.untyped ())
+                  n.type = "dir";
+              }
+              else
+              {
+                // Move the directory part, if any, from value to dir.
+                //
+                try
+                {
+                  n.canonicalize ();
+                }
+                catch (const invalid_path& e)
+                {
+                  fail (nloc) << "invalid path '" << e.path << "'";
+                }
+                catch (const invalid_argument&)
+                {
+                  fail (nloc) << "invalid pattern '" << n.value << "'";
+                }
+              }
+
               // If we have the directory, then it is the scope.
               //
               enter_scope sg;
               if (!n.dir.empty ())
+              {
+                if (path_pattern (n.dir))
+                  fail (nloc) << "pattern in directory "
+                              << n.dir.representation ();
+
                 sg = enter_scope (*this, move (n.dir));
+              }
 
               // Resolve target type. If none is specified or if it is '*',
-              // use the root of the hierarchy. So these are all equivalent:
+              // use the root of the target type hierarchy. So these are all
+              // equivalent:
               //
               // *: foo = bar
               // {*}: foo = bar
@@ -924,6 +994,12 @@ namespace build2
           if (at.first)
             fail (at.second) << "attributes before scope directory";
 
+          // Make sure it's not a pattern (see also the target case above and
+          // scope below).
+          //
+          if (ns[0].pattern)
+            fail (nloc) << "pattern in " << ns[0];
+
           if (ns.size () == 2)
           {
             d = move (ns[0].dir);
@@ -937,12 +1013,6 @@ namespace build2
             d = dir_path (ns[0].value, 0, p + 1);
             ns[0].value.erase (0, p + 1);
           }
-
-          // Make sure it's not a pattern (see also the target case above and
-          // scope below).
-          //
-          if (path_pattern (d))
-            fail (nloc) << "pattern in directory " << d.representation ();
         }
 
         const variable& var (parse_variable_name (move (ns), nloc));
@@ -980,13 +1050,11 @@ namespace build2
 
         if (next (t, tt) == type::lcbrace && peek () == type::newline)
         {
-          dir_path&& d (move (ns[0].dir));
-
           // Make sure not a pattern (see also the target and directory cases
           // above).
           //
-          if (path_pattern (d))
-            fail (nloc) << "pattern in directory " << d.representation ();
+          if (ns[0].pattern)
+            fail (nloc) << "pattern in " << ns[0];
 
           next (t, tt); // Newline.
           next (t, tt); // First token inside the block.
@@ -999,7 +1067,7 @@ namespace build2
           // Can contain anything that a top level can.
           //
           {
-            enter_scope sg (*this, move (d));
+            enter_scope sg (*this, move (ns[0].dir));
             parse_clause (t, tt);
           }
 
@@ -1060,7 +1128,7 @@ namespace build2
       attributes_push (t, tt);
 
       location nloc (get_location (t));
-      names ns (parse_names (t, tt, pattern_mode::ignore, "variable name"));
+      names ns (parse_names (t, tt, pattern_mode::preserve, "variable name"));
 
       if (tt != type::assign  &&
           tt != type::prepend &&
@@ -1199,7 +1267,7 @@ namespace build2
                 fail (t) << "expected c++ recipe version instead of " << t;
 
               location nloc (get_location (t));
-              names ns (parse_names (t, tt, pattern_mode::ignore));
+              names ns (parse_names (t, tt, pattern_mode::preserve));
 
               uint64_t ver;
               try
@@ -1586,10 +1654,9 @@ namespace build2
       if (n.qualified ())
         fail (tloc) << "project name in target " << n;
 
-      // Make sure none of our targets are patterns (maybe we will allow
-      // quoting later).
+      // Make sure none of our targets are patterns.
       //
-      if (path_pattern (n.value))
+      if (n.pattern)
         fail (tloc) << "pattern in target " << n;
 
       enter_target tg (*this,
@@ -2171,7 +2238,7 @@ namespace build2
     {
       args = convert<strings> (
         tt != type::newline && tt != type::eos
-        ? parse_names (t, tt, pattern_mode::ignore, "argument", nullptr)
+        ? parse_names (t, tt, pattern_mode::preserve, "argument", nullptr)
         : names ());
     }
     catch (const invalid_argument& e)
@@ -2535,7 +2602,7 @@ namespace build2
       ns = convert<strings> (
         tt != type::newline && tt != type::eos
         ? parse_names (t, tt,
-                       pattern_mode::ignore,
+                       pattern_mode::preserve,
                        "environment variable name",
                        nullptr)
         : names ());
@@ -2811,7 +2878,7 @@ namespace build2
     next (t, tt);
     const location l (get_location (t));
     names ns (tt != type::newline && tt != type::eos
-              ? parse_names (t, tt, pattern_mode::ignore, "module", nullptr)
+              ? parse_names (t, tt, pattern_mode::preserve, "module", nullptr)
               : names ());
 
     for (auto i (ns.begin ()); i != ns.end (); ++i)
@@ -3121,7 +3188,7 @@ namespace build2
       {
         next (t, tt);
         const location l (get_location (t));
-        names ns (parse_names (t, tt, pattern_mode::ignore, "function name"));
+        names ns (parse_names (t, tt, pattern_mode::preserve, "function name"));
 
         if (ns.empty () || ns[0].empty ())
           fail (l) << "function name expected after ':'";
@@ -3230,7 +3297,7 @@ namespace build2
           auto parse_pattern_with_attributes = [this] (token& t, type& tt)
           {
             return parse_value_with_attributes (
-              t, tt, pattern_mode::ignore, "pattern", nullptr);
+              t, tt, pattern_mode::preserve, "pattern", nullptr);
           };
 
           for (size_t i (0);; ++i)
@@ -3593,7 +3660,7 @@ namespace build2
     //
     names ns (tt != type::newline && tt != type::eos
               ? parse_names (t, tt,
-                             pattern_mode::ignore,
+                             pattern_mode::preserve,
                              "description",
                              nullptr)
               : names ());
@@ -3670,7 +3737,7 @@ namespace build2
     const location l (get_location (t));
     next (t, tt);
     names ns (tt != type::newline && tt != type::eos
-              ? parse_names (t, tt, pattern_mode::ignore)
+              ? parse_names (t, tt, pattern_mode::preserve)
               : names ());
 
     text (l) << "dump:";
@@ -4411,7 +4478,7 @@ namespace build2
 
       const location nl (get_location (t));
       next (t, tt);
-      value n (parse_value (t, tt, pattern_mode::ignore));
+      value n (parse_value (t, tt, pattern_mode::preserve));
 
       if (tt != type::rparen)
         fail (t) << "expected ')' after variable name";
@@ -4524,7 +4591,7 @@ namespace build2
         const location l (get_location (t));
 
         names ns (
-          parse_names (t, tt, pattern_mode::ignore, "attribute", nullptr));
+          parse_names (t, tt, pattern_mode::preserve, "attribute", nullptr));
 
         string n;
         value v;
@@ -4548,7 +4615,7 @@ namespace build2
           next (t, tt);
 
           v = (tt != type::comma && tt != type::rsbrace
-               ? parse_value (t, tt, pattern_mode::ignore, "attribute value")
+               ? parse_value (t, tt, pattern_mode::preserve, "attribute value")
                : value (names ()));
 
           expire_mode ();
@@ -4596,7 +4663,11 @@ namespace build2
   //
   static inline name&
   append_name (names& ns,
-               optional<project_name> p, dir_path d, string t, string v,
+               optional<project_name> p,
+               dir_path d,
+               string t,
+               string v,
+               bool pat,
                const location& loc)
   {
     // The directory/value must not be empty if we have a type.
@@ -4604,7 +4675,7 @@ namespace build2
     if (d.empty () && v.empty () && !t.empty ())
       fail (loc) << "typed empty name";
 
-    ns.emplace_back (move (p), move (d), move (t), move (v));
+    ns.emplace_back (move (p), move (d), move (t), move (v), pat);
     return ns.back ();
   }
 
@@ -4711,7 +4782,9 @@ namespace build2
           ns.push_back (ns[pairn - 1]);
       }
 
-      name& r (append_name (ns, move (p), move (d), move (t), move (v), loc));
+      name& r (
+        append_name (
+          ns, move (p), move (d), move (t), move (v), cn.pattern, loc));
       r.pair = cn.pair;
     }
 
@@ -5087,7 +5160,7 @@ namespace build2
                        bool cross)
   {
     if (pp)
-      pmode = pattern_mode::ignore;
+      pmode = pattern_mode::preserve;
 
     next (t, tt);                          // Get what's after '{'.
     const location loc (get_location (t)); // Start of names.
@@ -5113,7 +5186,7 @@ namespace build2
       // This can be an ordinary name group or a pattern (with inclusions and
       // exclusions). We want to detect which one it is since for patterns we
       // want just the list of simple names without pair/dir/type added (those
-      // are added after the pattern expansion in parse_names_pattern()).
+      // are added after the pattern expansion in expand_name_pattern()).
       //
       // Detecting which one it is is tricky. We cannot just peek at the token
       // and look for some wildcards since the pattern can be the result of an
@@ -5294,7 +5367,7 @@ namespace build2
     tracer trace ("parser::parse_names", &path_);
 
     if (pp)
-      pmode = pattern_mode::ignore;
+      pmode = pattern_mode::preserve;
 
     // Returned value NULL/type and pattern (see below).
     //
@@ -5387,12 +5460,12 @@ namespace build2
       }
     };
 
-    // Set the result pattern target type and switch to the ignore mode.
+    // Set the result pattern target type and switch to the preserve mode.
     //
     // The goal of the detect mode is to assemble the "raw" list (the pattern
     // itself plus inclusions/exclusions) that will then be passed to
-    // parse_names_pattern(). So clear pair, directory, and type (they will be
-    // added during pattern expansion) and change the mode to ignore (to
+    // expand_name_pattern(). So clear pair, directory, and type (they will be
+    // added during pattern expansion) and change the mode to preserve (to
     // prevent any expansions in inclusions/exclusions).
     //
     auto pattern_detected =
@@ -5403,7 +5476,7 @@ namespace build2
       pairn = 0;
       dp = nullptr;
       tp = nullptr;
-      pmode = pattern_mode::ignore;
+      pmode = pattern_mode::preserve;
       rpat = ttp;
     };
 
@@ -5833,59 +5906,73 @@ namespace build2
           }
         };
 
-        if (pmode != pattern_mode::ignore   &&
-            !*pp1                           && // Cannot be project-qualified.
-            !quoted                         && // Cannot be quoted.
-            ((dp != nullptr && dp->absolute ()) || pbase_ != nullptr) &&
-            (pattern () || (curly && val[0] == '+')))
+        bool pat (false);
+        if (pmode != pattern_mode::preserve)
         {
-          // Resolve the target if there is one. If we fail, then this is not
-          // a pattern.
-          //
-          const target_type* ttp (tp != nullptr && scope_ != nullptr
-                                  ? scope_->find_target_type (*tp)
-                                  : nullptr);
-
-          if (tp == nullptr || ttp != nullptr)
+          if (!*pp1    && // Cannot be project-qualified.
+              !quoted  && // Cannot be quoted.
+              ((dp != nullptr && dp->absolute ()) || pbase_ != nullptr) &&
+              (pattern () || (curly && val[0] == '+')))
           {
-            if (pmode == pattern_mode::detect)
+            // Resolve the target type if there is one. If we fail, then this
+            // is not a pattern.
+            //
+            const target_type* ttp (tp != nullptr && scope_ != nullptr
+                                    ? scope_->find_target_type (*tp)
+                                    : nullptr);
+
+            if (tp == nullptr || ttp != nullptr)
             {
-              // Strip the literal unquoted plus character for the first
-              // pattern in the group.
-              //
-              if (ppat)
+              if (pmode == pattern_mode::detect)
               {
-                assert (val[0] == '+');
+                // Strip the literal unquoted plus character for the first
+                // pattern in the group.
+                //
+                if (ppat)
+                {
+                  assert (val[0] == '+');
 
-                val.erase (0, 1);
-                ppat = pinc = false;
-              }
+                  val.erase (0, 1);
+                  ppat = pinc = false;
+                }
 
-              // Reset the detect pattern mode to expand if the pattern is not
-              // followed by the inclusion/exclusion pattern/match. Note that
-              // if it is '}' (i.e., the end of the group), then it is a single
-              // pattern and the expansion is what we want.
-              //
-              if (!pattern_prefix (peeked ()))
-                pmode = pattern_mode::expand;
-            }
+                // Reset the detect pattern mode to expand if the pattern is
+                // not followed by the inclusion/exclusion pattern/match. Note
+                // that if it is '}' (i.e., the end of the group), then it is
+                // a single pattern and the expansion is what we want.
+                //
+                if (!pattern_prefix (peeked ()))
+                  pmode = pattern_mode::expand;
+              }
 
-            if (pmode == pattern_mode::expand)
-            {
-              count = expand_name_pattern (get_location (t),
-                                           names {name (move (val))},
-                                           ns,
-                                           what,
-                                           pairn,
-                                           dp, tp, ttp);
-              continue;
-            }
+              if (pmode == pattern_mode::expand)
+              {
+                count = expand_name_pattern (get_location (t),
+                                             names {name (move (val))},
+                                             ns,
+                                             what,
+                                             pairn,
+                                             dp, tp, ttp);
+                continue;
+              }
 
-            pattern_detected (ttp);
+              pattern_detected (ttp);
 
-            // Fall through.
+              // Fall through.
+            }
           }
         }
+        else
+        {
+          // For the preserve mode we treat it as a pattern if it look like
+          // one syntactically. For now we also don't treat leading `+` in the
+          // curly context as an indication of a pattern.
+          //
+          if (!*pp1   && // Cannot be project-qualified.
+              !quoted && // Cannot be quoted.
+              pattern ())
+            pat = true;
+        }
 
         // If we are a second half of a pair, add another first half
         // unless this is the first instance.
@@ -5918,6 +6005,7 @@ namespace build2
             append_name (
               ns,
               *pp1, move (dir), (tp != nullptr ? *tp : string ()), string (),
+              pat,
               loc);
 
             continue;
@@ -5929,6 +6017,7 @@ namespace build2
                      (dp != nullptr ? *dp : dir_path ()),
                      (tp != nullptr ? *tp : string ()),
                      move (val),
+                     pat,
                      loc);
 
         continue;
@@ -6193,7 +6282,7 @@ namespace build2
           location l (get_location (t));
           value v (
             tt != type::rsbrace
-            ? parse_value (t, tt, pattern_mode::ignore, "value subscript")
+            ? parse_value (t, tt, pattern_mode::preserve, "value subscript")
             : value (names ()));
 
           if (tt != type::rsbrace)
@@ -6458,6 +6547,7 @@ namespace build2
                          (dp != nullptr ? *dp : dir_path ()),
                          (tp != nullptr ? *tp : string ()),
                          string (),
+                         false /* pattern */,
                          get_location (t));
             count = 1;
           }
@@ -6478,6 +6568,7 @@ namespace build2
                          (dp != nullptr ? *dp : dir_path ()),
                          (tp != nullptr ? *tp : string ()),
                          string (),
+                         false /* pattern */,
                          get_location (t));
             count = 0;
           }
@@ -6505,6 +6596,7 @@ namespace build2
                      (dp != nullptr ? *dp : dir_path ()),
                      (tp != nullptr ? *tp : string ()),
                      string (),
+                     false /* pattern */,
                      get_location (t));
         break;
       }
@@ -6523,6 +6615,7 @@ namespace build2
                    (dp != nullptr ? *dp : dir_path ()),
                    (tp != nullptr ? *tp : string ()),
                    string (),
+                   false /* pattern */,
                    get_location (t));
     }
 
@@ -6762,7 +6855,7 @@ namespace build2
           // specific (via pre-parse or some such).
           //
           params.push_back (tt != type::rparen
-                            ? parse_value (t, tt, pattern_mode::ignore)
+                            ? parse_value (t, tt, pattern_mode::preserve)
                             : value (names ()));
         }
 
diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx
index 1af79bf..d6044e9 100644
--- a/libbuild2/parser.hxx
+++ b/libbuild2/parser.hxx
@@ -108,9 +108,9 @@ namespace build2
     //
     enum class pattern_mode
     {
-      ignore, // Treat as ordinary names.
-      detect, // Ignore pair/dir/type if the first name is a pattern.
-      expand  // Expand to ordinary names.
+      preserve, // Preserve as name pattern.
+      expand,   // Expand to non-pattern names.
+      detect    // Implementation detail mode (see code for more information).
     };
 
     // If one is true then parse a single (logical) line (logical means it
@@ -400,8 +400,8 @@ namespace build2
     // As above but also handle value attributes.
     //
     value
-    parse_value_with_attributes (token& t, token_type& tt,
-                                 pattern_mode pmode,
+    parse_value_with_attributes (token&, token_type&,
+                                 pattern_mode,
                                  const char* what = "name",
                                  const string* separators = &name_separators,
                                  bool chunk = false);
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index c5eb17e..e16e773 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -327,7 +327,7 @@ namespace build2
       // If we are still looking for the cache, see if the original comes from
       // this scope. We check this before the overrides since it can come from
       // the target type/patter-specific variables, which is "more inner" than
-      // normal scope variables (see find_original()).
+      // normal scope variables (see lookup_original()).
       //
       if (inner_vars == nullptr && orig.defined () && belongs (orig))
       {
@@ -684,33 +684,26 @@ namespace build2
     else if (!v.empty ())
     {
       // Split the path into its directory part (if any) the name part, and
-      // the extension (if any). We cannot assume the name part is a valid
-      // filesystem name so we will have to do the splitting manually.
+      // the extension (if any).
       //
       // See also parser::expand_name_pattern() if changing anything here.
       //
-      size_t p (path::traits_type::rfind_separator (v));
-
-      if (p != string::npos)
+      try
       {
-        try
-        {
-          n.dir /= dir_path (v, p != 0 ? p : 1); // Special case: "/".
-        }
-        catch (const invalid_path& e)
-        {
-          fail (loc) << "invalid path '" << e.path << "'";
-        }
-
-        // This is probably too general of a place to ignore multiple trailing
-        // slashes and treat it as a directory (e.g., we don't want to
-        // encourage this sloppiness in buildfiles). We could, however, do it
-        // for certain contexts, such as buildspec. Maybe a lax flag?
+        n.canonicalize ();
+      }
+      catch (const invalid_path& e)
+      {
+        fail (loc) << "invalid path '" << e.path << "'";
+      }
+      catch (const invalid_argument&)
+      {
+        // This is probably too general of a place to ignore multiple
+        // trailing slashes and treat it as a directory (e.g., we don't want
+        // to encourage this sloppiness in buildfiles). We could, however,
+        // do it for certain contexts, such as buildspec. Maybe a lax flag?
         //
-        if (++p == v.size ())
-          fail (loc) << "invalid name '" << v << "'";
-
-        v.erase (0, p);
+        fail (loc) << "invalid name '" << v << "'";
       }
 
       // Extract the extension.
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index f78a565..5a853d9 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -132,9 +132,13 @@ namespace build2
     lookup_type
     lookup (const variable& var, const target_key& tk) const
     {
+      //@@ TODO: dir name
       return lookup (var, tk.type, tk.name).first;
     }
 
+    // Note for dir{} and fsdir{} target name is the directory leaf (without
+    // the trailing slash).
+    //
     lookup_type
     lookup (const variable& var, const target_type& tt, const string& tn) const
     {
diff --git a/libbuild2/script/parser.cxx b/libbuild2/script/parser.cxx
index ebfd5fc..4c8a377 100644
--- a/libbuild2/script/parser.cxx
+++ b/libbuild2/script/parser.cxx
@@ -34,7 +34,7 @@ namespace build2
       //
       return tt != type::newline && start_names (tt)
         ? parse_value (t, tt,
-                       pattern_mode::ignore,
+                       pattern_mode::preserve,
                        "variable value",
                        nullptr)
         : value (names ());
@@ -101,7 +101,7 @@ namespace build2
     {
       parse_names (t, tt,
                    ns,
-                   pattern_mode::ignore,
+                   pattern_mode::preserve,
                    true /* chunk */,
                    "command line",
                    nullptr);
@@ -1073,7 +1073,7 @@ namespace build2
             else
               parse_names (t, tt,
                            ns,
-                           pattern_mode::ignore,
+                           pattern_mode::preserve,
                            true /* chunk */,
                            "command line",
                            nullptr);
@@ -1319,7 +1319,7 @@ namespace build2
 
         parse_names (t, tt,
                      ns,
-                     pattern_mode::ignore,
+                     pattern_mode::preserve,
                      true /* chunk */,
                      "env builtin argument",
                      nullptr);
@@ -1473,7 +1473,7 @@ namespace build2
       next (t, tt);
       location l (get_location (t));
       names ns (parse_names (t, tt,
-                             pattern_mode::ignore,
+                             pattern_mode::preserve,
                              true,
                              "exit status",
                              nullptr));
@@ -1669,7 +1669,7 @@ namespace build2
         //
         names ns (tt != type::newline
                   ? parse_names (t, tt,
-                                 pattern_mode::ignore,
+                                 pattern_mode::preserve,
                                  false,
                                  "here-document line",
                                  nullptr)
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index fec522c..a5061e3 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -176,12 +176,13 @@ namespace build2
       }
     }
 
-    // Delegate to scope's find_original().
+    // Delegate to scope's lookup_original().
     //
     if (!r.first)
     {
       if (!target_only)
       {
+        //@@ TODO: dir name
         auto p (base_scope ().lookup_original (
                   var,
                   &type (),
@@ -232,7 +233,7 @@ namespace build2
         r.first = lookup_type (*p.first, p.second, vars);
     }
 
-    // Delegate to target's find_original().
+    // Delegate to target's lookup_original().
     //
     if (!r.first)
     {
diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx
index 9e21d7a..49d4563 100644
--- a/libbuild2/target.hxx
+++ b/libbuild2/target.hxx
@@ -141,7 +141,7 @@ namespace build2
     //
     const dir_path    dir;  // Absolute and normalized.
     const dir_path    out;  // Empty or absolute and normalized.
-    const string      name;
+    const string      name; // Empty for dir{} and fsdir{} targets.
     optional<string>* ext_; // Reference to value in target_key.
 
     const string* ext () const; // Return NULL if not specified.
diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx
index f9e60c1..451cb76 100644
--- a/libbuild2/variable.cxx
+++ b/libbuild2/variable.cxx
@@ -1820,8 +1820,7 @@ namespace build2
         //
         if (pat != "*")
         {
-          if (name.size () < pat.size () - 1 || // One for '*' or '?'.
-              !butl::path_match (name, pat))
+          if (!butl::path_match (name, pat))
             continue;
         }
 
diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx
index 805b93d..d40934c 100644
--- a/libbuild2/variable.hxx
+++ b/libbuild2/variable.hxx
@@ -1572,7 +1572,7 @@ namespace build2
 
     // Convert a lookup pointing to a value belonging to this variable map
     // to its non-const version. Note that this is only safe on the original
-    // values (see find_original()).
+    // values (see lookup_original()).
     //
     value&
     modify (const lookup_type& l)
@@ -1793,7 +1793,7 @@ namespace build2
     // "target identity" (as target type and target name). Note that while at
     // first it may seem like we don't need the target identity, we actually
     // do since the stem may itself be target-type/pattern-specific. See
-    // scope::find_original() for details.
+    // scope::lookup_original() for details.
     //
     mutable
     variable_cache<tuple<const value*, const target_type*, string>>