Skip to content

Commit 6d41c92

Browse files
mrexodiamrexodia
committed
Fix custom command review issues
1 parent 411eb03 commit 6d41c92

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
@@ -1222,6 +1222,14 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
12221222
}
12231223
}
12241224

1225+
auto resolved_target_type = target.type;
1226+
if (tmplate != nullptr) {
1227+
if (resolved_target_type != parser::target_template) {
1228+
throw_target_error("Unreachable code, unexpected target type for template");
1229+
}
1230+
resolved_target_type = tmplate->outline.type;
1231+
}
1232+
12251233
ConditionScope cs(gen, target.condition);
12261234

12271235
// Detect if there is cmake included before/after the target
@@ -1323,6 +1331,31 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
13231331
}
13241332
custom_commands.insert(custom_commands.end(), target.custom_commands.begin(), target.custom_commands.end());
13251333

1334+
auto normalize_generated_output_source = [](const std::string &output) {
1335+
auto starts_with = [](const std::string &value, const std::string &prefix) {
1336+
return value.rfind(prefix, 0) == 0;
1337+
};
1338+
if (fs::path(output).is_absolute()) {
1339+
return output;
1340+
}
1341+
if (starts_with(output, "${CMAKE_CURRENT_BINARY_DIR}") || starts_with(output, "${CMAKE_BINARY_DIR}") ||
1342+
starts_with(output, "${PROJECT_BINARY_DIR}") || starts_with(output, "${CMAKE_CURRENT_SOURCE_DIR}") ||
1343+
starts_with(output, "${CMAKE_SOURCE_DIR}") || starts_with(output, "${PROJECT_SOURCE_DIR}") ||
1344+
starts_with(output, "${CMAKE_CURRENT_LIST_DIR}") || starts_with(output, "$ENV{") || starts_with(output, "$CACHE{") ||
1345+
starts_with(output, "${")) {
1346+
return output;
1347+
}
1348+
return "${CMAKE_CURRENT_BINARY_DIR}/" + output;
1349+
};
1350+
1351+
parser::Condition<tsl::ordered_set<std::string>> mcustom_target_depends;
1352+
if (resolved_target_type == parser::target_custom) {
1353+
auto &unconditional_depends = mcustom_target_depends[""];
1354+
for (const auto &dep : custom_target.depends) {
1355+
unconditional_depends.insert(dep);
1356+
}
1357+
}
1358+
13261359
// Merge the sources from the template and the target. The sources
13271360
// without condition need to be processed first
13281361
parser::Condition<tsl::ordered_set<std::string>> msources;
@@ -1347,12 +1380,15 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
13471380
}
13481381
auto &condition_sources = msources[custom_command.condition];
13491382
for (const auto &output : custom_command.outputs) {
1350-
condition_sources.insert(output);
1383+
condition_sources.insert(normalize_generated_output_source(output));
1384+
if (resolved_target_type == parser::target_custom) {
1385+
mcustom_target_depends[custom_command.condition].insert(output);
1386+
}
13511387
}
13521388
}
13531389

13541390
// Improve IDE support
1355-
if (target.type != parser::target_interface) {
1391+
if (resolved_target_type != parser::target_interface) {
13561392
msources[""].insert("cmake.toml");
13571393
}
13581394

@@ -1386,7 +1422,7 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
13861422
}
13871423

13881424
// Make sure there are source files for the languages used by the project
1389-
switch (target.type) {
1425+
switch (resolved_target_type) {
13901426
case parser::target_executable:
13911427
case parser::target_library:
13921428
case parser::target_shared:
@@ -1430,14 +1466,7 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
14301466
}
14311467
});
14321468

1433-
auto target_type = target.type;
1434-
1435-
if (tmplate != nullptr) {
1436-
if (target_type != parser::target_template) {
1437-
throw_target_error("Unreachable code, unexpected target type for template");
1438-
}
1439-
target_type = tmplate->outline.type;
1440-
}
1469+
auto target_type = resolved_target_type;
14411470

14421471
std::string add_command;
14431472
std::string target_type_string;
@@ -1484,6 +1513,39 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
14841513
throw_target_error("Unimplemented enum value");
14851514
}
14861515

1516+
auto has_custom_target_depends = false;
1517+
auto custom_target_depends_var = target.name + "_DEPENDS";
1518+
auto custom_target_depends_with_set = true;
1519+
if (is_custom_target) {
1520+
for (const auto &itr : mcustom_target_depends) {
1521+
if (!itr.second.empty()) {
1522+
has_custom_target_depends = true;
1523+
break;
1524+
}
1525+
}
1526+
if (has_custom_target_depends && mcustom_target_depends[""].empty()) {
1527+
custom_target_depends_with_set = false;
1528+
cmd("set")(custom_target_depends_var, RawArg("\"\"")).endl();
1529+
}
1530+
gen.handle_condition(mcustom_target_depends, [&](const std::string &condition, const tsl::ordered_set<std::string> &depend_set) {
1531+
std::vector<std::string> depends;
1532+
depends.reserve(depend_set.size());
1533+
for (const auto &depend : depend_set) {
1534+
depends.push_back(depend);
1535+
}
1536+
1537+
if (custom_target_depends_with_set) {
1538+
if (!condition.empty()) {
1539+
throw_target_error("Unreachable code, make sure unconditional depends are first");
1540+
}
1541+
cmd("set")(custom_target_depends_var, depends);
1542+
custom_target_depends_with_set = false;
1543+
} else {
1544+
cmd("list")("APPEND", custom_target_depends_var, depends);
1545+
}
1546+
});
1547+
}
1548+
14871549
auto append_keyword = [](std::vector<RawArg> &args, const std::string &keyword) {
14881550
args.emplace_back(keyword);
14891551
};
@@ -1622,9 +1684,9 @@ void generate_cmake(const char *path, const parser::Project *parent_project) {
16221684
append_keyword(args, "ALL");
16231685
}
16241686
append_commands(args, custom_target.commands);
1625-
if (!custom_target.depends.empty()) {
1687+
if (has_custom_target_depends) {
16261688
append_keyword(args, "DEPENDS");
1627-
append_values(args, custom_target.depends);
1689+
append_value(args, "${" + custom_target_depends_var + "}");
16281690
}
16291691
if (!custom_target.byproducts.empty()) {
16301692
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)