Skip to content

[boost-modular-build] Fix lack of arm64-linux support#10814

Merged
strega-nil merged 7 commits intomicrosoft:masterfrom
ddavidhahn:master
Apr 27, 2020
Merged

[boost-modular-build] Fix lack of arm64-linux support#10814
strega-nil merged 7 commits intomicrosoft:masterfrom
ddavidhahn:master

Conversation

@ddavidhahn
Copy link
Contributor

This PR fixes this issue: #8644

This change updates boost modular build to utilize x64-linux build tools when building arm64-linux. Prior to this change, running boost-modular-build.cmake for arm64-linux would result in an error message suggesting the user installs x86-windows tools which are not available on Linux machines.

Note that arm64-linux here refers to a triplet which sets VCPKG_TARGET_ARCHITECTURE to "arm64" and VCPKG_CMAKE_SYSTEM_NAME to "Linux".

@ddavidhahn ddavidhahn changed the title Fix lack of arm64-linux support [boost-modular-build] Fix lack of arm64-linux support Apr 13, 2020
@PhoebeHui PhoebeHui self-assigned this Apr 14, 2020
@ddavidhahn
Copy link
Contributor Author

Note, the failure for argumentum:x86-windows in the PR test 33540 seems to be a regression introduced into master. I've seen several other pipeline runs fail due to it and I don't believe this change should impact the failure of that test.

@PhoebeHui
Copy link
Contributor

@ddavidhahn, thanks for the PR, I have rerun the CI test.

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

The changes looks good to me, since arm64-linux triplet is not support by default now, this is not test by CI, however, users might need some failure message when they try to use custome triplet arm64-linux

@PhoebeHui
Copy link
Contributor

@yurybura, would you like to review the change?

Copy link
Contributor

@yurybura yurybura left a comment

Choose a reason for hiding this comment

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

Please update version in ./boost-modular-build-helper/CONTROL

@msftclas
Copy link

msftclas commented Apr 14, 2020

CLA assistant check
All CLA requirements met.

@ddavidhahn
Copy link
Contributor Author

Thank you for taking a look @PhoebeHui @yurybura !

Went ahead and bumped up the version according to https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/control-files.md

Please let me know if that doesn't look right.

@ddavidhahn ddavidhahn requested a review from PhoebeHui April 14, 2020 22:27
@ddavidhahn
Copy link
Contributor Author

Looks like this failed due to an issue with the build agent. I think this failure is unrelated to the change. @PhoebeHui could you re-trigger the build? I wasn't able to see an option that would allow me to self-trigger.

@yurybura
Copy link
Contributor

Related to #6217

@ddavidhahn
Copy link
Contributor Author

@yurybura @PhoebeHui Are there any additional concerns with checking this in? I think it's a relatively small impact change.

@ddavidhahn
Copy link
Contributor Author

@PhoebeHui Gentle ping on this. Any reasons not to merge it? :)

@LilyWangL LilyWangL self-assigned this Apr 21, 2020
@LilyWangL LilyWangL requested a review from ras0219-msft April 21, 2020 02:13
@LilyWangL
Copy link
Contributor

@ras0219-msft Could you take a look this PR? Thanks.

@ddavidhahn
Copy link
Contributor Author

Gentle ping on this @ras0219-msft

@LilyWangL LilyWangL added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 27, 2020
@strega-nil
Copy link
Contributor

strega-nil commented Apr 27, 2020

Cool, LGTM :) Thanks @ddavidhahn

@strega-nil strega-nil merged commit 89d112b into microsoft:master Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants