From: Mikko Rasa <tdb@tdb.fi>
Date: Thu, 29 Dec 2022 16:36:26 +0000 (+0200)
Subject: Improve package brokenness checks and problem reporting
X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=2c42374a95a76dcb28eaf3e5d85d321fafe62b38;p=builder.git

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 9cf6266..48f3550 100644
--- 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 6dc29f0..28379ed 100644
--- 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 77fbe12..b98556c 100644
--- 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 7ddd972..816d2c1 100644
--- 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 2a4cf8f..25661fc 100644
--- 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 d13aa73..8e0c692 100644
--- 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 dbdd950..f13afb8 100644
--- 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 b756563..a6bd322 100644
--- 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;