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.

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

[vcpkg-make] Misc fixes, in particular: parallel builds #43249

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 13, 2025

No description provided.

@MonicaLiu0311 MonicaLiu0311 added the category:port-bug The issue is with a library, which is something the port should already support label Jan 14, 2025
@dg0yt dg0yt marked this pull request as ready for review January 14, 2025 05:59
vcpkg_execute_build_process(
COMMAND ${arg_SHELL} -c "${cmd}"
NO_PARALLEL_COMMAND ${arg_SHELL} -c "${no_par_cmd}"
${no_par_cmd}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Restarting without parallelism" shouldn't be displayed if the build was initiated without parallelism.
This is achieved by not passing the NO_PARALLEL_COMMAND option when arg_NO_PARALLEL_COMMAND is empty.

@@ -557,7 +557,6 @@ function(z_vcpkg_make_prepare_flags)
${flags_opts}
)
if(NOT DEFINED VCPKG_BUILD_TYPE)
list(APPEND all_buildtypes DEBUG)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused in port vcpkg_make.

@@ -97,7 +97,6 @@ function(vcpkg_make_configure)
set(opts "")
if(NOT arg_DISABLE_DEFAULT_OPTIONS)
z_vcpkg_make_default_path_and_configure_options(opts AUTOMAKE CONFIG "${configup}")
vcpkg_list(APPEND arg_OPTIONS ${opts})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appended the second config's options, to the first config's options, breaking the debug,relase build order.

@@ -81,26 +81,27 @@ function(vcpkg_make_install)
endif()

foreach(target IN LISTS arg_TARGETS)
string(REPLACE "/" "_" target_no_slash "${target}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most targets aren't "phony" but actual files. But filepaths must not be used to construct the log file name.

SHELL ${shell_cmd}
COMMAND ${configure_env} ${no_parallel_make_cmd_line}
COMMAND ${configure_env} ${make_cmd_line}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is why the build wasn't parallel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants