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

[boost-modular-build-helper] Ensure CMAKE_STATIC_LINKER_FLAGS makes it to the link.exe command line when building Boost as a static lib with MSVC. #20697

Closed
wants to merge 3 commits into from

Conversation

PDeets
Copy link
Member

@PDeets PDeets commented Oct 13, 2021

I have a custom triplet that builds static libs for ARM64EC (https://blogs.windows.com/windowsdeveloper/2021/06/28/announcing-arm64ec-building-native-and-interoperable-apps-for-windows-11-on-arm/). When building as ARM64EC, CMAKE_STATIC_LINKER_FLAGS is correctly populated to "/machine:ARM64X /nologo". However, this is passed to b2 via "linkflags". When b2 builds a static lib with MSVC, it uses "archiveflags" instead of "linkflags" for the command line settings for link.exe. This PR makes it so the CMAKE_STATIC_LINKER_FLAGS value is passed to b2 as "archiveflags" when building Boost as a static lib with MSVC. This enables me to build boost as ARM64EC.

  • What does your PR fix?

Fixes the build for boost when using a custom triplet for building a static lib targeting ARM64EC.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

This is for a custom triplet of my own which isn't currently checked into the vcpkg repo.

Yes. (I reviewed it and nothing seemed to apply in this case.)

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 050ee6cce0ea11b57cd2ae60857af839dfbe4be6 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/b-/boost-modular-build-helper.json b/versions/b-/boost-modular-build-helper.json
index 58454cf..8c415d0 100644
--- a/versions/b-/boost-modular-build-helper.json
+++ b/versions/b-/boost-modular-build-helper.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "0a69ebce8ad56686604bd3d5d790e531bcde5544",
+      "git-tree": "195c01960f4868776b587d13396942768774c417",
       "version": "1.77.0",
       "port-version": 3
     },

@PhoebeHui PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 13, 2021
@PhoebeHui PhoebeHui self-assigned this Oct 13, 2021
@PhoebeHui PhoebeHui changed the title Ensure CMAKE_STATIC_LINKER_FLAGS makes it to the link.exe command line when building Boost as a static lib with MSVC. [boost-modular-build-helper] Ensure CMAKE_STATIC_LINKER_FLAGS makes it to the link.exe command line when building Boost as a static lib with MSVC. Oct 13, 2021
@PhoebeHui
Copy link
Contributor

cc @yurybura

@PDeets PDeets force-pushed the pdeets/BoostStaticLibArm64EC branch from 2e97d09 to 41460bf Compare February 1, 2022 02:26
@PDeets
Copy link
Member Author

PDeets commented Feb 1, 2022

I just rebased this to be on the latest commit. I also fixed up the issues with the version files. If the PR build goes well, I plan to make this no longer be a draft.

(In my project that uses vcpkg, I've been overwriting this file with a patched copy as part of a script whenever I install vcpkg dependencies; so I had a workaround, but now I want to upstream this so I can remove my hack.)

@PDeets PDeets marked this pull request as ready for review February 2, 2022 01:04
@PDeets
Copy link
Member Author

PDeets commented Feb 2, 2022

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

The version issue is addressed now. I can't find any way to resolve the comment from the bot.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/boost-modular-build-helper/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

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 LGTM, however, I'm not fimiliar with flags. From the https://www.boost.org/doc/libs/1_78_0/tools/build/doc/html/index.html,
archiveflags: The value of this feature is passed without modification to the archiver tool when creating static libraries.

I'm not sure if it is correct solution for the case you mentioned, I didn't find a detail doc about it. Anyway, I marked this PR to 'reviewed'.

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Feb 8, 2022
@PDeets
Copy link
Member Author

PDeets commented Feb 8, 2022

Thank you.

@PDeets PDeets closed this Feb 8, 2022
@PDeets PDeets reopened this Feb 8, 2022
@PDeets
Copy link
Member Author

PDeets commented Feb 8, 2022

Sorry, I clicked "Close with comment" thinking that would merge the PR, but it just closed it without merging; so I reopened it.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.
If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/boost-modular-build-helper/vcpkg.json

Valid values for the license field are listed at https://spdx.org/licenses/

@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Feb 16, 2022
@BillyONeal
Copy link
Member

I believe we should do #23001 instead which solves the problem more generally for all build flags rather than only STATIC_LINKER_FLAGS. We'll see where that goes.

@PDeets
Copy link
Member Author

PDeets commented Feb 16, 2022

I tried out Robert's fix that you referred to. It doesn't seem to address my issue. The compile command lines are correct:

compile-c-c++ D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\alloc_lib.obj

    call "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\standalone\msvc\b8cab6c23bafdda38fa0399294b4cb12\msvc-setup.bat" x86_arm64 >nul
cl "..\src\alloc_lib.c" -c -Fo"D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\alloc_lib.obj"     -TC -nologo -DWIN32 -D_WINDOWS -W3 -utf-8 -MP -arm64EC -DWIN32 -D_AMD64_ -DAMD64 -D_ARM64EC_ -DARM64EC -D_ARM64EC_WORKAROUND_ -D_WINDOWS -D_DEBUG -MTd -Z7 -Ob0 -Od -RTC1 -D_ITERATOR_DEBUG_LEVEL=1 /Z7 /Od /Ob0 /W3 /MTd /Zc:forScope /Zc:wchar_t /Zc:inline -DBOOST_ALL_NO_LIB=1 -DBOOST_CONTAINER_STATIC_LINK=1 "-I..\include" "-ID:\git\validation.taef3\vcpkg\submodule\installed\arm64ec-windows-custom\include" 

alloc_lib.c

This correctly includes "-arm64EC" on the command line. However, the linker flags are not passed to link.exe:

msvc.archive D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\libboost_container-vc143-mt-sgd-a64-1_78.lib

        if exist "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\libboost_container-vc143-mt-sgd-a64-1_78.lib" DEL "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\libboost_container-vc143-mt-sgd-a64-1_78.lib"
        call "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\standalone\msvc\b8cab6c23bafdda38fa0399294b4cb12\msvc-setup.bat" x86_arm64 >nul
link /lib /NOLOGO  /out:"D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\libboost_container-vc143-mt-sgd-a64-1_78.lib" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\alloc_lib.obj" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\dlmalloc.obj" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\global_resource.obj" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\monotonic_buffer_resource.obj" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\pool_resource.obj" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\synchronized_pool_resource.obj" "D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\unsynchronized_pool_resource.obj"   
    
D:\git\validation.taef3\vcpkg\submodule\buildtrees\boost-container\arm64ec-windows-custom-dbg\boost\build\21132b4af385ed13a7e7d6293d937cec\alloc_lib.obj : fatal error LNK1392: Incompatible object type found. Did you mean /machine:arm64x or /machine:arm64ec?

There is no "/machine:arm64x" or "/machine:arm64ec" switch on the "link /lib" command line. My understand is this is because b2 is using "msvc.archive", which looks at archive_flags instead of link_flags.

The generated user-config.jam file passes the value via link_flags:

using msvc : 143 : "D:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.31.31103/bin/Hostx64/arm64/cl.exe"
    :
    <setup>"D:/git/validation.taef3/vcpkg/submodule/installed/x64-windows/share/boost-build/nothing.bat"
    <cxxflags>"-nologo -DWIN32 -D_WINDOWS -W3 -utf-8 -GR -EHsc -MP -arm64EC -DWIN32 -D_AMD64_ -DAMD64 -D_ARM64EC_ -DARM64EC -D_ARM64EC_WORKAROUND_ -D_WINDOWS -EHsc -D_DEBUG -MTd -Z7 -Ob0 -Od -RTC1 -D_ITERATOR_DEBUG_LEVEL=1"
    <cflags>"-nologo -DWIN32 -D_WINDOWS -W3 -utf-8 -MP -arm64EC -DWIN32 -D_AMD64_ -DAMD64 -D_ARM64EC_ -DARM64EC -D_ARM64EC_WORKAROUND_ -D_WINDOWS -D_DEBUG -MTd -Z7 -Ob0 -Od -RTC1 -D_ITERATOR_DEBUG_LEVEL=1"
    <asmflags>"-nologo -DWIN32 -D_WINDOWS -W3 -utf-8 -MP -arm64EC -DWIN32 -D_AMD64_ -DAMD64 -D_ARM64EC_ -DARM64EC -D_ARM64EC_WORKAROUND_ -D_WINDOWS -D_DEBUG -MTd -Z7 -Ob0 -Od -RTC1 -D_ITERATOR_DEBUG_LEVEL=1"
    <linkflags>"-machine:ARM64X -nologo"

    ;

That is the issue my PR is designed to address. It makes sure the link flags are passed as "archive_flags" when building a static lib.

@Neumann-A
Copy link
Contributor

There is no "/machine:arm64x" or "/machine:arm64ec" switch on the "link /lib" command line. My understand is this is because b2 is using "msvc.archive", which looks at archive_flags instead of link_flags.

shouldn't vcvars magically define the machine type so it isn't needed on the link line?

@PDeets
Copy link
Member Author

PDeets commented Feb 18, 2022

There is no "/machine:arm64x" or "/machine:arm64ec" switch on the "link /lib" command line. My understand is this is because b2 is using "msvc.archive", which looks at archive_flags instead of link_flags.

shouldn't vcvars magically define the machine type so it isn't needed on the link line?

ARM64EC is built with the same vcvars configuration as plain ARM64. You opt into ARM64EC using the -arm64EC command line switch to cl.exe, not by starting vcvars with a different parameter. If it was automatic based on vcvars, then it would pick /machine:arm64, which is not what is needed for ARM64EC. When using "link /lib", you need to use /machine:arm64ec or /machine:arm64x when building ARM64EC code.

@PDeets
Copy link
Member Author

PDeets commented Feb 19, 2022

Robert updated his change to address this. I tested the latest update and it does fix the issue; so I'm closing this merge request now since Robert's change will address it.

@PDeets PDeets closed this Feb 19, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants