Skip to content
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

[libc] Specify path for making include/ subdirs #66589

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Conversation

kaladron
Copy link
Contributor

When doing a clean build from vscode, it makes the subdirectories in the source tree rather than in the build folder. Elsehwere in LLVM, they prefix the MAKE_DIRECTORY calls, so this appears to be the correct approach.

@llvmbot llvmbot added the libc label Sep 17, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2023

@llvm/pr-subscribers-libc

Changes

When doing a clean build from vscode, it makes the subdirectories in the source tree rather than in the build folder. Elsehwere in LLVM, they prefix the MAKE_DIRECTORY calls, so this appears to be the correct approach.

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

1 Files Affected:

  • (modified) libc/include/CMakeLists.txt (+3-3)
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 2e57275e10bc4cf..06a342d28221561 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -78,7 +78,7 @@ add_gen_header(
 )
 
 # TODO: This should be conditional on POSIX networking being included.
-file(MAKE_DIRECTORY "arpa")
+file(MAKE_DIRECTORY ${LIBC_INCLUDE_DIR}/"arpa")
 
 add_gen_header(
   arpa_inet
@@ -280,7 +280,7 @@ add_gen_header(
 # TODO: Not all platforms will have a include/sys directory. Add the sys
 # directory and the targets for sys/*.h files conditional to the OS requiring
 # them.
-file(MAKE_DIRECTORY "sys")
+file(MAKE_DIRECTORY ${LIBC_INCLUDE_DIR}/sys)
 
 add_gen_header(
   sys_auxv
@@ -497,7 +497,7 @@ add_gen_header(
 )
 
 if(LIBC_TARGET_ARCHITECTURE_IS_GPU)
-  file(MAKE_DIRECTORY "gpu")
+  file(MAKE_DIRECTORY ${LIBC_INCLUDE_DIR}/"gpu")
 
   add_gen_header(
     gpu_rpc

When doing a clean build from vscode, it makes the subdirectories in the
source tree rather than in the build folder.  Elsehwere in LLVM, they
prefix the MAKE_DIRECTORY calls, so this appears to be the correct
approach.

Tested:
Build within vscode
Commandline build with:
cmake ../llvm -DLLVM_ENABLE_PROJECTS="libc" -DCMAKE_BUILD_TYPE=Debug \
-DLLVM_LIBC_FULL_BUILD=true -DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++
cmke --build . --target libc

(Note that this is Makefile build, not ninja)
@@ -78,7 +78,7 @@ add_gen_header(
)

# TODO: This should be conditional on POSIX networking being included.
file(MAKE_DIRECTORY "arpa")
file(MAKE_DIRECTORY ${LIBC_INCLUDE_DIR}/arpa)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is CMAKE_CURRENT_BINARY_DIR more appropriate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. In the libc/CMakeLists.txt there's LIBC_INCLUDE_DIR is redefined a few times, one of which is:

elseif(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND LIBC_ENABLE_USE_BY_CLANG)
set(LIBC_INCLUDE_DIR ${LLVM_BINARY_DIR}/include/${LLVM_DEFAULT_TARGET_TRIPLE})

Whereas LIBC_INCLUDE_DIR is implemented in terms of it:

set(LIBC_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)

Nothing else in this file refers to the CMAKE_CURRENT_BINARY_DIR, but there is other references to LIBC_INCLUDE_DIR:

target_include_directories(libc-headers SYSTEM INTERFACE ${LIBC_INCLUDE_DIR})

I'm not a CMake expert, though. I'm happy to adjust if that's better!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. ${LIBC_INCLUDE_PATH} is the correct path.

@kaladron kaladron merged commit acfb99d into llvm:main Sep 19, 2023
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
When doing a clean build from vscode, it makes the subdirectories in the
source tree rather than in the build folder. Elsehwere in LLVM, they
prefix the MAKE_DIRECTORY calls, so this appears to be the correct
approach.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants