Skip to content

[NFC] Eliminate duplicated create_dirs #41691

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

[NFC] Eliminate duplicated create_dirs #41691

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions stdlib/cmake/modules/SwiftSource.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,13 +844,17 @@ function(_compile_swift_files
endif()
endif()

# First generate the obj dirs
# First generate the obj dirs if not created before
list(REMOVE_DUPLICATES dirs_to_create)
add_custom_command_target(
create_dirs_dependency_target
COMMAND "${CMAKE_COMMAND}" -E make_directory ${dirs_to_create}
OUTPUT ${dirs_to_create}
COMMENT "Generating dirs for ${first_output}")
list(REMOVE_ITEM dirs_to_create ${CREATED_OBJ_DIRS})
if(NOT dirs_to_create STREQUAL "")
add_custom_command_target(
create_dirs_dependency_target
COMMAND "${CMAKE_COMMAND}" -E make_directory ${dirs_to_create}
OUTPUT ${dirs_to_create}
COMMENT "Generating dirs for ${first_output}")
set(CREATED_OBJ_DIRS ${CREATED_OBJ_DIRS} ${dirs_to_create} CACHE INTERNAL "Already created obj directories")
endif()
Comment on lines +849 to +857
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be my untested proposal that reuses previously already created targets, instead of relying on a cache variable (that would make us lose the dependency for the subsequent targets)

Suggested change
list(REMOVE_ITEM dirs_to_create ${CREATED_OBJ_DIRS})
if(NOT dirs_to_create STREQUAL "")
add_custom_command_target(
create_dirs_dependency_target
COMMAND "${CMAKE_COMMAND}" -E make_directory ${dirs_to_create}
OUTPUT ${dirs_to_create}
COMMENT "Generating dirs for ${first_output}")
set(CREATED_OBJ_DIRS ${CREATED_OBJ_DIRS} ${dirs_to_create} CACHE INTERNAL "Already created obj directories")
endif()
list(APPEND create_dirs_dependency_targets)
foreach(current_dir_to_create IN LISTS dirs_to_create)
add_custom_command_target(
create_dirs_dependency_target
IDEMPOTENT
CUSTOM_TARGET_NAME ${current_dir_to_create}
COMMAND "${CMAKE_COMMAND}" -E make_directory ${current_dir_to_create}
OUTPUT ${current_dir_to_create}
COMMENT "Generating dirs for ${first_output}")
list(APPEND create_dirs_dependency_targets "$create_dirs_dependency_target")
endforeach()
# use create_dirs_dependency_targets below instead of create_dirs_dependency_target

This likely could be simplified more (e.g. from the build ninja it seems only one directory is created), erring on the side of caution without doing builds/PR testing.

Another solution would be to alter the temporary path name to account for the module name, but that seems a slightly more invasive solution.


# Then we can compile both the object files and the swiftmodule files
# in parallel in this target for the object file, and ...
Expand Down