From c285dafe9e2b7e4bba3fddad3fa254e4bdbb02d3 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 9 Feb 2022 10:11:18 +0200 Subject: Don't use fallback file_rule to clean real targets --- libbuild2/algorithm.cxx | 16 ++++++++++++++-- libbuild2/rule.cxx | 5 ++++- libbuild2/target.hxx | 4 ++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index e15fcbc..9b6fd4e 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -967,9 +967,21 @@ namespace build2 // not seem like it will be easy to fix (we don't know whether // someone else will execute this target). // - // @@ What if we always do match & execute together? After all, + // What if we always do match & execute together? After all, // if a group can be resolved in apply(), then it can be - // resolved in match()! + // resolved in match()! Feels a bit drastic. + // + // But, this won't be a problem if the target returns noop_recipe. + // And perhaps it's correct to fail if it's not noop_recipe but + // nobody executed it? Maybe not. + // + // Another option would be to have a count for such "matched but + // may not be executed" targets and then make sure target_count + // is less than that at the end. Though this definitelt makes it + // less exact (since we can end up executed this target but not + // some other). Maybe we can increment and decrement such targets + // in a separate count (i.e., mark their recipe as special or some + // such). // // Apply (locked). diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx index 6dad685..c573339 100644 --- a/libbuild2/rule.cxx +++ b/libbuild2/rule.cxx @@ -56,10 +56,13 @@ namespace build2 // are not doing anything for this action so not checking if the file // exists seems harmless. // + // But we also don't want to match real targets and not cleaning their + // output files. + // switch (a) { case perform_clean_id: - return true; + return t.decl != target_decl::real; default: { // While normally we shouldn't do any of this in match(), no other diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 76c01b6..809bf8b 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -129,6 +129,10 @@ namespace build2 // @@ We have cases (like pkg-config extraction) where it should probably be // prereq_file rather than implied (also audit targets.insert<> calls). // + // @@ Also, synthesized dependency declarations (e.g., in cc::link_rule) are + // fuzzy: they feel more `real` than `implied`. Maybe introduce + // `synthesized` in-between? + // enum class target_decl: uint8_t { prereq_new, // Created from prerequisite (create_new_target()). -- cgit v1.1