From 02533ffdda609f1960f66ef72f593826d801d482 Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:00:34 +0800 Subject: [PATCH] Order discovered variants by name, not by interning order (fixes #175) discover_variants ended with std::sort(result.begin(), result.end()) on a Vec. StringId is an enum class over the interning handle, and the names are interned in read_directory order, so the sort ordered by raw readdir order. Same defect as #171, at the second and last site that reads a directory into StringIds and treats handle order as name order. Its one consumer is the build-directory hint, so the damage is cosmetic -- the order never reaches %f, command identity, or anything persisted -- but the same tree listed its candidates differently on different machines: before: found: build-yankee, build-mike, build-charlie, build-bravo, build-alpha, build-zeta after: found: build-alpha, build-bravo, build-charlie, build-mike, build-yankee, build-zeta (readdir order was yankee mike charlie bravo alpha zeta.) The test fixes interning order rather than trusting the filesystem to enumerate out of order, as the #171 test does. The remaining std::sort calls over StringId -- cmd_build.cpp, context.cpp, scheduler.cpp, glob_expand_all's exclusion list -- are sort+unique or binary_search set structures, where handle order is order-agnostic by construction. cmd_show.cpp and multi_variant.cpp already sort through pool.get. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01TLob4Ef3qyMd3Dnp9q3XmA --- src/core/layout.cpp | 3 ++- test/unit/test_layout.cpp | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/core/layout.cpp b/src/core/layout.cpp index 70b3f859..a4a9b0da 100644 --- a/src/core/layout.cpp +++ b/src/core/layout.cpp @@ -218,7 +218,8 @@ auto discover_variants( } } - std::sort(result.begin(), result.end()); + // StringId order is interning order, i.e. readdir order, not name order. + std::ranges::sort(result, {}, [&pool](StringId id) { return pool.get(id); }); return result; } diff --git a/test/unit/test_layout.cpp b/test/unit/test_layout.cpp index 9f0e8bab..e6a418cf 100644 --- a/test/unit/test_layout.cpp +++ b/test/unit/test_layout.cpp @@ -6,9 +6,11 @@ #include "pup/core/layout.hpp" #include "pup/core/string_pool.hpp" +#include #include #include #include +#include namespace fs = std::filesystem; @@ -199,3 +201,38 @@ TEST_CASE("discover_layout from build directory", "[e2e][layout]") REQUIRE(fs::canonical(std::string(global_pool().get(result->output_root))) == fs::canonical(tmp.path() / "build").string()); } } + +SCENARIO("discover_variants orders variants by name, not by interning order", "[e2e][layout]") +{ + GIVEN("build directories whose names were interned in reverse-lexicographic order") + { + // Names unique to this test, so the interning below decides their handles. + auto const names = std::array { + "dvar_alpha", + "dvar_bravo", + "dvar_mike", + "dvar_zeta", + }; + + auto tmp = TempDir {}; + tmp.create_file("Tupfile.ini"); + for (auto name : names) { + tmp.create_file(fs::path { name } / "tup.config"); + } + for (auto i = names.size(); i-- > 0;) { + (void)global_pool().intern(names[i]); + } + + WHEN("the source root is scanned for variants") + { + THEN("the variants are in lexicographic name order") + { + auto found = std::vector {}; + for (auto id : pup::discover_variants(tmp.path().string())) { + found.push_back(global_pool().get(id)); + } + REQUIRE(found == std::vector(names.begin(), names.end())); + } + } + } +}