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

Qt6: change variable setting from FEATURE_cxx to QT_FEATURE_cxx #25652

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

weynaa
Copy link

@weynaa weynaa commented Oct 18, 2024

Summary

Changes to recipe: qt/6.x.x

Motivation

See #25651
With this change the CMAKE_CXX_STANDARD property is propagated correctly to the Qtbase submodule and most likely others aswell.

D:\dev\conan-center-index\recipes\qt\6.x.x (fix-qt-cxx-flag -> fork)
λ conan create . --version=6.7.3 -s compiler.cppstd=20
....
qt/6.7.3: RUN: cmake -G "Ninja" -DCMAKE_TOOLCHAIN_FILE="D:/conan-cache/b/qtd417356601dd9/b/build/generators/conan_toolchain.cmake" -DCMAKE_INSTALL_PREFIX="D:/conan-cache/b/qtd417356601dd9/p" -DCMAKE_TRY_COMPILE_CONFIGURATION="Release" -DQT_USE_VCPKG="OFF" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" -DCMAKE_BUILD_TYPE="Release" "D:/conan-cache/qt9bbf649b16297/s/src"
conanvcvars.bat: Activating environment Visual Studio 17 - amd64 - winsdk_version=None - vcvars_ver=14.4
[vcvarsall.bat] Environment initialized for: 'x64'
-- Using Conan toolchain: D:/conan-cache/b/qtd417356601dd9/b/build/generators/conan_toolchain.cmake
-- Conan toolchain: Setting CMAKE_MSVC_RUNTIME_LIBRARY=$<$<CONFIG:Release>:MultiThreadedDLL>
-- Conan toolchain: C++ Standard 20 with extensions OFF
-- Conan toolchain: Setting BUILD_SHARED_LIBS = OFF
-- The CXX compiler identification is MSVC 19.41.34123.0
-- The C compiler identification is MSVC 19.41.34123.0
-- The ASM compiler identification is MSVC
-- Found assembler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.41.34120/bin/Hostx64/x64/cl.exe
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.41.34120/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.41.34120/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Checking dependencies of submodule 'qtbase'
-- Configuring submodule 'qtbase'
-- [QtBase] CMAKE_BUILD_TYPE was already explicitly set to: 'Release'
            -DCMAKE_C_FLAGS=/DWIN32 /D_WINDOWS
            -DCMAKE_C_FLAGS_DEBUG=/Zi /Ob0 /Od /RTC1
            -DCMAKE_C_FLAGS_RELEASE=/O2 /Ob2 /DNDEBUG
            -DCMAKE_C_FLAGS_RELWITHDEBINFO=/Zi /O2 /Ob1 /DNDEBUG
            -DCMAKE_CXX_FLAGS=/DWIN32 /D_WINDOWS /EHsc
            -DCMAKE_CXX_FLAGS_DEBUG=/Zi /Ob0 /Od /RTC1
            -DCMAKE_CXX_FLAGS_RELEASE=/O2 /Ob2 /DNDEBUG
            -DCMAKE_CXX_FLAGS_RELWITHDEBINFO=/Zi /O2 /Ob1 /DNDEBUG
            -DCMAKE_EXE_LINKER_FLAGS=/machine:x64
            -DCMAKE_TOOLCHAIN_FILE=D:/conan-cache/b/qtd417356601dd9/b/build/generators/conan_toolchain.cmake
            -DCMAKE_C_STANDARD=11
            -DCMAKE_C_STANDARD_REQUIRED=ON
            -DCMAKE_CXX_STANDARD=20
            -DCMAKE_CXX_STANDARD_REQUIRED=ON
            -DCMAKE_MODULE_PATH:STRING=D:/conan-cache/qt9bbf649b16297/s/src/qtbase/cmake/platforms

-- Configuration summary shown below. It has also been written to D:/conan-cache/b/qtd417356601dd9/b/build/config.summary
-- Configure with --log-level=STATUS or higher to increase CMake's message verbosity. The log level does not persist across reconfigurations.

-- Configure summary:

Building for: win32-msvc (x86_64, CPU features: )
Compiler: msvc 19.41.34123.0
Build options:

Details

Rename the defined cmake configuration variables for the c++ standard from FEATURE_cxx to QT_FEATURE_cxx. As far as I see it cannot propagate the CXX_STANDARD correctly with the previous name: https://github.com/qt/qtbase/blob/bab1fd8fc3f8d6af5ad4b603c0dabdca434759a4/cmake/QtFlagHandlingHelpers.cmake#L355-L368.

This flag for cppstd was originally added in #15098, @ericLemanissier I wonder if I missed something in my testing or if there was an upstream change in Qt that introduced or changed this. As far as i see, in version 6.4.0 it was also already named QT_FEATURE_cxx20 and a while before that.


@CLAassistant
Copy link

CLAassistant commented Oct 18, 2024

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

1 similar comment
@conan-center-bot

This comment has been minimized.

@weynaa weynaa mentioned this pull request Oct 18, 2024
@ericLemanissier
Copy link
Contributor

Hi, I just followed documentation https://github.com/qt/qtbase/blob/bab1fd8fc3f8d6af5ad4b603c0dabdca434759a4/cmake/configure-cmake-mapping.md?plain=1#L58
all features are documented to be passed as -DFEATURE_foo=BAR. You should report the problem on https://bugreports.qt.io/

@weynaa
Copy link
Author

weynaa commented Oct 18, 2024

Hi, Thanks for the link to the documentation. I agree i guess the user should use the FEAUTURE_xxx values without the QT_ prefix as that is supposed to be internal - still seems to be a bug from Qt though because the FEATURE_cxx20 variable does not propagate into the QT_FEATURE_cxx20 variable for some reason.

@weynaa weynaa closed this Oct 18, 2024
@valgur
Copy link
Contributor

valgur commented Oct 18, 2024

I think this PR still has merit until the issue has been fixed upstream, so please don't close. I would set both the QT_FEATURE_... and FEATURE_ variables in the CMakeToolchain as a workaround.

@weynaa weynaa reopened this Oct 18, 2024
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Warning

Conan Center will stop receiving updates for Conan 1.x packages soon - please see announcement.

Sorry, the build is only launched for Access Request users. You can request access writing in this issue.

Conan v2 pipeline ❌

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping @conan-io/barbarians on the PR and we will help you.

Sorry, the build is only launched for Access Request users. You can request access writing in this issue.

@weynaa
Copy link
Author

weynaa commented Oct 18, 2024

I think this PR still has merit until the issue has been fixed upstream, so please don't close. I would set both the QT_FEATURE_... and FEATURE_ variables in the CMakeToolchain as a workaround.

I opened a bug report with qt https://bugreports.qt.io/browse/QTBUG-126161. I got confused with the configure log because it wrongfully shows CXX_STANDARD=17 when you only set the FEATURE_cxx20 flag, but with the QT_FEATURE_cxx20 it shows the correct entry in the build log.

However, the actual -std:c++20 flag sent to the compiler looks to be unaffected by it after checking by just compiling qtbase verbose. Initially, i still got linkage errors as described in #15001 even with the compiler.cppstd=20 setting. I think that was caused by me being not careful and accidentally downloading an official package from conan-center and probabaly some other cached package/buildtree was used for my project:

> conan install --requires=qt/6.7.3 -s compiler.cppstd=20 -u
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=msvc
compiler.cppstd=20
compiler.runtime=dynamic
compiler.runtime_type=Release
compiler.version=194
os=Windows
[options]
with_qt=True
[conf]


Profile build:
[settings]
arch=x86_64
build_type=Release
compiler=msvc
compiler.cppstd=20
compiler.runtime=dynamic
compiler.runtime_type=Release
compiler.version=194
os=Windows
[conf]

....

qt/6.7.3: Found compatible package 'd096538b2a88cc969d69186b84eb615b739c2ed3': compiler.cppstd=17, compiler.version=193

Maybe there is a different problem here though: cppstd=17 and cppstd=20 are not ABI compatible in MSVC if you use coroutines (https://devblogs.microsoft.com/oldnewthing/20230111-00/?p=107694) but the recipie still thinks its compatible here and downloads a binary package online. If desired i can open a separate issue for that.

I was able to solve my issue by doing a clean build:

> conan remove qt*
> conan install --requires=qt/6.7.3 --build missing --build qt* -s compiler.cppstd=20

Feel free to close if no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants