From: Mikko Rasa <tdb@tdb.fi>
Date: Wed, 8 May 2013 09:36:22 +0000 (+0300)
Subject: Store problems at their source rather than globally
X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=0d95fee118a3fcd78f153dca5721d9fe19b6f6bf;p=builder.git

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 9f0bfcd..e7b5e81 100644
--- 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 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<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 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<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 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<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 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<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 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<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 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 <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 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<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 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<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;