Switch test manifests to embedded C++ (#5036)

Inconsistent execution environments make using a path-as-define
difficult, so switch to an embedded file.

Also fixes the lldb launch so that passing tests run cleanly, and adds
TEST_TARGET to gdb (but without testing there). I'm dropping `sourceMap`
because it's not handled quite correctly (also not great to be trying to
pass source mappings in two different ways), and `env` didn't seem to be
working as intended either; maybe specifying `initCommands` causes other
things to not be evaluated. But the straight `initCommands` looks like
it's working. I used lldb to validate execution of these changes.

```
Running initCommands:
(lldb) command script import external/+llvm_project+llvm-project/llvm/utils/lldbDataFormatters.py
(lldb) settings set target.source-map "." "/usr/local/google/home/jperkins/dev/carbon-lang"
(lldb) settings set target.source-map "/proc/self/cwd" "/usr/local/google/home/jperkins/dev/carbon-lang"
(lldb) env TEST_TARGET=//toolchain/testing:file_test
(lldb) env TEST_TMPDIR=/tmp
Running tests with 128 thread(s)
.
Done!
Note: Google Test filter = ToolchainFileTest.toolchain/check/testdata/const/collapse.carbon
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ToolchainFileTest
[ RUN      ] ToolchainFileTest.toolchain/check/testdata/const/collapse.carbon
[       OK ] ToolchainFileTest.toolchain/check/testdata/const/collapse.carbon (0 ms)
[----------] 1 test from ToolchainFileTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1 ms total)
[  PASSED  ] 1 test.
Process 3869310 exited with status = 0 (0x00000000) 
```

.vscode/gdb_launch.json

@@ -8,7 +8,10 @@
       "program": "bazel-bin/toolchain/testing/file_test",
       "programArgs": "--file_tests=${relativeFile}",
       "cwd": "${workspaceFolder}",
-      "env": { "TEST_TMPDIR": "/tmp" }
+      "env": {
+        "TEST_TARGET": "//toolchain/testing:file_test",
+        "TEST_TMPDIR": "/tmp"
+      }
     },
     {
       "type": "by-gdb",

.vscode/lldb_launch.json

@@ -10,12 +10,11 @@
       "debuggerRoot": "${workspaceFolder}",
       "initCommands": [
         "command script import external/+llvm_project+llvm-project/llvm/utils/lldbDataFormatters.py",
-        "settings set target.source-map \".\" \"${workspaceFolder}\""
-      ],
-      "env": { "TEST_TMPDIR": "/tmp" },
-      "sourceMap": {
-        "/proc/self/cwd/": "${workspaceFolder}"
-      }
+        "settings set target.source-map \".\" \"${workspaceFolder}\"",
+        "settings set target.source-map \"/proc/self/cwd\" \"${workspaceFolder}\"",
+        "env TEST_TARGET=//toolchain/testing:file_test",
+        "env TEST_TMPDIR=/tmp"
+      ]
     },
     {
       "type": "lldb-dap",
@@ -32,7 +31,8 @@
       "debuggerRoot": "${workspaceFolder}",
       "initCommands": [
         "command script import external/+llvm_project+llvm-project/llvm/utils/lldbDataFormatters.py",
-        "settings set target.source-map \".\" \"${workspaceFolder}\""
+        "settings set target.source-map \".\" \"${workspaceFolder}\"",
+        "settings set target.source-map \"/proc/self/cwd\" \"${workspaceFolder}\""
       ]
     }
   ]

bazel/manifest/defs.bzl

@@ -4,9 +4,7 @@
 
 """Rule for producing a manifest for a filegroup."""
 
-def _manifest(ctx):
-    out = ctx.actions.declare_file(ctx.label.name)
-
+def _get_files(ctx):
     files = []
     for src in ctx.attr.srcs:
         files.extend([f.path for f in src[DefaultInfo].files.to_list()])
@@ -17,11 +15,16 @@ def _manifest(ctx):
 
     if ctx.attr.strip_package_dir:
         package_dir = ctx.label.package + "/"
-        content = [f.removeprefix(package_dir) for f in files]
+        files_stripped = [f.removeprefix(package_dir) for f in files]
     else:
-        content = files
+        files_stripped = files
 
-    ctx.actions.write(out, "\n".join(content) + "\n")
+    return files_stripped
+
+def _manifest(ctx):
+    out = ctx.actions.declare_file(ctx.label.name)
+    files = _get_files(ctx)
+    ctx.actions.write(out, "\n".join(files) + "\n")
 
     return [
         DefaultInfo(
@@ -30,6 +33,7 @@ def _manifest(ctx):
         ),
     ]
 
+# Produces the manifest as a series of lines.
 manifest = rule(
     implementation = _manifest,
     attrs = {
@@ -37,3 +41,38 @@ manifest = rule(
         "strip_package_dir": attr.bool(default = False),
     },
 )
+
+def _manifest_as_cpp(ctx):
+    out = ctx.actions.declare_file(ctx.label.name)
+    files = _get_files(ctx)
+    lines = [
+        "// Auto-generated by manifest_as_cpp.",
+        "const char* {0}[] = {{".format(ctx.attr.var_name),
+    ]
+    lines += [
+        "    \"{0}\",".format(file)
+        for file in files
+    ]
+    lines += [
+        "    nullptr,",
+        "};",
+    ]
+    ctx.actions.write(out, "\n".join(lines) + "\n")
+
+    return [
+        DefaultInfo(
+            files = depset(direct = [out]),
+            runfiles = ctx.runfiles(files = [out]),
+        ),
+    ]
+
+# Produces the manifest as a nullptr-terminated `const char* var_name[]`.
+# Use with `extern const char* var_name[];`.
+manifest_as_cpp = rule(
+    implementation = _manifest_as_cpp,
+    attrs = {
+        "srcs": attr.label_list(allow_files = True, mandatory = True),
+        "strip_package_dir": attr.bool(default = False),
+        "var_name": attr.string(mandatory = True),
+    },
+)

explorer/BUILD

@@ -58,8 +58,11 @@ cc_binary(
         ":main",
         "//common:check",
         "//common:raw_string_ostream",
+        "//testing/base:file_helpers",
         "//testing/file_test:file_test_base",
+        "//testing/file_test:manifest",
         "@abseil-cpp//absl/flags:flag",
+        "@abseil-cpp//absl/strings",
         "@re2",
     ],
 )

explorer/file_test.cpp

@@ -3,10 +3,13 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 #include "absl/flags/flag.h"
+#include "absl/strings/str_split.h"
 #include "common/raw_string_ostream.h"
 #include "explorer/main.h"
 #include "re2/re2.h"
+#include "testing/base/file_helpers.h"
 #include "testing/file_test/file_test_base.h"
+#include "testing/file_test/manifest.h"
 
 ABSL_FLAG(bool, trace, false,
           "Set to true to run tests with tracing enabled, even if they don't "
@@ -115,8 +118,13 @@ class ExplorerFileTest : public FileTestBase {
 }  // namespace
 
 // Explorer uses a non-standard approach to getting the manifest path.
-auto GetFileTestManifestPath() -> std::filesystem::path {
-  return absl::GetFlag(FLAGS_explorer_test_targets_file);
+auto GetFileTestManifest() -> llvm::SmallVector<std::string> {
+  llvm::SmallVector<std::string> manifest;
+  auto content = ReadFile(absl::GetFlag(FLAGS_explorer_test_targets_file));
+  for (const auto& line : absl::StrSplit(*content, "\n", absl::SkipEmpty())) {
+    manifest.push_back(std::string(line));
+  }
+  return manifest;
 }
 
 CARBON_FILE_TEST_FACTORY(ExplorerFileTest)

scripts/fix_cc_deps.py

@@ -68,7 +68,7 @@ EXTERNAL_REPOS: dict[str, ExternalRepo] = {
 }
 
 IGNORE_SOURCE_FILE_REGEX = re.compile(
-    "^(third_party/clangd.*|common/version.*\\.cpp)$"
+    r"^(third_party/clangd.*|common/version.*\.cpp|.*_autogen_manifest\.cpp)$"
 )
 
 

testing/file_test/BUILD

@@ -7,9 +7,6 @@ load("rules.bzl", "file_test")
 
 package(default_visibility = ["//visibility:public"])
 
-# Used to access a `#define` with the manifest file, produced by rules.bzl.
-exports_files(["file_test_manifest.cpp"])
-
 cc_library(
     name = "autoupdate",
     testonly = 1,
@@ -42,6 +39,7 @@ cc_library(
     hdrs = ["file_test_base.h"],
     deps = [
         ":autoupdate",
+        ":manifest",
         "//common:check",
         "//common:error",
         "//common:exe_path",
@@ -69,3 +67,19 @@ file_test(
         "@llvm-project//llvm:Support",
     ],
 )
+
+# Note this is separate from the implementation; see the .h file.
+cc_library(
+    name = "manifest",
+    testonly = 1,
+    hdrs = ["manifest.h"],
+    deps = ["@llvm-project//llvm:Support"],
+)
+
+# Used through `file_test` in rules.bzl.
+cc_library(
+    name = "manifest_impl",
+    testonly = 1,
+    srcs = ["manifest.cpp"],
+    deps = [":manifest"],
+)

testing/file_test/file_test_base.cpp

@@ -29,7 +29,6 @@
 #include "absl/flags/flag.h"
 #include "absl/flags/parse.h"
 #include "absl/strings/str_join.h"
-#include "absl/strings/str_split.h"
 #include "common/check.h"
 #include "common/error.h"
 #include "common/exe_path.h"
@@ -42,7 +41,6 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/ThreadPool.h"
-#include "testing/base/file_helpers.h"
 #include "testing/file_test/autoupdate.h"
 #include "testing/file_test/run_test.h"
 #include "testing/file_test/test_file.h"
@@ -298,14 +296,9 @@ static auto RegisterTests(FileTestFactory* test_factory,
                           llvm::StringRef exe_path,
                           llvm::SmallVectorImpl<FileTestInfo>& tests)
     -> ErrorOr<Success> {
-  GetFileTestManifestPath();
-  CARBON_ASSIGN_OR_RETURN(auto test_manifest,
-                          ReadFile(GetFileTestManifestPath()));
-
   // Prepare the vector first, so that the location of entries won't change.
-  for (const auto& test_name :
-       absl::StrSplit(test_manifest, "\n", absl::SkipEmpty())) {
-    tests.push_back({.test_name = std::string(test_name)});
+  for (auto& test_name : GetFileTestManifest()) {
+    tests.push_back({.test_name = test_name});
   }
 
   // Amend entries with factory functions.

testing/file_test/file_test_base.h

@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "testing/file_test/autoupdate.h"
+#include "testing/file_test/manifest.h"
 
 namespace Carbon::Testing {
 
@@ -133,11 +134,6 @@ struct FileTestFactory {
 // to implement this function.
 extern auto GetFileTestFactory() -> FileTestFactory;
 
-// Returns the manifest path, which is provided by rules.bzl and
-// file_test_manifest.cpp. This is exposed so that the explorer sharding
-// approach can use a different implementation.
-auto GetFileTestManifestPath() -> std::filesystem::path;
-
 // Provides a standard GetFileTestFactory implementation.
 #define CARBON_FILE_TEST_FACTORY(Name)                                       \
   auto GetFileTestFactory() -> FileTestFactory {                             \

testing/file_test/file_test_manifest.cpp

@@ -1,13 +0,0 @@
-// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-// Exceptions. See /LICENSE for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-#include "testing/file_test/file_test_base.h"
-
-namespace Carbon::Testing {
-
-auto GetFileTestManifestPath() -> std::filesystem::path {
-  return CARBON_FILE_TEST_MANIFEST;
-}
-
-}  // namespace Carbon::Testing

testing/file_test/manifest.cpp

@@ -0,0 +1,21 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "testing/file_test/manifest.h"
+
+// The test manifest, produced by `manifest_as_cpp`.
+// NOLINTNEXTLINE(readability-identifier-naming): Constant in practice.
+extern const char* CarbonFileTestManifest[];
+
+namespace Carbon::Testing {
+
+auto GetFileTestManifest() -> llvm::SmallVector<std::string> {
+  llvm::SmallVector<std::string> manifest;
+  for (int i = 0; CarbonFileTestManifest[i]; ++i) {
+    manifest.push_back(CarbonFileTestManifest[i]);
+  }
+  return manifest;
+}
+
+}  // namespace Carbon::Testing

testing/file_test/manifest.h

@@ -0,0 +1,21 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TESTING_FILE_TEST_MANIFEST_H_
+#define CARBON_TESTING_FILE_TEST_MANIFEST_H_
+
+#include <string>
+
+#include "llvm/ADT/SmallVector.h"
+
+namespace Carbon::Testing {
+
+// Returns the manifest path, which is provided by rules.bzl and manifest.cpp.
+// This is exposed separately so that the explorer sharding approach can use a
+// different implementation.
+auto GetFileTestManifest() -> llvm::SmallVector<std::string>;
+
+}  // namespace Carbon::Testing
+
+#endif  // CARBON_TESTING_FILE_TEST_MANIFEST_H_

testing/file_test/rules.bzl

@@ -10,12 +10,13 @@ a file which can be accessed as a list. This avoids long argument parsing.
 
 load("@rules_cc//cc:defs.bzl", "cc_test")
 load("//bazel/cc_toolchains:defs.bzl", "cc_env")
-load("//bazel/manifest:defs.bzl", "manifest")
+load("//bazel/manifest:defs.bzl", "manifest", "manifest_as_cpp")
 
 def file_test(
         name,
         tests,
         srcs = [],
+        deps = [],
         data = [],
         args = [],
         prebuilt_binary = None,
@@ -30,6 +31,7 @@ def file_test(
       name: The base name of the tests.
       tests: The list of test files to use as data, typically a glob.
       srcs: Passed to cc_test.
+      deps: Passed to cc_test.
       data: Passed to cc_test.
       args: Passed to cc_test.
       prebuilt_binary: If set, specifies a prebuilt test binary to use instead
@@ -38,34 +40,41 @@ def file_test(
     """
 
     # Ensure tests are always a filegroup for tests_as_input_file_rule.
-    tests_file = "{0}.tests".format(name)
-    manifest(
-        name = tests_file,
-        srcs = tests,
-        testonly = 1,
-    )
-    data = [":" + tests_file] + tests + data
-
     if prebuilt_binary:
         # TODO: The prebuilt_binary support is only used by explorer. We should
         # remove this once explorer is removed, and think about better factoring
         # approaches if we need it later for toolchain.
+        tests_file = "{0}.tests".format(name)
+        manifest(
+            name = tests_file,
+            srcs = tests,
+            testonly = 1,
+        )
         args = ["--explorer_test_targets_file=$(rootpath :{0})".format(tests_file)] + args
+
         native.sh_test(
             name = name,
             srcs = srcs + [prebuilt_binary],
-            data = data,
+            deps = deps,
+            data = [":" + tests_file] + tests + data,
             args = args,
             env = cc_env(),
             **kwargs
         )
     else:
+        manifest_cpp = "{0}_autogen_manifest.cpp".format(name)
+        manifest_as_cpp(
+            name = manifest_cpp,
+            var_name = "CarbonFileTestManifest",
+            srcs = tests,
+            testonly = 1,
+        )
         cc_test(
             name = name,
-            srcs = srcs + ["//testing/file_test:file_test_manifest.cpp"],
-            data = data,
+            srcs = srcs + [":" + manifest_cpp],
+            deps = deps + ["//testing/file_test:manifest_impl"],
+            data = tests + data,
             args = args,
             env = cc_env(),
-            local_defines = ["CARBON_FILE_TEST_MANIFEST='\"$(rootpath :{0})\"'".format(tests_file)],
             **kwargs
         )