aboutsummaryrefslogtreecommitdiff
path: root/libbuild2
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2023-11-01 11:50:08 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2023-11-01 11:50:08 +0200
commitd0d4486702b045852dca36008746afeb8754ae85 (patch)
treeb4f3930c8a64b4650a183c59257a43adc854da46 /libbuild2
parent736c9f08b68b2735d85fe7eefdf2118de8b8c34e (diff)
Tighten/optimize cleaning of fsdir{} during match
Diffstat (limited to 'libbuild2')
-rw-r--r--libbuild2/algorithm.cxx11
-rw-r--r--libbuild2/algorithm.hxx5
-rw-r--r--libbuild2/algorithm.ixx2
-rw-r--r--libbuild2/rule.cxx29
4 files changed, 37 insertions, 10 deletions
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index 4ec4db5..1f2d88b 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -3137,7 +3137,14 @@ namespace build2
// Let's keep this as close to update_during_match() semantically as
// possible until we see a clear reason to deviate.
- assert (a == perform_clean_id);
+ // We have a problem with fsdir{}: if the directory is not empty because
+ // there are other targets that depend on it and we execute it here and
+ // now, it will not remove the directory (because it's not yet empty) but
+ // will cause the target to be in the executed state, which means that
+ // when other targets try to execute it, it will be a noop and the
+ // directory will be left behind.
+
+ assert (a == perform_clean_id && !t.is_a<fsdir> ());
target_state os (t.matched_state (a));
@@ -3194,6 +3201,8 @@ namespace build2
{
const target& pt (*p.target);
+ assert (!pt.is_a<fsdir> ()); // See above.
+
target_state os (pt.matched_state (a));
if (os != target_state::unchanged)
diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx
index 19f7db2..11e655b 100644
--- a/libbuild2/algorithm.hxx
+++ b/libbuild2/algorithm.hxx
@@ -740,6 +740,11 @@ namespace build2
// cleaning to normal execute and these functions should only be used in
// special cases where this is not possible.
//
+ // Note also that neither function should be called on fsdir{} since it's
+ // hard to guarantee such an execution won't be too early (see the
+ // implementation for details). If you do need to clean fsdir{} during
+ // match, use fsdir_rule::perform_clean_direct() instead.
+ //
LIBBUILD2_SYMEXPORT bool
clean_during_match (tracer&,
action, const target&);
diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx
index 09fc6d9..bae9151 100644
--- a/libbuild2/algorithm.ixx
+++ b/libbuild2/algorithm.ixx
@@ -482,7 +482,7 @@ namespace build2
// cannot change their mind).
//
if ((s == target_state::unchanged && t.group == nullptr) ||
- t[a].dependents.load (memory_order_consume) != 0)
+ t[a].dependents.load (memory_order_relaxed) != 0)
return make_pair (true, s);
break;
diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx
index 79ccb86..9f90919 100644
--- a/libbuild2/rule.cxx
+++ b/libbuild2/rule.cxx
@@ -354,6 +354,8 @@ namespace build2
void fsdir_rule::
perform_update_direct (action a, const fsdir& t)
{
+ assert (t.ctx.phase == run_phase::match);
+
// First create the parent directory. If present, it is always first.
//
if (const target* p = (t.prerequisite_targets[a].empty ()
@@ -397,18 +399,29 @@ namespace build2
void fsdir_rule::
perform_clean_direct (action a, const fsdir& t)
{
+ assert (t.ctx.phase == run_phase::match);
+
// The same code as in perform_clean() above.
//
- rmdir (t.dir, t, t.ctx.current_diag_noise ? 1 : 2);
-
- // Then clean the parent directory. If present, it is always first.
+ // Except that if there are other dependens of this fsdir{} then this will
+ // likely be a noop (because the directory won't be empty) and it makes
+ // sense to just defer cleaning to such other dependents. See
+ // clean_during_match() for backgound. This is similar logic as in
+ // unmatch::safe.
//
- if (const target* p = (t.prerequisite_targets[a].empty ()
- ? nullptr
- : t.prerequisite_targets[a][0]))
+ if (t[a].dependents.load (memory_order_relaxed) == 0)
{
- if (const fsdir* fp = p->is_a<fsdir> ())
- perform_clean_direct (a, *fp);
+ rmdir (t.dir, t, t.ctx.current_diag_noise ? 1 : 2);
+
+ // Then clean the parent directory. If present, it is always first.
+ //
+ if (const target* p = (t.prerequisite_targets[a].empty ()
+ ? nullptr
+ : t.prerequisite_targets[a][0]))
+ {
+ if (const fsdir* fp = p->is_a<fsdir> ())
+ perform_clean_direct (a, *fp);
+ }
}
}