Skip to content

[libc++] Rename %{module} substitution to %{modules-dir} #78310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ldionne opened this issue Jan 16, 2024 · 0 comments · Fixed by #80471
Closed

[libc++] Rename %{module} substitution to %{modules-dir} #78310

ldionne opened this issue Jan 16, 2024 · 0 comments · Fixed by #80471
Assignees
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite

Comments

@ldionne
Copy link
Member

ldionne commented Jan 16, 2024

We should rename %{module} to %{modules-dir}. In fact the other variables might need a renaming too:

diff --git a/libcxx/test/configs/cmake-bridge.cfg.in b/libcxx/test/configs/cmake-bridge.cfg.in
index 0e3c3040c964..93a7a9bb8129 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -25,12 +25,12 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
 # Add substitutions for bootstrapping the test suite configuration
 import shlex
 config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
-config.substitutions.append(('%{libcxx}', '@LIBCXX_SOURCE_DIR@'))
-config.substitutions.append(('%{include}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
-config.substitutions.append(('%{target-include}', '@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
-config.substitutions.append(('%{lib}', '@LIBCXX_LIBRARY_DIR@'))
-config.substitutions.append(('%{module}', '@LIBCXX_GENERATED_MODULE_DIR@'))
-config.substitutions.append(('%{test-tools}', '@LIBCXX_TEST_TOOLS_PATH@'))
+config.substitutions.append(('%{libcxx-dir}', '@LIBCXX_SOURCE_DIR@'))
+config.substitutions.append(('%{include-dir}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
+config.substitutions.append(('%{target-include-dir}', '@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
+config.substitutions.append(('%{lib-dir}', '@LIBCXX_LIBRARY_DIR@'))
+config.substitutions.append(('%{modules-dir}', '@LIBCXX_GENERATED_MODULE_DIR@'))
+config.substitutions.append(('%{test-tools-dir}', '@LIBCXX_TEST_TOOLS_PATH@'))
 
 # The test needs to manually rebuild the module. The compiler flags used in the
 # test need to be the same as the compiler flags used to generate the module.

This would make this a lot easier to read IMO.

Originally posted by @ldionne in #76246 (comment)

@ldionne ldionne added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 16, 2024
mordante added a commit to mordante/llvm-project that referenced this issue Feb 2, 2024
Using the `-dir` suffix for directories makes it easier to understand.

Fixes: llvm#78310
mordante added a commit to mordante/llvm-project that referenced this issue Feb 4, 2024
Using the `-dir` suffix for directories makes it easier to understand.

Fixes: llvm#78310
mordante added a commit to mordante/llvm-project that referenced this issue Feb 4, 2024
Using the `-dir` suffix for directories makes it easier to understand.

Fixes: llvm#78310
mordante added a commit that referenced this issue Feb 9, 2024
Using the `-dir` suffix for directories makes it easier to understand.

Fixes: #78310
blueboxd pushed a commit to blueboxd/libcxx that referenced this issue May 4, 2024
Using the `-dir` suffix for directories makes it easier to understand.

Fixes: llvm/llvm-project#78310
NOKEYCHECK=True
GitOrigin-RevId: 6d1396148977ca275df243a965ac504448bf5faa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants