[qt5-base] Move 'gui' and 'icu' out of 'core' feature #23668

dg0yt · 2022-03-20T13:02:40Z

What does your PR fix?

This PR creates separate gui and icu features, reducing the number of dependencies and the build time for the core feature.
- Adds an icu opt-in feature, reducing build time and deployment size of the default configuration.
  (It might make sense to leave it as a default feature to avoid a change for currents users.)
- Adds a gui opt-out feature to qt5-base and qt5-tools, meant to benefit cross builds which have a host dependency on qt5-base[core] or qt5-tools[core].
  (This PR is separated out of work to fix building qt5 for mingw, but android and other triplets might benefit as well.)
- Adds a quick opt-out feature to qt5-declarative, meant to benefit users which only need Qt Widgets, not Qt Quick.
In addition, there is some minor cleanup.
Which triplets are supported/not supported? Have you updated the CI baseline?

all, no
Does your PR follow the maintainer guide?

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

yes

dg0yt · 2022-03-21T08:12:18Z

This should be beneficial for cross builds which have a host dependency on qt5-base[core].

... if vcpkg wouldn't install default features for host dependencies: microsoft/vcpkg-tool#177.

Osyotr · 2022-03-24T15:51:46Z

ports/qt5-base/vcpkg.json

ICU can also be moved into a separate/gui feature.

Not sure how much work we want to put in Qt5. But I admit that I still use Qt5 without icu (and openssl) because I don't want to deploy it.

Elapsed time for package icu:x64-windows: 5.296 min
This would be a significant improvement IMHO

github-actions

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 5d66a98f64188bbd806b8e62e66c097d90eb3224 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index dc7d7dd..7e988d1 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5682,7 +5682,7 @@
     },
     "qt5-base": {
       "baseline": "5.15.3",
-      "port-version": 2
+      "port-version": 3
     },
     "qt5-canvas3d": {
       "baseline": "0",
@@ -5702,11 +5702,11 @@
     },
     "qt5-declarative": {
       "baseline": "5.15.3",
-      "port-version": 0
+      "port-version": 1
     },
     "qt5-doc": {
       "baseline": "5.15.3",
-      "port-version": 0
+      "port-version": 1
     },
     "qt5-gamepad": {
       "baseline": "5.15.3",
@@ -5762,7 +5762,7 @@
     },
     "qt5-script": {
       "baseline": "5.15.3",
-      "port-version": 0
+      "port-version": 1
     },
     "qt5-scxml": {
       "baseline": "5.15.3",
@@ -5786,15 +5786,15 @@
     },
     "qt5-svg": {
       "baseline": "5.15.3",
-      "port-version": 0
+      "port-version": 1
     },
     "qt5-tools": {
       "baseline": "5.15.3",
-      "port-version": 0
+      "port-version": 1
     },
     "qt5-translations": {
       "baseline": "5.15.3",
-      "port-version": 0
+      "port-version": 1
     },
     "qt5-virtualkeyboard": {
       "baseline": "5.15.3",
diff --git a/versions/q-/qt5-base.json b/versions/q-/qt5-base.json
index a14cf4e..7f87e3f 100644
--- a/versions/q-/qt5-base.json
+++ b/versions/q-/qt5-base.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "1c67b483e9292c1d113f15ea9115cf24b82d2cbe",
+      "version": "5.15.3",
+      "port-version": 3
+    },
     {
       "git-tree": "abaaa592badf33576f197d915ed1f4ddd91d0931",
       "version": "5.15.3",
diff --git a/versions/q-/qt5-declarative.json b/versions/q-/qt5-declarative.json
index 734ecb9..fa3b93e 100644
--- a/versions/q-/qt5-declarative.json
+++ b/versions/q-/qt5-declarative.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "aadf2445a49c7128dc94041c0340bd0a070a428e",
+      "version": "5.15.3",
+      "port-version": 1
+    },
     {
       "git-tree": "51388bc681cd6616f0d2ecb29b1f3901b2e66613",
       "version": "5.15.3",
diff --git a/versions/q-/qt5-doc.json b/versions/q-/qt5-doc.json
index df590ce..6594b60 100644
--- a/versions/q-/qt5-doc.json
+++ b/versions/q-/qt5-doc.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "040d157ef0edc633385b1c954361a71717d733ec",
+      "version": "5.15.3",
+      "port-version": 1
+    },
     {
       "git-tree": "5cecc56a1a87cf86f87dcde5bc650f8de1e53b60",
       "version": "5.15.3",
diff --git a/versions/q-/qt5-script.json b/versions/q-/qt5-script.json
index 491fdc3..c7d47a7 100644
--- a/versions/q-/qt5-script.json
+++ b/versions/q-/qt5-script.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "1fdb0d31262faf4d6fbd9a28b640bd07d7ff51a6",
+      "version": "5.15.3",
+      "port-version": 1
+    },
     {
       "git-tree": "450d4c10bbd324c74ddca59e7d16c6cb215aa077",
       "version": "5.15.3",
diff --git a/versions/q-/qt5-svg.json b/versions/q-/qt5-svg.json
index f57f54b..e8e2e1d 100644
--- a/versions/q-/qt5-svg.json
+++ b/versions/q-/qt5-svg.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "427484032aebaf36b9a199f512426d3cf1b6d424",
+      "version": "5.15.3",
+      "port-version": 1
+    },
     {
       "git-tree": "6a43bcf48e884abdd82502f747342962e2a2c4b8",
       "version": "5.15.3",
diff --git a/versions/q-/qt5-tools.json b/versions/q-/qt5-tools.json
index a5deb8a..cdfa063 100644
--- a/versions/q-/qt5-tools.json
+++ b/versions/q-/qt5-tools.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "8c04a30b3417432d2e5e5e3f1bc079af25a47e82",
+      "version": "5.15.3",
+      "port-version": 1
+    },
     {
       "git-tree": "0f3066692eacc724dde9d8f17d4cb67f019ecc75",
       "version": "5.15.3",
diff --git a/versions/q-/qt5-translations.json b/versions/q-/qt5-translations.json
index b2f540c..af7ba39 100644
--- a/versions/q-/qt5-translations.json
+++ b/versions/q-/qt5-translations.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "2af296af0544fcd8e3c1fa0977c57bcaab273732",
+      "version": "5.15.3",
+      "port-version": 1
+    },
     {
       "git-tree": "4a0df59e14f9c24d2584e7f0424424f50eb4c9f7",
       "version": "5.15.3",

github-actions

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 qt5-declarative but no changes to version or port version.
-- Version: 5.15.3#1
-- Old SHA: aadf2445a49c7128dc94041c0340bd0a070a428e
-- New SHA: b010a6c15d9a48abe23c23eae1787d76d917340a
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for qt5-tools but no changes to version or port version.
-- Version: 5.15.3#1
-- Old SHA: d328616f1d98d31b87c359c9d8670fcfb34f7a69
-- New SHA: 06285aadc34def5c56cca71330c987996a42ebc5
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

JackBoosY · 2022-04-22T02:13:45Z

cc @Neumann-A

dg0yt · 2022-04-22T06:04:58Z

It is basically good enough to review, although downstream ports will need changes to fully leverage qt5-base feature control.
In any case, this will be the base for the mingw (or more general: cross-build) changes which are another PR.

Neumann-A · 2022-04-22T10:15:06Z

ports/qt5-tools/vcpkg.json

need to be finer grained/renamed. Doesn't make sense for qttools to have a gui feature.

qt5-tools includes command-line host tools (lupdate, lrelease) as well as GUI target tools (Qt Linguist, Qt Assistant). AFAICT the target tools are barely usable on Android.
(There are also libs (Qt Help) which may include GUI features, even if not needed by the command line tools.)

I had a "quick" feature at an earlier stage of this PR, and I would be glad to to add it again later, to completely opt-out from the qt5-declarative dependency. But this isn't well supported in Qt5, so it is no longer part of this PR. (It is still visible in the libs variable in the portfile.)

I think you want an additional widgets feature in qt5-base which would automatically control the builds of the tools here.

dg0yt · 2022-07-26T17:48:28Z

Requires review, not author-response.

dg0yt · 2022-07-27T07:22:19Z

If I gate everything only by the qt5-base[gui] feature, it means that for example vcpkg install qt5-svg will be empty and not install Qt5 SVG if only qt5-base[core] is installed. Okay?

dg0yt · 2022-07-30T03:50:15Z

If I gate everything only by the qt5-base[gui] feature, it means that for example vcpkg install qt5-svg will be empty and not install Qt5 SVG if only qt5-base[core] is installed. Okay?

I would like to keep the feature design as proposed in this PR, with qt5-base[gui], qt5-declarative[quick] and qt5-tools[gui].
Gating only on qt5-base[gui] doesn't allow to cut dependencies from depending ports (e.g. qt5-imageformats).
If there are concerns about how the feature selection is passed into each port in absence of direct options, I could still add patches for these options.

JackBoosY · 2022-08-17T09:43:12Z

I think you should make agreement with @Neumann-A first, sorry for forgot this PR.

dg0yt · 2022-08-21T12:57:24Z

I think you should make agreement with @Neumann-A first, sorry for forgot this PR.

Waiting for signal from @Neumann-A.

Neumann-A · 2022-08-23T10:38:21Z

Waiting for signal from @Neumann-A.

I still don't get why you would want to separate qt5-base gui from qt5-tools gui. I mean for qml/quick stuff that makes sense because it drags in the dependency of qt5-declarative but if qt5-base is already build with gui why do you require qt5-tools to be not build wit gui? For me this is just a step into feature hell.

If I gate everything only by the qt5-base[gui] feature, it means that for example vcpkg install qt5-svg will be empty and not install Qt5 SVG if only qt5-base[core] is installed. Okay?

This means qt5-svg is dependent on the gui feature and need to add it as a dependency.

Gating only on qt5-base[gui] doesn't allow to cut dependencies from depending ports (e.g. qt5-imageformats).

What dependencies are you trying to cut? (i don't think it is qt5-imageformats which probably requires the gui feature?)

Neumann-A · 2022-08-23T10:42:04Z

ports/qt5-declarative/portfile.cmake

+if(NOT "quick" IN_LIST FEATURES)
+    vcpkg_list(APPEND OPTIONS "QT.gui.name=" "QT.widgets.name=")
+endif()


Influencing qtConfig(qml-animation) seems to be the correct way instead of manipulating qtHaveModule(gui)

IMO qml-animation is to control QML animation features, not QML GUI features in general.

Yeah that might be your understand of the variable. I just look at
https://github.com/qt/qtdeclarative/blob/104eae5b17b0ec700391e9539ee3a4f638588194/src/src.pro#L19-L31 and see qtConfig(qml-animation) controlling it all (meaning directly influencing qtHaveModule(quick) by not adding the quick subdir.)

(This is probably "Controlling features in qt5-declarative by properties which correlate but semantically mean a different thing.")

That being said that for that block controlling qml-animation is the same as "QT.gui.name=" but I haven't checked the rest of the build script to see if "QT.gui.name=" and "QT.widgets.name=" have more side effects than that.

JackBoosY · 2022-09-16T08:27:28Z

Any conclusion here?

dg0yt · 2022-09-16T11:01:45Z

Any conclusion here?

I feel that some requested changes to the current state are not really acceptable or sustainable.

Controlling features in qt5-tools by presence of features in qt5-base.
Controlling features in qt5-declarative by properties which correlate but semantically mean a different thing.

I'd appreciate confirmation in one or another direction.

dg0yt · 2022-10-05T06:16:10Z

I'd appreciate confirmation in one or another direction.

Ping.

JackBoosY · 2022-10-08T06:31:51Z

I think we might need to confirm these points with @Neumann-A .

dg0yt · 2022-10-27T06:16:01Z

Any conclusion here?

I feel that some requested changes to the current state are not really acceptable or sustainable.
* Controlling features in `qt5-tools` by presence of features in `qt5-base`.

* Controlling features in `qt5-declarative` by properties which correlate but semantically mean a different thing.
I'd appreciate confirmation in one or another direction.

Ping.

Adela0814 · 2022-10-28T07:00:51Z

I think we might need to confirm these points with @Neumann-A .

@Neumann-A could you please help to review this? Thanks.

Neumann-A · 2022-10-28T07:33:27Z

Controlling features in qt5-tools by presence of features in qt5-base.

@Adela0814 That is something you guys have to decide if you want ALL the features. I don't see a reason to build qt5-base with gui and qt5-tools without it.

    vcpkg_list(APPEND OPTIONS
        "QT.quick.name="
        "config.qttools.features.assistant.disable=true"
        "config.qttools.features.designer.disable=true"
        "config.qttools.features.distancefieldgenerator.disable=true"
        "config.qttools.features.pixeltool.disable=true"
        "config.qttools.features.qtdiag.disable=true"

Ah maybe I am just annoyed by the name of the feature because it actually doesn't use "QT.gui.name=" (but that is similar to what i said in #23668 (comment); maybe just go with the qtdeclarative approach and spin out the tools with gui as gui-tools instead and the cmd list tools just as single feature).
Or just follow https://code.qt.io/cgit/qt/qttools.git/tree/configure.json?h=v5.15.7-lts-lgpl and we don't have to discuss at all. (even if it adds 16 features where probably 3-5 are platform dependent and doesn't need to be features explicitly.)

dg0yt · 2023-01-25T08:37:14Z

Controlling features in qt5-tools by presence of features in qt5-base.

I take the approval of #29153 as an indication that we don't want that.

There: control qttools dbus by qtbase dbus presence.
Here: control qt5-tools gui by qt5-base gui presence.

I admit that gui is a cross-cutting concern, and dbusviewer isn't.

Neumann-A · 2023-01-25T08:47:47Z

I take the approval of #29153 as an indication that we don't want that.

@dg0yt

Or just follow https://code.qt.io/cgit/qt/qttools.git/tree/configure.json?h=v5.15.7-lts-lgpl and we don't have to discuss at all. (even if it adds 16 features where probably 3-5 are platform dependent and doesn't need to be features explicitly.)

#29153 is exactly that. upstream feature == vcpkg feature

Adela0814 · 2023-05-05T08:14:46Z

@dg0yt Please fix the conflicts.
Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". That way, I can be aware that you've responded since you can't modify the tags.

Adela0814 · 2024-01-08T08:37:47Z

Closing this PR since it seems that no progress is being made. Please ping us to reopen if work is still being done.

Adela0814 assigned JackBoosY Mar 21, 2022

JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Mar 21, 2022

dg0yt mentioned this pull request Mar 21, 2022

Dont enable default features for host dependencies microsoft/vcpkg-tool#177

Open

Osyotr reviewed Mar 24, 2022

View reviewed changes

dg0yt force-pushed the qt-min branch from 98e344b to a087156 Compare April 15, 2022 10:43

github-actions bot requested changes Apr 15, 2022

View reviewed changes

github-actions bot approved these changes Apr 15, 2022

View reviewed changes

dg0yt force-pushed the qt-min branch from 6c7e9f1 to 296be41 Compare April 15, 2022 10:57

github-actions bot approved these changes Apr 15, 2022

View reviewed changes

dg0yt force-pushed the qt-min branch from 296be41 to 4d48011 Compare April 15, 2022 12:28

github-actions bot approved these changes Apr 15, 2022

View reviewed changes

dg0yt force-pushed the qt-min branch from 4d48011 to 71a8d37 Compare April 15, 2022 12:31

github-actions bot approved these changes Apr 15, 2022

View reviewed changes

github-actions bot approved these changes Apr 18, 2022

View reviewed changes

dg0yt force-pushed the qt-min branch from 8f39a88 to 381fc47 Compare April 18, 2022 17:51

github-actions bot approved these changes Apr 18, 2022

View reviewed changes

dg0yt mentioned this pull request Apr 18, 2022

[vcpkg-ci] Current CI problems #21905

Open

5 tasks

github-actions bot approved these changes Apr 18, 2022

View reviewed changes

github-actions bot approved these changes Apr 19, 2022

View reviewed changes

dg0yt marked this pull request as ready for review April 19, 2022 05:33

dg0yt changed the title ~~[qt5-base] Move 'gui' out of 'core' feature~~ [qt5-base] Move 'gui' and icu out of 'core' feature Apr 19, 2022

dg0yt changed the title ~~[qt5-base] Move 'gui' and icu out of 'core' feature~~ [qt5-base] Move 'gui' and 'icu' out of 'core' feature Apr 19, 2022

dg0yt marked this pull request as draft April 20, 2022 06:54

github-actions bot requested changes Apr 21, 2022

View reviewed changes

github-actions bot previously approved these changes Apr 21, 2022

View reviewed changes

dg0yt marked this pull request as ready for review April 22, 2022 05:59

Neumann-A reviewed Apr 22, 2022

View reviewed changes

dg0yt force-pushed the qt-min branch from 0ce40eb to 9b2934f Compare July 22, 2022 18:52

github-actions bot approved these changes Jul 22, 2022

View reviewed changes

dg0yt marked this pull request as ready for review July 22, 2022 19:52

JackBoosY removed the requires:author-response label Jul 27, 2022

JackBoosY added the requires:author-response label Aug 19, 2022

Neumann-A reviewed Aug 23, 2022

View reviewed changes

dg0yt mentioned this pull request Oct 10, 2022

[qt5] Fix mingw builds #20309

Closed

JackBoosY assigned Adela0814 and unassigned JackBoosY Oct 14, 2022

Osyotr mentioned this pull request Mar 8, 2023

[qt5-base] Move 'icu' out of 'core' feature #30091

Merged

7 tasks

Adela0814 removed the requires:author-response label May 5, 2023

Adela0814 marked this pull request as draft May 5, 2023 08:14

Adela0814 closed this Jan 8, 2024

[qt5-base] Move 'gui' and 'icu' out of 'core' feature #23668

[qt5-base] Move 'gui' and 'icu' out of 'core' feature #23668

Uh oh!

Conversation

dg0yt commented Mar 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

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

Does your PR follow the maintainer guide?

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

Uh oh!

dg0yt commented Mar 21, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

JackBoosY commented Apr 22, 2022

Uh oh!

dg0yt commented Apr 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Jul 26, 2022

Uh oh!

dg0yt commented Jul 27, 2022

Uh oh!

dg0yt commented Jul 30, 2022

Uh oh!

JackBoosY commented Aug 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg0yt commented Aug 21, 2022

Uh oh!

Neumann-A commented Aug 23, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neumann-A Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JackBoosY commented Sep 16, 2022

Uh oh!

dg0yt commented Sep 16, 2022

Uh oh!

dg0yt commented Oct 5, 2022

Uh oh!

JackBoosY commented Oct 8, 2022

Uh oh!

dg0yt commented Oct 27, 2022

Uh oh!

Adela0814 commented Oct 28, 2022

Uh oh!

Neumann-A commented Oct 28, 2022

Uh oh!

dg0yt commented Jan 25, 2023

Uh oh!

Neumann-A commented Jan 25, 2023

Uh oh!

Adela0814 commented May 5, 2023

Uh oh!

Adela0814 commented Jan 8, 2024

Uh oh!

dg0yt commented Mar 20, 2022 •

edited

Loading

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

JackBoosY commented Aug 17, 2022 •

edited

Loading

Neumann-A Oct 28, 2022 •

edited

Loading