From 00c998301ad826715ee4845b9eb9e6a05ee5d93e Mon Sep 17 00:00:00 2001 From: pgdalmeida Date: Tue, 22 Sep 2026 18:27:43 +0200 Subject: [PATCH] Bound the report files too, and only prune what this program generated --- include/fgc/Paths.h | 21 +++++++- src/core/Paths.cpp | 61 ++++++++++++++++++++- tests/test_paths.cpp | 124 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 2 deletions(-) diff --git a/include/fgc/Paths.h b/include/fgc/Paths.h index 19cea78..17891b7 100644 --- a/include/fgc/Paths.h +++ b/include/fgc/Paths.h @@ -55,8 +55,27 @@ std::string defaultLogDir(); // "_YYYYMMDD-HHMMSS.log" using the local clock. std::string timestampedLogName(const std::string& prefix); +// How many _*.log reports to keep per kind. Generous: these are a few +// hundred bytes each and written by hand-run commands, so the cap is about +// keeping the directory greppable rather than about disk. +inline constexpr int kDefaultReportsKept = 50; + +// Delete all but the `keep` newest "_*.log" in defaultLogDir(). +// +// Only files this program GENERATED are eligible: the name must be exactly +// "_YYYYMMDD-HHMMSS.log", as timestampedLogName() builds it. A +// prefix-only match would also cover `[Test] baseline_file`, which is +// configurable and may live in this directory - the pinned reference every +// self-test compares against. keep <= 0 prunes nothing. +void pruneLogFiles(const std::string& prefix, int keep); + // Write `text` to `defaultLogDir()/`, creating the directory if needed. // Returns the full path written, or "" on failure (and logs a warning). -std::string writeLogFile(const std::string& name, const std::string& text); +// +// Then prunes older files of the same kind, the prefix being whatever precedes +// the first '_' in `name` - so `test_*.log`, `diag_*.log` and `pitch_setup_*.log` +// stay bounded without each caller having to remember. Pass keep = 0 to opt out. +std::string writeLogFile(const std::string& name, const std::string& text, + int keep = kDefaultReportsKept); } // namespace fgc::paths diff --git a/src/core/Paths.cpp b/src/core/Paths.cpp index 6c49316..af5b815 100644 --- a/src/core/Paths.cpp +++ b/src/core/Paths.cpp @@ -2,6 +2,9 @@ #include "fgc/Logger.h" +#include +#include +#include #include #include #include @@ -139,7 +142,56 @@ std::string timestampedLogName(const std::string& prefix) { return prefix + "_" + buf + ".log"; } -std::string writeLogFile(const std::string& name, const std::string& text) { +// True for "_YYYYMMDD-HHMMSS.log" and nothing else. `want` is the +// prefix including its trailing underscore. +static bool isGeneratedReportName(const std::string& name, const std::string& want) { + const std::string rest = name.substr(want.size()); + // "YYYYMMDD-HHMMSS.log" is exactly 19 characters. + if (rest.size() != 19) return false; + if (rest.compare(15, 4, ".log") != 0) return false; + if (rest[8] != '-') return false; + for (size_t i = 0; i < 15; ++i) { + if (i == 8) continue; + if (!std::isdigit(static_cast(rest[i]))) return false; + } + return true; +} + +void pruneLogFiles(const std::string& prefix, int keep) { + if (prefix.empty() || keep <= 0) return; + + std::error_code ec; + const fs::path dir = defaultLogDir(); + if (!fs::exists(dir, ec)) return; + + const std::string want = prefix + "_"; + std::vector files; + for (const auto& e : fs::directory_iterator(dir, ec)) { + if (ec) return; + if (!e.is_regular_file(ec)) continue; + const std::string n = e.path().filename().string(); + if (n.rfind(want, 0) != 0) continue; + // Only files this program GENERATED, i.e. exactly + // "_YYYYMMDD-HHMMSS.log" as timestampedLogName() builds them. + // + // Matching the prefix alone is not enough. `[Test] baseline_file` is + // configurable and may legitimately live here; the default + // `test_baseline.txt` happens to survive a looser match only because + // 'b' sorts above the digits of a year, which is luck rather than a + // guarantee - point it at `test_mybaseline.log` and a prefix-only match + // would delete the very reference every self-test compares against. + if (!isGeneratedReportName(n, want)) continue; + files.push_back(e.path().string()); + } + if (static_cast(files.size()) <= keep) return; + + // Names embed YYYYMMDD-HHMMSS, so lexicographic order is chronological. + std::sort(files.begin(), files.end(), std::greater()); + for (size_t i = static_cast(keep); i < files.size(); ++i) + fs::remove(files[i], ec); // best effort +} + +std::string writeLogFile(const std::string& name, const std::string& text, int keep) { std::error_code ec; fs::path dir = defaultLogDir(); fs::create_directories(dir, ec); @@ -155,6 +207,13 @@ std::string writeLogFile(const std::string& name, const std::string& text) { LOG_WARN << "failed to write logfile " << path.string(); return ""; } + + // Bound this kind of report. The prefix is whatever precedes the first '_', + // matching timestampedLogName()'s "_.log". Pruning AFTER the + // write means the file just created is the newest and is always retained. + const size_t us = name.find('_'); + if (us != std::string::npos && us > 0) pruneLogFiles(name.substr(0, us), keep); + return path.string(); } diff --git a/tests/test_paths.cpp b/tests/test_paths.cpp index 51b93a0..8df2ff7 100644 --- a/tests/test_paths.cpp +++ b/tests/test_paths.cpp @@ -3,6 +3,11 @@ #include "fgc/Paths.h" #include +#include +#include +#include + +#include using namespace fgc; @@ -63,3 +68,122 @@ TEST_CASE("resolveHardwarePath returns nothing when there is no hardware.ini") { unsetenv("FGC_HARDWARE"); CHECK_FALSE(paths::resolveHardwarePath("/nonexistent/nope.ini", "").has_value()); } + +// --- report pruning: the log dir must not grow without bound ---------------- + +namespace { +// Point defaultLogDir() at a scratch dir, restoring the environment afterwards +// even if a REQUIRE aborts the body. +struct ScopedDataHome { + std::string old_; + bool had_ = false; + std::filesystem::path dir; + explicit ScopedDataHome(const char* tag) { + if (const char* p = ::getenv("XDG_DATA_HOME")) { old_ = p; had_ = true; } + dir = std::filesystem::temp_directory_path() / + ("fgc-prune-" + std::string(tag) + "-" + std::to_string(::getpid())); + std::filesystem::remove_all(dir); + std::filesystem::create_directories(dir); + ::setenv("XDG_DATA_HOME", dir.c_str(), 1); + } + ~ScopedDataHome() { + if (had_) ::setenv("XDG_DATA_HOME", old_.c_str(), 1); + else ::unsetenv("XDG_DATA_HOME"); + std::error_code ec; + std::filesystem::remove_all(dir, ec); + } + int countMatching(const std::string& glob_prefix, const std::string& ext) const { + int n = 0; + std::error_code ec; + for (const auto& e : std::filesystem::directory_iterator(fgc::paths::defaultLogDir(), ec)) { + const std::string name = e.path().filename().string(); + if (name.rfind(glob_prefix, 0) == 0 && e.path().extension() == ext) ++n; + } + return n; + } +}; +} // namespace + +TEST_CASE("writing a report prunes older ones of the same kind") { + ScopedDataHome home("keep"); + for (int i = 0; i < 12; ++i) + fgc::paths::writeLogFile("test_2026092" + std::to_string(i / 10) + + "-00000" + std::to_string(i % 10) + ".log", + "body", /*keep=*/5); + CHECK(home.countMatching("test_", ".log") == 5); +} + +TEST_CASE("only files this program generated are eligible for pruning") { + // [Test] baseline_file is CONFIGURABLE and may live in this directory. It is + // the reference every self-test compares against, so pruning it would + // silently discard the comparison. + // + // The default `test_baseline.txt` survives a prefix-only match by accident - + // 'b' sorts above the digits of a year, so it never lands in the delete + // range. A baseline named `test_mybaseline.log` does not have that luck, and + // it is the case a prefix-only match would destroy. Hence: the name must + // look exactly like timestampedLogName()'s output. + ScopedDataHome home("baseline"); + std::filesystem::create_directories(fgc::paths::defaultLogDir()); + const auto dir = std::filesystem::path(fgc::paths::defaultLogDir()); + const auto dflt = dir / "test_baseline.txt"; + const auto custom = dir / "test_mybaseline.log"; // sorts BELOW the reports + { std::ofstream(dflt) << "pinned"; } + { std::ofstream(custom) << "pinned"; } + + for (int i = 0; i < 10; ++i) + fgc::paths::writeLogFile("test_20260922-00000" + std::to_string(i) + ".log", "b", + /*keep=*/2); + + CHECK(std::filesystem::exists(dflt)); + CHECK(std::filesystem::exists(custom)); + // Exactly the generated reports were pruned, and neither baseline consumed + // one of the retained slots. + CHECK(home.countMatching("test_2026", ".log") == 2); +} + +TEST_CASE("a name that is not timestampedLogName's shape is left alone") { + ScopedDataHome home("shape"); + std::filesystem::create_directories(fgc::paths::defaultLogDir()); + const auto dir = std::filesystem::path(fgc::paths::defaultLogDir()); + for (const char* odd : {"test_notes.log", "test_2026-09-22.log", "test_20260922-0000.log"}) + { std::ofstream(dir / odd) << "x"; } + + for (int i = 0; i < 6; ++i) + fgc::paths::writeLogFile("test_20260922-00000" + std::to_string(i) + ".log", "b", 1); + + for (const char* odd : {"test_notes.log", "test_2026-09-22.log", "test_20260922-0000.log"}) + CHECK(std::filesystem::exists(dir / odd)); + // Narrow enough to match ONLY the generated reports: "test_20260922-0000.log" + // is one of the planted oddities and shares a shorter prefix with them. + CHECK(home.countMatching("test_20260922-00000", ".log") == 1); +} + +TEST_CASE("pruning one kind leaves the others alone") { + ScopedDataHome home("kinds"); + for (int i = 0; i < 4; ++i) + fgc::paths::writeLogFile("diag_20260922-00000" + std::to_string(i) + ".log", "d", 10); + for (int i = 0; i < 6; ++i) + fgc::paths::writeLogFile("test_20260922-00000" + std::to_string(i) + ".log", "t", 2); + + CHECK(home.countMatching("test_", ".log") == 2); // pruned + CHECK(home.countMatching("diag_", ".log") == 4); // untouched +} + +TEST_CASE("the newest report survives, and it is the one just written") { + ScopedDataHome home("newest"); + std::string last; + for (int i = 0; i < 8; ++i) + last = fgc::paths::writeLogFile("test_20260922-00000" + std::to_string(i) + ".log", + "body-" + std::to_string(i), /*keep=*/3); + REQUIRE_FALSE(last.empty()); + CHECK(std::filesystem::exists(last)); +} + +TEST_CASE("keep = 0 opts out, preserving the old unbounded behaviour") { + ScopedDataHome home("optout"); + for (int i = 0; i < 7; ++i) + fgc::paths::writeLogFile("test_20260922-00000" + std::to_string(i) + ".log", "b", + /*keep=*/0); + CHECK(home.countMatching("test_", ".log") == 7); +}