Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for hostbuild executables when cross-compiling to non-windows on windows hosts #437

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Fix support for hostbuild executables when cross-compiling to non-windows on windows hosts #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8a6b26
Fix support for hostbuild executables when cross-compiling to non-win…
tronical Sep 8, 2023
85b8822
Attempt to limit deferred byproduct .exe suffix handling only to the …
tronical Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 54 additions & 9 deletions cmake/Corrosion.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,22 @@ function(_corrosion_set_imported_location_legacy target_name base_property filen
)
endfunction()


# Sets out_var to true if the byproduct copying and imported location is done in a deferred
# manner to respect target properties, etc. that may be set later.
function(_corrosion_determine_deferred_byproduct_copying_and_import_location_handling out_var)
set(${out_var} ${CORROSION_RESPECT_OUTPUT_DIRECTORY} PARENT_SCOPE)
endfunction()

function(_corrosion_bin_target_suffix target_name out_var_suffix)
get_target_property(hostbuild "${target_name}" ${_CORR_PROP_HOST_BUILD})
if(hostbuild AND CMAKE_HOST_WIN32)
set(${out_var_suffix} ".exe" PARENT_SCOPE)
elseif(Rust_CARGO_TARGET_OS STREQUAL "windows")
set(${out_var_suffix} ".exe" PARENT_SCOPE)
endif()
endfunction()

# Do not call this function directly!
#
# This function should be called deferred to evaluate target properties late in the configure stage.
Expand All @@ -144,6 +160,16 @@ function(_corrosion_set_imported_location_deferred target_name base_property out
set(output_dir_prop_target_name "${target_name}")
endif()

# Append .exe suffix for executable by-products if the target is windows or if it's a host
# build and the host is Windows.
get_target_property(target_type ${target_name} TYPE)
if(${target_type} STREQUAL "EXECUTABLE")
_corrosion_bin_target_suffix(${target_name} "suffix")
if(suffix)
set(filename "${filename}${suffix}")
endif()
endif()

get_target_property(output_directory "${output_dir_prop_target_name}" "${output_directory_property}")
message(DEBUG "Output directory property (target ${output_dir_prop_target_name}): ${output_directory_property} dir: ${output_directory}")

Expand Down Expand Up @@ -220,7 +246,8 @@ endfunction()
# artifact.
# - filename of the artifact.
function(_corrosion_set_imported_location target_name base_property output_directory_property filename)
if(CORROSION_RESPECT_OUTPUT_DIRECTORY)
_corrosion_determine_deferred_byproduct_copying_and_import_location_handling("defer")
if(defer)
_corrosion_call_set_imported_location_deferred("${target_name}" "${base_property}" "${output_directory_property}" "${filename}")
else()
_corrosion_set_imported_location_legacy("${target_name}" "${base_property}" "${filename}")
Expand Down Expand Up @@ -256,7 +283,7 @@ function(_corrosion_copy_byproduct_legacy target_name cargo_build_dir file_names
)
endfunction()

function(_corrosion_copy_byproduct_deferred target_name output_dir_prop_name cargo_build_dir file_names)
function(_corrosion_copy_byproduct_deferred target_name output_dir_prop_name cargo_build_dir file_names is_binary)
if(ARGN)
message(FATAL_ERROR "Unexpected additional arguments")
endif()
Expand Down Expand Up @@ -294,6 +321,16 @@ function(_corrosion_copy_byproduct_deferred target_name output_dir_prop_name car
endif()
endif()

# Append .exe suffix for executable by-products if the target is windows or if it's a host
# build and the host is Windows.
if(${is_binary})
_corrosion_bin_target_suffix(${target_name} "suffix")
if(suffix)
set(tmp_file_names "${file_names}")
list(TRANSFORM tmp_file_names APPEND "${suffix}" OUTPUT_VARIABLE file_names)
endif()
endif()

list(TRANSFORM file_names PREPEND "${cargo_build_dir}/" OUTPUT_VARIABLE src_file_names)
list(TRANSFORM file_names PREPEND "${output_dir}/" OUTPUT_VARIABLE dst_file_names)
message(DEBUG "Adding command to copy byproducts `${file_names}` to ${dst_file_names}")
Expand All @@ -313,7 +350,7 @@ function(_corrosion_copy_byproduct_deferred target_name output_dir_prop_name car
)
endfunction()

function(_corrosion_call_copy_byproduct_deferred target_name output_dir_prop_name cargo_build_dir file_names)
function(_corrosion_call_copy_byproduct_deferred target_name output_dir_prop_name cargo_build_dir file_names is_binary)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra parameter is rather ugly. I'm wondering if it's worth porting this over to cmake_parse_arguments. But I guess latest in this function it would all have to be unpacked and packed again, so maybe not worth it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, maybe we could just remove all named parameters to this function and pass [[${ARGN}]] to the deferred function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I’ll try that.

cmake_language(EVAL CODE "
cmake_language(DEFER
CALL
Expand All @@ -322,6 +359,7 @@ function(_corrosion_call_copy_byproduct_deferred target_name output_dir_prop_nam
[[${output_dir_prop_name}]]
[[${cargo_build_dir}]]
[[${file_names}]]
[[${is_binary}]]
)
")
endfunction()
Expand All @@ -334,9 +372,11 @@ endfunction()
# `RUNTIME_OUTPUT_DIRECTORY`)
# - cargo_build_dir: the directory cargo build places it's output artifacts in.
# - filenames: the file names of any output artifacts as a list.
function(_corrosion_copy_byproducts target_name output_dir_prop_name cargo_build_dir filenames)
if(CORROSION_RESPECT_OUTPUT_DIRECTORY)
_corrosion_call_copy_byproduct_deferred("${target_name}" "${output_dir_prop_name}" "${cargo_build_dir}" "${filenames}")
# - is_binary: TRUE if the byproducts are program executables.
function(_corrosion_copy_byproducts target_name output_dir_prop_name cargo_build_dir filenames is_binary)
_corrosion_determine_deferred_byproduct_copying_and_import_location_handling("defer")
if(defer)
_corrosion_call_copy_byproduct_deferred("${target_name}" "${output_dir_prop_name}" "${cargo_build_dir}" "${filenames}" "${is_binary}")
else()
_corrosion_copy_byproduct_legacy("${target_name}" "${cargo_build_dir}" "${filenames}")
endif()
Expand Down Expand Up @@ -520,10 +560,15 @@ function(_corrosion_add_bin_target workspace_manifest_path bin_name out_bin_bypr
set(${out_pdb_byproduct} "${pdb_name}" PARENT_SCOPE)
endif()

if(Rust_CARGO_TARGET_OS STREQUAL "windows")
set(bin_filename "${bin_name}.exe")
set(bin_filename "${bin_name}")
_corrosion_determine_deferred_byproduct_copying_and_import_location_handling("defer")
if(defer)
# .exe suffix will be added later, also depending on possible hostbuild
# target property
else()
set(bin_filename "${bin_name}")
if(Rust_CARGO_TARGET_OS STREQUAL "windows")
set(bin_filename "${bin_name}.exe")
endif()
endif()
set(${out_bin_byproduct} "${bin_filename}" PARENT_SCOPE)

Expand Down
10 changes: 5 additions & 5 deletions cmake/CorrosionGenerator.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,17 @@ function(_generator_add_package_targets)
)
if(archive_byproducts)
_corrosion_copy_byproducts(
${target_name} ARCHIVE_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${archive_byproducts}"
${target_name} ARCHIVE_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${archive_byproducts}" FALSE
)
endif()
if(shared_lib_byproduct)
_corrosion_copy_byproducts(
${target_name} LIBRARY_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${shared_lib_byproduct}"
${target_name} LIBRARY_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${shared_lib_byproduct}" FALSE
)
endif()
if(pdb_byproduct)
_corrosion_copy_byproducts(
${target_name} PDB_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${pdb_byproduct}"
${target_name} PDB_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${pdb_byproduct}" FALSE
)
endif()
list(APPEND corrosion_targets ${target_name})
Expand Down Expand Up @@ -174,11 +174,11 @@ function(_generator_add_package_targets)
${no_linker_override}
)
_corrosion_copy_byproducts(
${target_name} RUNTIME_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${bin_byproduct}"
${target_name} RUNTIME_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${bin_byproduct}" TRUE
)
if(pdb_byproduct)
_corrosion_copy_byproducts(
${target_name} PDB_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${pdb_byproduct}"
${target_name} PDB_OUTPUT_DIRECTORY "${cargo_build_out_dir}" "${pdb_byproduct}" FALSE
)
endif()
list(APPEND corrosion_targets ${target_name})
Expand Down