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

[Suggestion]: Remove /W3 being used when installing packages on Windows #26699

Closed
fsandhei opened this issue Sep 6, 2022 · 9 comments
Closed
Assignees
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed Stale

Comments

@fsandhei
Copy link

fsandhei commented Sep 6, 2022

Hi,

This might be the wrong place to post this, so here goes.

I have noticed that when vcpkg installs a package on Windows, it adds /W3 to the CXXFLAGS, see below.

set(CMAKE_CXX_FLAGS " /nologo /DWIN32 /D_WINDOWS /W3 ${CHARSET_FLAG} /GR /EHsc /MP ${VCPKG_CXX_FLAGS}" CACHE STRING "")

This causes the following warning to be generated by MSVC 2019 when we in our projects use /W4:

Command line warning D9025: overriding '/W3' with '/W4'

This warning will be generated for each translation unit. When looking through the logs for troubleshooting a failed installation, this warning produces noise.

Why does vcpkg set this warning at all? Should it not be up to the individual projects to decide their warning level preference?

Proposal
vcpkg should not set warning flags for any toolchain and leave that responsibility up to each package maintainer.

EDIT: Added link to line in question.

@Neumann-A
Copy link
Contributor

The problem actually is people having (non-default) warning levels in the build scripts instead of using what is externally supplied.

It is a upstream failure to not provide a way to suppress/deactivate their dev build flags.

@fsandhei
Copy link
Author

fsandhei commented Sep 6, 2022

The problem actually is people having (non-default) warning levels in the build scripts instead of using what is externally supplied.

Can you clarify who "people" refers to in this comment? If you mean vcpkg itself then I agree; it is wrong to set warning levels this way. If you mean the users of vcpkg, then I disagree; users should be able to build with whatever options they need.

@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Sep 6, 2022
@JackBoosY
Copy link
Contributor

The user's personal settings should be able to override this option.

@Neumann-A
Copy link
Contributor

Can you clarify who "people" refers to in this comment?

People writing build scripts for their libraries and hardcoding the warning options.

then I disagree; users should be able to build with whatever options they need.

You can. Provide a toolchain with the warning level you want and vcpkg will try to force it down all builds within vcpkg. But this will always generate warnings because people hardcode their warnings level in their build scripts ....
If you don't want the /W3 in the default windows toolchain just supply your own toolchain.

This causes the following warning to be generated by MSVC 2019 when we in our projects use /W4:

means you are hardcoding the warning level in your build script.

Should it not be up to the individual projects to decide their warning level preference?

If your project is within vcpkg as a port: No! Otherwise your project outside vcpkg using vcpkg: yes.

The user's personal settings should be able to override this option.

The toolchain is a personal setting within vcpkg, so it has to take control.

@craigscott-crascit
Copy link

CMake hasn't added /W3 by default since CMake 3.15. This behavior is controlled by CMake's policy settings (see CMP0092). For projects that update themselves to rely on that behavior, they now have to find some workaround for vcpkg forcing in a /W3 flag. This is vcpkg getting in the way, not being helpful. I shouldn't have to override CMake's defaults just to get..... its defaults!

The problem actually is people having (non-default) warning levels in the build scripts instead of using what is externally supplied.

I disagree. I see the problem as vcpkg making assumptions about the project that are wrong.

If you don't want the /W3 in the default windows toolchain just supply your own toolchain.

No, that's inconsiderate to users. I shouldn't even need a toolchain file at all, let alone having to provide one just to explicitly specify what amounts to CMake's default behavior.

means you are hardcoding the warning level in your build script.

And if the project wants to do that, it should be allowed to. It might even be a requirement from company policies. It's reasonably likely to be specified in CMake presets in such cases, for example.

@Neumann-A
Copy link
Contributor

@craigscott-crascit I think you are mixing outside/inside vcpkg. The issue has nothing to do with CMake or its defaults. This is purely inside vcpkg itself

I see the problem as vcpkg making assumptions about the project that are wrong.

vcpkg does not assume anything it just passes a CMAKE_TOOLCHAIN_FILE/VCPKG_CHAINLOAD_TOOLCHAIN_FILE with the warning flag set. (which is vcpkg/scripts/toolchains/windows.cmake)

No, that's inconsiderate to users. I shouldn't even need a toolchain file at all, let alone having to provide one just to explicitly specify what amounts to CMake's default behavior.

vcpkg internally always passes a (chainload) toolchain since that is the ground truth for the build of (nearly) all ports. If the users do not like the default behavior, they are free to supply their own via the triplet by setting VCPKG_CHAINLOAD_TOOLCHAIN_FILE

And if the project wants to do that, it should be allowed to. It might even be a requirement from company policies. It's reasonably likely to be specified in CMake presets in such cases, for example.

this is an issue external to vcpkg and I don't mind warning flags in CMake presets. Warning flags set within CMakeLists.txt are, in general, just plain wrong (especially /WX or -Werror) if there is no way to opt out of them. (but that is OT here)

they now have to find some workaround for vcpkg forcing in a /W3 flag.

vcpkg is not forcing /W3 to anything outside of vcpkg. Meaning if you use vcpkg.cmake as a toolchain, there is no change in compiler flags in your current CMakeLists.txt.
Your dependencies from the manifest (vcpkg.json) however will be build with the settings of the selected triplet.

@craigscott-crascit
Copy link

Well my situation is I'm working on an internal vcpkg registry, building an internal project. I hit exactly this problem because vcpkg automatically and unexpectedly added /W3. That internal repo enforces certain warning settings according to company directions. So this isn't something that is purely inside vcpkg. It affects all projects that build through vcpkg.

If the users do not like the default behavior, they are free to supply their own via the triplet by setting VCPKG_CHAINLOAD_TOOLCHAIN_FILE

As I stated before, in this case users want the default behavior.... CMake's default behavior, not the overridden "default" behavior that vcpkg applies as its replacement. One of vcpkg's stated goals is that projects shouldn't have to add vcpkg-specific logic, but that's precisely what this behavior is causing.

vcpkg is not forcing /W3 to anything outside of vcpkg. Meaning if you use vcpkg.cmake as a toolchain, there is no change in compiler flags in your current CMakeLists.txt.

That is not what I observe.

@Neumann-A
Copy link
Contributor

It affects all projects that build through vcpkg.

yeah because VCPKG_CHAINLOAD_TOOLCHAIN_FILE is the truth for stuff build internally by vcpkg.

As I stated before, in this case users want the default behavior.... CMake's default behavior, not the overridden "default" behavior that vcpkg applies as its replacement. One of vcpkg's stated goals is that projects shouldn't have to add vcpkg-specific logic, but that's precisely what this behavior is causing.

Setup a new triplet: x64-windows-cmake-default. Set VCPKG_CHAINLOAD_TOOLCHAIN_FILE to an empty file and also set VCPKG_LOAD_VCVARS_ENV to ON (to keep vcvars being loaded by vcpkg) -> get whatever cmake thinks is the default in that env. (also some cmake defaults doesn't make sense in the context of vcpkg, especially for debug builds.)

You don't need vcpkg specific logic in your project but you still need to setup vcpkg the way you want it to be. So you just need a triplet with the dev settings you want. I for example use https://github.com/Neumann-A/my-vcpkg-triplets/blob/master/x64-win-llvm.cmake to have a clang-cl build of everything.

So again: If you want your dev compiler flags in vcpkg you need to supply a triplet with a toolchain with those flags. Otherwise vcpkg will just use its default toolchain.

That is not what I observe.

Yeah because you are not outside of vcpkg. -> It affects all projects that build through vcpkg. -> that is inside vcpkg not outside. Outside vcpkg: VCPKG_CHAINLOAD_TOOLCHAIN_FILE is not set automagically so there is no way vcpkg can import its toolchain settings into the outside project. If you build your project through vcpkg that is always inside vcpkg and VCPKG_CHAINLOAD_TOOLCHAIN_FILE gets passed by vcpkg_configure_cmake/vcpkg_cmake_configure pulling in those flags.

Copy link

This is an automated message. Per our repo policy, stale issues get closed if there has been no activity in the past 180 days. The issue will be automatically closed in 14 days. If you wish to keep this issue open, please add a new comment.

@github-actions github-actions bot added the Stale label Nov 17, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed Stale
Projects
None yet
5 participants