diff --git a/implementations/Crafter.Build-Agents.cpp b/implementations/Crafter.Build-Agents.cpp index f3811ef..f26c8c2 100644 --- a/implementations/Crafter.Build-Agents.cpp +++ b/implementations/Crafter.Build-Agents.cpp @@ -2,6 +2,9 @@ // SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® module; +#include +#include +#include #include module Crafter.Build:Agents_impl; import std; @@ -25,6 +28,53 @@ namespace { 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 buffer; + for (;;) { + const auto got = ::read(fd, buffer.data(), buffer.size()); + if (got > 0) { + out.append(buffer.data(), static_cast(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 { std::string_view text; // without the newline and a '\r' before it 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); } - // Fenced code blocks as CommonMark has them: a run of three or more - // backticks or tildes, after at most three spaces, opens one, and a run - // of the same character at least as long, with nothing after it but - // spaces, closes it. Tracking the character and the length is what lets - // a ~~~ block quote a ``` one, the usual way to show a fenced example. + // A line with the block quote markers in front of it taken off. + struct Shape { + std::size_t quotes = 0; // `>` markers + std::size_t indent = 0; // columns of indentation after them + 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 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 { public: // Feed the lines in order. True for a fence and for every line // inside a block. bool InCode(std::string_view line) { - std::size_t pos = 0; - while (pos < line.size() && pos < 3 && line[pos] == ' ') ++pos; - const char ch = pos < line.size() ? line[pos] : '\0'; - std::size_t run = 0; - if (ch == '`' || ch == '~') { - while (pos + run < line.size() && line[pos + run] == ch) ++run; + const Shape shape = ShapeOf(line); + if (open_ != '\0') { + const bool blank = TrimEnd(shape.rest).empty(); + const bool left = !blank && (shape.quotes < quotes_ || shape.indent < content_); + if (!left) { + const std::optional 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 (open_ == '\0') { - if (!fence) return false; - open_ = ch; - length_ = run; - return true; + if (shape.indent > 3) return false; + std::size_t column = shape.indent; + std::string_view text = shape.rest; + std::size_t content = 0; + for (std::size_t marker = ListMarker(text); marker > 0; marker = ListMarker(text)) { + 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 = 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; } bool Open() const { return open_ != '\0'; } + // How many blocks have opened so far. + std::size_t Opened() const { return opened_; } private: char open_ = '\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 @@ -100,11 +236,10 @@ namespace { scan.lines = SplitLines(text); CodeFences fences; for (std::size_t i = 0; i < scan.lines.size(); ++i) { - const bool wasOpen = fences.Open(); - if (fences.InCode(scan.lines[i].text)) { - if (!wasOpen) scan.unclosedBlock = i; - continue; - } + const std::size_t opened = fences.Opened(); + const bool code = fences.InCode(scan.lines[i].text); + if (fences.Opened() != opened) scan.unclosedBlock = i; + if (code) continue; const std::string_view line = TrimEnd(scan.lines[i].text); if (line == BeginMarker) scan.begins.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 // 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. - 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); - if (!scan.begins.empty() || !scan.ends.empty()) { - std::size_t line = scan.lines.size(); - if (!scan.begins.empty()) line = scan.begins[0]; - if (!scan.ends.empty()) line = std::min(line, scan.ends[0]); - 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 + 1); + std::optional marker; + if (!scan.begins.empty()) marker = scan.begins[0]; + if (!scan.ends.empty()) marker = std::min(marker.value_or(scan.ends[0]), scan.ends[0]); + if (marker) return std::format("line {} is a dependency notes marker", *marker + firstLine); + if (scan.unclosedBlock) return std::format("line {} opens a code block that never closes", *scan.unclosedBlock + firstLine); return ""; } - std::string RenderSection(std::span notes) { - std::string section = std::format("{}\n{}\n", BeginMarker, Preamble); - for (const DependencyNote& note : notes) section += std::format("\n\n{}\n", note.source, note.text); - section += std::format("\n{}\n", EndMarker); + struct NormalizedNote { + std::string text; + // The line of the file `text` starts on. + 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(begin), '\n'); + return { .text = text.substr(begin, last + 1 - begin), .firstLine = static_cast(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 notes, std::string_view eol) { + std::string section = std::format("{}{}{}{}", BeginMarker, eol, Preamble, eol); + for (const DependencyNote& note : notes) section += std::format("{0}{0}{2}{0}", eol, note.name, WithLineEnds(note.text, eol)); + section += std::format("{0}{1}{0}", eol, EndMarker); return section; } - // Without leading blank lines and trailing whitespace, so the section - // spaces its notes the same whatever the files end with. - std::string TrimBlank(std::string_view text) { - 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 ReadFile(const fs::path& file) { - std::ifstream in(file, std::ios::binary); - if (!in) return std::nullopt; - std::string content((std::istreambuf_iterator(in)), std::istreambuf_iterator()); - 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(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 ReadNote(const fs::path& file, const std::string& url, const fs::path& projectDir) { + // The note beside a dependency's project file, or nullopt when there is + // none or it cannot be used. + std::optional ReadNote(const fs::path& file, const std::string& name) { std::error_code ec; if (!fs::is_regular_file(file, ec)) return std::nullopt; - const std::optional content = ReadFile(file); - if (!content) { - Warn(std::format("{} could not be read, so its notes are left out", file.string())); + std::string 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(), error)); return std::nullopt; } - std::string text = TrimBlank(*content); - if (text.empty()) return std::nullopt; - // On the file as it is, so the line numbers are the author's. - if (const std::string problem = NoteProblem(*content); !problem.empty()) { + NormalizedNote note = NormalizeNote(content); + if (note.text.empty()) return std::nullopt; + // On the very bytes the merge checks, so the two can never disagree, + // 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)); return std::nullopt; } - // A URL names the dependency the same on every machine, which a path - // 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; - return DependencyNote{ .source = std::move(source), .text = std::move(text) }; + return DependencyNote{ .name = name, .text = std::move(note.text) }; + } + + // The loaded projects `configs` lead to without passing through another + // 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 NextRing(std::span configs, std::unordered_set& walked) { + std::vector ring; + std::vector 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 - // project.dependencyNotes selects them. Breadth-first from the project's - // `#pragma crafter dependency` targets, its dependencies and its tests' - // dependencies, so the projects it names itself come first, in the - // order it names them, and a library several of them use is described - // once, where it is first reached. + // project.dependencyNotes selects them, ring by ring: first the loaded + // projects (the ones GitProject and LocalProject returned) that its + // `#pragma crafter dependency` lines and its configurations name, in + // that order, then the ones those depend on. Its own configurations, and + // a dependency's internal ones, are only walked through. Direct stops + // after the first ring. // - // A loaded project, one that GitProject or LocalProject returned, - // contributes the file beside its project file. Every other - // Configuration, the project's own library or a dependency's internal - // one, is only walked through. Direct stops at the first loaded project - // on each path. A dependency's own pragma tooling is never reached, - // being no `dependencies` edge, and a dependency's tests are never - // walked: nothing builds them for the consumer, and they may point at a - // Configuration their project file has since moved out of. - std::vector CollectNotes(const Configuration& project, const fs::path& absProject) { - const fs::path projectDir = absProject.parent_path(); - std::deque queue; - // Seen from the start: a test may list the project itself. - std::unordered_set seen = { &project }; - auto enqueue = [&](const Configuration* cfg) { - 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); - } - + // Never walked: the project's tests, since a plain build never touches + // them and a test's dependency can point at the Configuration the project + // file returned by value, which has since moved; and a dependency's own + // pragma tooling, which is about how the dependency is built, not how it + // is used, and is no `dependencies` edge to begin with. + std::vector CollectNotes(const Configuration& project, const fs::path& projectFile, const fs::path& projectDir) { + std::unordered_set walked = { &project }; + // The project's own notes are for its consumers, so they never go + // into its own AGENTS.md, whatever dependency they sit beside. Both + // places count for a symlinked project file. + std::unordered_set files = { projectDir / NotesFileName }; + std::error_code ec; + if (const fs::path canonical = fs::canonical(projectFile, ec); !ec) files.insert(canonical.parent_path() / NotesFileName); + std::unordered_set identities; std::vector notes; - std::unordered_set files; - while (!queue.empty()) { - const Configuration* cfg = queue.front(); - queue.pop_front(); - if (const std::optional loaded = LoadedProjectOf(*cfg)) { + + std::vector start; + for (const Configuration* dep : PragmaDependencies(projectFile)) start.push_back(dep); + start.insert(start.end(), project.dependencies.begin(), project.dependencies.end()); + for (std::vector ring = NextRing(start, walked); !ring.empty();) { + std::vector next; + for (const Configuration* cfg : ring) { + const std::optional loaded = LoadedProjectOf(*cfg); const fs::path file = loaded->projectFile.parent_path() / NotesFileName; - if (files.insert(file).second) { - if (std::optional note = ReadNote(file, loaded->url, projectDir)) notes.push_back(std::move(*note)); + // One library under two specs (a pin here, another revision + // 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 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; } + + // 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(diverge.in2 - wanted.begin()); done < wanted.size();) { + const auto wrote = ::pwrite(fd, wanted.data() + done, wanted.size() - done, static_cast(done)); + if (wrote > 0) { + done += static_cast(wrote); + } else if (wrote == 0) { + return "the write made no progress"; + } else if (errno != EINTR) { + return ErrnoMessage(); + } + } + if (::ftruncate(fd, static_cast(wanted.size())) != 0) return ErrnoMessage(); + if (::fsync(fd) != 0) return ErrnoMessage(); + return ""; + } } std::string Crafter::MergeDependencyNotes(std::string_view agentsMd, std::span 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); - 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.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.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); - if (scan.begins.empty()) { - if (notes.empty()) return std::string(agentsMd); - std::string merged(agentsMd); - if (!merged.empty()) merged += merged.ends_with('\n') ? "\n" : "\n\n"; - return merged + section; - } - 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 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); + const std::string rendered = RenderSection(notes, eol); + if (section) return std::format("{}{}{}", agentsMd.substr(0, scan.lines[scan.begins[0]].begin), rendered, agentsMd.substr(scan.lines[scan.ends[0]].next)); + // Appended, the section would sit inside that block and never be found + // again, so every build would add another. + if (scan.unclosedBlock) throw std::runtime_error(std::format("line {}: a code block opens here and never closes", *scan.unclosedBlock + 1)); + if (agentsMd.empty()) return rendered; + // On a line of its own, after a blank one. + return std::format("{}{}{}{}", agentsMd, agentsMd.ends_with('\n') ? std::string_view() : eol, eol, rendered); } bool Crafter::UpdateAgentsMd(const Configuration& project, const fs::path& projectFile) { 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; - 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; - const std::optional current = ReadFile(agents); - if (!current) { - Warn(std::format("{} could not be read, so its dependency notes were not updated", agents.string())); + const std::vector notes = project.dependencyNotes == DependencyNotes::None ? std::vector() : CollectNotes(project, projectFile, projectDir); + + // 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; } - const std::vector notes = project.dependencyNotes == DependencyNotes::None ? std::vector() : 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; try { - merged = MergeDependencyNotes(*current, notes); + merged = MergeDependencyNotes(current, notes); } catch (const std::exception& e) { Warn(std::format("{}: {}, so its dependency notes were not updated", agents.string(), e.what())); 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())); + if (merged == current) return false; + + // 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; } // A build that edits a file the user commits says so. diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index 2721c87..6d8820c 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -1701,7 +1701,7 @@ Clean (after the `clean` subcommand): and dependency artifact under those trees goes with it. 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 project.cpp, and write the file only when that section changes. An AGENTS.md 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 // write nothing, `format` rewrites the project's sources and nothing - // else, and `run` is for chores. Here, before compiling, because - // loading has just fetched every dependency, so the notes are - // current, and an agent fixing a build that fails needs them. - UpdateAgentsMd(config, projectFile); + // else, `run` is for chores and `test --list` only lists. Here, + // before compiling, because loading has just fetched every + // dependency, so the notes are current, and an agent fixing a build + // that fails needs them. + if (!testOpts.listOnly) UpdateAgentsMd(config, projectFile); if (runTests) { TestSummary summary = RunTests(config, testOpts, projectArgs); diff --git a/implementations/Crafter.Build-External.cpp b/implementations/Crafter.Build-External.cpp index 390ff73..f27dc81 100644 --- a/implementations/Crafter.Build-External.cpp +++ b/implementations/Crafter.Build-External.cpp @@ -360,6 +360,36 @@ namespace { for (Shader& s : cfg.shaders) s.path = abs(s.path); 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(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) { @@ -412,15 +442,13 @@ Configuration* Crafter::GitProject(const GitProjectSpec& spec) { std::vector argViews(spec.args.begin(), spec.args.end()); Configuration cfg = LoadProjectFromRoot(projectPath, cloneDir, argViews); - std::lock_guard lock(ProjectCacheMutex); - if (auto it = ProjectCache.find(fullHash); it != ProjectCache.end()) { - return it->second.get(); - } - auto stored = std::make_unique(std::move(cfg)); - Configuration* ptr = stored.get(); - ProjectCache.emplace(std::move(fullHash), std::move(stored)); - LoadedProjects.emplace(ptr, LoadedProject{ .projectFile = projectPath.lexically_normal(), .url = spec.source.url }); - return ptr; + const fs::path projectInRepo = spec.projectFile.lexically_normal(); + const fs::path below = projectInRepo.parent_path(); + return StoreProject(std::move(fullHash), std::move(cfg), { + .projectFile = projectPath.lexically_normal(), + .name = below.empty() ? name : std::format("{}/{}", name, below.generic_string()), + .identity = std::format("git|{}|{}", RepositoryKey(spec.source.url), projectInRepo.generic_string()), + }); } fs::path Crafter::GitFetch(const GitSource& source) { @@ -475,15 +503,11 @@ Configuration* Crafter::LocalProject(const LocalProjectSpec& spec) { std::vector argViews(spec.args.begin(), spec.args.end()); Configuration cfg = LoadProjectFromRoot(canonical, depRoot, argViews); - std::lock_guard lock(ProjectCacheMutex); - if (auto it = ProjectCache.find(fullHash); it != ProjectCache.end()) { - return it->second.get(); - } - auto stored = std::make_unique(std::move(cfg)); - Configuration* ptr = stored.get(); - ProjectCache.emplace(std::move(fullHash), std::move(stored)); - LoadedProjects.emplace(ptr, LoadedProject{ .projectFile = canonical, .url = "" }); - return ptr; + return StoreProject(std::move(fullHash), std::move(cfg), { + .projectFile = canonical, + .name = depRoot.filename().string(), + .identity = std::format("local|{}", canonical.string()), + }); } std::optional Crafter::LoadedProjectOf(const Configuration& cfg) { diff --git a/interfaces/Crafter.Build-Agents.cppm b/interfaces/Crafter.Build-Agents.cppm index 55b6da5..77e3c4e 100644 --- a/interfaces/Crafter.Build-Agents.cppm +++ b/interfaces/Crafter.Build-Agents.cppm @@ -20,7 +20,7 @@ export namespace Crafter { // The projects this one builds against, and the projects those build // against, all the way down. 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. Direct, // 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. struct DependencyNote { - // Where it came from, written into the comment above it: the git URL, - // or for a local project the file's path relative to the consumer. - std::string source; + // What the comment above the note calls the dependency: its + // repository's name, or a local project's directory name. + std::string name; // 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; }; // `agentsMd` with its dependency notes section holding `notes`, in order. - // A file without a section gets one appended after a blank line; one - // with a section has it rewritten in place, wherever it was moved to; - // and empty `notes` remove it together with that blank line, so a file - // that only ever gained the section gets its own bytes back. + // 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; and + // empty `notes` remove it. Removing a section that ends the document takes + // 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 // blocks, which is what lets a document quote them. Throws, naming the - // line, when the markers are unbalanced or the document ends inside a - // code block: guessing which part of someone's file is generated would - // overwrite their text, and missing the section would add a second one - // on every build. 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. + // line, when there are notes to write and the markers are unbalanced, or + // when the section would have to be appended to a document that ends + // inside a code block: guessing which part of someone's file is generated + // would overwrite their text, and missing the section would add another on + // every build. With no notes it never throws, since leaving the document + // 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 notes); // 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 // file returned, after LoadProject: loading is what found the // dependencies. Does nothing when there is no AGENTS.md, which is never - // created. Writes only when the text changes, and returns whether it - // did. An unreadable file, a failed write, malformed markers and a - // dependency's unusable notes are warnings: the notes never fail a build. + // created. Writes only when the text changes, in place, so hard links, + // ownership and a read-only AGENTS.md are respected; returns whether it + // 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); } diff --git a/interfaces/Crafter.Build-External.cppm b/interfaces/Crafter.Build-External.cppm index e93a1ed..f60e17d 100644 --- a/interfaces/Crafter.Build-External.cppm +++ b/interfaces/Crafter.Build-External.cppm @@ -107,7 +107,17 @@ namespace Crafter { // consumer's directory. struct LoadedProject { 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 // LocalProject returned; nullopt for any other: the project crafter-build