test(external): race BuildExternal as well as GitFetch

ConcurrentExternalFetch raced eight threads through GitFetch only.
BuildExternal takes the same lock on a line of its own, so that line
could go without any test noticing. Now that the configurations of one
pass share a single BuildExternal, its lock is what keeps two
crafter-builds, or two passes, out of each other's pulls.

The same threads now fetch through both, each into a clone of its own,
and both clones must end at the last round. With the lock in
BuildExternal removed, the test failed in two of three runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tests/ConcurrentExternalFetch/main.cpp

@@ -7,17 +7,19 @@ namespace fs = std::filesystem;
 using namespace Crafter;
 
 // Builds that run at the same time share their dependencies' clones in the
-// cache: the tests `crafter-build test --jobs=N` builds side by side each
+// cache, and two crafter-builds can run at once. Fetching a clone that
-// declare the same ExternalDependency, and two crafter-builds can run at
+// already exists ran `git pull` (and the CMake build) in it with nothing to
-// once. Fetching a clone that already exists ran `git pull` (and the CMake
+// keep a second fetch out, and the second git fails on the first one's
-// build) in it with nothing to keep a second fetch out, and the second git
+// index.lock. A clone's fetch and build now hold a lock of their own. Within
-// fails on the first one's index.lock. A clone's fetch and build now hold a
+// one run, the configurations that declare the same dependency share one
-// lock of their own.
+// fetch of it instead (SharedExternalFetch).
 //
 // Eight threads fetch the same tracked branch of a local repository at
 // once, a few rounds over, the branch moving between rounds so every pull
-// has work to do; every fetch has to succeed. Everything is local;
+// has work to do; every fetch has to succeed. Each thread fetches through
-// XDG_CACHE_HOME keeps the clone out of the real cache.
+// GitFetch and through BuildExternal, which every build of an
+// ExternalDependency calls, each into a clone of its own. Everything is
+// local; XDG_CACHE_HOME keeps the clones out of the real cache.
 
 extern "C" int setenv(const char* name, const char* value, int overwrite);
 
@@ -42,6 +44,13 @@ namespace {
         Git(repo, "add round.txt");
         Git(repo, std::format("commit -q -m round-{}", round));
     }
+
+    std::uint32_t Round(const fs::path& clone) {
+        std::ifstream in(clone / "round.txt");
+        std::uint32_t round = 0;
+        in >> round;
+        return round;
+    }
 }
 
 int main() {
@@ -60,6 +69,12 @@ int main() {
     Commit(repo, 0);
     const GitSource source { .url = std::format("file://{}", repo.string()), .branch = "main" };
     const fs::path clone = GitFetch(source);
+    // A clone of its own: BuildExternal keys the cache differently.
+    ExternalDependency dep;
+    dep.name = "shared";
+    dep.source = source;
+    const std::string cloned = BuildExternal(dep, HostTarget());
+    Check(cloned.empty(), std::format("BuildExternal clones the repository: {}", cloned));
 
     constexpr std::uint32_t threads = 8;
     constexpr std::uint32_t rounds = 4;
@@ -70,20 +85,23 @@ int main() {
         std::vector<std::jthread> fetchers;
         for (std::uint32_t i = 0; i < threads; ++i) {
             fetchers.emplace_back([&] {
+                std::string error;
                 try {
                     (void)GitFetch(source);
+                    error = BuildExternal(dep, HostTarget());
                 } catch (const std::exception& e) {
-                    std::lock_guard lock(errorsMutex);
+                    error = e.what();
-                    errors.push_back(e.what());
                 }
+                if (error.empty()) return;
+                std::lock_guard lock(errorsMutex);
+                errors.push_back(std::move(error));
             });
         }
     }
-    Check(errors.empty(), std::format("{} fetches at once, {} rounds: every one succeeds ({} failed{}{})", threads, rounds, errors.size(), errors.empty() ? "" : ", the first: ", errors.empty() ? "" : errors.front()));
+    Check(errors.empty(), std::format("{} threads fetching at once, {} rounds: every fetch succeeds ({} failed{}{})", threads, rounds, errors.size(), errors.empty() ? "" : ", the first: ", errors.empty() ? "" : errors.front()));
-    std::ifstream round(clone / "round.txt");
+    Check(Round(clone) == rounds, std::format("and GitFetch's clone ends at the last round ({})", Round(clone)));
-    std::uint32_t last = 0;
+    const fs::path externalClone = ExternalCloneDir(dep, HostTarget());
-    round >> last;
+    Check(Round(externalClone) == rounds, std::format("and so does BuildExternal's ({})", Round(externalClone)));
-    Check(last == rounds, std::format("and the clone ends at the last round ({})", last));
 
     std::error_code ec;
     fs::remove_all(scratch, ec);