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 scripts]: Include --build in autoconf xcompile #43703

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

Conversation

Krzmbrzl
Copy link
Contributor

@Krzmbrzl Krzmbrzl commented Feb 8, 2025

According to the autconf manual (source), when using the --host option, one should always also specify the --build option ("for historic reasons").

This commit ensures that vcpkg follows this recommendation by letting config.guess determine the build triplet to use.

This is a more general version of #28374 and it fixes the immediate issue observed in #41130 though. Whether the issue is fully fixed, I can't tell but I can tell that on a M1 Mac, trying to cross-compile to x64 Mac is still not possible but this is due to CPython not supporting that: python/cpython#90905 (though thanks to #27830 it might starting from 3.14).

According to the autconf manual [1], when using the --host option, one
should always also specify the --build option ("for historic reasons").

This commit ensures that vcpkg follows this recommendation by letting
config.guess determine the build triplet to use.

[1]: https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Specifying-Target-Triplets.html
@dg0yt
Copy link
Contributor

dg0yt commented Feb 8, 2025

All autoconf changes must go to ports/vcpkg-make.

@Krzmbrzl
Copy link
Contributor Author

Krzmbrzl commented Feb 8, 2025

@dg0yt the directory you mentioned seems to be about the installation of autoconf via vcpkg. But my changes affect the way vcpkg invokes autoconf instead. Furthermore, all other handling of target triplet options is in the file that I edited so it would seem odd to me if I was to add my change somewhere else 👀

@dg0yt
Copy link
Contributor

dg0yt commented Feb 8, 2025

You are trying to change the unversioned script the vpckg owners were reluctant to change, requesting a versioned script port instead.
vcpkg-make is that versioned port.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 8, 2025

... and in triggers a lot of CI activity for that.

@Krzmbrzl
Copy link
Contributor Author

Krzmbrzl commented Feb 8, 2025

Okay after having read through the discussion in #31228 I now understand what you mean. Does this effectively mean that we just don't fix ports that are broken due to issues in those never-to-be-changed scripts? That's rather ridiculous imo.
However, from the linked PR i know that you are the wrong person to complain to about this.

I'll redirect my changes to the port file then even though this means that the thing this was originally fixing now remains unfixed... 😕

@dg0yt
Copy link
Contributor

dg0yt commented Feb 8, 2025

Does this effectively mean that we just don't fix ports that are broken due to issues in those never-to-be-changed scripts?

Ports are fixed by making them use the shiny new maintainer functions from vcpkg-make.
(Well, still needs quite some polishing before being shiny...)

@Krzmbrzl
Copy link
Contributor Author

Krzmbrzl commented Feb 8, 2025

Hm. As I see it, z_vcpkg_make_get_configure_triplets would be the function to modify. However, it seems like it could be called from any context and I don't see a way to obtain the path to a potentially existing config.guess file from in there. I could just rely on the CWD being the directory in which the configure script is located but that seems a bit wonky 🤔

I guess instead of calling this script, we could also just assemble the triplets based on VCPKG_HOST_IS_LINUX and friends. Though I guess that might be less general 👀

Any thoughts on that?

@dg0yt
Copy link
Contributor

dg0yt commented Feb 8, 2025

I have rough ideas... some elaboration might be needed.

  • In z_vcpkg_make_get_configure_triplets, set build_triplet to @CONFIG_GUESS@ when VCPKG_CROSSCOMPILING and not having a better guess.
  • In vcpkg_make_configure, substitute @CONFIG_GUESS@ with
    • the result of config.guess or an shell-expanded call to config.guess (*), or
    • the empty string if there is no config.guess in SOURCE_PATH.

*) In libffi, there is:

set(configure_triplets DETERMINE_BUILD_TRIPLET)
if(VCPKG_TARGET_IS_EMSCRIPTEN)
set(configure_triplets BUILD_TRIPLET "--host=wasm32-unknown-emscripten --build=\$(\$SHELL \"${SOURCE_PATH}/config.guess\")")
endif()

@Krzmbrzl
Copy link
Contributor Author

Krzmbrzl commented Feb 8, 2025

Good idea. On further thought however, I feel like given z_vcpkg_make_get_configure_triplets already determines (at least potentially) --host and --build values, it seems odd if the caller of that function has to inspect them in order to figure out whether they need to do it themselves after all 🤔

I think what I am going to do is to add a parameter to z_vcpkg_make_get_configure_triplets that allows passing the path to the config.guess file into the function. Then the determination can be entirely within z_vcpkg_make_get_configure_triplets by either calling config.guess (if given) or assembling the value based on the VCPKG_HOST_* variables.

@Krzmbrzl Krzmbrzl force-pushed the autoconf-fix-crosscompile branch from f7118a8 to 94eba20 Compare February 8, 2025 19:07
@Krzmbrzl Krzmbrzl force-pushed the autoconf-fix-crosscompile branch from 94eba20 to 40f9b74 Compare February 8, 2025 19:10
@Krzmbrzl Krzmbrzl force-pushed the autoconf-fix-crosscompile branch from 40f9b74 to 54d9957 Compare February 8, 2025 19:15
Comment on lines +187 to +192
elseif (VCPKG_HOST_IS_WINDOWS)
set(build_triplet_opt "--build=${BUILD_ARCH}-pc-mingw32")
elseif (VCPKG_HOST_IS_LINUX)
set(build_triplet_opt "--build=${BUILD_ARCH}-pc-linux")
elseif (VCPKG_HOST_IS_IOS OR VCPKG_HOST_IS_OSX)
set(build_triplet_opt "--build=${BUILD_ARCH}-apple-darwin")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet sure whether doing this "estimated guessing" or just leaving it blank would be better. The latter would certainly be more conservative of a change 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

In vcpkg, ports are meant to not build any binary for the host.* (Host binaries must be done via host dependencies,) The guess just needs to be good enough for running the build scripts correctly,

@WangWeiLin-MV WangWeiLin-MV added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants