Store problems at their source rather than globally

This allows finer granularity in determining whether problems are
preventing a build.  In some cases it may be desirable to build just one
component of a package, even if others have problems.

diff --git a/source/binary.cpp b/source/binary.cpp
index 9f0bfcd2e5aa273fd11fc878ec1bfe4c8d9a22a3..e7b5e818d4e42f3f78272347d1258ec7e883cb49 100644 (file)
--- a/source/binary.cpp
+++ b/source/binary.cpp
@@ -33,6 +33,7 @@ void Binary::find_dependencies()
 
        list<const Component *> queue;
        list<Target *> dep_libs;
+       set<string> missing_libs;
        queue.push_back(component);
        while(!queue.empty())
        {
@@ -56,8 +57,8 @@ void Binary::find_dependencies()
                                else
                                        dep_libs.push_back(lib);
                        }
-                       else
-                               builder.problem(package->get_name(), format("Couldn't find library %s for %s", *i, name));
+                       else if(missing_libs.insert(*i).second)
+                               problems.push_back(format("Required library %s not found", *i));
                }
        }
 

diff --git a/source/binarypackage.cpp b/source/binarypackage.cpp
index 22a415457e75844e52e3c8675953e3e351c8338c..4c5382191661b7c627c0ef0e94db63fa34ab8ec9 100644 (file)
--- a/source/binarypackage.cpp
+++ b/source/binarypackage.cpp
@@ -72,7 +72,8 @@ void BinaryPackage::do_prepare()
 
        if(base_path.empty())
        {
-               builder.problem(name, "Cannot locate files");
+               // TODO report which files were not found
+               problems.push_back("Cannot locate files");
                return;
        }
 

diff --git a/source/builder.cpp b/source/builder.cpp
index bbb95b6634cb03292ae1d37a017bd82094ce6eba..81f4d8e3d481f30b236abd8cdd90e389705cb4b5 100644 (file)
--- a/source/builder.cpp
+++ b/source/builder.cpp
@@ -98,9 +98,57 @@ void Builder::set_logger(const Logger *l)
        logger = (l ? l : &default_logger);
 }
 
-void Builder::problem(const string &p, const string &d)
+list<string> Builder::collect_problems() const
 {
-       problems.push_back(Problem(p, d));
+       list<string> problems;
+       set<const Package *> broken_packages;
+       set<const Component *> broken_components;
+       set<const Tool *> broken_tools;
+
+       const BuildGraph::TargetMap &targets = build_graph.get_targets();
+       for(BuildGraph::TargetMap::const_iterator i=targets.begin(); i!=targets.end(); ++i)
+               if(i->second->is_broken())
+               {
+                       const list<string> &tgt_problems = i->second->get_problems();
+                       for(list<string>::const_iterator j=tgt_problems.begin(); j!=tgt_problems.end(); ++j)
+                               problems.push_back(format("%s: %s", i->second->get_name(), *j));
+
+                       const Package *package = i->second->get_package();
+                       if(package && !package->get_problems().empty())
+                               broken_packages.insert(package);
+
+                       const Component *component = i->second->get_component();
+                       if(component && !component->get_problems().empty())
+                               broken_components.insert(component);
+
+                       const Tool *tool = i->second->get_tool();
+                       if(tool && !tool->get_problems().empty())
+                               broken_tools.insert(tool);
+               }
+
+       // TODO Sort components after their packages, and targets last
+       for(set<const Package *>::const_iterator i=broken_packages.begin(); i!=broken_packages.end(); ++i)
+       {
+               const list<string> &pkg_problems = (*i)->get_problems();
+               for(list<string>::const_iterator j=pkg_problems.begin(); j!=pkg_problems.end(); ++j)
+                       problems.push_back(format("%s: %s", (*i)->get_name(), *j));
+       }
+
+       for(set<const Component *>::const_iterator i=broken_components.begin(); i!=broken_components.end(); ++i)
+       {
+               const list<string> &comp_problems = (*i)->get_problems();
+               for(list<string>::const_iterator j=comp_problems.begin(); j!=comp_problems.end(); ++j)
+                       problems.push_back(format("%s/%s: %s", (*i)->get_package().get_name(), (*i)->get_name(), *j));
+       }
+
+       for(set<const Tool *>::const_iterator i=broken_tools.begin(); i!=broken_tools.end(); ++i)
+       {
+               const list<string> &tool_problems = (*i)->get_problems();
+               for(list<string>::const_iterator j=tool_problems.begin(); j!=tool_problems.end(); ++j)
+                       problems.push_back(format("%s: %s", (*i)->get_tag(), *j));
+       }
+
+       return problems;
 }
 
 void Builder::load_build_file(const FS::Path &fn, const Config::InputOptions *opts, bool all)

diff --git a/source/builder.h b/source/builder.h
index 4c78a3dffd579d1054fa02846c771c3eae62fd3c..3004ed245ecedb860da199e28f8f4bdaecf2eb60 100644 (file)
--- a/source/builder.h
+++ b/source/builder.h
@@ -12,7 +12,6 @@
 #include "config.h"
 #include "logger.h"
 #include "packagemanager.h"
-#include "problem.h"
 #include "target.h"
 #include "toolchain.h"
 #include "virtualfilesystem.h"
@@ -46,9 +45,6 @@ private:
                void package(const std::string &);
        };
 
-public:
-       typedef std::list<Problem> ProblemList;
-
 private:
        typedef std::map<std::string, BuildType> BuildTypeMap;
 
@@ -64,7 +60,6 @@ private:
        Logger default_logger;
        const Logger *logger;
 
-       ProblemList problems;
        Msp::FS::Path prefix;
        Msp::FS::Path tempdir;
 
@@ -93,8 +88,7 @@ public:
        void set_logger(const Logger *);
        const Logger &get_logger() const { return *logger; }
 
-       void problem(const std::string &, const std::string &);
-       const ProblemList &get_problems() const { return problems; }
+       std::list<std::string> collect_problems() const;
 
        /** 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

diff --git a/source/buildercli.cpp b/source/buildercli.cpp
index 629231814a0012a152a0222b29800dd4765b267c..a7dd1e23ff2402f94ab07a1b67ce85f0fb446801 100644 (file)
--- a/source/buildercli.cpp
+++ b/source/buildercli.cpp
@@ -255,12 +255,12 @@ int BuilderCLI::main()
        if(analyzer)
                analyzer->analyze();
 
-       const Builder::ProblemList &problems = builder.get_problems();
-       if(!problems.empty())
+       if(build_graph.get_goals().is_broken())
        {
+               list<string> problems = builder.collect_problems();
                IO::print(IO::cerr, "The following problems were detected:\n");
-               for(Builder::ProblemList::const_iterator i=problems.begin(); i!=problems.end(); ++i)
-                       IO::print(IO::cerr, "  %s: %s\n", i->package, i->descr);
+               for(list<string>::const_iterator i=problems.begin(); i!=problems.end(); ++i)
+                       IO::print(IO::cerr, "  %s\n", *i);
                if(!analyzer)
                        IO::print(IO::cerr, "Please fix them and try again.\n");
                return 1;

diff --git a/source/component.cpp b/source/component.cpp
index d0cc1e9822922726f5a5cce8f2af3c5fa0a7b7af..5e918c841719967b99f904c0d3c5fae05aaeb158 100644 (file)
--- a/source/component.cpp
+++ b/source/component.cpp
@@ -348,6 +348,8 @@ void Component::Loader::require(const string &n)
        Package *req = obj.package.get_builder().get_package_manager().find_package(n);
        if(req)
                obj.requires.push_back(req);
+       else
+               obj.problems.push_back(format("Required package %s not found", n));
 }
 
 void Component::Loader::source(const string &s)

diff --git a/source/component.h b/source/component.h
index 3a743d0c41987324ab7da79818cab037059984e4..7cbd1e13cbe1f0099522161e4e2eb74c708fcad3 100644 (file)
--- a/source/component.h
+++ b/source/component.h
@@ -60,6 +60,7 @@ protected:
        UseList uses;
        bool deflt;
        InstallMap install_map;
+       std::list<std::string> problems;
 
 public:
        Component(SourcePackage &, Type, const std::string &);
@@ -84,6 +85,7 @@ public:
        const Package::Requirements &get_required_packages() const { return requires; }
        const UseList &get_used_components() const { return uses; }
        bool is_default() const { return deflt; }
+       const std::list<std::string> &get_problems() const { return problems; }
 
        /** Prepares any required packages. */
        void prepare();

diff --git a/source/package.cpp b/source/package.cpp
index 05bebcc55f929971cb5d1780ba98200b6730bcca..aa9160e11b8e62ab03c16a0e961dc579bbe0894f 100644 (file)
--- a/source/package.cpp
+++ b/source/package.cpp
@@ -49,4 +49,6 @@ void Package::Loader::require(const string &n)
        Package *req = obj.builder.get_package_manager().find_package(n);
        if(req)
                obj.requires.push_back(req);
+       else
+               obj.problems.push_back(format("Required package %s not found", n));
 }

diff --git a/source/package.h b/source/package.h
index aed1fd9c061b1f37fde116c42ec23ee539d3a3a5..cd2d021129dcaee30411ff9fc6a16a90a57c8bad 100644 (file)
--- a/source/package.h
+++ b/source/package.h
@@ -37,6 +37,7 @@ protected:
        Requirements requires;
        BuildInfo export_binfo;
        bool prepared;
+       std::list<std::string> problems;
 
        bool use_pkgconfig;
 
@@ -63,6 +64,8 @@ protected:
 public:
        bool is_prepared() const { return prepared; }
 
+       const std::list<std::string> &get_problems() const { return problems; }
+
        virtual void save_caches() { }
 };
 

diff --git a/source/packagemanager.cpp b/source/packagemanager.cpp
index 0e2d8a0f578805c4fc0427cc3d87a5433658a16d..ee07987d0186e86641cf43cf8aa11f801d3ea18f 100644 (file)
--- a/source/packagemanager.cpp
+++ b/source/packagemanager.cpp
@@ -116,7 +116,6 @@ Package *PackageManager::find_package(const string &name)
        }
        catch(...)
        {
-               builder.problem(name, "not found");
                not_found.insert(name);
                return 0;
        }

diff --git a/source/problem.h b/source/problem.h
deleted file mode 100644 (file)
index d3d10d4..0000000
--- a/source/problem.h
+++ /dev/null
@@ -1,15 +0,0 @@
-#ifndef PROBLEM_H_
-#define PROBLEM_H_
-
-#include <list>
-#include <string>
-
-struct Problem
-{
-       std::string package;
-       std::string descr;
-
-       Problem(const std::string &p, const std::string &d): package(p), descr(d) { }
-};
-
-#endif

diff --git a/source/target.cpp b/source/target.cpp
index 87b0d0078e16bcb565898c55ba6923e807b328c1..7f031ec2dd384a1b86e0e4f61f7797d4f0803798 100644 (file)
--- a/source/target.cpp
+++ b/source/target.cpp
@@ -97,23 +97,40 @@ void Target::prepare()
                return;
        if(state==PREPARING)
        {
-               builder.problem((package ? package->get_name() : string()), "Dependency cycle detected at "+name);
+               problems.push_back("Dependency cycle detected");
+               state = BROKEN;
                return;
        }
 
        state = PREPARING;
        if(tool)
                tool->prepare();
+
        find_dependencies();
+       bool broken = !problems.empty();
+
        if(tool)
+       {
                if(FileTarget *tool_exe = tool->get_executable())
                        add_dependency(*tool_exe);
+               broken |= !tool->get_problems().empty();
+
+               // 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());
+       }
 
        for(Dependencies::iterator i=depends.begin(); i!=depends.end(); ++i)
+       {
                (*i)->prepare();
+               broken |= (*i)->is_broken();
+       }
 
        check_rebuild();
-       if(state==PREPARING)
+       if(broken)
+               state = BROKEN;
+       else if(state==PREPARING)
                state = UPTODATE;
 
        for(Dependencies::iterator i=depends.begin(); i!=depends.end(); ++i)

diff --git a/source/target.h b/source/target.h
index eda77d00dcd126b1c587768f04af4b8e7b162b2b..7c1d9a74b7ea90bc2ec881254da2cda61861f39b 100644 (file)
--- a/source/target.h
+++ b/source/target.h
@@ -30,7 +30,8 @@ protected:
                PREPARING,
                REBUILD,
                BUILDING,
-               UPTODATE
+               UPTODATE,
+               BROKEN
        };
 
 public:
@@ -45,6 +46,7 @@ protected:
        Tool *tool;
        State state;
        std::string rebuild_reason;
+       std::list<std::string> problems;
 
        Dependencies depends;
        Dependencies side_effects;
@@ -123,6 +125,10 @@ protected:
        virtual void check_rebuild() = 0;
 
 public:
+       bool is_broken() const { return state==BROKEN; }
+
+       const std::list<std::string> &get_problems() const { return problems; }
+
        /** Prepares the target by finding dependencies, recursively preparing them
        and then checking whether rebuilding is needed. */
        void prepare();

diff --git a/source/tool.cpp b/source/tool.cpp
index 65a45041286e6a054649461bd65770cccc614d69..8aa75dcafd982646980871b59ff74c796d5a00ca 100644 (file)
--- a/source/tool.cpp
+++ b/source/tool.cpp
@@ -56,7 +56,7 @@ void Tool::set_executable(const string &command, bool cross)
 
        executable = builder.get_vfs().find_binary(command);
        if(!executable)
-               builder.problem(string(), format("Can't find executable %s for tool %s", command, tag));
+               problems.push_back(format("Can't find executable %s", command));
 }
 
 

diff --git a/source/tool.h b/source/tool.h
index 93e2196189ca1f110f3a7d5774f4523420f03a28..042199e54b79342c10e967f7b6de00a6e474554c 100644 (file)
--- a/source/tool.h
+++ b/source/tool.h
@@ -32,6 +32,7 @@ protected:
        SuffixList aux_suffixes;
        SearchPath system_path;
        bool prepared;
+       std::list<std::string> problems;
 
        Tool(Builder &, const std::string &);
        Tool(Builder &, const Architecture &, const std::string &);
@@ -91,6 +92,8 @@ protected:
        void set_executable(const std::string &command, bool cross = false);
 
 public:
+       const std::list<std::string> &get_problems() const { return problems; }
+
        /** Invokes the tool to build a target.  This should not be called directly;
        use Target::build() instead. */
        virtual Task *run(const Target &) const = 0;