Bound the report files too, and only prune what this program generated

This commit is contained in:
pgdalmeida 2026-09-22 18:27:43 +02:00
parent fd1df9e93a
commit 00c998301a
Signed by: pedro.almeida
GPG Key ID: D4A6C394DF13F1D7
3 changed files with 204 additions and 2 deletions

View File

@ -55,8 +55,27 @@ std::string defaultLogDir();
// "<prefix>_YYYYMMDD-HHMMSS.log" using the local clock. // "<prefix>_YYYYMMDD-HHMMSS.log" using the local clock.
std::string timestampedLogName(const std::string& prefix); std::string timestampedLogName(const std::string& prefix);
// How many <prefix>_*.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 "<prefix>_*.log" in defaultLogDir().
//
// Only files this program GENERATED are eligible: the name must be exactly
// "<prefix>_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()/<name>`, creating the directory if needed. // Write `text` to `defaultLogDir()/<name>`, creating the directory if needed.
// Returns the full path written, or "" on failure (and logs a warning). // 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 } // namespace fgc::paths

View File

@ -2,6 +2,9 @@
#include "fgc/Logger.h" #include "fgc/Logger.h"
#include <algorithm>
#include <vector>
#include <cctype>
#include <cstdio> #include <cstdio>
#include <cstdlib> #include <cstdlib>
#include <ctime> #include <ctime>
@ -139,7 +142,56 @@ std::string timestampedLogName(const std::string& prefix) {
return prefix + "_" + buf + ".log"; return prefix + "_" + buf + ".log";
} }
std::string writeLogFile(const std::string& name, const std::string& text) { // True for "<prefix>_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<unsigned char>(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<std::string> 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
// "<prefix>_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<int>(files.size()) <= keep) return;
// Names embed YYYYMMDD-HHMMSS, so lexicographic order is chronological.
std::sort(files.begin(), files.end(), std::greater<std::string>());
for (size_t i = static_cast<size_t>(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; std::error_code ec;
fs::path dir = defaultLogDir(); fs::path dir = defaultLogDir();
fs::create_directories(dir, ec); 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(); LOG_WARN << "failed to write logfile " << path.string();
return ""; return "";
} }
// Bound this kind of report. The prefix is whatever precedes the first '_',
// matching timestampedLogName()'s "<prefix>_<stamp>.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(); return path.string();
} }

View File

@ -3,6 +3,11 @@
#include "fgc/Paths.h" #include "fgc/Paths.h"
#include <cstdlib> #include <cstdlib>
#include <filesystem>
#include <fstream>
#include <string>
#include <unistd.h>
using namespace fgc; using namespace fgc;
@ -63,3 +68,122 @@ TEST_CASE("resolveHardwarePath returns nothing when there is no hardware.ini") {
unsetenv("FGC_HARDWARE"); unsetenv("FGC_HARDWARE");
CHECK_FALSE(paths::resolveHardwarePath("/nonexistent/nope.ini", "").has_value()); 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);
}