[luajit] Fix cross compile for macOS #29003
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
LilyWangLL
added
category:port-bug
ports/luajit/portfile.cmake
Outdated
| if(TARGET_ARCHITECTURE STREQUAL x64) | ||
| set(TARGET_ARCHITECTURE x86_64) | ||
| endif() | ||
| execute_process(COMMAND uname -m OUTPUT_VARIABLE HOST_ARCHITECTURE) |
There was a problem hiding this comment.
Should this strip trailing whitespace? #28939
There was a problem hiding this comment.
There is string(STRIP ) below this line.
But do we need the host architecture at all?
- There is
VCPKG_CROSSCOMPILINGto detect cross-triplet builds. - The usage of
HOST_...FLAGSindicates the build of host tools during the target build, which is not how vcpkg works.
There was a problem hiding this comment.
Should this strip trailing whitespace? #28939
There is string(Strip ...)
There is
string(STRIP )below this line.But do we need the host architecture at all?
- There is
VCPKG_CROSSCOMPILINGto detect cross-triplet builds.- The usage of
HOST_...FLAGSindicates the build of host tools during the target build, which is not how vcpkg works.
We need the host architecture. Since there are minilua and buildvm executable (host tools) which participated in the build process but not installed. These tools need to be runable on host machine.
There was a problem hiding this comment.
We need the host architecture. Since there are
miniluaandbuildvmexecutable (host tools) which participated in the build process but not installed. These tools need to be runable on host machine.
If this scenario is happening then when host compiling they should be included and when cross compiling they should not be. See Google Protocol Buffers for an example that does this: when the target triplet is the host triplet it builds protoc, otherwise it parachutes in the protoc when built for the host: https://github.com/microsoft/vcpkg/blob/master/ports/protobuf/portfile.cmake
This arrangement is fairly important because what the host triplet is is a user setting.
There was a problem hiding this comment.
Not the same case. protoc need to be installed since it participated in the user project build process. But minilua and buildvm only participated in the luajit build process. They should not be installed. This port has another executable named luajit and it is TARGET_ARCHITECTURE. The HOST_ARCHITECTURE just make sure minilua and buildvm is runnable on host machine, otherwise the build will fails.
There was a problem hiding this comment.
Em... wait, It seems HOST_ARCHITECTURE can be omited. I will update this pr.
There was a problem hiding this comment.
AFAICT the host dependency requirement in vcpkg is also for tools which are used only during the port build process. Unfortunately, this means patching in most cases, and actually installing the internal tools. Some ports use a manual-tools dir for this purpose.
(IMO this could be resolved in a better way, but there is no proof of concept.)
BillyONeal
added
requires:author-response
and removed
info:reviewed
xiaozhuai
dismissed stale reviews from LilyWangLL and github-actions[bot]
via
d35890c
There was a problem hiding this comment.
I'm approving this despite the outstanding concerns about doing cross compilation correctly, because while this is not the correct behavior, it's an improvement on the status quo, and the path to doing it correctly won't be damaged by this being merged first.
|
@xiaozhuai We would really appreciate you plumbing in cross support correctly but I'm going to merge this as is for now. Thank you! |
BillyONeal
added
category:port-feature
Describe the pull request
What does your PR fix?
Fix #28848