From d74b2a50c421bd4a0fd8753848d3796029fcff43 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 20 Oct 2022 10:21:21 +0200 Subject: Diagnose incorrect output directory specification --- libbuild2/buildfile | 8 ++++---- libbuild2/parser.cxx | 25 ++++++++++++++++++++++++- libbuild2/scope.cxx | 33 ++++++++++++++++++++++++++------- libbuild2/search.cxx | 46 ++++++++++++++++++++++++++++++++++++---------- 4 files changed, 90 insertions(+), 22 deletions(-) (limited to 'libbuild2') 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, -- cgit v1.1