Skip to content

Commit

Permalink
CMake: fix custom target dependencies
Browse files Browse the repository at this point in the history
This fixes dependency issues where a dependency of a custom command
on a custom target guarantees only the order of building and does
not guarantee that the custom command would rerun every time
when the custom target is rebuilt.

The approach to fix this is for custom targets to record in the
OUTPUTS target property their output files. Then the dependent
custom command depends on the contents of the OUTPUTS property
using generator expressions. This creates a file level dependencies
that triggers rebuilds on change.
  • Loading branch information
sogartar committed May 6, 2022
1 parent ac34498 commit 9f5fd38
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 11 deletions.
17 changes: 11 additions & 6 deletions build_tools/cmake/cg_filegroup.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,19 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

include_guard(GLOBAL)
include(cg_target_outputs)

function(cg_filegroup)
cmake_parse_arguments(_ARG "PUBLIC" "NAME" "FILES;DEPENDS" ${ARGN})
rename_bazel_targets(_NAME "${_ARG_NAME}")
add_custom_target(${_NAME})

rename_bazel_targets(_DEPS "${_ARG_DEPENDS}")
cg_target_outputs(TARGETS ${_DEPS} RESULT _DEPS_OUTPUTS)

unset(_OUTPUTS)

foreach(FILE_ ${_ARG_FILES})
if(IS_ABSOLUTE "${FILE_}")
set(_INPUT_PATH "${FILE_}")
Expand All @@ -29,22 +37,19 @@ function(cg_filegroup)
COMMAND
${CMAKE_COMMAND} -E create_symlink "${_INPUT_PATH}"
"${_OUTPUT_PATH}"
DEPENDS "${_INPUT_PATH}"
DEPENDS "${_INPUT_PATH}" ${_DEPS_OUTPUTS}
)
endif()
add_custom_target(${_TARGET} DEPENDS "${_OUTPUT_PATH}")
endif()
list(APPEND _OUTPUTS "${_OUTPUT_PATH}")

add_dependencies(${_NAME} ${_TARGET})
endforeach()

if(_ARG_DEPENDS)
rename_bazel_targets(_DEPS "${_ARG_DEPENDS}")
add_dependencies(${_NAME} ${_DEPS})
endif()

set_target_properties(
${_NAME}
PROPERTIES IS_FILEGROUP TRUE OUTPUTS "${_SRCS}"
)
set_property(TARGET ${_NAME} PROPERTY OUTPUTS ${_OUTPUTS})
endfunction()
6 changes: 4 additions & 2 deletions build_tools/cmake/cg_genrule.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ include_guard(GLOBAL)

include(CMakeParseArguments)
include(cg_macros)
include(cg_target_outputs)

# cg_genrule()
#
Expand Down Expand Up @@ -55,10 +56,11 @@ function(cg_genrule)
string(REPLACE "$(@D)" "${_OUTS_DIR}" _CMD "${_CMD}")

if(_OUTS)
cg_target_outputs(TARGETS ${_DEPS} RESULT _DEPS_OUTPUTS)
add_custom_command(
OUTPUT ${_OUTS}
COMMAND bash -c "${_CMD}"
DEPENDS ${_DEPS} ${_SRCS}
DEPENDS ${_DEPS} ${_DEPS_OUTPUTS} ${_SRCS}
VERBATIM
USES_TERMINAL
)
Expand All @@ -83,7 +85,7 @@ function(cg_genrule)
)
endif()

set_target_properties(${_NAME} PROPERTIES OUTPUTS "${_OUTS}")
set_property(TARGET ${_NAME} PROPERTY OUTPUTS ${_OUTS})

list(LENGTH _OUTS _OUTS_LENGTH)
if(_OUTS_LENGTH EQUAL "1")
Expand Down
11 changes: 8 additions & 3 deletions build_tools/cmake/cg_py_library.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function(cg_py_library)

# TODO(boian): remove this renaming when call sites do not include ":" in target dependency names
rename_bazel_targets(_RULE_DEPS "${_RULE_DEPS}")
cg_target_outputs(TARGETS ${_RULE_DEPS} RESULT _DEPS_OUTPUTS)

# Prefix the library with the package name, so we get: cg_package_name.
rename_bazel_targets(_NAME "${_RULE_NAME}")
Expand All @@ -64,19 +65,23 @@ function(cg_py_library)
COMMAND
${CMAKE_COMMAND} -E create_symlink
"${CMAKE_CURRENT_SOURCE_DIR}/${_SRC_FILE}" "${_SRC_BIN_PATH}"
DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/${_SRC_FILE}"
DEPENDS
"${CMAKE_CURRENT_SOURCE_DIR}/${_SRC_FILE}"
${_RULE_DEPS}
${_DEPS_OUTPUTS}
${_RULE_GENERATED_SRCS}
VERBATIM
)
list(APPEND _BIN_PATHS "${_SRC_BIN_PATH}")
endforeach()

list(APPEND _BIN_PATHS ${_RULE_GENERATED_SRCS})

set(_DEPS ${_RULE_DEPS} ${_BIN_PATHS})
add_custom_target(${_NAME} ALL DEPENDS ${_DEPS})

cg_add_data_dependencies(NAME ${_RULE_NAME} DATA ${_RULE_DATA})

set_property(TARGET ${_NAME} PROPERTY OUTPUTS ${_BIN_PATHS})

# If only one src file set the LOCATION target property to point to it.
list(LENGTH _BIN_PATHS _BIN_PATHS_LENGTH)
if(_BIN_PATHS_LENGTH EQUAL "1")
Expand Down
16 changes: 16 additions & 0 deletions build_tools/cmake/cg_target_outputs.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

include_guard(GLOBAL)
include(CMakeParseArguments)

function(cg_target_outputs)
cmake_parse_arguments(ARG "" "RESULT" "TARGETS" ${ARGN})
unset(RES_)
foreach(TARGET_ ${ARG_TARGETS})
list(APPEND RES_ $<TARGET_PROPERTY:${TARGET_},OUTPUTS>)
endforeach()
set("${ARG_RESULT}" ${RES_} PARENT_SCOPE)
endfunction()
5 changes: 5 additions & 0 deletions build_tools/cmake/grpc.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ include_guard(GLOBAL)
include(CMakeParseArguments)
include(cg_macros)
include(cg_py_library)
include(cg_target_outputs)
include(protobuf)

function(get_cc_grpc_proto_out_files _PROTO_FILENAME _RESULT)
Expand Down Expand Up @@ -43,6 +44,7 @@ function(cc_grpc_library)
endif()

rename_bazel_targets(_DEPS "${_RULE_DEPS}")
cg_target_outputs(TARGETS ${_DEPS} RESULT _DEPS_OUTPUTS)
rename_bazel_targets(_NAME "${_RULE_NAME}")
rename_bazel_targets(_SRCS "${_RULE_SRCS}")

Expand Down Expand Up @@ -84,6 +86,7 @@ function(cc_grpc_library)
"${_DESCRIPTOR_SET_FILE}"
"${_PROTO_FILE}"
${_DEPS}
${_DEPS_OUTPUTS}
VERBATIM
)

Expand Down Expand Up @@ -119,6 +122,7 @@ function(py_grpc_library)
cmake_parse_arguments(_RULE "" "NAME;SRCS" "DEPS" ${ARGN})

rename_bazel_targets(_DEPS "${_RULE_DEPS}")
cg_target_outputs(TARGETS ${_DEPS} RESULT _DEPS_OUTPUTS)
rename_bazel_targets(_SRCS "${_RULE_SRCS}")

get_target_property(_DESCRIPTOR_SET_FILE ${_SRCS} PROTO_DESCRIPTOR_SETS)
Expand Down Expand Up @@ -150,6 +154,7 @@ function(py_grpc_library)
"${_DESCRIPTOR_SET_FILE}"
"${_PROTO_FILE}"
${_DEPS}
${_DEPS_OUTPUTS}
VERBATIM
)

Expand Down

0 comments on commit 9f5fd38

Please sign in to comment.