Qt: default options should not be dependent on CI environment #23911
+2
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🤖 Beep Boop! This pull request is making changes to 'recipes/qt//'. 👋 @ericLemanissier @jwillikers @MartinDelille @paulharris you might be interested. 😉 |
|
@jcar87 you want to set the option to False by default or the build time and space will be too important for C3I |
Was the recipe building those Qt modules before that change was put in place? Or is the "essential" modules group a completely different set of modules compared to the previous defaults? Unrelated to this PR but will also make sure we update the Xcode 13 compiler on CI so that it can be built on macOS as well. That one is easy to fix |
|
I detected other pull requests that are modifying qt/6.x.x recipe:
This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This was referenced May 7, 2024
|
I detected other pull requests that are modifying qt/5.x.x recipe:
This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
|
Setting the option to False will build only qtbase, which is what was built until I introduced this new option. Regarding the usefulness of a qtbase only build, it is very unlikely that consumers use only qtbase. Most projects also use other modules like qttools, qtquick etc.. so the binaries produced by C3I are very close to useless regardless ot the default value of options outside C3I Rules regarding the use of this environment variable should be documented (with a short explanation of the motivation) to avoid future problems. I can change the default value in my PR on qt which is waiting for team review #23571 |
Apologies that the PR that introduced this use was approved and merged - this should not have happened. This one is on us! We want to avoid recipes having logic that is "exclusive" to Conan Center CI - CI is a moving target, so it's unlikely that the logic stands the test of time anyway, and it's an anti-pattern to have the recipes contain logic to work around or bypass CI issues or limitations. We are working on a solution where if CI needs to be skipped for any configuration, we can do that outside of the recipe logic - but otherwise, and for avoidance of doubt: this variable must not be used other than the one place in the Qt recipe in the Troubleshooting issues like this: conan-io/conan#16215 is very time consuming, as we do not expect the default options to be dependent on the environment. Users rely con |
What is a good default for Qt in terms of modules to build? |
This comment has been minimized.
This comment has been minimized.
|
By default (outside of conan), qt builds all modules for which mandatory dependencies are detected. On my machine calling So even non-essential modules like qtmultimedia are enabled by default. |
|
I always end up having to enable Wayland and EGL for Qt for our use cases. |
This was referenced May 10, 2024
jcar87
commented
May 21, 2024
jcar87
commented
May 21, 2024
Conan v1 pipeline ✔️All green in build 3 (
Conan v2 pipeline ✔️
All green in build 3 (
|
Hooks produced the following warnings for commit d9704abqt/5.15.11@#0549bf9b418d3d156e6fa0edbc4d948dqt/5.15.12@#b4d1432b04bfbba8eced7e4edc508821qt/5.15.10@#5820cebd347eafd74a24cf77cc46529bqt/5.15.13@#1e3e0c28041f4a150d3bc0033f3cae4cqt/5.15.9@#097767e6fb873285ce7ab3e96cf035e1qt/6.7.0@#09d03f6d1c35f1879ad2f2629bdefaefqt/6.6.2@#5f538bd98c62ff8f7dcb0833960165e8qt/6.6.3@#e6a381f2f800d61d1acd41d4b37f418fqt/6.6.1@#bda3f2cc90913192268482568f10eec0qt/6.6.0@#1c4e6644b80d20c9e005f3d5ab9b262dqt/6.5.3@#70aa885db5bdea39ce9a1461ad50253eqt/6.3.2@#35e4ddada939f22eca110323d858e8d5qt/6.4.2@#4006db2923e767bebb2435e61c498e3e |
ericLemanissier
approved these changes
May 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Qt: fix issue where CI generates binaries that differ from user environments. The environment variable was added for a specific case where CI should not skip binary generation for reasons external to the recipe itself (like CI) to raise an InvalidConfiguration (this is a stop-gap measure until we have other mechanisms to skip binary generation for specific recipes without having CI fail). In any case, the recipes in Conan Center Index must not have default values for options depend on environment variables. If CI cannot generate binaries this needs to be reflected a different way altogheter.