From ffcce4b0af0652ac343b085e446c615bf0dc47ba Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 11:05:31 +0200 Subject: [PATCH 1/9] Fix dtest -f relative paths under bazel run. Literal paths like hands/list1.txt were only checked against the runfiles cwd; resolve them via BUILD_WORKING_DIRECTORY / BUILD_WORKSPACE_DIRECTORY and relative to the binary, matching the numeric list shorthand. Co-authored-by: Cursor --- library/tests/args.cpp | 81 +++++++++++++++++++++++++++---------- library/tests/args.hpp | 10 +++-- library/tests/args_test.cpp | 51 +++++++++++++++++++++++ 3 files changed, 116 insertions(+), 26 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index 66ce180e..2db76914 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -90,10 +90,10 @@ void usage( cout << "Usage: " << basename << " [options]\n\n" << "-f, --file s Input file, or the number n;\n" << - " '100' means hands/list100.txt under the current\n" << - " directory (or BUILD_WORKING_DIRECTORY /\n" << - " BUILD_WORKSPACE_DIRECTORY under bazel run), else\n" << - " relative to the dtest binary\n" << + " Relative paths (and '100' → hands/list100.txt) are\n" << + " resolved under the current directory, then under\n" << + " BUILD_WORKING_DIRECTORY / BUILD_WORKSPACE_DIRECTORY\n" << + " (bazel run), else relative to the dtest binary\n" << " (bazel-bin/library/tests/).\n" << " (Default: input.txt)\n" << "\n" << @@ -333,39 +333,76 @@ string resolve_dtest_input_file( return cwd_candidate; // bazel run moves CWD into the runfiles tree; it exports the invoke-time - // shell cwd and the workspace root so we can still find hands/. - auto from_env_dir = [&](const char* env_name) -> string + // shell cwd and the workspace root so relative -f paths still resolve. + auto from_env_dir = [&](const char* env_name, const fs::path& rel) -> string { const char* dir = std::getenv(env_name); if (dir == nullptr || dir[0] == '\0') return string(); - const string candidate = normalize_logical_path( - (fs::path(dir) / "hands" / list_name).string()); + const string candidate = + normalize_logical_path((fs::path(dir) / rel).string()); if (path_exists(candidate)) return candidate; return string(); }; - if (const string found = from_env_dir("BUILD_WORKING_DIRECTORY"); !found.empty()) - return found; - if (const string found = from_env_dir("BUILD_WORKSPACE_DIRECTORY"); !found.empty()) - return found; - // Climb parents in the path *string* (do not use "/../" with filesystem // resolution — that follows a bazel-bin symlink into the execroot and misses // the workspace hands/ directory). bazel-bin/library/tests/dtest → four // parent_path steps to the repo root. - fs::path dir(absolute_path_logical(argv0)); - for (unsigned i = 0; i < 4; ++i) + auto workspace_root_from_argv0 = [&]() -> fs::path { - const fs::path parent = dir.parent_path(); - if (parent.empty()) - return string(); - dir = parent; + fs::path dir(absolute_path_logical(argv0)); + for (unsigned i = 0; i < 4; ++i) + { + const fs::path parent = dir.parent_path(); + if (parent.empty()) + return {}; + dir = parent; + } + return dir; + }; + + if (!is_absolute_path(arg)) + { + if (const string found = + from_env_dir("BUILD_WORKING_DIRECTORY", arg); !found.empty()) + { + return found; + } + if (const string found = + from_env_dir("BUILD_WORKSPACE_DIRECTORY", arg); !found.empty()) + { + return found; + } + } + + const fs::path list_rel = fs::path("hands") / list_name; + if (const string found = + from_env_dir("BUILD_WORKING_DIRECTORY", list_rel); !found.empty()) + { + return found; + } + if (const string found = + from_env_dir("BUILD_WORKSPACE_DIRECTORY", list_rel); !found.empty()) + { + return found; + } + + const fs::path root = workspace_root_from_argv0(); + if (root.empty()) + return string(); + + if (!is_absolute_path(arg)) + { + const string bin_literal = + normalize_logical_path((root / arg).string()); + if (path_exists(bin_literal)) + return bin_literal; } const string bin_candidate = - normalize_logical_path((dir / "hands" / list_name).string()); + normalize_logical_path((root / "hands" / list_name).string()); if (path_exists(bin_candidate)) return bin_candidate; @@ -431,8 +468,8 @@ void read_args( } cout << "Input file '" << optarg << "' not found\n"; - cout << "Also tried hands/list" << optarg << - ".txt under the current directory, " + cout << "Also tried that path (and hands/list" << optarg << + ".txt) under the current directory, " "BUILD_WORKING_DIRECTORY, BUILD_WORKSPACE_DIRECTORY, " "and relative to the dtest binary\n"; nextToken -= 2; diff --git a/library/tests/args.hpp b/library/tests/args.hpp index dde54022..2889cf52 100644 --- a/library/tests/args.hpp +++ b/library/tests/args.hpp @@ -32,10 +32,12 @@ void print_options(); /// Resolve `-f` / `--file` to an existing regular file. /// -/// Order: (1) `arg` as a literal path; (2) `hands/list{arg}.txt` under the -/// current working directory; (3) the same under `BUILD_WORKING_DIRECTORY` / -/// `BUILD_WORKSPACE_DIRECTORY` (set by `bazel run`); (4) relative to the -/// directory of `argv0` (the usual `bazel-bin/library/tests/dtest` layout). +/// Order: (1) `arg` as a literal path under the current working directory; +/// (2) `hands/list{arg}.txt` under cwd; (3) the same literal relative path, +/// then `hands/list{arg}.txt`, under `BUILD_WORKING_DIRECTORY` / +/// `BUILD_WORKSPACE_DIRECTORY` (set by `bazel run`); (4) those same two forms +/// relative to the directory of `argv0` (the usual `bazel-bin/library/tests/dtest` +/// layout). Absolute paths skip the bazel / argv0 relative retries. /// Directories are not accepted (avoids treating e.g. `-f hands` as a file). /// @return Resolved path, or empty if no regular file is found std::string resolve_dtest_input_file( diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index cb49c94f..ee79a39b 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -264,6 +264,25 @@ TEST_F(HandsLayoutFixture, ResolveNumericUsesBazelWorkingDirectory) root_ + "hands/list42.txt")); } +TEST_F(HandsLayoutFixture, ResolveLiteralRelativeUsesBazelWorkingDirectory) +{ + // bazelisk run //library/tests:dtest -- -f hands/list1.txt must find the + // path relative to the invoke-time shell cwd, not the runfiles tree. + const std::string runfiles = + std::string(::testing::TempDir()) + "dtest_hands_runfiles_lit/"; + ASSERT_TRUE(make_dir(runfiles)); + ASSERT_EQ(change_dir(runfiles.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(root_.c_str()); + workspace.set(nullptr); + + EXPECT_TRUE(same_path( + resolve_dtest_input_file("hands/list42.txt", "dtest"), + root_ + "hands/list42.txt")); +} + TEST_F(HandsLayoutFixture, ResolveNumericUsesBazelWorkspaceDirectory) { const std::string runfiles = @@ -281,6 +300,38 @@ TEST_F(HandsLayoutFixture, ResolveNumericUsesBazelWorkspaceDirectory) root_ + "hands/list42.txt")); } +TEST_F(HandsLayoutFixture, ResolveLiteralRelativeUsesBazelWorkspaceDirectory) +{ + const std::string runfiles = + std::string(::testing::TempDir()) + "dtest_hands_runfiles_ws_lit/"; + ASSERT_TRUE(make_dir(runfiles)); + ASSERT_EQ(change_dir(runfiles.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(nullptr); + workspace.set(root_.c_str()); + + EXPECT_TRUE(same_path( + resolve_dtest_input_file("hands/list42.txt", "dtest"), + root_ + "hands/list42.txt")); +} + +TEST_F(HandsLayoutFixture, ResolveLiteralRelativeFallsBackRelativeToBinary) +{ + // Same layout as numeric binary-relative lookup, but with an explicit path. + ASSERT_EQ(change_dir(original_cwd_.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(nullptr); + workspace.set(nullptr); + + EXPECT_TRUE(same_path( + resolve_dtest_input_file("hands/list42.txt", binary_path_), + root_ + "hands/list42.txt")); +} + TEST_F(HandsLayoutFixture, ResolveNumericWithRelativeArgv0FromOtherCwd) { // Mimic running `../bazel-bin/library/tests/dtest -f 42` from a sibling of From ec3a7199c2ae86e69855cc5442a6a6aa2c670e51 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 11:27:24 +0200 Subject: [PATCH 2/9] Clarify dtest -f docs and missing-file hint. Spell out that list shorthand is for numeric ids, that absolute paths only try the literal path, and avoid inventing hands/list.txt in the error text when -f is already a path. Co-authored-by: Cursor --- library/tests/args.cpp | 6 +++--- library/tests/args.hpp | 13 +++++++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index 2db76914..269f1314 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -468,10 +468,10 @@ void read_args( } cout << "Input file '" << optarg << "' not found\n"; - cout << "Also tried that path (and hands/list" << optarg << - ".txt) under the current directory, " + cout << "Also tried that path under the current directory, " "BUILD_WORKING_DIRECTORY, BUILD_WORKSPACE_DIRECTORY, " - "and relative to the dtest binary\n"; + "and relative to the dtest binary; " + "for numeric -f N, also hands/listN.txt\n"; nextToken -= 2; errFlag = true; break; diff --git a/library/tests/args.hpp b/library/tests/args.hpp index 2889cf52..3c7fbaa4 100644 --- a/library/tests/args.hpp +++ b/library/tests/args.hpp @@ -32,12 +32,13 @@ void print_options(); /// Resolve `-f` / `--file` to an existing regular file. /// -/// Order: (1) `arg` as a literal path under the current working directory; -/// (2) `hands/list{arg}.txt` under cwd; (3) the same literal relative path, -/// then `hands/list{arg}.txt`, under `BUILD_WORKING_DIRECTORY` / -/// `BUILD_WORKSPACE_DIRECTORY` (set by `bazel run`); (4) those same two forms -/// relative to the directory of `argv0` (the usual `bazel-bin/library/tests/dtest` -/// layout). Absolute paths skip the bazel / argv0 relative retries. +/// Tries, in order: the literal path under the current working directory; +/// `hands/list{arg}.txt` under cwd (list shorthand, intended for numeric +/// ids such as `"100"`); the literal relative path, then that list form, +/// under `BUILD_WORKING_DIRECTORY` / `BUILD_WORKSPACE_DIRECTORY` (set by +/// `bazel run`); then those same two forms relative to the directory of +/// `argv0` (the usual `bazel-bin/library/tests/dtest` layout). Absolute +/// paths only attempt the literal path as given. /// Directories are not accepted (avoids treating e.g. `-f hands` as a file). /// @return Resolved path, or empty if no regular file is found std::string resolve_dtest_input_file( From e9b2814a4960ccd755841f72700b9d4ca7333de8 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 14:30:36 +0200 Subject: [PATCH 3/9] Stop absolute -f paths from falling through to list shorthand. A missing absolute path must fail rather than searching hands/list{arg}.txt under cwd, bazel env dirs, or the dtest binary, matching the documented contract. Co-authored-by: Cursor --- library/tests/args.cpp | 32 ++++++++++++++------------------ library/tests/args_test.cpp | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index 269f1314..cb07430c 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -324,6 +324,8 @@ string resolve_dtest_input_file( { if (path_exists(arg)) return arg; + if (is_absolute_path(arg)) + return string(); const string list_name = "list" + arg + ".txt"; // Keep generic separators so cwd hits match the documented hands/listN.txt form. @@ -363,18 +365,15 @@ string resolve_dtest_input_file( return dir; }; - if (!is_absolute_path(arg)) + if (const string found = + from_env_dir("BUILD_WORKING_DIRECTORY", arg); !found.empty()) { - if (const string found = - from_env_dir("BUILD_WORKING_DIRECTORY", arg); !found.empty()) - { - return found; - } - if (const string found = - from_env_dir("BUILD_WORKSPACE_DIRECTORY", arg); !found.empty()) - { - return found; - } + return found; + } + if (const string found = + from_env_dir("BUILD_WORKSPACE_DIRECTORY", arg); !found.empty()) + { + return found; } const fs::path list_rel = fs::path("hands") / list_name; @@ -393,13 +392,10 @@ string resolve_dtest_input_file( if (root.empty()) return string(); - if (!is_absolute_path(arg)) - { - const string bin_literal = - normalize_logical_path((root / arg).string()); - if (path_exists(bin_literal)) - return bin_literal; - } + const string bin_literal = + normalize_logical_path((root / arg).string()); + if (path_exists(bin_literal)) + return bin_literal; const string bin_candidate = normalize_logical_path((root / "hands" / list_name).string()); diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index ee79a39b..5c08db23 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -382,6 +382,26 @@ TEST_F(HandsLayoutFixture, ResolveRejectsDirectoryAsLiteralPath) EXPECT_TRUE(resolve_dtest_input_file(root_ + "hands", "dtest").empty()); } +TEST_F(HandsLayoutFixture, ResolveAbsoluteMissingDoesNotUseListShorthand) +{ + // A missing absolute -f must not fall through to hands/list{arg}.txt + // (list shorthand concatenates the absolute path into a nested name). + ASSERT_EQ(change_dir(original_cwd_.c_str()), 0); + ASSERT_TRUE(make_dir(root_ + "hands/list")); + { + std::ofstream out(root_ + "hands/list/no_such_dtest_abs.txt"); + out << "trap\n"; + } + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(nullptr); + workspace.set(nullptr); + + EXPECT_TRUE( + resolve_dtest_input_file("/no_such_dtest_abs", binary_path_).empty()); +} + TEST(Args, AbsolutePathDetection) { EXPECT_FALSE(is_dtest_absolute_path("tmp/dtest")); From b17c58673cc3b42de3914b7986b6a303f841b821 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 14:52:35 +0200 Subject: [PATCH 4/9] Align dtest -f docs with argv0 workspace climb. Document that binary-relative lookup climbs four parents from argv0 to the workspace root, and build the absolute-path regression trap from a unique missing path so the test does not depend on a hardcoded absolute name. Co-authored-by: Cursor --- library/tests/args.cpp | 6 +++--- library/tests/args.hpp | 7 ++++--- library/tests/args_test.cpp | 31 ++++++++++++++++++++++++++++--- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index cb07430c..d637f325 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -93,8 +93,8 @@ void usage( " Relative paths (and '100' → hands/list100.txt) are\n" << " resolved under the current directory, then under\n" << " BUILD_WORKING_DIRECTORY / BUILD_WORKSPACE_DIRECTORY\n" << - " (bazel run), else relative to the dtest binary\n" << - " (bazel-bin/library/tests/).\n" << + " (bazel run), else under the workspace root inferred\n" << + " from the dtest binary (bazel-bin/library/tests/).\n" << " (Default: input.txt)\n" << "\n" << "-s, --solver One of: solve, calc, play, par, dealerpar.\n" << @@ -466,7 +466,7 @@ void read_args( cout << "Input file '" << optarg << "' not found\n"; cout << "Also tried that path under the current directory, " "BUILD_WORKING_DIRECTORY, BUILD_WORKSPACE_DIRECTORY, " - "and relative to the dtest binary; " + "and under the workspace root inferred from the dtest binary; " "for numeric -f N, also hands/listN.txt\n"; nextToken -= 2; errFlag = true; diff --git a/library/tests/args.hpp b/library/tests/args.hpp index 3c7fbaa4..0f32d9b0 100644 --- a/library/tests/args.hpp +++ b/library/tests/args.hpp @@ -36,9 +36,10 @@ void print_options(); /// `hands/list{arg}.txt` under cwd (list shorthand, intended for numeric /// ids such as `"100"`); the literal relative path, then that list form, /// under `BUILD_WORKING_DIRECTORY` / `BUILD_WORKSPACE_DIRECTORY` (set by -/// `bazel run`); then those same two forms relative to the directory of -/// `argv0` (the usual `bazel-bin/library/tests/dtest` layout). Absolute -/// paths only attempt the literal path as given. +/// `bazel run`); then those same two forms under the workspace root inferred +/// by climbing four parents from `argv0` (the usual +/// `bazel-bin/library/tests/dtest` layout). Absolute paths only attempt the +/// literal path as given. /// Directories are not accepted (avoids treating e.g. `-f hands` as a file). /// @return Resolved path, or empty if no regular file is found std::string resolve_dtest_input_file( diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index 5c08db23..1c89d7f8 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -3,7 +3,9 @@ #include +#include #include +#include #include #include #include @@ -387,9 +389,32 @@ TEST_F(HandsLayoutFixture, ResolveAbsoluteMissingDoesNotUseListShorthand) // A missing absolute -f must not fall through to hands/list{arg}.txt // (list shorthand concatenates the absolute path into a nested name). ASSERT_EQ(change_dir(original_cwd_.c_str()), 0); - ASSERT_TRUE(make_dir(root_ + "hands/list")); + + // Pick a unique absolute path that is not present on this machine. + const std::string token = + "no_such_dtest_abs_" + + std::to_string(static_cast( + reinterpret_cast(this))); +#ifdef _WIN32 + const std::string missing_abs = "\\" + token; +#else + const std::string missing_abs = "/" + token; +#endif + ASSERT_FALSE(std::filesystem::exists(missing_abs)); + + // Trap file at the binary-relative list-shorthand location that a buggy + // fallthrough would incorrectly accept. + const std::filesystem::path trap = + std::filesystem::path(root_) / "hands" / + ("list" + missing_abs + ".txt"); + { + std::error_code ec; + std::filesystem::create_directories(trap.parent_path(), ec); + ASSERT_FALSE(ec) << ec.message(); + } { - std::ofstream out(root_ + "hands/list/no_such_dtest_abs.txt"); + std::ofstream out(trap); + ASSERT_TRUE(out) << trap.string(); out << "trap\n"; } @@ -399,7 +424,7 @@ TEST_F(HandsLayoutFixture, ResolveAbsoluteMissingDoesNotUseListShorthand) workspace.set(nullptr); EXPECT_TRUE( - resolve_dtest_input_file("/no_such_dtest_abs", binary_path_).empty()); + resolve_dtest_input_file(missing_abs, binary_path_).empty()); } TEST(Args, AbsolutePathDetection) From 3b2d43b5f84e4ff2951cd6a3887b0596e6cc085c Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 15:15:07 +0200 Subject: [PATCH 5/9] Match ResolveLiteralRelative test comment to list42.txt. Co-authored-by: Cursor --- library/tests/args_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index 1c89d7f8..9f9f7c2a 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -268,7 +268,7 @@ TEST_F(HandsLayoutFixture, ResolveNumericUsesBazelWorkingDirectory) TEST_F(HandsLayoutFixture, ResolveLiteralRelativeUsesBazelWorkingDirectory) { - // bazelisk run //library/tests:dtest -- -f hands/list1.txt must find the + // bazelisk run //library/tests:dtest -- -f hands/list42.txt must find the // path relative to the invoke-time shell cwd, not the runfiles tree. const std::string runfiles = std::string(::testing::TempDir()) + "dtest_hands_runfiles_lit/"; From 65f83c40da7e0c7e02350183a9fd315b2b143314 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 18:51:53 +0200 Subject: [PATCH 6/9] Prefer list shorthand over bare names under bazel paths. Under BUILD_* dirs and the argv0 workspace root, try hands/listN.txt before a literal relative arg so numeric -f N is not shadowed by a workspace-root file named N. Co-authored-by: Cursor --- library/tests/args.cpp | 23 ++++++++++--------- library/tests/args.hpp | 2 +- library/tests/args_test.cpp | 44 +++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index d637f325..cb5824b5 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -365,25 +365,28 @@ string resolve_dtest_input_file( return dir; }; + const fs::path list_rel = fs::path("hands") / list_name; if (const string found = - from_env_dir("BUILD_WORKING_DIRECTORY", arg); !found.empty()) + from_env_dir("BUILD_WORKING_DIRECTORY", list_rel); !found.empty()) { return found; } if (const string found = - from_env_dir("BUILD_WORKSPACE_DIRECTORY", arg); !found.empty()) + from_env_dir("BUILD_WORKSPACE_DIRECTORY", list_rel); !found.empty()) { return found; } - const fs::path list_rel = fs::path("hands") / list_name; + // Prefer list shorthand under bazel dirs / argv0 before a bare relative + // name (so -f 42 keeps resolving to hands/list42.txt even if a file named + // "42" exists at the workspace root). if (const string found = - from_env_dir("BUILD_WORKING_DIRECTORY", list_rel); !found.empty()) + from_env_dir("BUILD_WORKING_DIRECTORY", arg); !found.empty()) { return found; } if (const string found = - from_env_dir("BUILD_WORKSPACE_DIRECTORY", list_rel); !found.empty()) + from_env_dir("BUILD_WORKSPACE_DIRECTORY", arg); !found.empty()) { return found; } @@ -392,16 +395,16 @@ string resolve_dtest_input_file( if (root.empty()) return string(); - const string bin_literal = - normalize_logical_path((root / arg).string()); - if (path_exists(bin_literal)) - return bin_literal; - const string bin_candidate = normalize_logical_path((root / "hands" / list_name).string()); if (path_exists(bin_candidate)) return bin_candidate; + const string bin_literal = + normalize_logical_path((root / arg).string()); + if (path_exists(bin_literal)) + return bin_literal; + return string(); } diff --git a/library/tests/args.hpp b/library/tests/args.hpp index 0f32d9b0..30ec6711 100644 --- a/library/tests/args.hpp +++ b/library/tests/args.hpp @@ -34,7 +34,7 @@ void print_options(); /// /// Tries, in order: the literal path under the current working directory; /// `hands/list{arg}.txt` under cwd (list shorthand, intended for numeric -/// ids such as `"100"`); the literal relative path, then that list form, +/// ids such as `"100"`); that list form, then the literal relative path, /// under `BUILD_WORKING_DIRECTORY` / `BUILD_WORKSPACE_DIRECTORY` (set by /// `bazel run`); then those same two forms under the workspace root inferred /// by climbing four parents from `argv0` (the usual diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index 9f9f7c2a..56a21e43 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -266,6 +266,30 @@ TEST_F(HandsLayoutFixture, ResolveNumericUsesBazelWorkingDirectory) root_ + "hands/list42.txt")); } +TEST_F(HandsLayoutFixture, ResolveNumericPrefersListOverLiteralUnderBazelWorking) +{ + // A workspace-root file named "42" must not win over hands/list42.txt when + // resolving numeric -f under BUILD_WORKING_DIRECTORY. + { + std::ofstream out(root_ + "42"); + out << "literal-trap\n"; + } + + const std::string runfiles = + std::string(::testing::TempDir()) + "dtest_hands_runfiles_num_pref/"; + ASSERT_TRUE(make_dir(runfiles)); + ASSERT_EQ(change_dir(runfiles.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(root_.c_str()); + workspace.set(nullptr); + + EXPECT_TRUE(same_path( + resolve_dtest_input_file("42", "dtest"), + root_ + "hands/list42.txt")); +} + TEST_F(HandsLayoutFixture, ResolveLiteralRelativeUsesBazelWorkingDirectory) { // bazelisk run //library/tests:dtest -- -f hands/list42.txt must find the @@ -334,6 +358,26 @@ TEST_F(HandsLayoutFixture, ResolveLiteralRelativeFallsBackRelativeToBinary) root_ + "hands/list42.txt")); } +TEST_F(HandsLayoutFixture, ResolveNumericPrefersListOverLiteralRelativeToBinary) +{ + // Workspace-root file "42" must not shadow hands/list42.txt for numeric -f + // when falling back via argv0. + { + std::ofstream out(root_ + "42"); + out << "literal-trap\n"; + } + ASSERT_EQ(change_dir(original_cwd_.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(nullptr); + workspace.set(nullptr); + + EXPECT_TRUE(same_path( + resolve_dtest_input_file("42", binary_path_), + root_ + "hands/list42.txt")); +} + TEST_F(HandsLayoutFixture, ResolveNumericWithRelativeArgv0FromOtherCwd) { // Mimic running `../bazel-bin/library/tests/dtest -f 42` from a sibling of From 8e2db09dd21355f80c9c0e0035b4f455792aac1d Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sun, 16 Aug 2026 19:07:14 +0200 Subject: [PATCH 7/9] Reject drive-relative -f joins; fix absolute missing hint. Windows drive-relative paths like C:foo are not joined under BUILD_* or argv0 (operator/ can discard the base). The "Also tried" hint is only printed for portable relative -f values. Co-authored-by: Cursor --- library/tests/args.cpp | 19 +++++++++++++++---- library/tests/args.hpp | 4 ++-- library/tests/args_test.cpp | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index cb5824b5..46fa469e 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -326,6 +326,10 @@ string resolve_dtest_input_file( return arg; if (is_absolute_path(arg)) return string(); + // Windows drive-relative "C:foo" has a root-name; fs::path join may discard + // the BUILD_* / argv0 base. Only the literal path is attempted. + if (fs::path(arg).has_root_name()) + return string(); const string list_name = "list" + arg + ".txt"; // Keep generic separators so cwd hits match the documented hands/listN.txt form. @@ -338,6 +342,8 @@ string resolve_dtest_input_file( // shell cwd and the workspace root so relative -f paths still resolve. auto from_env_dir = [&](const char* env_name, const fs::path& rel) -> string { + if (rel.has_root_name() || rel.has_root_directory()) + return string(); const char* dir = std::getenv(env_name); if (dir == nullptr || dir[0] == '\0') return string(); @@ -467,10 +473,15 @@ void read_args( } cout << "Input file '" << optarg << "' not found\n"; - cout << "Also tried that path under the current directory, " - "BUILD_WORKING_DIRECTORY, BUILD_WORKSPACE_DIRECTORY, " - "and under the workspace root inferred from the dtest binary; " - "for numeric -f N, also hands/listN.txt\n"; + // Absolute / drive-relative args only attempt the literal path. + if (!is_dtest_absolute_path(optarg) && + !fs::path(optarg).has_root_name()) + { + cout << "Also tried that path under the current directory, " + "BUILD_WORKING_DIRECTORY, BUILD_WORKSPACE_DIRECTORY, " + "and under the workspace root inferred from the dtest binary; " + "for numeric -f N, also hands/listN.txt\n"; + } nextToken -= 2; errFlag = true; break; diff --git a/library/tests/args.hpp b/library/tests/args.hpp index 30ec6711..c1f1a46e 100644 --- a/library/tests/args.hpp +++ b/library/tests/args.hpp @@ -38,8 +38,8 @@ void print_options(); /// under `BUILD_WORKING_DIRECTORY` / `BUILD_WORKSPACE_DIRECTORY` (set by /// `bazel run`); then those same two forms under the workspace root inferred /// by climbing four parents from `argv0` (the usual -/// `bazel-bin/library/tests/dtest` layout). Absolute paths only attempt the -/// literal path as given. +/// `bazel-bin/library/tests/dtest` layout). Absolute paths, and Windows +/// drive-relative forms like `C:foo`, only attempt the literal path as given. /// Directories are not accepted (avoids treating e.g. `-f hands` as a file). /// @return Resolved path, or empty if no regular file is found std::string resolve_dtest_input_file( diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index 56a21e43..abd09447 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -471,6 +471,38 @@ TEST_F(HandsLayoutFixture, ResolveAbsoluteMissingDoesNotUseListShorthand) resolve_dtest_input_file(missing_abs, binary_path_).empty()); } +#ifdef _WIN32 +TEST_F(HandsLayoutFixture, ResolveDriveRelativeMissingDoesNotJoinUnderBazelDirs) +{ + // Drive-relative "X:foo" is not absolute, but fs::path join can discard the + // BUILD_* base. Missing drive-relative -f must not resolve via env joins. + ASSERT_GE(root_.size(), 2u); + ASSERT_EQ(root_[1], ':'); + + const std::string token = + "no_such_dtest_drive_rel_" + + std::to_string(static_cast( + reinterpret_cast(this))); + const std::string missing_drive_rel = + std::string(1, root_[0]) + ":" + token; + ASSERT_FALSE(is_dtest_absolute_path(missing_drive_rel)); + ASSERT_FALSE(std::filesystem::exists(missing_drive_rel)); + + const std::string runfiles = + std::string(::testing::TempDir()) + "dtest_hands_drive_rel/"; + ASSERT_TRUE(make_dir(runfiles)); + ASSERT_EQ(change_dir(runfiles.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(root_.c_str()); + workspace.set(root_.c_str()); + + EXPECT_TRUE( + resolve_dtest_input_file(missing_drive_rel, binary_path_).empty()); +} +#endif + TEST(Args, AbsolutePathDetection) { EXPECT_FALSE(is_dtest_absolute_path("tmp/dtest")); From 845ad905b9b9e4575201a152c20c013f04b1a95f Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Wed, 19 Aug 2026 22:18:19 +0200 Subject: [PATCH 8/9] Limit list shorthand to purely numeric -f args. Path-like -f values no longer probe hands/list{arg}.txt candidates that concatenate directory components into nonsense paths. Co-authored-by: Cursor --- library/tests/args.cpp | 64 ++++++++++++++++++++++++++----------- library/tests/args.hpp | 4 +-- library/tests/args_test.cpp | 26 +++++++++++++++ 3 files changed, 73 insertions(+), 21 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index 46fa469e..8f92cfca 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -318,6 +318,21 @@ bool is_dtest_absolute_path(const string& path) } +bool is_dtest_list_shorthand_arg(const string& arg) +{ + if (arg.empty()) + return false; + for (unsigned char c : arg) + { + if (c == '/' || c == '\\') + return false; + if (!std::isdigit(c)) + return false; + } + return true; +} + + string resolve_dtest_input_file( const string& arg, const string& argv0) @@ -331,12 +346,17 @@ string resolve_dtest_input_file( if (fs::path(arg).has_root_name()) return string(); - const string list_name = "list" + arg + ".txt"; - // Keep generic separators so cwd hits match the documented hands/listN.txt form. - const string cwd_candidate = - (fs::path("hands") / list_name).generic_string(); - if (path_exists(cwd_candidate)) - return cwd_candidate; + const bool use_list_shorthand = is_dtest_list_shorthand_arg(arg); + const string list_name = use_list_shorthand ? "list" + arg + ".txt" : string(); + + if (use_list_shorthand) + { + // Keep generic separators so cwd hits match the documented hands/listN.txt form. + const string cwd_candidate = + (fs::path("hands") / list_name).generic_string(); + if (path_exists(cwd_candidate)) + return cwd_candidate; + } // bazel run moves CWD into the runfiles tree; it exports the invoke-time // shell cwd and the workspace root so relative -f paths still resolve. @@ -371,16 +391,19 @@ string resolve_dtest_input_file( return dir; }; - const fs::path list_rel = fs::path("hands") / list_name; - if (const string found = - from_env_dir("BUILD_WORKING_DIRECTORY", list_rel); !found.empty()) + if (use_list_shorthand) { - return found; - } - if (const string found = - from_env_dir("BUILD_WORKSPACE_DIRECTORY", list_rel); !found.empty()) - { - return found; + const fs::path list_rel = fs::path("hands") / list_name; + if (const string found = + from_env_dir("BUILD_WORKING_DIRECTORY", list_rel); !found.empty()) + { + return found; + } + if (const string found = + from_env_dir("BUILD_WORKSPACE_DIRECTORY", list_rel); !found.empty()) + { + return found; + } } // Prefer list shorthand under bazel dirs / argv0 before a bare relative @@ -401,10 +424,13 @@ string resolve_dtest_input_file( if (root.empty()) return string(); - const string bin_candidate = - normalize_logical_path((root / "hands" / list_name).string()); - if (path_exists(bin_candidate)) - return bin_candidate; + if (use_list_shorthand) + { + const string bin_candidate = + normalize_logical_path((root / "hands" / list_name).string()); + if (path_exists(bin_candidate)) + return bin_candidate; + } const string bin_literal = normalize_logical_path((root / arg).string()); diff --git a/library/tests/args.hpp b/library/tests/args.hpp index c1f1a46e..5d8989b4 100644 --- a/library/tests/args.hpp +++ b/library/tests/args.hpp @@ -33,8 +33,8 @@ void print_options(); /// Resolve `-f` / `--file` to an existing regular file. /// /// Tries, in order: the literal path under the current working directory; -/// `hands/list{arg}.txt` under cwd (list shorthand, intended for numeric -/// ids such as `"100"`); that list form, then the literal relative path, +/// for purely numeric `arg` (e.g. `"100"`), `hands/list{arg}.txt` under cwd; +/// that list form, then the literal relative path, /// under `BUILD_WORKING_DIRECTORY` / `BUILD_WORKSPACE_DIRECTORY` (set by /// `bazel run`); then those same two forms under the workspace root inferred /// by climbing four parents from `argv0` (the usual diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index abd09447..987a6e80 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -290,6 +290,32 @@ TEST_F(HandsLayoutFixture, ResolveNumericPrefersListOverLiteralUnderBazelWorking root_ + "hands/list42.txt")); } +TEST_F(HandsLayoutFixture, ResolvePathLikeArgDoesNotUseListShorthand) +{ + // A path-like -f must not probe nonsense list-shorthand candidates such as + // hands/listhands/list42.txt.txt when the literal path is missing. + const std::string trap = root_ + "hands/listhands/list42.txt.txt"; + ASSERT_TRUE(make_dir(root_ + "hands/listhands")); + { + std::ofstream out(trap); + out << "trap\n"; + } + std::filesystem::remove(root_ + "hands/list42.txt"); + + const std::string runfiles = + std::string(::testing::TempDir()) + "dtest_hands_runfiles_no_shorthand/"; + ASSERT_TRUE(make_dir(runfiles)); + ASSERT_EQ(change_dir(runfiles.c_str()), 0); + + const EnvVarGuard working("BUILD_WORKING_DIRECTORY"); + const EnvVarGuard workspace("BUILD_WORKSPACE_DIRECTORY"); + working.set(root_.c_str()); + workspace.set(nullptr); + + EXPECT_TRUE( + resolve_dtest_input_file("hands/list42.txt", "dtest").empty()); +} + TEST_F(HandsLayoutFixture, ResolveLiteralRelativeUsesBazelWorkingDirectory) { // bazelisk run //library/tests:dtest -- -f hands/list42.txt must find the From cc593ac2d73df389620f0fa986f1f2ddc8866ab4 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Thu, 20 Aug 2026 21:52:17 +0200 Subject: [PATCH 9/9] Address Copilot notes on list-shorthand linkage and remove(). Keep is_dtest_list_shorthand_arg TU-local in the unnamed namespace, and use the non-throwing filesystem::remove overload in the path-like shorthand test. Co-authored-by: Cursor --- library/tests/args.cpp | 16 ++++++++-------- library/tests/args_test.cpp | 6 +++++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/library/tests/args.cpp b/library/tests/args.cpp index 8f92cfca..c59c259c 100644 --- a/library/tests/args.cpp +++ b/library/tests/args.cpp @@ -309,14 +309,6 @@ string absolute_path_logical(const string& path) return normalize_logical_path((cwd / path).string()); } -} // namespace - - -bool is_dtest_absolute_path(const string& path) -{ - return is_absolute_path(path); -} - bool is_dtest_list_shorthand_arg(const string& arg) { @@ -332,6 +324,14 @@ bool is_dtest_list_shorthand_arg(const string& arg) return true; } +} // namespace + + +bool is_dtest_absolute_path(const string& path) +{ + return is_absolute_path(path); +} + string resolve_dtest_input_file( const string& arg, diff --git a/library/tests/args_test.cpp b/library/tests/args_test.cpp index 987a6e80..9451aa28 100644 --- a/library/tests/args_test.cpp +++ b/library/tests/args_test.cpp @@ -300,7 +300,11 @@ TEST_F(HandsLayoutFixture, ResolvePathLikeArgDoesNotUseListShorthand) std::ofstream out(trap); out << "trap\n"; } - std::filesystem::remove(root_ + "hands/list42.txt"); + { + std::error_code ec; + std::filesystem::remove(root_ + "hands/list42.txt", ec); + ASSERT_FALSE(ec) << ec.message(); + } const std::string runfiles = std::string(::testing::TempDir()) + "dtest_hands_runfiles_no_shorthand/";