Diagnose incorrect output directory specification

4 files changed, 90 insertions, 22 deletions

diff --git a/libbuild2/buildfile b/libbuild2/buildfile
index 52252d6..b4f420c 100644
--- a/libbuild2/buildfile
+++ b/libbuild2/buildfile
@@ -304,11 +304,11 @@ else
 {
   # No install for the pre-generated case.
   #
-  script/hxx{builtin-options}@./ \
-  script/ixx{builtin-options}@./: install = false
+  script/hxx{builtin-options}@script/ \
+  script/ixx{builtin-options}@script/: install = false
 
-  build/script/hxx{builtin-options}@./ \
-  build/script/ixx{builtin-options}@./: install = false
+  build/script/hxx{builtin-options}@build/script/ \
+  build/script/ixx{builtin-options}@build/script/: install = false
 }
 
 # Install into the libbuild2/ subdirectory of, say, /usr/include/
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index a13dc41..d167200 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -2491,7 +2491,7 @@ namespace build2
       // Current dir collapses to an empty one.
       //
       if (!n.dir.empty ())
-        n.dir.normalize (false, true);
+        n.dir.normalize (false /* actual */, true);
 
       // @@ OUT: for now we assume the prerequisite's out is undetermined. The
       // only way to specify an src prerequisite will be with the explicit
@@ -2514,6 +2514,29 @@ namespace build2
           fail (ploc) << "expected directory after '@'";
 
         o.dir.normalize (); // Note: don't collapse current to empty.
+
+        // Make sure out and src are parallel.
+        //
+        // See similar code for targets in scope::find_target_type().
+        //
+        // For now we require that both are either relative or absolute.
+        //
+        if (n.dir.empty () && o.dir.current ())
+          ;
+        else if (o.dir.relative () &&
+                 n.dir.relative () &&
+                 o.dir == n.dir)
+          ;
+        else if (o.dir.absolute () &&
+                 n.dir.absolute () &&
+                 o.dir.sub (root_->out_path ()) &&
+                 n.dir.sub (root_->src_path ()) &&
+                 o.dir.leaf (root_->out_path ()) ==
+                 n.dir.leaf (root_->src_path ()))
+          ;
+        else
+          fail (ploc) << "prerequisite output directory " << o.dir
+                      << " must be parallel to source directory " << n.dir;
       }
 
       prerequisite p (move (n.proj),
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index 5811bd1..0d952f0 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -806,26 +806,45 @@ namespace build2
         fail (loc) << "expected directory after '@'";
     }
 
-    dir_path& d (n.dir);
+    dir_path& dir (n.dir);
 
     const dir_path& sd (src_path ());
     const dir_path& od (out_path ());
 
-    if (d.empty ())
-      d = src ? sd : od; // Already dormalized.
+    if (dir.empty ())
+      dir = src ? sd : od; // Already dormalized.
     else
     {
-      if (d.relative ())
-        d = (src ? sd : od) / d;
+      if (dir.relative ())
+        dir = (src ? sd : od) / dir;
 
-      d.normalize ();
+      dir.normalize ();
     }
 
     dir_path out;
-    if (src && sd != od) // If in source build, then out must be empty.
+    if (src)
     {
       out = o.dir.relative () ? od / o.dir : move (o.dir);
       out.normalize ();
+
+      // Make sure out and src are parallel.
+      //
+      // See similar code for prerequisites in parser::parse_dependency().
+      //
+      if (out.sub (root_->out_path ()) &&
+          dir.sub (root_->src_path ()) &&
+          out.leaf (root_->out_path ()) == dir.leaf (root_->src_path ()))
+        ;
+      else
+        // @@ TMP change warn to fail after 0.16.0 release.
+        //
+        warn (loc) << "target output directory " << out
+                   << " must be parallel to source directory " << dir;
+
+      // If in source build, then out must be empty.
+      //
+      if (sd == od)
+        out.clear ();
     }
     o.dir = move (out); // Result.
 
diff --git a/libbuild2/search.cxx b/libbuild2/search.cxx
index 19385b0..c2f7de4 100644
--- a/libbuild2/search.cxx
+++ b/libbuild2/search.cxx
@@ -58,8 +58,11 @@ namespace build2
       else
       {
         o = pk.scope->out_path ();
-        o /= *tk.out;
-        o.normalize ();
+        if (!tk.out->current ())
+        {
+          o /= *tk.out;
+          o.normalize ();
+        }
       }
 
       // Drop out if it is the same as src (in-src build).
@@ -169,11 +172,7 @@ namespace build2
     // will be from the src tree.
     //
     // In the other two cases we use the prerequisite's out (in case it is
-    // relative, we need to complete it, which is @@ OUT TODO). Note that we
-    // blindly trust the user's value which can be used for some interesting
-    // tricks, for example:
-    //
-    // ../cxx{foo}@./
+    // relative, we need to complete it).
     //
     dir_path out;
 
@@ -183,7 +182,24 @@ namespace build2
         out = out_src (d, *s->root_scope ());
     }
     else
-      out = *tk.out;
+    {
+      if (tk.out->absolute ())
+        out = *tk.out; // Already normalized.
+      else
+      {
+        out = pk.scope->out_path ();
+        if (!tk.out->current ())
+        {
+          out /= *tk.out;
+          out.normalize ();
+        }
+      }
+
+      // Drop out if it is the same as src (in-src build).
+      //
+      if (out == d)
+        out.clear ();
+    }
 
     // Find or insert. Note that we are using our updated extension.
     //
@@ -235,8 +251,13 @@ namespace build2
     //
     // More often insert than find, so skip find in insert().
     //
-    // @@ OUT: same story as in search_existing_target() re out.
+    // @@ OUT: same story as in search_existing_target() re out. Maybe not:
+    //         if out is present, then it means the target is in src and we
+    //         shouldn't be creating new targets in src, should we? Feels
+    //         like this should not even be called in out is not empty.
     //
+    //assert (tk.out->empty ()); @@ TMP
+
     auto r (ctx.targets.insert (*tk.type,
                                 move (d),
                                 *tk.out,
@@ -279,8 +300,13 @@ namespace build2
     //
     // More often insert than find, so skip find in insert_locked().
     //
-    // @@ OUT: same story as in search_existing_target() re out.
+    // @@ OUT: same story as in search_existing_target() re out. Maybe not:
+    //         if out is present, then it means the target is in src and we
+    //         shouldn't be creating new targets in src, should we? Feels
+    //         like this should not even be called in out is not empty.
     //
+    //assert (tk.out->empty ()); @@ TMP
+
     auto r (ctx.targets.insert_locked (*tk.type,
                                        move (d),
                                        *tk.out,