Skip to content

[boost-modular-build-helper] Wrap compiler flags with quotes#22371

Closed
ahojnnes wants to merge 4 commits intomicrosoft:masterfrom
ahojnnes:user/joschonb/boost-modular-build-helper
Closed

[boost-modular-build-helper] Wrap compiler flags with quotes#22371
ahojnnes wants to merge 4 commits intomicrosoft:masterfrom
ahojnnes:user/joschonb/boost-modular-build-helper

Conversation

@ahojnnes
Copy link
Contributor

@ahojnnes ahojnnes commented Jan 5, 2022

Without this PR, building boost packages under Windows with custom compiler flags fails as follows:

notice: Loading user-config configuration file 'user-config.jam' from '.../vcpkg/buildtrees/boost-container/x64-windows-rel'.
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:6: Unescaped special character in argument <cxxflags>/guard:cf
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:7: Unescaped special character in argument <cflags>/guard:cf
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/machine:x64
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/INCREMENTAL:NO
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/OPT:REF
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/OPT:ICF
...\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/guard:cf

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!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for boost-modular-build-helper but no changes to version or port version.
-- Version: 1.77.0#4
-- Old SHA: aea8b4dbb8063db29d8ac843ef6aac35478bebaa
-- New SHA: 749698488fe29b5dcf41abc98e3b8dba3cc47341
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

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!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for boost-modular-build-helper but no changes to version or port version.
-- Version: 1.77.0#4
-- Old SHA: aea8b4dbb8063db29d8ac843ef6aac35478bebaa
-- New SHA: fc56d2cf8e172d909ee1341effe945fea6c72541
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@PhoebeHui
Copy link
Contributor

@ahojnnes, could you report an issue for this PR? what compiler flag do you apply?

@ahojnnes
Copy link
Contributor Author

ahojnnes commented Jan 6, 2022

I am trying to be a good citizen :-) and enable /guard:cf and other related flags to address binskim suggestions.

@PhoebeHui PhoebeHui added the category:port-bug The issue is with a library, which is something the port should already support label Jan 6, 2022
@PhoebeHui
Copy link
Contributor

@ahojnnes, I reproed the issue when I added the flag to triplets\x64-window.cmake, and test boost-container.

set(VCPKG_CXX_FLAGS "/guard:cf")
set(VCPKG_C_FLAGS "/guard:cf")

cc @yurybura

@ahojnnes
Copy link
Contributor Author

ahojnnes commented Jan 6, 2022

@PhoebeHui Yes, exactly, that's what I was trying to achieve.

@yurybura
Copy link
Contributor

yurybura commented Jan 6, 2022

@PhoebeHui @ahojnnes If I understand correctly build doesn't fail.
Boost.Build reports warning but applies corresponding flags.
My log:

[...]
D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:6: Unescaped special character in argument <cxxflags>/guard:cf
D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:7: Unescaped special character in argument <cflags>/guard:cf
D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/machine:x64
D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/INCREMENTAL:NO
D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/OPT:REF
D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\user-config.jam:8: Unescaped special character in argument <linkflags>/OPT:ICF
[...]
    call "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\standalone\msvc\19858a0c092d6daed3b7579665d68f18\msvc-setup.bat" amd64 >nul
 cl "..\src\dlmalloc.cpp" -c -Fo"D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\dlmalloc.obj"     -TP /wd4675 /EHs /GR /Zc:throwingNew /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /GR /EHsc /MP /guard:cf /MD /O2 /Oi /Gy /DNDEBUG /Z7 /O2 /Z7 /Ob2 /W3 /MD /Zc:forScope /Zc:wchar_t /Zc:inline /Gw /favor:blend -DBOOST_ALL_NO_LIB=1 -DBOOST_CONTAINER_DYN_LINK=1 -DNDEBUG "-I..\include" "-ID:\libs\vcpkg\installed\x64-windows\include" 
[...]
        call "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\standalone\msvc\19858a0c092d6daed3b7579665d68f18\msvc-setup.bat" amd64 >nul
 link /NOLOGO /INCREMENTAL:NO "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\alloc_lib.obj" "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\dlmalloc.obj" "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\global_resource.obj" "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\monotonic_buffer_resource.obj" "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\pool_resource.obj" "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\synchronized_pool_resource.obj" "D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\unsynchronized_pool_resource.obj"      /DEBUG /MACHINE:X64 /MANIFEST:EMBED /OPT:REF,ICF /subsystem:console /out:"D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\boost_container-vc142-mt-x64-1_78.dll"   /DLL /IMPLIB:"D:\libs\vcpkg\buildtrees\boost-container\x64-windows-rel\boost\build\aa84e6e880a9d383ddf248bc50c22218\boost_container-vc142-mt-x64-1_78.lib" 
[...]

@ahojnnes
Copy link
Contributor Author

ahojnnes commented Jan 6, 2022

@yurybura The warnings are not a blocker, but see the annotated code line above. Without this, the linker flags are just ignored.

@yurybura
Copy link
Contributor

yurybura commented Jan 6, 2022

@yurybura The warnings are not a blocker, but see the annotated code line above. Without this, the linker flags are just ignored.

@ahojnnes Ok, thank you. I think this is the real issue - Boost.Build ignores linker flags defined for msvc toolset in user_config.jam.

@yurybura
Copy link
Contributor

yurybura commented Jan 7, 2022

@yurybura The warnings are not a blocker, but see the annotated code line above. Without this, the linker flags are just ignored.

@ahojnnes Ok, thank you. I think this is the real issue - Boost.Build ignores linker flags defined for msvc toolset in user_config.jam.

@Kojoley Could you help to understand why linker flags defined in the user_config.jam are ignored? Thanks in advance.

@Kojoley
Copy link

Kojoley commented Jan 8, 2022

@Kojoley Could you help to understand why linker flags defined in the user_config.jam are ignored? Thanks in advance.

That's a bug in B2, and it seems that there should be the same bug for asmflags. For some historical reasons msvc.jam uses different names than common.handle-options and other toolsets use.

@PhoebeHui
Copy link
Contributor

Relate to #20697

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/

@PhoebeHui PhoebeHui added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 30, 2022
@ras0219-msft
Copy link
Contributor

Adding this doc link for posterity: https://grafikrobot.github.io/b2doc/#jam.language.lexical

@ras0219-msft
Copy link
Contributor

Looking at the documentation, it seems like we can actually take a much simpler approach here.

using gcc : 5 : : <cxxflags>"-std=c++14 -O2" ;

is claimed to work. This means that instead of trying to split on spaces, we should be able to do a very simple fix of:

  1. Get everything concatenated into a single string a la CXXFLAGS/CFLAGS like all the other buildsystems
  2. Escape \ and " with \ (either as one step via regex or two simple string replacements)
  3. Wrap the whole enchilada in quotes and prepend the appropriate <fooflags>.

I believe this should fix multiple current issues, including this one and #22840.

I think when I originally wrote this code I didn't find that part of the docs and assumed that boost build would apply its own layers of escaping to the arguments (such as surrounding every <cxxflags> entry in quotes to ensure it was passed as a single argument to the compiler). That's why we didn't previously take the approach I've described above.

@BillyONeal BillyONeal removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Feb 3, 2022
@PhoebeHui
Copy link
Contributor

Ping @ahojnnes for your response!

@ahojnnes
Copy link
Contributor Author

ahojnnes commented Feb 8, 2022

What @ras0219-msft proposes seems like the right approach and simplifies the whole script. I am supporting to take this approach, but I don't have any cycles to tackle the issues anytime soon to be honest.

@PhoebeHui
Copy link
Contributor

Closing in favor of #23001.

@PhoebeHui PhoebeHui closed this Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants