Various improvements to cli module

7 files changed, 76 insertions, 57 deletions

diff --git a/build/cli/module.cxx b/build/cli/module.cxx
index 96da40d..30f8d3e 100644
--- a/build/cli/module.cxx
+++ b/build/cli/module.cxx
@@ -97,22 +97,15 @@ namespace build
             process pr (args, false, false, true);
             ifdstream is (pr.in_ofd);
 
-            for (bool first (true); !is.eof (); )
-            {
-              string l;
-              getline (is, l);
-
-              if (first)
-              {
-                // The version is the last word on the first line.
-                //
-                auto p (l.rfind (' '));
-                if (p != string::npos)
-                  ver = string (l, p + 1);
-
-                first = false;
-              }
-            }
+            // The version should be the last word on the first line.
+            //
+            string l;
+            getline (is, l);
+            auto p (l.rfind (' '));
+            if (p != string::npos)
+              ver = string (l, p + 1);
+
+            is.close (); // Don't block the other end.
 
             if (!pr.wait ())
               throw failed ();
diff --git a/build/cli/rule.cxx b/build/cli/rule.cxx
index c0c8fe0..889e6a2 100644
--- a/build/cli/rule.cxx
+++ b/build/cli/rule.cxx
@@ -23,8 +23,6 @@ namespace build
 {
   namespace cli
   {
-    using config::append_options;
-
     match_result compile::
     match (action a, target& xt, const std::string&) const
     {
@@ -32,6 +30,8 @@ namespace build
 
       if (cli_cxx* pt = xt.is_a<cli_cxx> ())
       {
+        // The cli.cxx{} group.
+        //
         cli_cxx& t (*pt);
 
         // See if we have a .cli source file.
@@ -41,7 +41,15 @@ namespace build
         {
           if (p.is_a<cli> ())
           {
-            //@@ Need to verify input and output stems match.
+            // Check that the stems match.
+            //
+            if (t.name != p.name ())
+            {
+              level3 ([&]{trace << ".cli file stem '" << p.name () << "' "
+                                << "doesn't match target " << t;});
+              return r;
+            }
+
             r = p;
             break;
           }
@@ -50,11 +58,11 @@ namespace build
         if (!r)
         {
           level3 ([&]{trace << "no .cli source file for target " << t;});
-          return nullptr;
+          return r;
         }
 
         // If we still haven't figured out the member list, we can do
-        // that now. Specifically, at this stage no further changes to
+        // that now. Specifically, at this stage, no further changes to
         // cli.options are possible and we can determine whether the
         // --suppress-inline option is present.
         //
@@ -63,23 +71,10 @@ namespace build
           t.h = &search<cxx::hxx> (t.dir, t.name, nullptr, nullptr);
           t.h->group = &t;
 
-          t.c = & search<cxx::cxx> (t.dir, t.name, nullptr, nullptr);
+          t.c = &search<cxx::cxx> (t.dir, t.name, nullptr, nullptr);
           t.c->group = &t;
 
-          bool inl (true);
-          if (auto val = t["cli.options"])
-          {
-            for (const name& n: val.template as<const list_value&> ())
-            {
-              if (n.value == "--suppress-inline")
-              {
-                inl = false;
-                break;
-              }
-            }
-          }
-
-          if (inl)
+          if (!config::find_option ("--suppress-inline", t, "cli.options"))
           {
             t.i = &search<cxx::ixx> (t.dir, t.name, nullptr, nullptr);
             t.i->group = &t;
@@ -100,21 +95,29 @@ namespace build
         if (t.group != nullptr)
           return t.group->is_a<cli_cxx> ();
 
-        // Then see if there is a corresponding cli.cxx{} group.
+        // Then check if there is a corresponding cli.cxx{} group.
         //
         cli_cxx* g (targets.find<cli_cxx> (t.dir, t.name));
 
-        // Finally, if this target has a cli{} prerequisite, synthesize
+        // If not but this target has a cli{} prerequisite, synthesize
         // the group.
         //
         if (g == nullptr)
         {
           for (prerequisite_member p: group_prerequisite_members (a, t))
           {
-            if (p.is_a<cli> ()) // @@ Need to check that stems match.
+            if (p.is_a<cli> ())
             {
-              g = &targets.insert<cli_cxx> (t.dir, t.name, trace);
-              g->prerequisites.emplace_back (p.as_prerequisite (trace));
+              // Check that the stems match.
+              //
+              if (t.name == p.name ())
+              {
+                g = &targets.insert<cli_cxx> (t.dir, t.name, trace);
+                g->prerequisites.emplace_back (p.as_prerequisite (trace));
+              }
+              else
+                level3 ([&]{trace << ".cli file stem '" << p.name () << "' "
+                                  << "doesn't match target " << t;});
               break;
             }
           }
@@ -182,18 +185,18 @@ namespace build
     static void
     append_extension (vector<const char*>& args,
                       path_target& t,
-                      const char* opt,
-                      const char* def)
+                      const char* option,
+                      const char* default_extension)
     {
       assert (t.ext != nullptr); // Should have been figured out in apply().
 
-      if (*t.ext != def)
+      if (*t.ext != default_extension)
       {
         // CLI needs the extension with the leading dot (unless it is empty)
         // while we store the extension without. But if there is an extension,
         // then we can get it (with the dot) from the file name.
         //
-        args.push_back (opt);
+        args.push_back (option);
         args.push_back (t.ext->empty ()
                         ? t.ext->c_str ()
                         : t.path ().extension () - 1);
@@ -212,7 +215,7 @@ namespace build
       if (s == nullptr)
         return target_state::unchanged;
 
-      // Translate source path to relative (to working directory). This
+      // Translate paths to relative (to working directory). This
       // results in easier to read diagnostics.
       //
       path relo (relative (t.dir));
@@ -230,7 +233,7 @@ namespace build
       if (t.i != nullptr)
         append_extension (args, *t.i, "--ixx-suffix", "ixx");
 
-      append_options (args, t, "cli.options");
+      config::append_options (args, t, "cli.options");
 
       if (!relo.empty ())
       {
@@ -277,7 +280,6 @@ namespace build
       // prerequisites. Also update timestamp in case there are operations
       // after us that could use the information.
       //
-      //
       bool r (false);
 
       if (t.i != nullptr)
diff --git a/build/cli/target b/build/cli/target
index a32f718..06bc311 100644
--- a/build/cli/target
+++ b/build/cli/target
@@ -33,7 +33,7 @@ namespace build
         // It is theoretically possible that the compiler will add
         // padding between the members of this struct. This would
         // mean that the optimal alignment for a pointer is greater
-        // than its size (and that an array of pointer is sub-
+        // than its size (and that an array of pointers is sub-
         // optimally aligned). We will deal with such a beast of
         // an architecture when we see it.
         //
diff --git a/build/cli/target.cxx b/build/cli/target.cxx
index b77252d..d8280f2 100644
--- a/build/cli/target.cxx
+++ b/build/cli/target.cxx
@@ -49,12 +49,12 @@ namespace build
     static target*
     cli_cxx_factory (dir_path d, string n, const string* e)
     {
-      tracer trace ("cli::cli_cxx::factory");
+      tracer trace ("cli::cli_cxx_factory");
 
       // Pre-enter (potential) members as targets. The main purpose
       // of doing this is to avoid searching for existing files in
-      // src_base if the buildfile mentions one of them explicitly
-      // as a prerequisite.
+      // src_base if the buildfile mentions some of them explicitly
+      // as prerequisites.
       //
       targets.insert<cxx::hxx> (d, n, trace);
       targets.insert<cxx::cxx> (d, n, trace);
@@ -71,7 +71,7 @@ namespace build
       &cli_cxx_factory,
       nullptr,
       &search_target,
-      true // See through default semantics.
+      true // "See through" default iteration mode.
     };
   }
 }
diff --git a/build/config/utility b/build/config/utility
index e59f0e5..c2209e4 100644
--- a/build/config/utility
+++ b/build/config/utility
@@ -63,6 +63,24 @@ namespace build
         }
       }
     }
+
+    // Check if a specified option is present. T is either target or scope.
+    //
+    template <typename T>
+    bool
+    find_option (const char* option, T& s, const char* var)
+    {
+      if (auto val = s[var])
+      {
+        for (const name& n: val.template as<const list_value&> ())
+        {
+          if (n.simple () && n.value == option)
+            return true;
+        }
+      }
+
+      return false;
+    }
   }
 }
 
diff --git a/build/target b/build/target
index 39f6679..4131338 100644
--- a/build/target
+++ b/build/target
@@ -506,6 +506,12 @@ namespace build
       return target != nullptr ? target->type () : prerequisite.get ().type;
     }
 
+    const std::string&
+    name () const
+    {
+      return target != nullptr ? target->name : prerequisite.get ().name;
+    }
+
     target_type&
     search () const
     {
diff --git a/tests/cli/simple/buildfile b/tests/cli/simple/buildfile
index a7e6e80..80882c6 100644
--- a/tests/cli/simple/buildfile
+++ b/tests/cli/simple/buildfile
@@ -4,8 +4,8 @@ ixx.ext = ipp
 
 cxx.poptions = -I$out_root
 
-#exe{driver}: cxx{driver} cxx{test}
-#cxx{test} hxx{test}: cli{test}
+exe{driver}: cxx{driver} cxx{test}
+cxx{test} hxx{test}: cli{test}
 
-exe{driver}: cxx{driver} cli.cxx{test}
-cli.cxx{test}: cli{test}
+#exe{driver}: cxx{driver} cli.cxx{test}
+#cli.cxx{test}: cli{test}