diff options
Diffstat (limited to 'libbuild2')
| -rw-r--r-- | libbuild2/algorithm.cxx | 11 | ||||
| -rw-r--r-- | libbuild2/algorithm.hxx | 5 | ||||
| -rw-r--r-- | libbuild2/algorithm.ixx | 2 | ||||
| -rw-r--r-- | libbuild2/rule.cxx | 29 |
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); + } } } |