[lldb] Remove .noindex suffix from test output directory for non-Darwin

[speednoisemovement]

.noindex is only needed for macOS Spotlight. Given that some platforms limit path length, there's a benefit to omitting it except for Darwin.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: Leonard Grey (speednoisemovement)

Changes

.noindex doesn't prevent indexing on Windows. Given that Windows limits path length to 260 by default, there's a benefit to omitting it.

---

Full diff: https://github.com/llvm/llvm-project/pull/197237.diff

4 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/dotest_args.py (+3-1)
• (modified) lldb/test/API/sanity/TestModuleCacheSanity.py (+6-1)
• (modified) lldb/test/CMakeLists.txt (+6-1)
• (modified) lldb/test/Shell/Settings/TestModuleCacheSanity.test (+1-1)

diff --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index f3b0837bdc4ab..626b9b7046619 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -234,7 +234,9 @@ def create_parser():
         "--build-dir",
         dest="test_build_dir",
         metavar="Test build directory",
-        default="lldb-test-build.noindex",
+        default=(
+            "lldb-test-build" if sys.platform == "win32" else "lldb-test-build.noindex"
+        ),
         help="The root build directory for the tests. It will be removed before running.",
     )
     group.add_argument(
diff --git a/lldb/test/API/sanity/TestModuleCacheSanity.py b/lldb/test/API/sanity/TestModuleCacheSanity.py
index 1d3a6b3158898..42baf2968cfc8 100644
--- a/lldb/test/API/sanity/TestModuleCacheSanity.py
+++ b/lldb/test/API/sanity/TestModuleCacheSanity.py
@@ -4,6 +4,8 @@
 """
 
 
+import sys
+
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.lldbtest import *
@@ -13,7 +15,10 @@ class ModuleCacheSanityTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     def test(self):
+        build_dir_name = (
+            "lldb-test-build" if sys.platform == "win32" else "lldb-test-build.noindex"
+        )
         self.expect(
             "settings show symbols.clang-modules-cache-path",
-            substrs=["lldb-test-build.noindex", "module-cache-lldb"],
+            substrs=[build_dir_name, "module-cache-lldb"],
         )
diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 86974d86bbec6..81ae17a57ad0e 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -47,7 +47,12 @@ endif()
 # based on the UUID embedded in a binary, and because the UUID is a
 # hash of filename and .text section, there *will* be conflicts inside
 # the build directory.
-set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" CACHE PATH "The build root for building tests.")
+# Omit it for Windows since it's a no-op and exacerbates path-length issues.
+if(WIN32)
+  set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build" CACHE PATH "The build root for building tests.")
+else()
+  set(LLDB_TEST_BUILD_DIRECTORY "${PROJECT_BINARY_DIR}/lldb-test-build.noindex" CACHE PATH "The build root for building tests.")
+endif()
 
 # Configure and create module cache directories.
 set(LLDB_TEST_MODULE_CACHE_LLDB "${LLDB_TEST_BUILD_DIRECTORY}/module-cache-lldb" CACHE PATH "The Clang module cache used by the Clang embedded in LLDB while running tests.")
diff --git a/lldb/test/Shell/Settings/TestModuleCacheSanity.test b/lldb/test/Shell/Settings/TestModuleCacheSanity.test
index f16f40d47a6bf..37ce87da50e19 100644
--- a/lldb/test/Shell/Settings/TestModuleCacheSanity.test
+++ b/lldb/test/Shell/Settings/TestModuleCacheSanity.test
@@ -1,4 +1,4 @@
 # This is a sanity check that verifies that the module cache path is set
 # correctly and points inside the default test build directory.
 RUN: %lldb -o 'settings show symbols.clang-modules-cache-path' | FileCheck  %s
-CHECK: lldb-test-build.noindex{{.*}}module-cache-lldb
+CHECK: lldb-test-build{{(\.noindex)?}}{{.*}}module-cache-lldb

[adrian-prantl]

Suggested change

	"lldb-test-build" if sys.platform == "win32" else "lldb-test-build.noindex"
	"lldb-test-build" if sys.platform != "darwin" else "lldb-test-build.noindex"

[adrian-prantl]

The no index suffix really only exists for macOS Spotlight, so we could make the default on all non-Darwin platforms.

[speednoisemovement]

Updated

[JDevlieghere]

This seems like a lot of complexity for saving 8 characters. Can we save them some other way, for example by trimming lldb-test-build to test.noindex or something?

[speednoisemovement]

This seems like a lot of complexity for saving 8 characters. Can we save them some other way, for example by trimming lldb-test-build to test.noindex or something?

This was definitely motivated by saving characters, but I think it makes sense on its own since the .noindex is only meaningful on Darwin, and the complexity is limited to two ternaries (plus test changes). Probably shortening elsewhere is also good, but the only thing special about this path is the .noindex

[kastiglione]

I think it would be fine to simplify this to:

substrs=["lldb-test-build", "module-cache-lldb"]

This works whether the name is "lldb-test-build.noindex" or "lldb-test-build", and makes this file a 8 character change.

[kastiglione]

this could also be simplified

CHECK: lldb-test-build{{.*}}module-cache-lldb

[kastiglione]

@adrian-prantl @JDevlieghere What if, instead of using a .noindex suffix to solve the problem described in this comment, we instead add -random_uuid as a linker flag on macOS/darwin?

[kastiglione]

for consideration #199385