Ninja: Allow compilation before generation of dependencies' private sources

This requires knowing when a generated header is public, which we can
model using file sets.  Add policy CMP0154 to treat generated sources
as private by default in targets with file sets.  Generated public
headers can be specified in public file sets.

Fixes: #24959
Issue: #15555
This commit is contained in:
Martin Duffy
2023-09-14 13:13:38 -04:00
committed by Brad King
parent 123cdf9816
commit ec2ba29ac5
33 changed files with 320 additions and 15 deletions

View File

@@ -87,6 +87,11 @@ The options are:
:ref:`Target-dependent expressions <Target-Dependent Queries>` are not
permitted.
.. versionchanged:: 3.28
In targets using :ref:`file sets`, custom command byproducts are now
considered private unless they are listed in a non-private file set.
See policy :policy:`CMP0154`.
``COMMAND``
Specify the command-line(s) to execute at build time.
If more than one ``COMMAND`` is specified they will be executed in order,
@@ -270,6 +275,11 @@ The options are:
:ref:`Target-dependent expressions <Target-Dependent Queries>` are not
permitted.
.. versionchanged:: 3.28
In targets using :ref:`file sets`, custom command outputs are now
considered private unless they are listed in a non-private file set.
See policy :policy:`CMP0154`.
``USES_TERMINAL``
.. versionadded:: 3.2

View File

@@ -63,6 +63,11 @@ The options are:
:ref:`Target-dependent expressions <Target-Dependent Queries>` are not
permitted.
.. versionchanged:: 3.28
In custom targets using :ref:`file sets`, byproducts are now
considered private unless they are listed in a non-private file set.
See policy :policy:`CMP0154`.
``COMMAND``
Specify the command-line(s) to execute at build time.
If more than one ``COMMAND`` is specified they will be executed in order,

View File

@@ -60,6 +60,8 @@ expressions to ensure the sources are correctly assigned to the target.
See the :manual:`cmake-buildsystem(7)` manual for more on defining
buildsystem properties.
.. _`File Sets`:
File Sets
^^^^^^^^^

View File

@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.28
.. toctree::
:maxdepth: 1
CMP0154: Generated files are private by default in targets using file sets. </policy/CMP0154>
CMP0153: The exec_program command should not be called. </policy/CMP0153>
CMP0152: file(REAL_PATH) resolves symlinks before collapsing ../ components. </policy/CMP0152>

53
Help/policy/CMP0154.rst Normal file
View File

@@ -0,0 +1,53 @@
CMP0154
-------
.. versionadded:: 3.28
Generated files are private by default in targets using :ref:`file sets`.
CMake 3.27 and below assume that any file generated as an output or byproduct
of :command:`add_custom_command` or :command:`add_custom_target` may be a
public header file meant for inclusion by dependents' source files. This
requires :ref:`Ninja Generators` to add conservative order-only dependencies
that prevent a target's source files from compiling before custom commands
from the target's dependencies are finished, even if those custom commands
only produce sources private to their own target.
:ref:`File Sets`, introduced by CMake 3.23, provide a way to express the
visibility of generated header files. CMake 3.28 and above prefer to
assume that, in targets using file sets, generated files are private to
their own target by default. Generated public headers must be specified
as members of a ``PUBLIC`` (or ``INTERFACE``) ``FILE_SET``, typically of
type ``HEADERS``. With this information, :ref:`Ninja Generators` may omit
the above-mentioned conservative dependencies and produce more efficient
build graphs.
This policy provides compatibility for projects using file sets in targets
with generated header files that have not been updated. Such projects
should be updated to express generated public headers in a file set.
For example:
.. code-block:: cmake
add_custom_command(
OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/foo.h
...
)
target_sources(foo
PUBLIC FILE_SET HEADERS
BASE_DIRS ${CMAKE_CURRENT_BINARY_DIR}
FILES ${CMAKE_CURRENT_BINARY_DIR}/foo.h
)
The ``OLD`` behavior for this policy is to assume generated files are
public, even in targets using file sets, and for :ref:`Ninja Generators`
to produce conservative build graphs. The ``NEW`` behavior for this
policy is to assume generated files are private in targets using file sets,
and for :ref:`Ninja Generators` to produce more efficient build graphs.
This policy was introduced in CMake version 3.28. Use the
:command:`cmake_policy` command to set it to ``OLD`` or ``NEW`` explicitly.
Unlike many policies, CMake version |release| does *not* warn
when this policy is not set and simply uses ``OLD`` behavior.
.. include:: DEPRECATED.txt

View File

@@ -0,0 +1,7 @@
fileset-private-dep
-------------------
* Generated files, in targets using :ref:`file sets`, are now considered
private by default. Generated public headers must be specified using
file sets. This allows :ref:`Ninja Generators` to produce more
efficient build graphs. See policy :policy:`CMP0154`.

View File

@@ -1278,6 +1278,13 @@ std::string cmGlobalNinjaGenerator::OrderDependsTargetForTarget(
return cmStrCat("cmake_object_order_depends_target_", target->GetName());
}
std::string cmGlobalNinjaGenerator::OrderDependsTargetForTargetPrivate(
cmGeneratorTarget const* target, const std::string& config) const
{
return cmStrCat(this->OrderDependsTargetForTarget(target, config),
"_private");
}
void cmGlobalNinjaGenerator::AppendTargetOutputs(
cmGeneratorTarget const* target, cmNinjaDeps& outputs,
const std::string& config, cmNinjaTargetDepends depends) const
@@ -3176,3 +3183,10 @@ std::string cmGlobalNinjaMultiGenerator::OrderDependsTargetForTarget(
return cmStrCat("cmake_object_order_depends_target_", target->GetName(), '_',
cmSystemTools::UpperCase(config));
}
std::string cmGlobalNinjaMultiGenerator::OrderDependsTargetForTargetPrivate(
cmGeneratorTarget const* target, const std::string& config) const
{
return cmStrCat(this->OrderDependsTargetForTarget(target, config),
"_private");
}

View File

@@ -349,6 +349,9 @@ public:
virtual std::string OrderDependsTargetForTarget(
cmGeneratorTarget const* target, const std::string& config) const;
virtual std::string OrderDependsTargetForTargetPrivate(
cmGeneratorTarget const* target, const std::string& config) const;
void AppendTargetOutputs(cmGeneratorTarget const* target,
cmNinjaDeps& outputs, const std::string& config,
cmNinjaTargetDepends depends) const;
@@ -738,6 +741,9 @@ public:
std::string OrderDependsTargetForTarget(
cmGeneratorTarget const* target, const std::string& config) const override;
std::string OrderDependsTargetForTargetPrivate(
cmGeneratorTarget const* target, const std::string& config) const override;
protected:
bool OpenBuildFileStreams() override;
void CloseBuildFileStreams() override;

View File

@@ -38,6 +38,7 @@
#include "cmNinjaNormalTargetGenerator.h"
#include "cmNinjaUtilityTargetGenerator.h"
#include "cmOutputConverter.h"
#include "cmPolicies.h"
#include "cmRange.h"
#include "cmRulePlaceholderExpander.h"
#include "cmSourceFile.h"
@@ -169,6 +170,13 @@ std::string cmNinjaTargetGenerator::OrderDependsTargetForTarget(
this->GeneratorTarget, config);
}
std::string cmNinjaTargetGenerator::OrderDependsTargetForTargetPrivate(
const std::string& config)
{
return this->GetGlobalGenerator()->OrderDependsTargetForTargetPrivate(
this->GeneratorTarget, config);
}
// TODO: Most of the code is picked up from
// void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink),
// void cmMakefileTargetGenerator::WriteTargetLanguageFlags()
@@ -1020,6 +1028,45 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements(
cmStrCat("Order-only phony target for ", this->GetTargetName());
build.Outputs.push_back(this->OrderDependsTargetForTarget(config));
// Gather order-only dependencies on custom command outputs.
std::vector<std::string> ccouts;
std::vector<std::string> ccouts_private;
bool usePrivateGeneratedSources = false;
if (this->GeneratorTarget->Target->HasFileSets()) {
switch (this->GetGeneratorTarget()->GetPolicyStatusCMP0154()) {
case cmPolicies::WARN:
case cmPolicies::OLD:
break;
case cmPolicies::REQUIRED_ALWAYS:
case cmPolicies::REQUIRED_IF_USED:
case cmPolicies::NEW:
usePrivateGeneratedSources = true;
break;
}
}
for (cmCustomCommand const* cc : customCommands) {
cmCustomCommandGenerator ccg(*cc, config, this->GetLocalGenerator());
const std::vector<std::string>& ccoutputs = ccg.GetOutputs();
const std::vector<std::string>& ccbyproducts = ccg.GetByproducts();
ccouts.insert(ccouts.end(), ccoutputs.begin(), ccoutputs.end());
ccouts.insert(ccouts.end(), ccbyproducts.begin(), ccbyproducts.end());
if (usePrivateGeneratedSources) {
auto it = ccouts.begin();
while (it != ccouts.end()) {
cmFileSet const* fileset =
this->GeneratorTarget->GetFileSetForSource(
config, this->Makefile->GetOrCreateGeneratedSource(*it));
if (fileset &&
fileset->GetVisibility() != cmFileSetVisibility::Private) {
++it;
} else {
ccouts_private.push_back(*it);
it = ccouts.erase(it);
}
}
}
}
cmNinjaDeps& orderOnlyDeps = build.OrderOnlyDeps;
this->GetLocalGenerator()->AppendTargetDepends(
this->GeneratorTarget, orderOnlyDeps, config, fileConfig,
@@ -1029,17 +1076,8 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements(
cm::append(orderOnlyDeps, this->Configs[config].ExtraFiles);
// Add order-only dependencies on custom command outputs.
for (cmCustomCommand const* cc : customCommands) {
cmCustomCommandGenerator ccg(*cc, config, this->GetLocalGenerator());
const std::vector<std::string>& ccoutputs = ccg.GetOutputs();
const std::vector<std::string>& ccbyproducts = ccg.GetByproducts();
std::transform(ccoutputs.begin(), ccoutputs.end(),
std::back_inserter(orderOnlyDeps),
this->MapToNinjaPath());
std::transform(ccbyproducts.begin(), ccbyproducts.end(),
std::back_inserter(orderOnlyDeps),
this->MapToNinjaPath());
}
std::transform(ccouts.begin(), ccouts.end(),
std::back_inserter(orderOnlyDeps), this->MapToNinjaPath());
std::sort(orderOnlyDeps.begin(), orderOnlyDeps.end());
orderOnlyDeps.erase(
@@ -1060,8 +1098,32 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements(
this->GetGlobalGenerator()->WriteBuild(this->GetImplFileStream(fileConfig),
build);
}
// Add order-only dependencies on custom command outputs that are
// private to this target.
this->HasPrivateGeneratedSources = !ccouts_private.empty();
if (this->HasPrivateGeneratedSources) {
cmNinjaBuild buildPrivate("phony");
cmNinjaDeps& orderOnlyDepsPrivate = buildPrivate.OrderOnlyDeps;
orderOnlyDepsPrivate.push_back(
this->OrderDependsTargetForTarget(config));
buildPrivate.Outputs.push_back(
this->OrderDependsTargetForTargetPrivate(config));
std::transform(ccouts_private.begin(), ccouts_private.end(),
std::back_inserter(orderOnlyDepsPrivate),
this->MapToNinjaPath());
std::sort(orderOnlyDepsPrivate.begin(), orderOnlyDepsPrivate.end());
orderOnlyDepsPrivate.erase(
std::unique(orderOnlyDepsPrivate.begin(), orderOnlyDepsPrivate.end()),
orderOnlyDepsPrivate.end());
this->GetGlobalGenerator()->WriteBuild(
this->GetImplFileStream(fileConfig), buildPrivate);
}
}
{
std::vector<cmSourceFile const*> objectSources;
this->GeneratorTarget->GetObjectSources(objectSources, config);
@@ -1388,7 +1450,13 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement(
this->MapToNinjaPath());
}
objBuild.OrderOnlyDeps.push_back(this->OrderDependsTargetForTarget(config));
if (this->HasPrivateGeneratedSources) {
objBuild.OrderOnlyDeps.push_back(
this->OrderDependsTargetForTargetPrivate(config));
} else {
objBuild.OrderOnlyDeps.push_back(
this->OrderDependsTargetForTarget(config));
}
// If the source file is GENERATED and does not have a custom command
// (either attached to this source file or another one), assume that one of

View File

@@ -81,6 +81,7 @@ protected:
bool CompileWithDefines(std::string const& lang) const;
std::string OrderDependsTargetForTarget(const std::string& config);
std::string OrderDependsTargetForTargetPrivate(const std::string& config);
std::string ComputeOrderDependsForTarget();
@@ -228,6 +229,7 @@ protected:
private:
cmLocalNinjaGenerator* LocalGenerator;
bool HasPrivateGeneratedSources = false;
struct ScanningFiles
{

View File

@@ -465,7 +465,11 @@ class cmMakefile;
"file(REAL_PATH) resolves symlinks before collapsing ../ components.", 3, \
28, 0, cmPolicies::WARN) \
SELECT(POLICY, CMP0153, "The exec_program command should not be called.", \
3, 28, 0, cmPolicies::WARN)
3, 28, 0, cmPolicies::WARN) \
SELECT( \
POLICY, CMP0154, \
"Generated files are private by default in targets using file sets.", 3, \
28, 0, cmPolicies::WARN)
#define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
#define CM_FOR_EACH_POLICY_ID(POLICY) \
@@ -503,7 +507,8 @@ class cmMakefile;
F(CMP0113) \
F(CMP0119) \
F(CMP0131) \
F(CMP0142)
F(CMP0142) \
F(CMP0154)
#define CM_FOR_EACH_CUSTOM_COMMAND_POLICY(F) \
F(CMP0116) \

View File

@@ -3201,6 +3201,11 @@ std::vector<std::string> cmTarget::GetAllInterfaceFileSets() const
return result;
}
bool cmTarget::HasFileSets() const
{
return !this->impl->FileSets.empty();
}
bool cmTargetInternals::CheckImportedLibName(std::string const& prop,
std::string const& value) const
{

View File

@@ -321,6 +321,8 @@ public:
static std::string GetFileSetsPropertyName(const std::string& type);
static std::string GetInterfaceFileSetsPropertyName(const std::string& type);
bool HasFileSets() const;
private:
template <typename ValueType>
void StoreProperty(const std::string& prop, ValueType value);

View File

@@ -242,6 +242,9 @@ if(CMAKE_GENERATOR MATCHES "Ninja")
add_RunCMake_test(NinjaMultiConfig)
set_property(TEST RunCMake.NinjaMultiConfig APPEND
PROPERTY LABELS "CUDA")
add_RunCMake_test(NinjaPrivateDeps
-DCMAKE_C_OUTPUT_EXTENSION=${CMAKE_C_OUTPUT_EXTENSION}
-DRunCMake_GENERATOR_IS_MULTI_CONFIG=${_isMultiConfig})
endif()
add_RunCMake_test(CTest)

View File

@@ -0,0 +1,3 @@
if (EXISTS ${RunCMake_TEST_BINARY_DIR}/empty.cpp)
set(RunCMake_TEST_FAILED "Compiled source generated before compilation of consumers.")
endif()

View File

@@ -0,0 +1,3 @@
if (NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/empty.cpp)
set(RunCMake_TEST_FAILED "Compiled source did not generate.")
endif()

View File

@@ -0,0 +1,3 @@
if (NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/none.cpp)
set(RunCMake_TEST_FAILED "Private source dependency used for target without filesets.")
endif()

View File

@@ -0,0 +1,3 @@
if (EXISTS ${RunCMake_TEST_BINARY_DIR}/private.h)
set(RunCMake_TEST_FAILED "Private header generated before compilation.")
endif()

View File

@@ -0,0 +1,3 @@
if (NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/private.h)
set(RunCMake_TEST_FAILED "Private header generated before compilation.")
endif()

View File

@@ -0,0 +1,3 @@
if (NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/public.h)
set(RunCMake_TEST_FAILED "Public header did not generate before compilation.")
endif()

View File

@@ -0,0 +1,2 @@
cmake_policy(SET CMP0154 NEW)
include(CMP0154-common.cmake)

View File

@@ -0,0 +1,3 @@
if (NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/empty.cpp)
set(RunCMake_TEST_FAILED "Policy CMP0154 set to OLD but using new behavior compiled sources.")
endif()

View File

@@ -0,0 +1,3 @@
if (NOT EXISTS ${RunCMake_TEST_BINARY_DIR}/private.h)
set(RunCMake_TEST_FAILED "Policy CMP0154 set to OLD but using new behavior private headers.")
endif()

View File

@@ -0,0 +1,2 @@
cmake_policy(SET CMP0154 OLD)
include(CMP0154-common.cmake)

View File

@@ -0,0 +1,45 @@
enable_language(CXX)
function(copy_file file dest)
add_custom_command(
OUTPUT ${CMAKE_BINARY_DIR}/${dest}
COMMAND ${CMAKE_COMMAND} -E copy
${CMAKE_SOURCE_DIR}/${file} ${CMAKE_BINARY_DIR}/${dest}
)
endfunction()
copy_file(header.h.in private.h)
copy_file(header.h.in public.h)
copy_file(source.cpp.in empty.cpp)
copy_file(source.cpp.in none.cpp)
add_library(HelloLib_PrivateFileSet STATIC hello_lib.cpp)
target_sources(HelloLib_PrivateFileSet
PRIVATE FILE_SET HEADERS
BASE_DIRS ${CMAKE_BINARY_DIR}
FILES ${CMAKE_BINARY_DIR}/private.h
)
add_library(HelloLib_PublicFileSet STATIC hello_lib.cpp)
target_sources(HelloLib_PublicFileSet
PUBLIC FILE_SET HEADERS
BASE_DIRS ${CMAKE_BINARY_DIR}
FILES ${CMAKE_BINARY_DIR}/public.h
)
add_library(HelloLib_EmptyFileSet STATIC hello_lib.cpp empty.cpp)
target_sources(HelloLib_EmptyFileSet
PUBLIC FILE_SET HEADERS
)
add_library(HelloLib_NoFileSet STATIC hello_lib.cpp none.cpp)
function(hello_executable name)
add_executable(Hello_${name} hello.cpp)
target_link_libraries(Hello_${name} PRIVATE HelloLib_${name})
endfunction()
hello_executable(PrivateFileSet)
hello_executable(PublicFileSet)
hello_executable(EmptyFileSet)
hello_executable(NoFileSet)

View File

@@ -0,0 +1,3 @@
cmake_minimum_required(VERSION 3.26)
project(${RunCMake_TEST} NONE)
include(${RunCMake_TEST}.cmake)

View File

@@ -0,0 +1,30 @@
include(RunCMake)
function(compile_source test target)
if (RunCMake_GENERATOR_IS_MULTI_CONFIG)
set(config "Debug/")
endif()
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build)
set(RunCMake_TEST_NO_CLEAN 1)
run_cmake_command(${test}-Build-${target}-Source ${CMAKE_COMMAND} --build .
--target CMakeFiles/Hello_${target}.dir/${config}hello.cpp${CMAKE_C_OUTPUT_EXTENSION})
endfunction()
function(compile_target test target)
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build)
set(RunCMake_TEST_NO_CLEAN 1)
run_cmake_command(${test}-Build-${target} ${CMAKE_COMMAND} --build .
--target Hello_${target})
endfunction()
run_cmake(CMP0154-OLD)
compile_source(CMP0154-OLD PrivateFileSet)
compile_source(CMP0154-OLD EmptyFileSet)
run_cmake(CMP0154-NEW)
compile_source(CMP0154-NEW PrivateFileSet)
compile_target(CMP0154-NEW PrivateFileSet)
compile_source(CMP0154-NEW PublicFileSet)
compile_source(CMP0154-NEW NoFileSet)
compile_source(CMP0154-NEW EmptyFileSet)
compile_target(CMP0154-NEW EmptyFileSet)

View File

@@ -0,0 +1,6 @@
#include "hello_lib.h"
int main()
{
say_hello();
}

View File

@@ -0,0 +1,8 @@
#include "hello_lib.h"
#include <iostream>
void say_hello()
{
std::cout << "hello" << std::endl;
}

View File

@@ -0,0 +1 @@
void say_hello();

View File

@@ -37,6 +37,7 @@
\* CMP0119
\* CMP0131
\* CMP0142
\* CMP0154
Call Stack \(most recent call first\):
CMakeLists.txt:3 \(include\)