Skip to content

Commit 9feac49

Browse files
mrexodiamrexodia
committed
Fix custom command review issues
1 parent 570ec4c commit 9feac49

6 files changed

Lines changed: 181 additions & 21 deletions

File tree

‎docs/cmake-toml.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,15 +300,15 @@ Use `[[target.<name>.custom-command]]` to map directly to [`add_custom_command`]
300300
# OUTPUT form (add_custom_command(OUTPUT ...))
301301
[[target.mytarget.custom-command]]
302302
condition = "mycondition"
303-
outputs = ["${CMAKE_CURRENT_BINARY_DIR}/generated.cpp"]
303+
outputs = ["generated.cpp"] # relative paths are interpreted from ${CMAKE_CURRENT_BINARY_DIR}
304304
command = [
305305
"${CMAKE_COMMAND}",
306306
"-DOUTPUT=${CMAKE_CURRENT_BINARY_DIR}/generated.cpp",
307307
"-P",
308308
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/generate.cmake",
309309
]
310310
depends = ["cmake/generate.cmake"]
311-
byproducts = ["${CMAKE_CURRENT_BINARY_DIR}/generated.hpp"]
311+
byproducts = ["generated.hpp"]
312312
main-dependency = "cmake/generate.cmake"
313313
implicit-depends = [["CXX", "src/input.idl"]]
314314
working-directory = "${CMAKE_CURRENT_BINARY_DIR}"
@@ -335,7 +335,7 @@ command-expand-lists = false
335335
uses-terminal = false
336336
```
337337

338-
For each custom command entry, exactly one of `outputs` or `build-event` is required.
338+
For each custom command entry, exactly one of `outputs` or `build-event` is required. Relative `outputs`/`byproducts` paths follow CMake and are interpreted from the current binary directory. `build-event` is only valid for non-`interface` targets, and `pre-link` is not supported for `type = "custom"`.
339339

340340
A table mapping the cmkr features to the relevant CMake construct and the relevant documentation pages:
341341

‎docs/examples/custom-command.md‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ sources = ["src/main.cpp"]
4646
include-directories = ["${CMAKE_CURRENT_BINARY_DIR}/generated"]
4747

4848
# Output form: This custom command generates source files before building.
49+
# Relative outputs follow CMake and are interpreted from the binary directory.
4950
# The outputs are automatically added as sources to the target.
5051
[[target.custom-command.custom-command]]
51-
outputs = ["${CMAKE_CURRENT_BINARY_DIR}/generated/generated.cpp"]
52-
byproducts = ["${CMAKE_CURRENT_BINARY_DIR}/generated/generated.hpp"]
52+
outputs = ["generated/generated.cpp"]
53+
byproducts = ["generated/generated.hpp"]
5354
depends = ["cmake/generate_source.cmake"]
5455
command = [
5556
"${CMAKE_COMMAND}",
@@ -92,6 +93,39 @@ command = [
9293
byproducts = ["${CMAKE_CURRENT_BINARY_DIR}/custom-target.stamp"]
9394
comment = "Run custom target"
9495
verbatim = true
96+
97+
# -----------------------------------------------------------------------------
98+
# Template-based custom target
99+
# -----------------------------------------------------------------------------
100+
101+
# Custom target options should also work when the target type comes from a template.
102+
[template.generated-custom-target]
103+
type = "custom"
104+
all = true
105+
verbatim = true
106+
107+
[target.custom-codegen-template]
108+
type = "generated-custom-target"
109+
command = [
110+
"${CMAKE_COMMAND}",
111+
"-DINPUT=${CMAKE_CURRENT_BINARY_DIR}/template-custom-target.stamp",
112+
"-P",
113+
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/verify_file_exists.cmake",
114+
]
115+
comment = "Verify the generated stamp exists"
116+
117+
# The output-form custom command should automatically become a dependency of the
118+
# custom target, so the verification command sees the stamp file.
119+
[[target.custom-codegen-template.custom-command]]
120+
outputs = ["template-custom-target.stamp"]
121+
command = [
122+
"${CMAKE_COMMAND}",
123+
"-E",
124+
"touch",
125+
"${CMAKE_CURRENT_BINARY_DIR}/template-custom-target.stamp",
126+
]
127+
comment = "Create a stamp file for the template-based custom target"
128+
verbatim = true
95129
```
96130

97131

‎src/cmake_generator.cpp‎

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,6 +1216,14 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
12161216
}
12171217
}
12181218

1219+
auto resolved_target_type = target.type;
1220+
if (tmplate != nullptr) {
1221+
if (resolved_target_type != parser::target_template) {
1222+
throw_target_error("Unreachable code, unexpected target type for template");
1223+
}
1224+
resolved_target_type = tmplate->outline.type;
1225+
}
1226+
12191227
ConditionScope cs(gen, target.condition);
12201228

12211229
// Detect if there is cmake included before/after the target
@@ -1317,6 +1325,31 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
13171325
}
13181326
custom_commands.insert(custom_commands.end(), target.custom_commands.begin(), target.custom_commands.end());
13191327

1328+
auto normalize_generated_output_source = [](const std::string &output) {
1329+
auto starts_with = [](const std::string &value, const std::string &prefix) {
1330+
return value.rfind(prefix, 0) == 0;
1331+
};
1332+
if (fs::path(output).is_absolute()) {
1333+
return output;
1334+
}
1335+
if (starts_with(output, "${CMAKE_CURRENT_BINARY_DIR}") || starts_with(output, "${CMAKE_BINARY_DIR}") ||
1336+
starts_with(output, "${PROJECT_BINARY_DIR}") || starts_with(output, "${CMAKE_CURRENT_SOURCE_DIR}") ||
1337+
starts_with(output, "${CMAKE_SOURCE_DIR}") || starts_with(output, "${PROJECT_SOURCE_DIR}") ||
1338+
starts_with(output, "${CMAKE_CURRENT_LIST_DIR}") || starts_with(output, "$ENV{") || starts_with(output, "$CACHE{") ||
1339+
starts_with(output, "${")) {
1340+
return output;
1341+
}
1342+
return "${CMAKE_CURRENT_BINARY_DIR}/" + output;
1343+
};
1344+
1345+
parser::Condition<tsl::ordered_set<std::string>> mcustom_target_depends;
1346+
if (resolved_target_type == parser::target_custom) {
1347+
auto &unconditional_depends = mcustom_target_depends[""];
1348+
for (const auto &dep : custom_target.depends) {
1349+
unconditional_depends.insert(dep);
1350+
}
1351+
}
1352+
13201353
// Merge the sources from the template and the target. The sources
13211354
// without condition need to be processed first
13221355
parser::Condition<tsl::ordered_set<std::string>> msources;
@@ -1341,12 +1374,15 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
13411374
}
13421375
auto &condition_sources = msources[custom_command.condition];
13431376
for (const auto &output : custom_command.outputs) {
1344-
condition_sources.insert(output);
1377+
condition_sources.insert(normalize_generated_output_source(output));
1378+
if (resolved_target_type == parser::target_custom) {
1379+
mcustom_target_depends[custom_command.condition].insert(output);
1380+
}
13451381
}
13461382
}
13471383

13481384
// Improve IDE support
1349-
if (target.type != parser::target_interface) {
1385+
if (resolved_target_type != parser::target_interface) {
13501386
msources[""].insert("cmake.toml");
13511387
}
13521388

@@ -1380,7 +1416,7 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
13801416
}
13811417

13821418
// Make sure there are source files for the languages used by the project
1383-
switch (target.type) {
1419+
switch (resolved_target_type) {
13841420
case parser::target_executable:
13851421
case parser::target_library:
13861422
case parser::target_shared:
@@ -1424,14 +1460,7 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
14241460
}
14251461
});
14261462

1427-
auto target_type = target.type;
1428-
1429-
if (tmplate != nullptr) {
1430-
if (target_type != parser::target_template) {
1431-
throw_target_error("Unreachable code, unexpected target type for template");
1432-
}
1433-
target_type = tmplate->outline.type;
1434-
}
1463+
auto target_type = resolved_target_type;
14351464

14361465
std::string add_command;
14371466
std::string target_type_string;
@@ -1478,6 +1507,39 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
14781507
throw_target_error("Unimplemented enum value");
14791508
}
14801509

1510+
auto has_custom_target_depends = false;
1511+
auto custom_target_depends_var = target.name + "_DEPENDS";
1512+
auto custom_target_depends_with_set = true;
1513+
if (is_custom_target) {
1514+
for (const auto &itr : mcustom_target_depends) {
1515+
if (!itr.second.empty()) {
1516+
has_custom_target_depends = true;
1517+
break;
1518+
}
1519+
}
1520+
if (has_custom_target_depends && mcustom_target_depends[""].empty()) {
1521+
custom_target_depends_with_set = false;
1522+
cmd("set")(custom_target_depends_var, RawArg("\"\"")).endl();
1523+
}
1524+
gen.handle_condition(mcustom_target_depends, [&](const std::string &condition, const tsl::ordered_set<std::string> &depend_set) {
1525+
std::vector<std::string> depends;
1526+
depends.reserve(depend_set.size());
1527+
for (const auto &depend : depend_set) {
1528+
depends.push_back(depend);
1529+
}
1530+
1531+
if (custom_target_depends_with_set) {
1532+
if (!condition.empty()) {
1533+
throw_target_error("Unreachable code, make sure unconditional depends are first");
1534+
}
1535+
cmd("set")(custom_target_depends_var, depends);
1536+
custom_target_depends_with_set = false;
1537+
} else {
1538+
cmd("list")("APPEND", custom_target_depends_var, depends);
1539+
}
1540+
});
1541+
}
1542+
14811543
auto append_keyword = [](std::vector<RawArg> &args, const std::string &keyword) {
14821544
args.emplace_back(keyword);
14831545
};
@@ -1616,9 +1678,9 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
16161678
append_keyword(args, "ALL");
16171679
}
16181680
append_commands(args, custom_target.commands);
1619-
if (!custom_target.depends.empty()) {
1681+
if (has_custom_target_depends) {
16201682
append_keyword(args, "DEPENDS");
1621-
append_values(args, custom_target.depends);
1683+
append_value(args, "${" + custom_target_depends_var + "}");
16221684
}
16231685
if (!custom_target.byproducts.empty()) {
16241686
append_keyword(args, "BYPRODUCTS");

‎src/project_parser.cpp‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,18 @@ static std::string normalize_build_event(std::string build_event) {
297297
return build_event;
298298
}
299299

300+
static TargetType resolve_target_type(const Target &target, const std::vector<Template> &templates) {
301+
if (target.type != target_template) {
302+
return target.type;
303+
}
304+
for (const auto &tmplate : templates) {
305+
if (target.type_name == tmplate.outline.name) {
306+
return tmplate.outline.type;
307+
}
308+
}
309+
return target.type;
310+
}
311+
300312
Project::Project(const Project *parent, const std::string &path, bool build) : parent(parent) {
301313
const auto toml_path = fs::path(path) / "cmake.toml";
302314
if (!fs::exists(toml_path)) {
@@ -816,6 +828,8 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p
816828

817829
t.optional("dependencies", target.dependencies);
818830

831+
const auto resolved_target_type = resolve_target_type(target, templates);
832+
819833
if (t.contains("all")) {
820834
target.custom_target.has_all = true;
821835
target.custom_target.all = t.find("all").as_boolean();
@@ -986,6 +1000,13 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p
9861000
custom_command.build_event != "POST_BUILD") {
9871001
throw_key_error("build-event must be one of: pre-build, pre-link, post-build", "build-event", custom.find("build-event"));
9881002
}
1003+
if (resolved_target_type == target_interface) {
1004+
throw_key_error("build-event cannot be used with type = \"interface\"", "build-event", custom.find("build-event"));
1005+
}
1006+
if (resolved_target_type == target_custom && custom_command.build_event == "PRE_LINK") {
1007+
throw_key_error("build-event = \"pre-link\" cannot be used with type = \"custom\"", "build-event",
1008+
custom.find("build-event"));
1009+
}
9891010
if (custom_command.has_append || custom_command.has_main_dependency || !custom_command.depends.empty() ||
9901011
!custom_command.implicit_depends.empty() || custom_command.has_depfile || custom_command.has_job_pool ||
9911012
custom_command.has_job_server_aware || custom_command.has_codegen || custom_command.has_depends_explicit_only) {
@@ -1013,7 +1034,7 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p
10131034
}
10141035
}
10151036

1016-
if (target.type != target_custom && !target.custom_target.empty()) {
1037+
if (resolved_target_type != target_custom && !target.custom_target.empty()) {
10171038
const char *custom_key = "all";
10181039
if (target.custom_target.has_command) {
10191040
custom_key = "command";

‎tests/custom-command/cmake.toml‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,11 @@ sources = ["src/main.cpp"]
3333
include-directories = ["${CMAKE_CURRENT_BINARY_DIR}/generated"]
3434

3535
# Output form: This custom command generates source files before building.
36+
# Relative outputs follow CMake and are interpreted from the binary directory.
3637
# The outputs are automatically added as sources to the target.
3738
[[target.custom-command.custom-command]]
38-
outputs = ["${CMAKE_CURRENT_BINARY_DIR}/generated/generated.cpp"]
39-
byproducts = ["${CMAKE_CURRENT_BINARY_DIR}/generated/generated.hpp"]
39+
outputs = ["generated/generated.cpp"]
40+
byproducts = ["generated/generated.hpp"]
4041
depends = ["cmake/generate_source.cmake"]
4142
command = [
4243
"${CMAKE_COMMAND}",
@@ -79,3 +80,36 @@ command = [
7980
byproducts = ["${CMAKE_CURRENT_BINARY_DIR}/custom-target.stamp"]
8081
comment = "Run custom target"
8182
verbatim = true
83+
84+
# -----------------------------------------------------------------------------
85+
# Template-based custom target
86+
# -----------------------------------------------------------------------------
87+
88+
# Custom target options should also work when the target type comes from a template.
89+
[template.generated-custom-target]
90+
type = "custom"
91+
all = true
92+
verbatim = true
93+
94+
[target.custom-codegen-template]
95+
type = "generated-custom-target"
96+
command = [
97+
"${CMAKE_COMMAND}",
98+
"-DINPUT=${CMAKE_CURRENT_BINARY_DIR}/template-custom-target.stamp",
99+
"-P",
100+
"${CMAKE_CURRENT_SOURCE_DIR}/cmake/verify_file_exists.cmake",
101+
]
102+
comment = "Verify the generated stamp exists"
103+
104+
# The output-form custom command should automatically become a dependency of the
105+
# custom target, so the verification command sees the stamp file.
106+
[[target.custom-codegen-template.custom-command]]
107+
outputs = ["template-custom-target.stamp"]
108+
command = [
109+
"${CMAKE_COMMAND}",
110+
"-E",
111+
"touch",
112+
"${CMAKE_CURRENT_BINARY_DIR}/template-custom-target.stamp",
113+
]
114+
comment = "Create a stamp file for the template-based custom target"
115+
verbatim = true

‎tests/custom-command/cmake/verify_file_exists.cmake‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
if(NOT DEFINED INPUT)
2+
message(FATAL_ERROR "INPUT is required")
3+
endif()
4+
5+
if(NOT EXISTS "${INPUT}")
6+
message(FATAL_ERROR "Expected file not found: ${INPUT}")
7+
endif()
8+
9+
message(STATUS "Verified file exists: ${INPUT}")

0 commit comments

Comments
 (0)