cmake: extensions: use INTERFACE_SOURCES as property for code relocation

[danieldegrasse]

In order to enable code relocation, we use a custom target (code_data_relocation_target), and add files we wish to relocate, as well as which sections should be relocated to the COMPILE_DEFINITIONS property for the target.

This approach has been fragile, because COMPILE_DEFINITIONS can also be added to for all targets using add_definitions. This means if another part of the project uses add_definitions and
CONFIG_CODE_DATA_RELOCATION is on, a warning will appear about the "file" not being found. The "file" of course, is just the definition added by add_definitions.

To work around this, switch to overloading the INTERFACE_SOURCES property. This property should be a bit more robust, because nobody else will add sources to the code_data_relocation_target.

However, this approach has the downside that the CMake documentation pstates targets created with add_custom_target() (which the code_data_relocation_target is) do not have an INTERFACE scope for their sources- so while this approach works, it is not officially supported by CMake

Fixes #60220

[danieldegrasse]

@tejlmand I'm interested to hear your opinion on this fix- I really dislike the fact that if we use INTERFACE_SOURCES as the property to overload here, we are effectively relying on the fact that CMake does not validate the following claim in its documentation:

A target created by add_custom_target() can only have PRIVATE scope.

By that documentation, the INTERFACE_SOURCES property shouldn't exist on the code_data_relocation_target, yet it seems to be present and usable. My concern is that CMake could break this behavior. However, in my search for a target property that:

• does not have validation run by CMake to make sure the files added to the target exist (since we aren't adding files here)
• would not be modified by other CMake commands like add_definition
• supports generator expressions

INTERFACE_SOURCES is the only one I've found so far (SOURCES expects all files added to the target to exist, and will validate that they do)

[57300]

I got a custom property to work by wrapping the genex with:

# either
$<GENEX_EVAL:$<TARGET_PROPERTY:code_data_relocation_target,RELOCATE_FILES>>
# or
$<TARGET_GENEX_EVAL:code_data_relocation_target,$<TARGET_PROPERTY:code_data_relocation_target,RELOCATE_FILES>>

I think that both will work equivalently for the code relocation implementation. The only difference is that TARGET_GENEX_EVAL supports expressions with implicit target names, like $<TARGET_PROPERTY:prop> as opposed to $<TARGET_PROPERTY:tgt,prop>. See: https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#multi-level-expression-evaluation

[tejlmand]

@tejlmand I'm interested to hear your opinion on this fix- I really dislike the fact that if we use INTERFACE_SOURCES as the property to overload here

A target created by add_custom_target() can only have PRIVATE scope.

By that documentation, the INTERFACE_SOURCES property shouldn't exist on the code_data_relocation_target, yet it seems to be present and usable.

That documentation refers to the use of target_sources(), meaning that if you want to use that function together with a custom target, then you can only use the private scope.

The property itself is still available on the target, as you have noticed.
The docs also states that it's a valid property for a target: https://cmake.org/cmake/help/latest/manual/cmake-properties.7.html#properties-on-targets

I share the feeling of this being sub-optimal, but unfortunately CMake doesn't allow generator expressions on custom properties, which means that the idea given in #81249 (comment) of using a custom property will not work because we rely on generator expressions in the property value, ref:

zephyr/cmake/modules/extensions.cmake

Lines 1478 to 1489 in 65511ea

	set(genex_src_dir "$<TARGET_PROPERTY:${CODE_REL_LIBRARY},SOURCE_DIR>")
	set(genex_src_list "$<TARGET_PROPERTY:${CODE_REL_LIBRARY},SOURCES>")
	if(CMAKE_HOST_WIN32)
	# Note that this assumes windows absolute filenames start with a letter and colon, this does
	# not support \\x network paths and is untested under the likes of msys2/cygwin
	set(src_list_abs "$<FILTER:${genex_src_list},INCLUDE,^[A-Za-z]\:>")
	set(src_list_rel "$<FILTER:${genex_src_list},EXCLUDE,^[A-Za-z]\:>")
	else()
	set(src_list_abs "$<FILTER:${genex_src_list},INCLUDE,^/>")
	set(src_list_rel "$<FILTER:${genex_src_list},EXCLUDE,^/>")
	endif()

This actually indicates that INTERFACE_SOURCES might be the best choice in current situation, because:

• The INTERFACE_SOURCES property will not be accidentially be populated, because target_sources() will now allow that for a custom property. Thus only dedicated set_property() calls can set the INTERFACE_SOURCES on the code relocate target. The less likely to hit a user mistake.
• The INTERFACE_SOURCES property supports genex'es, which the code relocation implementation depends upon.

[tejlmand]

I got a custom property to work by wrapping the genex with:

it might be worth a try, but iirc then the issue with this approach when I tested at an earlier occasion was that the property is containing multiple levels of generator expressions.
Did you test all lib relocation combination ?

Also worth remembering is the linker consumer needs to be aware that the property retrieved must be expended in a GENEX_EVALor TARGET_GENEX_EVAL which makes it a bit harder for oot toolchains / linkers maintainers to apply, compared to just extract a single property.
Though this is a minor concern if we have good templating code available in ld/target_relocation.cmake.

[57300]

Did you test all lib relocation combination ?

@tejlmand I haven't tested very thoroughly, just settled on building tests/application_development/code_relocation.

But I concluded that as long as the custom property value would only contain things like $<TARGET_PROPERTY:lib,SOURCE_DIR> and $<TARGET_PROPERTY:lib,SOURCES> - both of which should get fully expanded as standard target properties - plus trivial combinations of FILTER, JOIN, etc., then a single GENEX_EVAL would be enough to expand the whole value. On the other hand, if the custom property referenced other custom properties, then you'd need multiple evaluations.

Still, if INTERFACE_SOURCES is safe to use by your own analysis, then it's probably the better choice, especially accounting for any future implementation changes that could break my own assumptions.

[danieldegrasse]

Still, if INTERFACE_SOURCES is safe to use by your own analysis, then it's probably the better choice, especially accounting for any future implementation changes that could break my own assumptions.

Personally if we are ok with INTERFACE_SOURCES I'd prefer to use that- based on the documentation @tejlmand linked, it seems like a "safe" property for this use case

[tejlmand]

But I concluded that as long as the custom property value would only contain things like $<TARGET_PROPERTY:lib,SOURCE_DIR> and $<TARGET_PROPERTY:lib,SOURCES> - both of which should get fully expanded as standard target properties - plus trivial combinations of FILTER, JOIN, etc., then a single GENEX_EVAL would be enough to expand the whole value. On the other hand, if the custom property referenced other custom properties, then you'd need multiple evaluations.

@57300 Yes, as long as we can be sure we only have simple constructs.
However, simple constructs tends to develop into complex once, and suddenly code starts to break 😞 .
This is why i'm don't fully truste GENEX_EVAL.

Developers must be aware that they need additional genex eval calls if the extend the code.

For example this fails:

add_custom_target(foo)
add_custom_target(bar)
add_custom_target(baz)

set_property(TARGET foo PROPERTY CUSTOM_PROPERTY "foo_custom_value")
set_property(TARGET bar PROPERTY CUSTOM_PROPERTY "$<TARGET_PROPERTY:foo,CUSTOM_PROPERTY>")
set_property(TARGET baz PROPERTY CUSTOM_PROPERTY "$<TARGET_PROPERTY:bar,CUSTOM_PROPERTY>")

add_custom_target(baz_print ${CMAKE_COMMAND} -E echo $<GENEX_EVAL:$<TARGET_PROPERTY:baz,CUSTOM_PROPERTY>>)

because one more level of generator expression is used in sub-property.

So would need to be:

add_custom_target(foo)
add_custom_target(bar)
add_custom_target(baz)

set_property(TARGET foo PROPERTY CUSTOM_PROPERTY "foo_custom_value")
set_property(TARGET bar PROPERTY CUSTOM_PROPERTY "$<TARGET_PROPERTY:foo,CUSTOM_PROPERTY>")
set_property(TARGET baz PROPERTY CUSTOM_PROPERTY "$<GENEX_EVAL:$<TARGET_PROPERTY:bar,CUSTOM_PROPERTY>>")

add_custom_target(baz_print ${CMAKE_COMMAND} -E echo $<GENEX_EVAL:$<TARGET_PROPERTY:baz,CUSTOM_PROPERTY>>)

Meaning there is a risk that at some point in time, something will break.

Sticking to a property which themselves allows generator expression within them, like INTERFACE_SOURCES always works, and no need to consider use of GENEX_EVAL.