From 0d95fee118a3fcd78f153dca5721d9fe19b6f6bf Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Wed, 8 May 2013 12:36:22 +0300 Subject: [PATCH] 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. --- source/binary.cpp | 5 ++-- source/binarypackage.cpp | 3 ++- source/builder.cpp | 52 +++++++++++++++++++++++++++++++++++++-- source/builder.h | 8 +----- source/buildercli.cpp | 8 +++--- source/component.cpp | 2 ++ source/component.h | 2 ++ source/package.cpp | 2 ++ source/package.h | 3 +++ source/packagemanager.cpp | 1 - source/problem.h | 15 ----------- source/target.cpp | 21 ++++++++++++++-- source/target.h | 8 +++++- source/tool.cpp | 2 +- source/tool.h | 3 +++ 15 files changed, 99 insertions(+), 36 deletions(-) delete mode 100644 source/problem.h diff --git a/source/binary.cpp b/source/binary.cpp index 9f0bfcd..e7b5e81 100644 --- a/source/binary.cpp +++ b/source/binary.cpp @@ -33,6 +33,7 @@ void Binary::find_dependencies() list queue; list dep_libs; + set 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 22a4154..4c53821 100644 --- 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 bbb95b6..81f4d8e 100644 --- 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 Builder::collect_problems() const { - problems.push_back(Problem(p, d)); + list problems; + set broken_packages; + set broken_components; + set 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 &tgt_problems = i->second->get_problems(); + for(list::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_iterator i=broken_packages.begin(); i!=broken_packages.end(); ++i) + { + const list &pkg_problems = (*i)->get_problems(); + for(list::const_iterator j=pkg_problems.begin(); j!=pkg_problems.end(); ++j) + problems.push_back(format("%s: %s", (*i)->get_name(), *j)); + } + + for(set::const_iterator i=broken_components.begin(); i!=broken_components.end(); ++i) + { + const list &comp_problems = (*i)->get_problems(); + for(list::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_iterator i=broken_tools.begin(); i!=broken_tools.end(); ++i) + { + const list &tool_problems = (*i)->get_problems(); + for(list::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 4c78a3d..3004ed2 100644 --- 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 ProblemList; - private: typedef std::map 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 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 6292318..a7dd1e2 100644 --- 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 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::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 d0cc1e9..5e918c8 100644 --- 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 3a743d0..7cbd1e1 100644 --- a/source/component.h +++ b/source/component.h @@ -60,6 +60,7 @@ protected: UseList uses; bool deflt; InstallMap install_map; + std::list 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 &get_problems() const { return problems; } /** Prepares any required packages. */ void prepare(); diff --git a/source/package.cpp b/source/package.cpp index 05bebcc..aa9160e 100644 --- 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 aed1fd9..cd2d021 100644 --- a/source/package.h +++ b/source/package.h @@ -37,6 +37,7 @@ protected: Requirements requires; BuildInfo export_binfo; bool prepared; + std::list problems; bool use_pkgconfig; @@ -63,6 +64,8 @@ protected: public: bool is_prepared() const { return prepared; } + const std::list &get_problems() const { return problems; } + virtual void save_caches() { } }; diff --git a/source/packagemanager.cpp b/source/packagemanager.cpp index 0e2d8a0..ee07987 100644 --- 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 index d3d10d4..0000000 --- a/source/problem.h +++ /dev/null @@ -1,15 +0,0 @@ -#ifndef PROBLEM_H_ -#define PROBLEM_H_ - -#include -#include - -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 87b0d00..7f031ec 100644 --- 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 eda77d0..7c1d9a7 100644 --- 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 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 &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 65a4504..8aa75dc 100644 --- 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 93e2196..042199e 100644 --- a/source/tool.h +++ b/source/tool.h @@ -32,6 +32,7 @@ protected: SuffixList aux_suffixes; SearchPath system_path; bool prepared; + std::list 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 &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; -- 2.43.0