Keep real staged compile errors navigable under warning floods
This commit is contained in:
@@ -7,9 +7,10 @@
|
||||
namespace faset::editor {
|
||||
|
||||
// Parse compiler, MSVC/clang-cl and Lua output into bounded Editor-facing rows.
|
||||
// Only sources under the current project's Scripts directory receive a file
|
||||
// key, so clients cannot offer navigation to unrelated filesystem paths.
|
||||
// Only sources under the project Scripts directory or the verified immutable
|
||||
// Scripts snapshot supplied by BuildService receive a project-relative file key.
|
||||
Json parse_build_diagnostics(std::string_view raw_log, std::string_view phase,
|
||||
const std::filesystem::path& project_root);
|
||||
const std::filesystem::path& project_root,
|
||||
const std::filesystem::path& snapshot_scripts = {});
|
||||
|
||||
} // namespace faset::editor
|
||||
|
||||
@@ -44,7 +44,8 @@ std::string lower_ascii(std::string value) {
|
||||
character = static_cast<char>(std::tolower(static_cast<unsigned char>(character)));
|
||||
return value;
|
||||
}
|
||||
std::string project_source(std::string raw, const std::filesystem::path& project_root) {
|
||||
std::string project_source(std::string raw, const std::filesystem::path& project_root,
|
||||
const std::filesystem::path& snapshot_scripts) {
|
||||
if (raw.starts_with("lua: "))
|
||||
raw.erase(0, 5);
|
||||
std::replace(raw.begin(), raw.end(), '\\', '/');
|
||||
@@ -52,19 +53,29 @@ std::string project_source(std::string raw, const std::filesystem::path& project
|
||||
if (component == "..")
|
||||
return {};
|
||||
const auto path = normalize_path(std::move(raw));
|
||||
const auto root = normalize_path(path_to_utf8(project_root));
|
||||
std::string relative;
|
||||
if (!path.empty() && path[0] != '/' && !has_windows_drive(path))
|
||||
relative = path;
|
||||
else {
|
||||
const auto windows = has_windows_drive(path) && has_windows_drive(root);
|
||||
const auto comparable_path = windows ? lower_ascii(path) : path;
|
||||
const auto comparable_root = windows ? lower_ascii(root) : root;
|
||||
if (comparable_path.size() <= comparable_root.size() ||
|
||||
!comparable_path.starts_with(comparable_root) ||
|
||||
comparable_path[comparable_root.size()] != '/')
|
||||
return {};
|
||||
relative = path.substr(root.size() + 1);
|
||||
const auto beneath = [&](const std::filesystem::path& directory) -> std::string {
|
||||
if (directory.empty())
|
||||
return {};
|
||||
const auto root = normalize_path(path_to_utf8(directory));
|
||||
const auto windows = has_windows_drive(path) && has_windows_drive(root);
|
||||
const auto comparable_path = windows ? lower_ascii(path) : path;
|
||||
const auto comparable_root = windows ? lower_ascii(root) : root;
|
||||
if (comparable_path.size() <= comparable_root.size() ||
|
||||
!comparable_path.starts_with(comparable_root) ||
|
||||
comparable_path[comparable_root.size()] != '/')
|
||||
return {};
|
||||
return path.substr(root.size() + 1);
|
||||
};
|
||||
relative = beneath(project_root);
|
||||
if (!relative.starts_with("Scripts/")) {
|
||||
relative = beneath(snapshot_scripts);
|
||||
if (!relative.empty())
|
||||
relative = "Scripts/" + relative;
|
||||
}
|
||||
}
|
||||
if (!relative.starts_with("Scripts/") || relative.size() <= 8)
|
||||
return {};
|
||||
@@ -83,7 +94,8 @@ int positive_number(const std::string& text) {
|
||||
} // namespace
|
||||
|
||||
Json parse_build_diagnostics(std::string_view raw_log, std::string_view phase,
|
||||
const std::filesystem::path& project_root) {
|
||||
const std::filesystem::path& project_root,
|
||||
const std::filesystem::path& snapshot_scripts) {
|
||||
static const std::regex msvc(
|
||||
R"(^(.+)\(([0-9]+)(?:,([0-9]+))?\)\s*:\s*(fatal error|error|warning|note)\s*([A-Za-z]+[0-9]+)?\s*:\s*(.*)$)");
|
||||
static const std::regex clang_column(
|
||||
@@ -140,7 +152,8 @@ Json parse_build_diagnostics(std::string_view raw_log, std::string_view phase,
|
||||
diagnostic["column"] = column;
|
||||
if (!code.empty())
|
||||
diagnostic["code"] = code;
|
||||
if (auto relative = project_source(file, project_root); !relative.empty())
|
||||
if (auto relative = project_source(file, project_root, snapshot_scripts);
|
||||
!relative.empty())
|
||||
diagnostic["file"] = std::move(relative);
|
||||
rows.push_back(std::move(diagnostic));
|
||||
}
|
||||
|
||||
@@ -295,15 +295,14 @@ struct BuildService::Impl {
|
||||
if (job.status.log.size() > max_log_bytes)
|
||||
job.status.log.erase(0, job.status.log.size() - max_log_bytes);
|
||||
}
|
||||
std::string run(Job& job, std::vector<std::string> arguments, const fs::path& cwd) {
|
||||
std::string run(Job& job, std::vector<std::string> arguments, const fs::path& cwd,
|
||||
const fs::path& snapshot_scripts = {}) {
|
||||
if (job.cancelled)
|
||||
throw Cancelled{};
|
||||
std::string phase;
|
||||
std::size_t diagnostics_before;
|
||||
{
|
||||
std::lock_guard lock(job.mutex);
|
||||
phase = job.status.stage;
|
||||
diagnostics_before = job.status.diagnostics.size();
|
||||
}
|
||||
std::string description = "$";
|
||||
for (const auto& argument : arguments)
|
||||
@@ -313,30 +312,40 @@ struct BuildService::Impl {
|
||||
Process process({std::move(arguments), cwd, {}});
|
||||
std::string output;
|
||||
std::string pending;
|
||||
bool saw_error = false;
|
||||
const auto append_diagnostic = [&](const Json& row) {
|
||||
const bool error = row.value("severity", "") == "error";
|
||||
saw_error |= error;
|
||||
std::lock_guard lock(job.mutex);
|
||||
if (job.status.diagnostics.size() >= 200) {
|
||||
if (!error)
|
||||
return;
|
||||
auto previous = std::find_if(job.status.diagnostics.begin(),
|
||||
job.status.diagnostics.end(), [](const Json& entry) {
|
||||
return entry.value("severity", "") != "error";
|
||||
});
|
||||
if (previous == job.status.diagnostics.end())
|
||||
previous = job.status.diagnostics.begin();
|
||||
job.status.diagnostics.erase(previous);
|
||||
}
|
||||
job.status.diagnostics.push_back(row);
|
||||
};
|
||||
const auto collect_diagnostics = [&](std::string_view chunk, bool final) {
|
||||
pending.append(chunk);
|
||||
std::size_t newline;
|
||||
while ((newline = pending.find('\n')) != std::string::npos) {
|
||||
const auto line = pending.substr(0, newline + 1);
|
||||
pending.erase(0, newline + 1);
|
||||
const auto found = parse_build_diagnostics(line, phase, config.project_root);
|
||||
if (found.empty())
|
||||
continue;
|
||||
std::lock_guard lock(job.mutex);
|
||||
for (const auto& row : found) {
|
||||
if (job.status.diagnostics.size() >= 200)
|
||||
break;
|
||||
job.status.diagnostics.push_back(row);
|
||||
}
|
||||
const auto found = parse_build_diagnostics(
|
||||
line, phase, config.project_root, snapshot_scripts);
|
||||
for (const auto& row : found)
|
||||
append_diagnostic(row);
|
||||
}
|
||||
if (final && !pending.empty()) {
|
||||
const auto found = parse_build_diagnostics(pending, phase, config.project_root);
|
||||
std::lock_guard lock(job.mutex);
|
||||
for (const auto& row : found) {
|
||||
if (job.status.diagnostics.size() >= 200)
|
||||
break;
|
||||
job.status.diagnostics.push_back(row);
|
||||
}
|
||||
const auto found = parse_build_diagnostics(
|
||||
pending, phase, config.project_root, snapshot_scripts);
|
||||
for (const auto& row : found)
|
||||
append_diagnostic(row);
|
||||
pending.clear();
|
||||
} else if (pending.size() > 8192)
|
||||
pending.erase(0, pending.size() - 8192);
|
||||
@@ -356,14 +365,8 @@ struct BuildService::Impl {
|
||||
output.erase(0, output.size() - max_log_bytes);
|
||||
if (!poll.running) {
|
||||
if (poll.exit_code.value_or(1) != 0) {
|
||||
std::lock_guard lock(job.mutex);
|
||||
bool has_error = false;
|
||||
for (std::size_t index = diagnostics_before;
|
||||
index < job.status.diagnostics.size(); ++index)
|
||||
if (job.status.diagnostics[index].value("severity", "") == "error")
|
||||
has_error = true;
|
||||
if (!has_error && job.status.diagnostics.size() < 200)
|
||||
job.status.diagnostics.push_back(
|
||||
if (!saw_error)
|
||||
append_diagnostic(
|
||||
{{"severity", "error"},
|
||||
{"phase", phase},
|
||||
{"message", "Process exited with code " +
|
||||
@@ -459,13 +462,13 @@ struct BuildService::Impl {
|
||||
// whether the packaged game has a Lua VM linked into it.
|
||||
arguments.push_back(std::string("-DFASET_ENABLE_LUA=") +
|
||||
(job.lua.enabled() ? "ON" : "OFF"));
|
||||
run(job, std::move(arguments), config.project_root);
|
||||
run(job, std::move(arguments), config.project_root, staged_scripts);
|
||||
const auto configured = std::chrono::steady_clock::now();
|
||||
checkpoint(job, "Compiling and linking Player", .25);
|
||||
run(job,
|
||||
{config.cmake, "--build", path_to_utf8(native_directory), "--config", configuration,
|
||||
"--parallel", "4", "--target", "faset_player", "faset_schema_exporter"},
|
||||
config.project_root);
|
||||
config.project_root, staged_scripts);
|
||||
const auto compiled = std::chrono::steady_clock::now();
|
||||
auto player = build_executable(native_directory, configuration, "faset_player");
|
||||
auto exporter = build_executable(native_directory, configuration, "faset_schema_exporter");
|
||||
@@ -532,7 +535,8 @@ struct BuildService::Impl {
|
||||
export_arguments.insert(export_arguments.end(),
|
||||
{"--project", path_to_utf8(staging)});
|
||||
}
|
||||
run(job, std::move(export_arguments), config.project_root);
|
||||
run(job, std::move(export_arguments), config.project_root,
|
||||
job.lua.enabled() ? staging / "Scripts" : staged_scripts);
|
||||
auto schema = read_json(schema_file);
|
||||
if (schema.value("format", "") != "faset.schema" || schema.value("version", 0) != 1 ||
|
||||
!schema.contains("types") || !schema.at("types").is_array())
|
||||
|
||||
@@ -50,6 +50,25 @@ void contracts() {
|
||||
"compile", project);
|
||||
check(rows.size() == 2 && !rows[0].contains("file") && !rows[1].contains("file"),
|
||||
"External and traversing sources are never navigable");
|
||||
const auto snapshot = fs::path("/cache/source-snapshots/verified/Scripts");
|
||||
rows = editor::parse_build_diagnostics(
|
||||
"/cache/source-snapshots/verified/Scripts/Nested/Game.cpp:21:3: error: broken\n"
|
||||
"/cache/source-snapshots/verified/Scripts-other/Game.cpp:4:2: error: outside\n",
|
||||
"compile", project, snapshot);
|
||||
check(rows.size() == 2 && rows[0].at("file") == "Scripts/Nested/Game.cpp" &&
|
||||
!rows[1].contains("file"),
|
||||
"Only the verified staged Scripts subtree maps back to project source");
|
||||
const auto nested_snapshot = project / ".faset/cache/source-snapshots/verified/Scripts";
|
||||
rows = editor::parse_build_diagnostics(
|
||||
"/project/.faset/cache/source-snapshots/verified/Scripts/Game.cpp:8:2: error: broken\n",
|
||||
"compile", project, nested_snapshot);
|
||||
check(rows.size() == 1 && rows[0].at("file") == "Scripts/Game.cpp",
|
||||
"Project-contained build snapshot does not mask its verified Scripts mapping");
|
||||
rows = editor::parse_build_diagnostics(
|
||||
R"(C:\cache\snapshots\verified\Scripts\Game.cpp(17,4): error C2143: syntax error)" "\n",
|
||||
"compile", windows, path_from_utf8(R"(C:\cache\snapshots\verified\Scripts)"));
|
||||
check(rows.size() == 1 && rows[0].at("file") == "Scripts/Game.cpp",
|
||||
"Windows staged source paths keep drive and Unicode navigation support");
|
||||
rows = editor::parse_build_diagnostics(
|
||||
"Scripts/../Scripts/Game.cpp:4:2: error: disguised traversal\n"
|
||||
"Scripts/Game.cpp:4:0: error: invalid column\n",
|
||||
|
||||
@@ -232,6 +232,16 @@ int test_main(int argc, char** argv) {
|
||||
compile_failed.log.find("fixture compile failure") != std::string::npos,
|
||||
"Failed compiler output retains raw log and structured source location");
|
||||
fs::remove(config.project_root / "emit-clang-error-and-fail-build");
|
||||
atomic_write(config.project_root / "flood-warnings-and-fail-build", "fixture\n");
|
||||
const auto flooded = builds.wait(builds.start_build());
|
||||
bool retained_error = false;
|
||||
for (const auto& diagnostic : flooded.diagnostics)
|
||||
retained_error |= diagnostic.value("severity", "") == "error" &&
|
||||
diagnostic.value("file", "") == "Scripts/Gameplay.cpp";
|
||||
check(flooded.state == "failed" && flooded.diagnostics.size() <= 200 &&
|
||||
retained_error && read_text(last_build) == previous_pointer,
|
||||
"Compiler errors survive bounded third-party warning floods");
|
||||
fs::remove(config.project_root / "flood-warnings-and-fail-build");
|
||||
atomic_write(config.project_root / "fail-unparseable-build", "fixture\n");
|
||||
const auto generic_failed = builds.wait(builds.start_build());
|
||||
check(generic_failed.state == "failed" &&
|
||||
|
||||
@@ -40,6 +40,14 @@ int tool_main(int argc, char** argv) {
|
||||
return 0;
|
||||
}
|
||||
if (argc > 2 && std::string_view(argv[1]) == "--build") {
|
||||
if (fs::exists("flood-warnings-and-fail-build")) {
|
||||
for (int index = 0; index < 250; ++index)
|
||||
std::cerr << "/external/library.cpp:1:1: warning: dependency warning "
|
||||
<< index << '\n';
|
||||
std::cerr << path_to_utf8(fs::current_path() / "Scripts/Gameplay.cpp")
|
||||
<< ":7:3: error: gameplay error after warnings\n";
|
||||
return 1;
|
||||
}
|
||||
if (fs::exists("emit-clang-error-and-fail-build")) {
|
||||
std::cerr << path_to_utf8(fs::current_path() / "Scripts/Gameplay.cpp")
|
||||
<< ":7:3: error: fixture compile failure\n";
|
||||
|
||||
@@ -345,6 +345,13 @@ int integration(const fs::path& root) {
|
||||
source + "\n#error intentional_build_failure\n");
|
||||
auto failed = service.wait(service.start_build());
|
||||
require(failed.state == "failed", "Invalid user C++ must fail build");
|
||||
bool navigable_cpp_error = false;
|
||||
for (const auto& diagnostic : failed.diagnostics)
|
||||
navigable_cpp_error |= diagnostic.value("severity", "") == "error" &&
|
||||
diagnostic.value("file", "") == "Scripts/Gameplay.cpp" &&
|
||||
diagnostic.value("line", 0) > 0;
|
||||
require(navigable_cpp_error,
|
||||
"Real staged C++ compile failure maps to a navigable project source");
|
||||
require(read_text(config.cache_root / "last_build.json") == last,
|
||||
"Failed compile preserved last good build");
|
||||
atomic_write(config.project_root / "Scripts" / "Gameplay.cpp", source);
|
||||
|
||||
Reference in New Issue
Block a user