Catcrafts/Crafter.Build
Watch
2
0
Fork
You've already forked Crafter.Build
 0
Code Issues 5 Pull requests Projects Releases 7 Packages Wiki Activity Actions

fix(agents): what the review of dependency notes found

Browse source 
A review of the feature turned up the following, fixed here.

Writing. AGENTS.md was replaced through a temporary renamed over it,
which swaps in another file: a read-only AGENTS.md was rewritten
anyway, a hard link such as a CLAUDE.md beside it kept the old text,
ACLs and ownership were dropped, and without an fsync a power cut could
still leave it empty. The temporary was named after the PID, which
every Flatpak instance shares as PID 2. The file is now written in
place, from the first byte that differs, then truncated and synced,
under an flock on the file itself. A write that fails partway is undone
where the disk allows; with the section last, where it is appended,
only the section can be damaged. A read-only file is opened for writing
only when something would change, so it is refused then and silent
otherwise.

Reading. libc++'s filebuf reports a failed read as the end of the file,
so an I/O error came back as a short file and the section was written
over what was left. Reads go through read(2).

Code fences. ```make``` was taken for a fence, and a fence in a list
item was missed while its indented closer opened one, so markers a user
quoted in a code block could be taken for the section, or the real one
hidden and appended again. Backtick info strings, block quotes and list
items are now followed, and a block ends with its container.

Warnings with nothing to write. With no notes, nothing short of a
well-formed section is reported, so an AGENTS.md that ends in an open
code block, or a project set to None, no longer warns on every build.
An unclosed fence after a section no longer stops it being rewritten
in place.

Labels. The `from` line carried the git URL, credentials and all, or a
path relative to the consumer that changed with the machine, the cache
location and --local. It is now the repository's or the directory's
name. Notes are deduplicated by repository rather than by clone, so one
library pinned two ways is described once, as the nearer spec has it.

The walk. It no longer looks at the project's tests: a plain build
never did, and a test's dependency can point at the Configuration the
project file returned by value. It goes ring by ring over projects
rather than configurations, so what the project names itself comes
first even with the library/executable split. The project's own
AGENTS.consumer.md is excluded by path, also when a dependency's
project file sits beside the project's.

Removing a section dropped a blank line even from between two
paragraphs, joining them; only a section that ends the file takes it
now. A CRLF file gets a CRLF section.

A note was checked on its raw bytes and again, under other whitespace
rules, on the trimmed ones, so it could pass the first check and fail
the merge, which then dropped every note. It is normalised once and
checked on exactly what is merged.

`test --list` only lists, and no longer writes AGENTS.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Jorijn van der Graaf 2026-10-31 05:02:28 +01:00
commit 7815856d60
Signed by: jorijnvdgraaf
GPG key ID: 2937E59CDCC1BCFB
5 changed files with 446 additions and 194 deletions
Download patch file Download diff file Expand all files Collapse all files

528
implementations/Crafter.Build-Agents.cpp
View file

@ -2,6 +2,9 @@
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® // SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
module; module;
#include <errno.h>
#include <fcntl.h>
#include <sys/file.h>
#include <unistd.h> #include <unistd.h>
module Crafter.Build:Agents_impl; module Crafter.Build:Agents_impl;
import std; import std;
@ -25,6 +28,53 @@ namespace {
Progress::Diagnostic(std::format("warning: {}", message)); Progress::Diagnostic(std::format("warning: {}", message));
} }
std::string ErrnoMessage() {
return std::error_code(errno, std::generic_category()).message();
}
// Closes on the way out of scope, which also drops an flock taken
// through it.
class Descriptor {
public:
explicit Descriptor(std::int32_t fd) : fd_(fd) {}
~Descriptor() {
if (fd_ >= 0) ::close(fd_);
}
Descriptor(const Descriptor&) = delete;
Descriptor& operator=(const Descriptor&) = delete;
std::int32_t Get() const { return fd_; }
bool Valid() const { return fd_ >= 0; }
private:
std::int32_t fd_;
};
// Everything `fd` reads, into `out`; empty on success, otherwise the
// error. Through read(2) rather than a stream: libc++'s filebuf reports a
// failed read as the end of the file, so an I/O error came back as a
// short read that looked successful, and the section was then written
// over a truncated AGENTS.md.
std::string ReadAll(std::int32_t fd, std::string& out) {
out.clear();
std::array<char, 16384> buffer;
for (;;) {
const auto got = ::read(fd, buffer.data(), buffer.size());
if (got > 0) {
out.append(buffer.data(), static_cast<std::size_t>(got));
} else if (got == 0) {
return "";
} else if (errno != EINTR) {
return ErrnoMessage();
}
}
}
std::string ReadFile(const fs::path& file, std::string& out) {
const Descriptor in(::open(file.c_str(), O_RDONLY | O_CLOEXEC));
if (!in.Valid()) return ErrnoMessage();
return ReadAll(in.Get(), out);
}
struct Line { struct Line {
std::string_view text; // without the newline and a '\r' before it std::string_view text; // without the newline and a '\r' before it
std::size_t begin = 0; // offset of the line's first byte std::size_t begin = 0; // offset of the line's first byte
@ -51,38 +101,124 @@ namespace {
return last == std::string_view::npos ? std::string_view() : text.substr(0, last + 1); return last == std::string_view::npos ? std::string_view() : text.substr(0, last + 1);
} }
// Fenced code blocks as CommonMark has them: a run of three or more // A line with the block quote markers in front of it taken off.
// backticks or tildes, after at most three spaces, opens one, and a run struct Shape {
// of the same character at least as long, with nothing after it but std::size_t quotes = 0; // `>` markers
// spaces, closes it. Tracking the character and the length is what lets std::size_t indent = 0; // columns of indentation after them
// a ~~~ block quote a ``` one, the usual way to show a fenced example. std::string_view rest; // the line from its first other character
};
Shape ShapeOf(std::string_view line) {
Shape shape;
std::size_t pos = 0;
for (;;) {
std::size_t probe = pos;
while (probe < line.size() && probe - pos < 3 && line[probe] == ' ') ++probe;
if (probe >= line.size() || line[probe] != '>') break;
pos = probe + 1;
if (pos < line.size() && line[pos] == ' ') ++pos;
++shape.quotes;
}
for (; pos < line.size() && (line[pos] == ' ' || line[pos] == '\t'); ++pos) {
shape.indent += line[pos] == '\t' ? 4 - shape.indent % 4 : 1;
}
shape.rest = line.substr(pos);
return shape;
}
// The columns a list item marker at the start of `text` takes, with the
// spaces after it: `-`, `+` or `*`, or up to nine digits and `.` or `)`,
// then at least one space. Zero when `text` starts with none.
std::size_t ListMarker(std::string_view text) {
std::size_t pos = 0;
if (!text.empty() && (text[0] == '-' || text[0] == '+' || text[0] == '*')) {
pos = 1;
} else {
while (pos < text.size() && pos < 9 && text[pos] >= '0' && text[pos] <= '9') ++pos;
if (pos == 0 || pos >= text.size() || (text[pos] != '.' && text[pos] != ')')) return 0;
++pos;
}
const std::size_t marker = pos;
while (pos < text.size() && text[pos] == ' ') ++pos;
return pos == marker ? 0 : pos;
}
struct Fence {
char ch = '\0';
std::size_t length = 0;
std::string_view after; // the rest of the line behind the run
};
// The run of three or more backticks or tildes `text` starts with.
std::optional<Fence> FenceRun(std::string_view text) {
if (text.empty() || (text[0] != '`' && text[0] != '~')) return std::nullopt;
std::size_t length = 0;
while (length < text.size() && text[length] == text[0]) ++length;
if (length < 3) return std::nullopt;
return Fence{ .ch = text[0], .length = length, .after = text.substr(length) };
}
// Fenced code blocks, close to how CommonMark reads them. A run of three
// or more backticks or tildes opens one after at most three spaces of
// indentation; a backtick run only when no backtick follows it on the
// line, since ```make``` is inline code. A run of the same character at
// least as long, with nothing behind it but spaces, closes it. Keeping
// the character and the length is what lets a ~~~ block quote a ``` one.
//
// Block quotes and list items are followed as far as fences need: the
// `>` markers come off every line, list markers off the line that opens
// a block, and a block ends with the container it sits in, at a line with
// fewer `>` or one indented less than its list item's content. Read
// without them, `- ```bash` was missed while its indented closer was
// taken for an opener, and the rest of the file came out inside out. Not
// followed: indented code blocks, and the continuation lines of a list
// item before a fence that opens there.
class CodeFences { class CodeFences {
public: public:
// Feed the lines in order. True for a fence and for every line // Feed the lines in order. True for a fence and for every line
// inside a block. // inside a block.
bool InCode(std::string_view line) { bool InCode(std::string_view line) {
std::size_t pos = 0; const Shape shape = ShapeOf(line);
while (pos < line.size() && pos < 3 && line[pos] == ' ') ++pos; if (open_ != '\0') {
const char ch = pos < line.size() ? line[pos] : '\0'; const bool blank = TrimEnd(shape.rest).empty();
std::size_t run = 0; const bool left = !blank && (shape.quotes < quotes_ || shape.indent < content_);
if (ch == '`' || ch == '~') { if (!left) {
while (pos + run < line.size() && line[pos + run] == ch) ++run; const std::optional<Fence> fence = FenceRun(shape.rest);
if (fence && fence->ch == open_ && fence->length >= length_ && TrimEnd(fence->after).empty() && shape.indent <= content_ + 3) open_ = '\0';
return true;
}
// The container ended, and the block with it; the line is
// read again as one outside any block.
open_ = '\0';
} }
const bool fence = run >= 3; if (shape.indent > 3) return false;
if (open_ == '\0') { std::size_t column = shape.indent;
if (!fence) return false; std::string_view text = shape.rest;
open_ = ch; std::size_t content = 0;
length_ = run; for (std::size_t marker = ListMarker(text); marker > 0; marker = ListMarker(text)) {
return true; column += marker;
text.remove_prefix(marker);
content = column;
} }
if (fence && ch == open_ && run >= length_ && TrimEnd(line.substr(pos + run)).empty()) open_ = '\0'; const std::optional<Fence> fence = FenceRun(text);
if (!fence || (fence->ch == '`' && fence->after.contains('`'))) return false;
open_ = fence->ch;
length_ = fence->length;
quotes_ = shape.quotes;
content_ = content;
++opened_;
return true; return true;
} }
bool Open() const { return open_ != '\0'; } bool Open() const { return open_ != '\0'; }
// How many blocks have opened so far.
std::size_t Opened() const { return opened_; }
private: private:
char open_ = '\0'; char open_ = '\0';
std::size_t length_ = 0; std::size_t length_ = 0;
std::size_t quotes_ = 0; // the block quote depth it opened at
std::size_t content_ = 0; // its list item's content column; 0 outside a list
std::size_t opened_ = 0;
}; };
// Where a text's marker lines are, outside code blocks, as indices into // Where a text's marker lines are, outside code blocks, as indices into
@ -100,11 +236,10 @@ namespace {
scan.lines = SplitLines(text); scan.lines = SplitLines(text);
CodeFences fences; CodeFences fences;
for (std::size_t i = 0; i < scan.lines.size(); ++i) { for (std::size_t i = 0; i < scan.lines.size(); ++i) {
const bool wasOpen = fences.Open(); const std::size_t opened = fences.Opened();
if (fences.InCode(scan.lines[i].text)) { const bool code = fences.InCode(scan.lines[i].text);
if (!wasOpen) scan.unclosedBlock = i; if (fences.Opened() != opened) scan.unclosedBlock = i;
continue; if (code) continue;
}
const std::string_view line = TrimEnd(scan.lines[i].text); const std::string_view line = TrimEnd(scan.lines[i].text);
if (line == BeginMarker) scan.begins.push_back(i); if (line == BeginMarker) scan.begins.push_back(i);
if (line == EndMarker) scan.ends.push_back(i); if (line == EndMarker) scan.ends.push_back(i);
@ -117,194 +252,269 @@ namespace {
// marker line of its own would end or restart the section once the file // marker line of its own would end or restart the section once the file
// is read back, and a code block left open would hide the end marker: // is read back, and a code block left open would hide the end marker:
// either way the next build could no longer find what this one wrote. // either way the next build could no longer find what this one wrote.
std::string NoteProblem(std::string_view text) { // Lines are numbered from `firstLine`.
std::string NoteProblem(std::string_view text, std::size_t firstLine) {
const MarkerScan scan = ScanMarkers(text); const MarkerScan scan = ScanMarkers(text);
if (!scan.begins.empty() || !scan.ends.empty()) { std::optional<std::size_t> marker;
std::size_t line = scan.lines.size(); if (!scan.begins.empty()) marker = scan.begins[0];
if (!scan.begins.empty()) line = scan.begins[0]; if (!scan.ends.empty()) marker = std::min(marker.value_or(scan.ends[0]), scan.ends[0]);
if (!scan.ends.empty()) line = std::min(line, scan.ends[0]); if (marker) return std::format("line {} is a dependency notes marker", *marker + firstLine);
return std::format("line {} is a dependency notes marker", line + 1); if (scan.unclosedBlock) return std::format("line {} opens a code block that never closes", *scan.unclosedBlock + firstLine);
}
if (scan.unclosedBlock) return std::format("line {} opens a code block that never closes", *scan.unclosedBlock + 1);
return ""; return "";
} }
std::string RenderSection(std::span<const DependencyNote> notes) { struct NormalizedNote {
std::string section = std::format("{}\n{}\n", BeginMarker, Preamble); std::string text;
for (const DependencyNote& note : notes) section += std::format("\n<!-- from {} -->\n{}\n", note.source, note.text); // The line of the file `text` starts on.
section += std::format("\n{}\n", EndMarker); std::size_t firstLine = 1;
};
// A notes file as it goes into the section: LF line ends, no blank lines
// in front and no whitespace behind, so the section spaces its notes the
// same whatever the files end with.
NormalizedNote NormalizeNote(std::string_view raw) {
std::string text;
text.reserve(raw.size());
for (std::size_t i = 0; i < raw.size(); ++i) {
if (raw[i] == '\r' && i + 1 < raw.size() && raw[i + 1] == '\n') continue;
text.push_back(raw[i]);
}
const std::size_t last = text.find_last_not_of(" \t\r\n");
if (last == std::string::npos) return {};
const std::size_t first = text.find_first_not_of(" \t\r\n");
const std::size_t newline = text.rfind('\n', first);
const std::size_t begin = newline == std::string::npos ? 0 : newline + 1;
const auto skipped = std::count(text.begin(), text.begin() + static_cast<std::ptrdiff_t>(begin), '\n');
return { .text = text.substr(begin, last + 1 - begin), .firstLine = static_cast<std::size_t>(skipped) + 1 };
}
// `text` with `eol` ending its lines, whether they ended in LF or CRLF.
std::string WithLineEnds(std::string_view text, std::string_view eol) {
std::string out;
out.reserve(text.size());
for (std::size_t i = 0; i < text.size(); ++i) {
if (text[i] == '\r' && i + 1 < text.size() && text[i + 1] == '\n') continue;
if (text[i] == '\n') {
out += eol;
} else {
out.push_back(text[i]);
}
}
return out;
}
std::string RenderSection(std::span<const DependencyNote> notes, std::string_view eol) {
std::string section = std::format("{}{}{}{}", BeginMarker, eol, Preamble, eol);
for (const DependencyNote& note : notes) section += std::format("{0}<!-- from {1} -->{0}{2}{0}", eol, note.name, WithLineEnds(note.text, eol));
section += std::format("{0}{1}{0}", eol, EndMarker);
return section; return section;
} }
// Without leading blank lines and trailing whitespace, so the section // The note beside a dependency's project file, or nullopt when there is
// spaces its notes the same whatever the files end with. // none or it cannot be used.
std::string TrimBlank(std::string_view text) { std::optional<DependencyNote> ReadNote(const fs::path& file, const std::string& name) {
const std::size_t last = text.find_last_not_of(" \t\r\n");
if (last == std::string_view::npos) return "";
const std::size_t first = text.find_first_not_of(" \t\r\n");
const std::size_t newline = text.rfind('\n', first);
const std::size_t begin = newline == std::string_view::npos ? 0 : newline + 1;
return std::string(text.substr(begin, last + 1 - begin));
}
std::optional<std::string> ReadFile(const fs::path& file) {
std::ifstream in(file, std::ios::binary);
if (!in) return std::nullopt;
std::string content((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
if (in.bad()) return std::nullopt;
return content;
}
// Replace the content of `file` through a temporary beside it: a write
// cut short (a full disk, a crash) leaves the old AGENTS.md rather than
// part of the new one, and a reader never sees a half-written file. The
// rename lands on the symlink's target, so an AGENTS.md that is a link
// stays one, and the target's permissions are carried over.
bool ReplaceFile(const fs::path& file, std::string_view content) {
std::error_code ec;
const fs::path target = fs::canonical(file, ec);
if (ec) return false;
const fs::path temporary = target.parent_path() / std::format(".{}.{}.tmp", target.filename().string(), getpid());
{
std::ofstream out(temporary, std::ios::binary | std::ios::trunc);
out.write(content.data(), static_cast<std::streamsize>(content.size()));
out.close();
if (!out) {
fs::remove(temporary, ec);
return false;
}
}
if (const fs::file_status status = fs::status(target, ec); !ec) fs::permissions(temporary, status.permissions(), ec);
fs::rename(temporary, target, ec);
if (ec) {
std::error_code ignored;
fs::remove(temporary, ignored);
return false;
}
return true;
}
// The note in `file`, beside the project file of a dependency loaded
// from `url` (empty for a local one); nullopt when there is none or it
// cannot be used.
std::optional<DependencyNote> ReadNote(const fs::path& file, const std::string& url, const fs::path& projectDir) {
std::error_code ec; std::error_code ec;
if (!fs::is_regular_file(file, ec)) return std::nullopt; if (!fs::is_regular_file(file, ec)) return std::nullopt;
const std::optional<std::string> content = ReadFile(file); std::string content;
if (!content) { if (const std::string error = ReadFile(file, content); !error.empty()) {
Warn(std::format("{} could not be read, so its notes are left out", file.string())); Warn(std::format("{} could not be read ({}), so its notes are left out", file.string(), error));
return std::nullopt; return std::nullopt;
} }
std::string text = TrimBlank(*content); NormalizedNote note = NormalizeNote(content);
if (text.empty()) return std::nullopt; if (note.text.empty()) return std::nullopt;
// On the file as it is, so the line numbers are the author's. // On the very bytes the merge checks, so the two can never disagree,
if (const std::string problem = NoteProblem(*content); !problem.empty()) { // and numbered as the author's file is.
if (const std::string problem = NoteProblem(note.text, note.firstLine); !problem.empty()) {
Warn(std::format("{}: {}, so its notes are left out", file.string(), problem)); Warn(std::format("{}: {}, so its notes are left out", file.string(), problem));
return std::nullopt; return std::nullopt;
} }
// A URL names the dependency the same on every machine, which a path return DependencyNote{ .name = name, .text = std::move(note.text) };
// into the cache or a sibling checkout does not, and the section is }
// committed along with the rest of AGENTS.md.
std::string source = url.empty() ? file.lexically_relative(projectDir).generic_string() : url; // The loaded projects `configs` lead to without passing through another
return DependencyNote{ .source = std::move(source), .text = std::move(text) }; // loaded project, in the order the configurations name them: one ring
// further out. `walked` spans the whole collection, so each
// Configuration is looked at once however many paths reach it.
std::vector<const Configuration*> NextRing(std::span<const Configuration* const> configs, std::unordered_set<const Configuration*>& walked) {
std::vector<const Configuration*> ring;
std::vector<const Configuration*> pending(configs.rbegin(), configs.rend());
while (!pending.empty()) {
const Configuration* cfg = pending.back();
pending.pop_back();
if (!cfg || !walked.insert(cfg).second) continue;
if (LoadedProjectOf(*cfg)) {
ring.push_back(cfg);
continue;
}
for (auto dep = cfg->dependencies.rbegin(); dep != cfg->dependencies.rend(); ++dep) pending.push_back(*dep);
}
return ring;
} }
// The AGENTS.consumer.md of every project `project` depends on, as // The AGENTS.consumer.md of every project `project` depends on, as
// project.dependencyNotes selects them. Breadth-first from the project's // project.dependencyNotes selects them, ring by ring: first the loaded
// `#pragma crafter dependency` targets, its dependencies and its tests' // projects (the ones GitProject and LocalProject returned) that its
// dependencies, so the projects it names itself come first, in the // `#pragma crafter dependency` lines and its configurations name, in
// order it names them, and a library several of them use is described // that order, then the ones those depend on. Its own configurations, and
// once, where it is first reached. // a dependency's internal ones, are only walked through. Direct stops
// after the first ring.
// //
// A loaded project, one that GitProject or LocalProject returned, // Never walked: the project's tests, since a plain build never touches
// contributes the file beside its project file. Every other // them and a test's dependency can point at the Configuration the project
// Configuration, the project's own library or a dependency's internal // file returned by value, which has since moved; and a dependency's own
// one, is only walked through. Direct stops at the first loaded project // pragma tooling, which is about how the dependency is built, not how it
// on each path. A dependency's own pragma tooling is never reached, // is used, and is no `dependencies` edge to begin with.
// being no `dependencies` edge, and a dependency's tests are never std::vector<DependencyNote> CollectNotes(const Configuration& project, const fs::path& projectFile, const fs::path& projectDir) {
// walked: nothing builds them for the consumer, and they may point at a std::unordered_set<const Configuration*> walked = { &project };
// Configuration their project file has since moved out of. // The project's own notes are for its consumers, so they never go
std::vector<DependencyNote> CollectNotes(const Configuration& project, const fs::path& absProject) { // into its own AGENTS.md, whatever dependency they sit beside. Both
const fs::path projectDir = absProject.parent_path(); // places count for a symlinked project file.
std::deque<const Configuration*> queue; std::unordered_set<fs::path> files = { projectDir / NotesFileName };
// Seen from the start: a test may list the project itself. std::error_code ec;
std::unordered_set<const Configuration*> seen = { &project }; if (const fs::path canonical = fs::canonical(projectFile, ec); !ec) files.insert(canonical.parent_path() / NotesFileName);
auto enqueue = [&](const Configuration* cfg) { std::unordered_set<std::string> identities;
if (cfg && seen.insert(cfg).second) queue.push_back(cfg);
};
for (const Configuration* dep : PragmaDependencies(absProject)) enqueue(dep);
for (const Configuration* dep : project.dependencies) enqueue(dep);
for (const Test& test : project.tests) {
for (const Configuration* dep : test.config.dependencies) enqueue(dep);
}
std::vector<DependencyNote> notes; std::vector<DependencyNote> notes;
std::unordered_set<fs::path> files;
while (!queue.empty()) { std::vector<const Configuration*> start;
const Configuration* cfg = queue.front(); for (const Configuration* dep : PragmaDependencies(projectFile)) start.push_back(dep);
queue.pop_front(); start.insert(start.end(), project.dependencies.begin(), project.dependencies.end());
if (const std::optional<LoadedProject> loaded = LoadedProjectOf(*cfg)) { for (std::vector<const Configuration*> ring = NextRing(start, walked); !ring.empty();) {
std::vector<const Configuration*> next;
for (const Configuration* cfg : ring) {
const std::optional<LoadedProject> loaded = LoadedProjectOf(*cfg);
const fs::path file = loaded->projectFile.parent_path() / NotesFileName; const fs::path file = loaded->projectFile.parent_path() / NotesFileName;
if (files.insert(file).second) { // One library under two specs (a pin here, another revision
if (std::optional<DependencyNote> note = ReadNote(file, loaded->url, projectDir)) notes.push_back(std::move(*note)); // further down) is described once, as the ring nearest the
// project has it.
if (identities.insert(loaded->identity).second && files.insert(file).second) {
if (std::optional<DependencyNote> note = ReadNote(file, loaded->name)) notes.push_back(std::move(*note));
} }
if (project.dependencyNotes == DependencyNotes::Direct) continue; next.insert(next.end(), cfg->dependencies.begin(), cfg->dependencies.end());
} }
for (const Configuration* dep : cfg->dependencies) enqueue(dep); if (project.dependencyNotes == DependencyNotes::Direct) break;
ring = NextRing(next, walked);
} }
return notes; return notes;
} }
// Bring `fd`, which holds `current`, to `wanted`: from the first byte the
// two differ at, so the text in front of the section is never rewritten,
// then cut to length and synced. Empty on success, otherwise the error.
//
// In place rather than through a temporary renamed over the file: a
// rename swaps in another file, which drops whatever belongs to the old
// one (a hard link such as a CLAUDE.md beside it, ACLs, ownership, being
// read-only) and needs fsyncs to be crash safe at all. The price is that
// a reader can catch the section half written, and that a write failing
// partway, on a full disk say, leaves it so. With the section last,
// where it is appended, only the section can be damaged, and the next
// build reports its markers instead of guessing.
std::string WriteChanged(std::int32_t fd, std::string_view current, std::string_view wanted) {
const auto diverge = std::ranges::mismatch(current, wanted);
for (auto done = static_cast<std::size_t>(diverge.in2 - wanted.begin()); done < wanted.size();) {
const auto wrote = ::pwrite(fd, wanted.data() + done, wanted.size() - done, static_cast<off_t>(done));
if (wrote > 0) {
done += static_cast<std::size_t>(wrote);
} else if (wrote == 0) {
return "the write made no progress";
} else if (errno != EINTR) {
return ErrnoMessage();
}
}
if (::ftruncate(fd, static_cast<off_t>(wanted.size())) != 0) return ErrnoMessage();
if (::fsync(fd) != 0) return ErrnoMessage();
return "";
}
} }
std::string Crafter::MergeDependencyNotes(std::string_view agentsMd, std::span<const DependencyNote> notes) { std::string Crafter::MergeDependencyNotes(std::string_view agentsMd, std::span<const DependencyNote> notes) {
for (const DependencyNote& note : notes) { for (const DependencyNote& note : notes) {
if (const std::string problem = NoteProblem(note.text); !problem.empty()) throw std::runtime_error(std::format("the notes from {}: {}", note.source, problem)); if (const std::string problem = NoteProblem(note.text, 1); !problem.empty()) throw std::runtime_error(std::format("the notes from {}: {}", note.name, problem));
} }
const MarkerScan scan = ScanMarkers(agentsMd); const MarkerScan scan = ScanMarkers(agentsMd);
if (scan.unclosedBlock) throw std::runtime_error(std::format("line {}: a code block opens here and never closes", *scan.unclosedBlock + 1)); const bool section = scan.begins.size() == 1 && scan.ends.size() == 1 && scan.begins[0] < scan.ends[0];
// The file's own line ending, so a CRLF file does not end up mixed.
const std::string_view eol = agentsMd.contains("\r\n") ? "\r\n" : "\n";
if (notes.empty()) {
// With nothing to write, leaving the file alone loses nothing, so
// anything short of a well-formed section is no concern of this one.
if (!section) return std::string(agentsMd);
std::string_view before = agentsMd.substr(0, scan.lines[scan.begins[0]].begin);
const std::string_view after = agentsMd.substr(scan.lines[scan.ends[0]].next);
// The blank line a section at the end was appended after goes with
// it. One the user moved keeps the text around it apart.
if (after.empty() && before.ends_with(std::format("{0}{0}", eol))) before.remove_suffix(eol.size());
return std::format("{}{}", before, after);
}
if (scan.begins.size() > 1) throw std::runtime_error(std::format("line {}: a second dependency notes begin marker (the first is on line {})", scan.begins[1] + 1, scan.begins[0] + 1)); if (scan.begins.size() > 1) throw std::runtime_error(std::format("line {}: a second dependency notes begin marker (the first is on line {})", scan.begins[1] + 1, scan.begins[0] + 1));
if (scan.ends.size() > 1) throw std::runtime_error(std::format("line {}: a second dependency notes end marker (the first is on line {})", scan.ends[1] + 1, scan.ends[0] + 1)); if (scan.ends.size() > 1) throw std::runtime_error(std::format("line {}: a second dependency notes end marker (the first is on line {})", scan.ends[1] + 1, scan.ends[0] + 1));
if (!scan.ends.empty() && (scan.begins.empty() || scan.ends[0] < scan.begins[0])) throw std::runtime_error(std::format("line {}: a dependency notes end marker with no begin marker before it", scan.ends[0] + 1)); if (!scan.ends.empty() && (scan.begins.empty() || scan.ends[0] < scan.begins[0])) throw std::runtime_error(std::format("line {}: a dependency notes end marker with no begin marker before it", scan.ends[0] + 1));
if (!scan.begins.empty() && scan.ends.empty()) throw std::runtime_error(std::format("line {}: a dependency notes begin marker with no end marker after it", scan.begins[0] + 1)); if (!scan.begins.empty() && scan.ends.empty()) throw std::runtime_error(std::format("line {}: a dependency notes begin marker with no end marker after it", scan.begins[0] + 1));
const std::string section = notes.empty() ? std::string() : RenderSection(notes); const std::string rendered = RenderSection(notes, eol);
if (scan.begins.empty()) { if (section) return std::format("{}{}{}", agentsMd.substr(0, scan.lines[scan.begins[0]].begin), rendered, agentsMd.substr(scan.lines[scan.ends[0]].next));
if (notes.empty()) return std::string(agentsMd); // Appended, the section would sit inside that block and never be found
std::string merged(agentsMd); // again, so every build would add another.
if (!merged.empty()) merged += merged.ends_with('\n') ? "\n" : "\n\n"; if (scan.unclosedBlock) throw std::runtime_error(std::format("line {}: a code block opens here and never closes", *scan.unclosedBlock + 1));
return merged + section; if (agentsMd.empty()) return rendered;
} // On a line of its own, after a blank one.
std::string_view before = agentsMd.substr(0, scan.lines[scan.begins[0]].begin); return std::format("{}{}{}{}", agentsMd, agentsMd.ends_with('\n') ? std::string_view() : eol, eol, rendered);
const std::string_view after = agentsMd.substr(scan.lines[scan.ends[0]].next);
// The blank line the section was appended after goes with it.
if (notes.empty() && before.ends_with("\n\n")) before.remove_suffix(1);
return std::format("{}{}{}", before, section, after);
} }
bool Crafter::UpdateAgentsMd(const Configuration& project, const fs::path& projectFile) { bool Crafter::UpdateAgentsMd(const Configuration& project, const fs::path& projectFile) {
std::error_code ec; std::error_code ec;
const fs::path absProject = fs::canonical(projectFile, ec); // Beside the project file as it was named, so a symlinked project.cpp
// keeps the AGENTS.md beside the link.
const fs::path projectDir = fs::absolute(projectFile, ec).lexically_normal().parent_path();
if (ec) return false; if (ec) return false;
const fs::path agents = absProject.parent_path() / "AGENTS.md"; const fs::path agents = projectDir / "AGENTS.md";
if (!fs::is_regular_file(agents, ec)) return false; if (!fs::is_regular_file(agents, ec)) return false;
const std::optional<std::string> current = ReadFile(agents); const std::vector<DependencyNote> notes = project.dependencyNotes == DependencyNotes::None ? std::vector<DependencyNote>() : CollectNotes(project, projectFile, projectDir);
if (!current) {
Warn(std::format("{} could not be read, so its dependency notes were not updated", agents.string())); // Locked from the read through the write, so two builds never interleave
// their writes, and on the file itself, which also covers two projects
// that link one AGENTS.md. A read-only descriptor takes the lock as
// well, so a read-only AGENTS.md is still read without complaint.
const Descriptor reading(::open(agents.c_str(), O_RDONLY | O_CLOEXEC));
if (!reading.Valid()) {
Warn(std::format("{} could not be read ({}), so its dependency notes were not updated", agents.string(), ErrnoMessage()));
return false; return false;
} }
const std::vector<DependencyNote> notes = project.dependencyNotes == DependencyNotes::None ? std::vector<DependencyNote>() : CollectNotes(project, absProject); while (::flock(reading.Get(), LOCK_EX) != 0 && errno == EINTR) {}
std::string current;
if (const std::string error = ReadAll(reading.Get(), current); !error.empty()) {
Warn(std::format("{} could not be read ({}), so its dependency notes were not updated", agents.string(), error));
return false;
}
std::string merged; std::string merged;
try { try {
merged = MergeDependencyNotes(*current, notes); merged = MergeDependencyNotes(current, notes);
} catch (const std::exception& e) { } catch (const std::exception& e) {
Warn(std::format("{}: {}, so its dependency notes were not updated", agents.string(), e.what())); Warn(std::format("{}: {}, so its dependency notes were not updated", agents.string(), e.what()));
return false; return false;
} }
if (merged == *current) return false; if (merged == current) return false;
if (!ReplaceFile(agents, merged)) {
Warn(std::format("{} could not be written, so its dependency notes were not updated", agents.string())); // Only now, with something to write: a read-only AGENTS.md is refused
// here, and not on every build that leaves it as it is.
const Descriptor writing(::open(agents.c_str(), O_WRONLY | O_CLOEXEC));
if (!writing.Valid()) {
Warn(std::format("{} could not be opened for writing ({}), so its dependency notes were not updated", agents.string(), ErrnoMessage()));
return false;
}
if (const std::string error = WriteChanged(writing.Get(), current, merged); !error.empty()) {
// Put back what was there. A full disk that refused the new bytes
// usually still takes the old ones, whose space was just freed.
if (WriteChanged(writing.Get(), merged, current).empty()) {
Warn(std::format("{} could not be written ({}), so its dependency notes were not updated", agents.string(), error));
} else {
Warn(std::format("{} could not be written ({}) and may now end in part of the section; the next build reports it if so", agents.string(), error));
}
return false; return false;
} }
// A build that edits a file the user commits says so. // A build that edits a file the user commits says so.

11
implementations/Crafter.Build-Clang.cpp
View file

@ -1701,7 +1701,7 @@ Clean (after the `clean` subcommand):
and dependency artifact under those trees goes with it. and dependency artifact under those trees goes with it.
AGENTS.md: AGENTS.md:
When an AGENTS.md sits beside the project file, a build and `test` keep a When an AGENTS.md sits beside the project file, a build and a test run keep a
section in it holding the AGENTS.consumer.md each dependency ships beside its section in it holding the AGENTS.consumer.md each dependency ships beside its
project.cpp, and write the file only when that section changes. An AGENTS.md project.cpp, and write the file only when that section changes. An AGENTS.md
is never created. cfg.dependencyNotes picks the dependencies: Transitive (the is never created. cfg.dependencyNotes picks the dependencies: Transitive (the
@ -1877,10 +1877,11 @@ std::int32_t Crafter::Run(std::int32_t argc, char** argv) {
// Builds and test runs only: lint and `format --check` promise to // Builds and test runs only: lint and `format --check` promise to
// write nothing, `format` rewrites the project's sources and nothing // write nothing, `format` rewrites the project's sources and nothing
// else, and `run` is for chores. Here, before compiling, because // else, `run` is for chores and `test --list` only lists. Here,
// loading has just fetched every dependency, so the notes are // before compiling, because loading has just fetched every
// current, and an agent fixing a build that fails needs them. // dependency, so the notes are current, and an agent fixing a build
UpdateAgentsMd(config, projectFile); // that fails needs them.
if (!testOpts.listOnly) UpdateAgentsMd(config, projectFile);
if (runTests) { if (runTests) {
TestSummary summary = RunTests(config, testOpts, projectArgs); TestSummary summary = RunTests(config, testOpts, projectArgs);

60
implementations/Crafter.Build-External.cpp
View file

@ -360,6 +360,36 @@ namespace {
for (Shader& s : cfg.shaders) s.path = abs(s.path); for (Shader& s : cfg.shaders) s.path = abs(s.path);
return cfg; return cfg;
} }
// Into ProjectCache under `fullHash`, with `loaded` recorded beside it.
// When another thread stored the same spec first, its Configuration
// wins and `cfg` is dropped.
Configuration* StoreProject(std::string fullHash, Configuration cfg, LoadedProject loaded) {
std::lock_guard lock(ProjectCacheMutex);
if (auto it = ProjectCache.find(fullHash); it != ProjectCache.end()) {
return it->second.get();
}
auto stored = std::make_unique<Configuration>(std::move(cfg));
Configuration* ptr = stored.get();
ProjectCache.emplace(std::move(fullHash), std::move(stored));
LoadedProjects.emplace(ptr, std::move(loaded));
return ptr;
}
// `url` as it names a repository: without user info, which is where a
// token goes, and without trailing slashes or ".git", so specs that only
// spell the repository differently name the same one.
std::string RepositoryKey(std::string_view url) {
std::string key(url);
if (const std::size_t scheme = key.find("://"); scheme != std::string::npos) {
const std::size_t host = scheme + 3;
const std::size_t at = key.rfind('@', key.find('/', host));
if (at != std::string::npos && at >= host) key.erase(host, at + 1 - host);
}
while (key.ends_with('/')) key.pop_back();
if (key.ends_with(".git")) key.resize(key.size() - 4);
return key;
}
} }
Configuration* Crafter::GitProject(const GitProjectSpec& spec) { Configuration* Crafter::GitProject(const GitProjectSpec& spec) {
@ -412,15 +442,13 @@ Configuration* Crafter::GitProject(const GitProjectSpec& spec) {
std::vector<std::string_view> argViews(spec.args.begin(), spec.args.end()); std::vector<std::string_view> argViews(spec.args.begin(), spec.args.end());
Configuration cfg = LoadProjectFromRoot(projectPath, cloneDir, argViews); Configuration cfg = LoadProjectFromRoot(projectPath, cloneDir, argViews);
std::lock_guard lock(ProjectCacheMutex); const fs::path projectInRepo = spec.projectFile.lexically_normal();
if (auto it = ProjectCache.find(fullHash); it != ProjectCache.end()) { const fs::path below = projectInRepo.parent_path();
return it->second.get(); return StoreProject(std::move(fullHash), std::move(cfg), {
} .projectFile = projectPath.lexically_normal(),
auto stored = std::make_unique<Configuration>(std::move(cfg)); .name = below.empty() ? name : std::format("{}/{}", name, below.generic_string()),
Configuration* ptr = stored.get(); .identity = std::format("git|{}|{}", RepositoryKey(spec.source.url), projectInRepo.generic_string()),
ProjectCache.emplace(std::move(fullHash), std::move(stored)); });
LoadedProjects.emplace(ptr, LoadedProject{ .projectFile = projectPath.lexically_normal(), .url = spec.source.url });
return ptr;
} }
fs::path Crafter::GitFetch(const GitSource& source) { fs::path Crafter::GitFetch(const GitSource& source) {
@ -475,15 +503,11 @@ Configuration* Crafter::LocalProject(const LocalProjectSpec& spec) {
std::vector<std::string_view> argViews(spec.args.begin(), spec.args.end()); std::vector<std::string_view> argViews(spec.args.begin(), spec.args.end());
Configuration cfg = LoadProjectFromRoot(canonical, depRoot, argViews); Configuration cfg = LoadProjectFromRoot(canonical, depRoot, argViews);
std::lock_guard lock(ProjectCacheMutex); return StoreProject(std::move(fullHash), std::move(cfg), {
if (auto it = ProjectCache.find(fullHash); it != ProjectCache.end()) { .projectFile = canonical,
return it->second.get(); .name = depRoot.filename().string(),
} .identity = std::format("local|{}", canonical.string()),
auto stored = std::make_unique<Configuration>(std::move(cfg)); });
Configuration* ptr = stored.get();
ProjectCache.emplace(std::move(fullHash), std::move(stored));
LoadedProjects.emplace(ptr, LoadedProject{ .projectFile = canonical, .url = "" });
return ptr;
} }
std::optional<LoadedProject> Crafter::LoadedProjectOf(const Configuration& cfg) { std::optional<LoadedProject> Crafter::LoadedProjectOf(const Configuration& cfg) {

43
interfaces/Crafter.Build-Agents.cppm
View file

@ -20,7 +20,7 @@ export namespace Crafter {
// The projects this one builds against, and the projects those build // The projects this one builds against, and the projects those build
// against, all the way down. // against, all the way down.
Transitive, Transitive,
// Only the projects this one's own configurations, tests and // Only the projects this one's own configurations and
// `#pragma crafter dependency` lines name. // `#pragma crafter dependency` lines name.
Direct, Direct,
// None. A section an earlier build wrote is removed. // None. A section an earlier build wrote is removed.
@ -29,28 +29,33 @@ export namespace Crafter {
// One dependency's AGENTS.consumer.md, ready to merge. // One dependency's AGENTS.consumer.md, ready to merge.
struct DependencyNote { struct DependencyNote {
// Where it came from, written into the comment above it: the git URL, // What the comment above the note calls the dependency: its
// or for a local project the file's path relative to the consumer. // repository's name, or a local project's directory name.
std::string source; std::string name;
// The file's content, without leading blank lines or trailing // The file's content, without leading blank lines or trailing
// whitespace. // whitespace. Lines end in LF; the merge writes the line ending the
// document uses.
std::string text; std::string text;
}; };
// `agentsMd` with its dependency notes section holding `notes`, in order. // `agentsMd` with its dependency notes section holding `notes`, in order.
// A file without a section gets one appended after a blank line; one // A document without a section gets one appended after a blank line; one
// with a section has it rewritten in place, wherever it was moved to; // with a section has it rewritten in place, wherever it was moved to; and
// and empty `notes` remove it together with that blank line, so a file // empty `notes` remove it. Removing a section that ends the document takes
// that only ever gained the section gets its own bytes back. // the blank line in front of it too, so a document that ended in a newline
// before it gained the section gets its own bytes back. The section uses
// the document's line ending, so a CRLF file stays CRLF.
// //
// The two marker lines count only as whole lines outside fenced code // The two marker lines count only as whole lines outside fenced code
// blocks, which is what lets a document quote them. Throws, naming the // blocks, which is what lets a document quote them. Throws, naming the
// line, when the markers are unbalanced or the document ends inside a // line, when there are notes to write and the markers are unbalanced, or
// code block: guessing which part of someone's file is generated would // when the section would have to be appended to a document that ends
// overwrite their text, and missing the section would add a second one // inside a code block: guessing which part of someone's file is generated
// on every build. Also throws for a note that would break the section // would overwrite their text, and missing the section would add another on
// when it is read back: one with a marker line of its own, or a code // every build. With no notes it never throws, since leaving the document
// block it never closes. // alone loses nothing. Also throws for a note that would break the section
// when it is read back: one with a marker line of its own, or a code block
// it never closes.
std::string MergeDependencyNotes(std::string_view agentsMd, std::span<const DependencyNote> notes); std::string MergeDependencyNotes(std::string_view agentsMd, std::span<const DependencyNote> notes);
// Refresh the dependency notes section of the AGENTS.md beside // Refresh the dependency notes section of the AGENTS.md beside
@ -58,8 +63,10 @@ export namespace Crafter {
// project.dependencyNotes selects. `project` is the Configuration that // project.dependencyNotes selects. `project` is the Configuration that
// file returned, after LoadProject: loading is what found the // file returned, after LoadProject: loading is what found the
// dependencies. Does nothing when there is no AGENTS.md, which is never // dependencies. Does nothing when there is no AGENTS.md, which is never
// created. Writes only when the text changes, and returns whether it // created. Writes only when the text changes, in place, so hard links,
// did. An unreadable file, a failed write, malformed markers and a // ownership and a read-only AGENTS.md are respected; returns whether it
// dependency's unusable notes are warnings: the notes never fail a build. // wrote. A read or write that fails, a section that cannot be placed and
// a dependency's unusable notes are warnings: the notes never fail a
// build, and nothing is reported when nothing would have been written.
bool UpdateAgentsMd(const Configuration& project, const fs::path& projectFile); bool UpdateAgentsMd(const Configuration& project, const fs::path& projectFile);
} }

12
interfaces/Crafter.Build-External.cppm
View file

@ -107,7 +107,17 @@ namespace Crafter {
// consumer's directory. // consumer's directory.
struct LoadedProject { struct LoadedProject {
fs::path projectFile; // absolute fs::path projectFile; // absolute
std::string url; // the git source; empty for a LocalProject // What a consumer's AGENTS.md calls it: the repository's name for a
// git project, with the path below the clone's root for one that is
// not at it, and the directory's name for a local one. The same on
// every machine and for a --local checkout and its clone alike,
// which a path into the cache or a sibling checkout is not, and free
// of what a URL can carry, credentials included.
std::string name;
// Which library this is, whatever the revision: the repository and
// the project file in it for git, the project file for a local one.
// Two specs that pin one library differently are one library.
std::string identity;
}; };
// The LoadedProject behind a Configuration that GitProject or // The LoadedProject behind a Configuration that GitProject or
// LocalProject returned; nullopt for any other: the project crafter-build // LocalProject returned; nullopt for any other: the project crafter-build