Skip to content

Add reporting of the reason why a rule's result is invalid to clients #1017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add reporting of the reason why a rule's result is invalid to clients #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion include/llbuild/BuildSystem/BuildSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ class BuildSystemDelegate {
/// \param reason - Describes why the rule needs to run. For example, because it has never run or because an input was rebuilt.
///
/// \param inputRule - If `reason` is `InputRebuilt`, the rule for the rebuilt input, else `nullptr`.
virtual void determinedRuleNeedsToRun(core::Rule* ruleNeedingToRun, core::Rule::RunReason reason, core::Rule* inputRule) = 0;
///
/// \param details - Unstructured additional detail explaining why the rule needs to run.
virtual void determinedRuleNeedsToRun(core::Rule* ruleNeedingToRun, core::Rule::RunReason reason, core::Rule* inputRule, StringRef details) = 0;
};

/// The BuildSystem class is used to perform builds using the native build
Expand Down
4 changes: 3 additions & 1 deletion include/llbuild/BuildSystem/BuildSystemFrontend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ class BuildSystemFrontendDelegate : public BuildSystemDelegate {
/// \param reason - Describes why the rule needs to run. For example, because it has never run or because an input was rebuilt.
///
/// \param inputRule - If `reason` is `InputRebuilt`, the rule for the rebuilt input, else `nullptr`.
virtual void determinedRuleNeedsToRun(core::Rule* ruleNeedingToRun, core::Rule::RunReason reason, core::Rule* inputRule) override;
///
/// \param details - Unstructured additional detail explaining why the rule needs to run.
virtual void determinedRuleNeedsToRun(core::Rule* ruleNeedingToRun, core::Rule::RunReason reason, core::Rule* inputRule, StringRef details) override;

/// Called when a cycle is detected by the build engine and it cannot make
/// forward progress.
Expand Down
5 changes: 3 additions & 2 deletions include/llbuild/BuildSystem/Command.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace llbuild {
namespace core {

class TaskInterface;
class Rule;

}

Expand Down Expand Up @@ -141,8 +142,8 @@ class Command : public basic::JobDescriptor {
///
/// @{

virtual bool isResultValid(BuildSystem& system, const BuildValue& value) = 0;
virtual core::Rule::ValidationResult isResultValid(BuildSystem& system, const BuildValue& value) = 0;

virtual void start(BuildSystem& system, core::TaskInterface ti) = 0;

virtual void providePriorValue(BuildSystem& system, core::TaskInterface ti,
Expand Down
4 changes: 2 additions & 2 deletions include/llbuild/BuildSystem/ExternalCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ class ExternalCommand : public Command {

virtual BuildValue getResultForOutput(Node* node,
const BuildValue& value) override;
virtual bool isResultValid(BuildSystem&, const BuildValue& value) override;

virtual core::Rule::ValidationResult isResultValid(BuildSystem&, const BuildValue& value) override;

virtual void start(BuildSystem& system, core::TaskInterface ti) override;

Expand Down
18 changes: 16 additions & 2 deletions include/llbuild/Core/BuildEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,17 @@ class Rule {
Forced = 4
};

struct ValidationResult {
/// Whether the rule's value is valid.
bool isValid;

/// Optional unstructured description of why the value is invalid.
std::string details;

ValidationResult(bool valid, const std::string& details = "")
: isValid(valid), details(details) {}
};

public:
/// The key computed by the rule.
const KeyType key;
Expand All @@ -358,7 +369,7 @@ class Rule {
/// state managed externally to the build engine. For example, a rule which
/// computes something on the file system may use this to verify that the
/// computed output has not changed since it was built.
virtual bool isResultValid(BuildEngine&, const ValueType&) = 0;
virtual ValidationResult isResultValid(BuildEngine& engine, const ValueType& value) = 0;

/// Called to indicate a change in the rule status.
virtual void updateStatus(BuildEngine&, StatusKind);
Expand Down Expand Up @@ -388,7 +399,10 @@ class BuildEngineDelegate {
/// \param reason - Describes why the rule needs to run. For example, because it has never run or because an input was rebuilt.
///
/// \param inputRule - If `reason` is `InputRebuilt`, the rule for the rebuilt input, else `nullptr`.
virtual void determinedRuleNeedsToRun(Rule* ruleNeedingToRun, Rule::RunReason reason, Rule* inputRule);
///
/// \param details - Optional unstructured additional detail about why the rule needs to run.
virtual void determinedRuleNeedsToRun(Rule* ruleNeedingToRun, Rule::RunReason reason, Rule* inputRule,
StringRef details);

/// Called when a cycle is detected by the build engine to check if it should
/// attempt to resolve the cycle and continue
Expand Down
Loading