Improve package brokenness checks and problem reporting

Packages are now considered broken if any required packages are broken,
and problems are reported for required packages as well.

diff --git a/source/lib/builder.cpp b/source/lib/builder.cpp
index 9cf6266af8fef3d03db31b9d6d81e33310220bfc..48f35500667edcf391f6153f64c584fef18604cc 100644 (file)
--- a/source/lib/builder.cpp
+++ b/source/lib/builder.cpp
@@ -159,46 +159,71 @@ void Builder::set_logger(const Logger *l)
 
 vector<string> Builder::collect_problems() const
 {
+       vector<string> target_problems;
        vector<string> problems;
-       set<const Package *> broken_packages;
-       set<const Component *> broken_components;
-       set<const Tool *> broken_tools;
+       vector<const Package *> broken_packages;
+       vector<const Component *> broken_components;
+       vector<const Tool *> broken_tools;
 
        for(const auto &kvp: build_graph.get_targets())
                if(kvp.second->is_broken())
                {
-                       for(const string &p: kvp.second->get_problems())
-                               problems.push_back(format("%s: %s", kvp.second->get_name(), p));
-
                        const Package *package = kvp.second->get_package();
-                       if(package && !package->get_problems().empty())
-                               broken_packages.insert(package);
+                       if(package && package->is_broken())
+                               collect_broken_packages(*package, broken_packages);
 
                        const Component *component = kvp.second->get_component();
-                       if(component && !component->get_problems().empty())
-                               broken_components.insert(component);
+                       if(component && component->is_broken() && !any_equals(broken_components, component))
+                       {
+                               broken_components.push_back(component);
+                               collect_broken_packages(component->get_package(), broken_packages);
+                               for(const Package *r: component->get_required_packages())
+                                       if(r->is_broken())
+                                               collect_broken_packages(*r, broken_packages);
+                       }
 
                        const Tool *tool = kvp.second->get_tool();
-                       if(tool && !tool->get_problems().empty())
-                               broken_tools.insert(tool);
+                       if(tool && tool->is_broken() && !any_equals(broken_tools, tool))
+                       {
+                               broken_tools.push_back(tool);
+                               for(const string &p: tool->get_problems())
+                                       problems.push_back(format("%s: %s", tool->get_tag(), p));
+                       }
+
+                       for(const string &p: kvp.second->get_problems())
+                               target_problems.push_back(format("%s: %s", kvp.second->get_name(), p));
                }
 
-       // TODO Sort components after their packages, and targets last
        for(const Package *p: broken_packages)
+       {
                for(const string &b: p->get_problems())
                        problems.push_back(format("%s: %s", p->get_name(), b));
 
-       for(const Component *c: broken_components)
-               for(const string &b: c->get_problems())
-                       problems.push_back(format("%s/%s: %s", c->get_package().get_name(), c->get_name(), b));
+               for(const Component *c: broken_components)
+                       if(&c->get_package()==p)
+                       {
+                               for(const string &b: c->get_problems())
+                                       problems.push_back(format("%s/%s: %s", p->get_name(), c->get_name(), b));
+                       }
+       }
 
-       for(const Tool *t: broken_tools)
-               for(const string &b: t->get_problems())
-                       problems.push_back(format("%s: %s", t->get_tag(), b));
+       problems.insert(problems.end(), make_move_iterator(target_problems.begin()), make_move_iterator(target_problems.end()));
 
        return problems;
 }
 
+void Builder::collect_broken_packages(const Package &pkg, vector<const Package *> &broken) const
+{
+       if(any_equals(broken, &pkg))
+               return;
+
+       broken.push_back(&pkg);
+
+       for(const Package *r: pkg.get_required_packages())
+               if(r->is_broken())
+                       collect_broken_packages(*r, broken);
+}
+
 void Builder::load_build_file(const FS::Path &fn, const Config::InputOptions *opts, bool all)
 {
        IO::BufferedFile in(fn.str());

diff --git a/source/lib/builder.h b/source/lib/builder.h
index 6dc29f098d2a22a714594c4dab1262de2a65df72..28379ed73a452409ad66715fc134e3a60365f754 100644 (file)
--- a/source/lib/builder.h
+++ b/source/lib/builder.h
@@ -111,7 +111,10 @@ public:
        const Logger &get_logger() const { return *logger; }
 
        std::vector<std::string> collect_problems() const;
+private:
+       void collect_broken_packages(const Package &, std::vector<const Package *> &) const;
 
+public:
        /** Loads a build file.  If opts is not null, it is used to configure any
        packages loaded from this file.  If all is true, external packages are also
        configured. */

diff --git a/source/lib/component.cpp b/source/lib/component.cpp
index 77fbe122dc062dcfe9628bb8446e0d6034b7cd68..b98556c20476c1a92e0096bd9256a54f741c2c01 100644 (file)
--- a/source/lib/component.cpp
+++ b/source/lib/component.cpp
@@ -14,7 +14,13 @@ using namespace Msp;
 void Component::prepare()
 {
        for(Package *r: requires)
+       {
                r->prepare();
+               broken |= r->is_broken();
+       }
+
+       if(!problems.empty())
+               broken = true;
 }
 
 void Component::create_build_info()

diff --git a/source/lib/component.h b/source/lib/component.h
index 7ddd97297f267b96e865b82c5ef3f24b1390aa18..816d2c11803c1eb947fb6244590548b71f5fcd84 100644 (file)
--- a/source/lib/component.h
+++ b/source/lib/component.h
@@ -42,6 +42,7 @@ protected:
        Package::Requirements requires;
        bool deflt = true;
        InstallMap install_map;
+       bool broken = false;
        std::vector<std::string> problems;
 
        Component(SourcePackage &p, const std::string &n): package(p), name(n) { }
@@ -66,6 +67,7 @@ public:
        const InstallMap &get_install_map() const { return install_map; }
        const Package::Requirements &get_required_packages() const { return requires; }
        bool is_default() const { return deflt; }
+       bool is_broken() const { return broken; }
        const std::vector<std::string> &get_problems() const { return problems; }
 
        /** Prepares any required packages. */

diff --git a/source/lib/package.cpp b/source/lib/package.cpp
index 2a4cf8f3303ef10ec9bc2d86cea7f48eb0ea27a1..25661fc40bb70fdc57ae4d5dfe9bc88f0f46705d 100644 (file)
--- a/source/lib/package.cpp
+++ b/source/lib/package.cpp
@@ -21,9 +21,13 @@ void Package::prepare()
        prepared = true;
 
        for(Package *r: requires)
+       {
                r->prepare();
+               broken |= r->is_broken();
+       }
 
        do_prepare();
+       broken |= !problems.empty();
 }
 
 

diff --git a/source/lib/package.h b/source/lib/package.h
index d13aa737b985a71a8a99242445bb5915472c66d3..8e0c6924fc33b85b461cca7b808cc856237adde2 100644 (file)
--- a/source/lib/package.h
+++ b/source/lib/package.h
@@ -38,6 +38,7 @@ protected:
        Requirements requires;
        BuildInfo export_binfo;
        bool prepared = false;
+       bool broken = false;
        std::vector<std::string> problems;
 
        bool use_pkgconfig = true;
@@ -66,6 +67,7 @@ protected:
 public:
        bool is_prepared() const { return prepared; }
 
+       bool is_broken() const { return broken; }
        const std::vector<std::string> &get_problems() const { return problems; }
 
        virtual void save_caches() { }

diff --git a/source/lib/target.cpp b/source/lib/target.cpp
index dbdd950e4a3bd7570c920b6122b0ee3cfcecf3e2..f13afb8d07bb68c8da074a217dbd6c6bbd5ee293 100644 (file)
--- a/source/lib/target.cpp
+++ b/source/lib/target.cpp
@@ -136,12 +136,11 @@ void Target::prepare()
        {
                if(FileTarget *tool_exe = tool->get_executable())
                        add_dependency(*tool_exe);
-               broken |= !tool->get_problems().empty();
+               broken |= tool->is_broken();
 
-               // Only check package and component problems for buildable targets
-               // XXX How to propagate nested package problems?
-               broken |= (package && !package->get_problems().empty());
-               broken |= (component && !component->get_problems().empty());
+               // Only check package and component brokenness for buildable targets
+               broken |= (package && package->is_broken());
+               broken |= (component && component->is_broken());
        }
 
        /* Now that all dependencies are known, prepare them again.  This will do

diff --git a/source/lib/tool.h b/source/lib/tool.h
index b756563242ba4099152f0ce125bd93a6504d9ffb..a6bd322a762b0bce5ccc609990870efe1c78a030 100644 (file)
--- a/source/lib/tool.h
+++ b/source/lib/tool.h
@@ -142,6 +142,7 @@ protected:
        virtual void do_prepare(ToolData &) const { }
 
 public:
+       bool is_broken() const { return !problems.empty(); }
        const std::vector<std::string> &get_problems() const { return problems; }
 
        /** Invokes the tool to build a target.  This should not be called directly;