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

Only use prebuilt musl libc binary with x86_64 arch #30364

Merged
merged 2 commits into from Apr 18, 2023

Conversation

danifv
Copy link
Contributor

@danifv danifv commented Mar 22, 2023

Fixes #30170

As described in #30170, bootstrap-vcpkg.sh currently incorrectly downloads x86_64 binaries for arm hosts, if the host is running alpine linux, regardless of the host architecture.
This PR fixes this issue, by checking the platform architecture along /etc/alpine-release. If the architecture is not x86_64, it will default to compiling the source, as it already happens on other distributions, e.g. ubuntu.

@danifv danifv changed the title Only use musl libc binary with x86_64 arch Only use prebuilt musl libc binary with x86_64 arch Mar 22, 2023
@jimwang118 jimwang118 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Mar 23, 2023
@jimwang118
Copy link
Contributor

Thank you for mentioning this PR, our ci does not have a corresponding machine to test this change, so please make sure that the local test is ok.

Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

I wonder if the download block wouldn't be better served by a case pattern as commonly found in configure scripts:

case "${UNAME},${ARCH},${vcpkgUseMuslC}" in
Darwin,*)
...
Linux,x86_64,ON)
...
Linux,x86_64,*)
...
*)
...
esac

This could be easily extended to support arm, bsd or similar.

Comment on lines 96 to 99
if [ -e /etc/alpine-release -a "$ARCH" = "x86_64" ]; then
vcpkgUseSystem="ON"
vcpkgUseMuslC="ON"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

This change also removes vcpkgUseSystem="ON" from non-x86_64. This doesn't seem right to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using current vcpkg, I get the following message if I try bootstrapping on an alpine host, that lacks the required packages:

./vcpkg/bootstrap-vcpkg.sh -disableMetrics
On Alpine:
  apk add build-base cmake ninja zip unzip curl git
  (and export VCPKG_FORCE_SYSTEM_BINARIES=1)

If I install the required packages, without setting VCPKG_FORCE_SYSTEM_BINARIES to 1, and then proceed to installing a package, then I get a note saying, that a suitable version of cmake was not found, which is followed by a download of Kitware/CMake.
However, if I set VCPKG_FORCE_SYSTEM_BINARIES=1, then my system installation of CMake is used, so vcpkgUseSystem by itself doesn't seem to have an effect.

If I bootstrap on aarch64 without VCPKG_FORCE_SYSTEM_BINARIES, an x64_86 distribution is downloaded, so not only is vcpkgUseSystem without effect, but an incompatible binary is downloaded.

What is the relation between VCPKG_FORCE_SYSTEM_BINARIES and vcpkgUseSystem? What is the expected behavoiur?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dg0yt Please help to make sure this modification is correct.

@danifv danifv requested review from dg0yt and removed request for Adela0814 March 27, 2023 09:09
@jimwang118
Copy link
Contributor

@danifv Please make sure that this modification only works on x64 and has no effect on other platforms. If you can confirm that the revision is correct, I will submit the application for integration.

@danifv
Copy link
Contributor Author

danifv commented Apr 14, 2023

@danifv Please make sure that this modification only works on x64 and has no effect on other platforms. If you can confirm that the revision is correct, I will submit the application for integration.

I've changed the code, so that vcpkgUseSystem="ON" remains set on alpine for all archs.

Otherwise I can confirm, that behaviour on x64 remains unchanged, and that on other archs a correct binary is compiled instead of an incorrect one being downloaded.

@jimwang118 jimwang118 added the info:reviewed Pull Request changes follow basic guidelines label Apr 18, 2023
@BillyONeal
Copy link
Member

Can you help us get instructions on how to build a real arm64 copy so that people can get something to use directly? We would like folks to not have to pay the compilation time to build us but don't have arm64-linux experience ourselves...

@BillyONeal BillyONeal merged commit 9c7c664 into microsoft:master Apr 18, 2023
12 checks passed
@danifv
Copy link
Contributor Author

danifv commented Apr 18, 2023

Can you help us get instructions on how to build a real arm64 copy so that people can get something to use directly? We would like folks to not have to pay the compilation time to build us but don't have arm64-linux experience ourselves...

Sure! Do you have a pipeline for building the x64 musl binaries? Would this pipeline work on arm64?
We basically did the following on our arm-based build host:

  1. patched vcpkg with our custom bootstrap-vcpkg.sh
  2. upon use a suitable binary was built

@BillyONeal
Copy link
Member

Sure! Do you have a pipeline for building the x64 musl binaries?

Yes.

Would this pipeline work on arm64?

I don't believe so. We don't have arm64 hardware.

@danifv
Copy link
Contributor Author

danifv commented Apr 27, 2023

Would this pipeline work on arm64?

I don't believe so. We don't have arm64 hardware.

Unfortunately I don't have personal access to arm64 hardware, and as far as I've checked github.com actions/gitlab.com runners don't provide arm64 hardware either.
I'm not familiar with your infrastructure, but isn't qemu an option for you? Wouldn't this be easier to set up than to depend on 3rd party builds?

@BillyONeal
Copy link
Member

I'm not familiar with your infrastructure, but isn't qemu an option for you?

I really don't have any idea. (This is kind of a "I lost my yo-yo." "Where did you have it last?" "Hey! If I knew that, I would still have my yo-yo!" situation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bootstrap.sh downloads x64 binary when run under arm64 Alpine Linux
4 participants