[scripts-audit] CMake buildsystem#18843
Conversation
There was a problem hiding this comment.
Are these variables the lists which are created a few lines before?
If yes:
- They should be appended to here.
- There is
_paramvs._argsinconsistency.
There was a problem hiding this comment.
Given that this is an audit:
What is the point of ADD_BIN_TO_PATH with the given paths? No port installs to these paths: Every port uses paths that include ${PORT}. Cf. #17607 for the full painful story.
There was a problem hiding this comment.
arg_ADD_BIN_TO_PATH needs still to be disabled in crossbuilds.
There was a problem hiding this comment.
Is it necessary to add ninja to the PATH?
There was a problem hiding this comment.
probably not since it is passed as CMAKE_MAKE_PROGRAM
There was a problem hiding this comment.
this should be its own helper function because it is used more often.
There was a problem hiding this comment.
also wondering if this could use:
https://cmake.org/cmake/help/latest/command/cmake_host_system_information.html
There was a problem hiding this comment.
I don't believe so, since it doesn't seem to read through PROCESSOR_ARCHITEW6432, it'll use the architecture of the process rather than the chip.
There was a problem hiding this comment.
Move into own function ? z_vcpkg_select_default_toolchain I still think this should be defined in the triplet instead or selected earlier outside of vcpkg_configure_cmake because it has nothing to do with this function
567835a to
63898d8
Compare
cae784b to
30b217b
Compare
This does an actual audit of the vcpkg_*_cmake functions, rather than just replacing them like #16377
Depends on PR #18397