Automatically install desktop qt when required for android/ios qt installations #540

ddalcino · 2022-07-24T07:54:29Z

I think this should fix #528.

This adds a CLI flag, --autodesktop, that automatically installs the required desktop version of Qt if it is not already installed. I chose not to turn it on by default, in order to preserve the existing behavior.
This adds a warning when --autodesktop is not turned on and the expected desktop version of Qt is not present. The warning prints an example command that could be used to install the desktop version of Qt.

Todo:

Fix the Updater for Windows. Currently, for Android installations on Windows, the Updater assumes that the desktop version of Qt is mingw81_64, regardless of what is already installed, what was installed by --autodesktop, and what is actually available. This is a serious bug: if aqt automatically installs mingw73_64, but patches the android files to look for some other version, the patch won't help anyone.
Improve test coverage.
Add 1-2 jobs to the Azure Pipelines that use this flag. I don't want to add too much here, because the --autodesktop flag does not allow the user to filter out archives; this means that every new job will download hundreds of MBs and take much longer to run than most of the other jobs.

aqt/installer.py

ddalcino · 2022-07-24T18:00:26Z

aqt/metadata.py

+        arches = [d.name for d in directories]
+        selected = QtRepoProperty.select_default_mingw(arches, is_dir=True)
+        return installed_qt_version_dir / selected if selected else None
+


QtRepoProperty is now 100 lines, and it's all static methods. It doesn't really have anything to do with the MetadataFactory. Maybe it should be a module instead of a class, and exist in its own file?

tests/test_cli.py

tests/test_install.py

ddalcino · 2022-07-31T00:33:17Z

I'm not sure what's going on with the CI run here: https://github.com/miurahr/aqtinstall/runs/7594667290?check_suite_focus=true#step:7:68

File "/home/runner/work/aqtinstall/aqtinstall/.tox/check/lib/python3.8/site-packages/flake8_isort.py", line 3, in <module>
      from isort import SortImports
  ImportError: cannot import name 'SortImports' from 'isort' (/home/runner/work/aqtinstall/aqtinstall/.tox/check/lib/python3.8/site-packages/isort/__init__.py)

The tox run is failing because of a problem loading isort; it's being tracked in several places, including here: gforcada/flake8-isort#115

There's a workaround people are talking about, and I think it involves pinning the versions of flake8 and flake8-isort. I am unable to get it to work though.

Edit: resolved

ddalcino · 2022-08-02T14:44:40Z

I think this PR is done for now, except for a couple of things:

It should probably be squashed into fewer commits
I’m not really happy about the way QtRepoProperty is turning out. It definitely needs its own file, but right now it cannot be moved without breaking out Version and a couple other things without creating dependency cycles. This would split metadata.py into at least 4 different files.
There’s some question about whether—autodesktop should be opt-in or opt-out. Ideally, it should be opt-out, but that cannot be done without breaking current interface.
This PR provides a way to automatically choose an architecture on Windows. Right now, install-qt for Windows desktop is the only command that fails if the user doesn’t select an architecture. If we are going to make breaking changes to the interface anyway, I would like aqt to use this code to automatically select an architecture when the user does not. This makes the interface more consistent.

Edit: oops, I did not intentionally close this, but it my smartphone had other ideas!

miurahr · 2022-08-03T03:40:36Z

I'd like to publish this improvements as v3.0.0beta/rc versions and ask users to test their environments.
It is better to recreate commit history for future maintenance before merge.

miurahr · 2022-08-17T00:30:40Z

I'd like to merge here to master. @ddalcino could you re-create commit history and fix merge conflict?

miurahr · 2022-08-17T02:08:00Z

I think it worth squashing commits to merge.

This change also causes aqt to emit a warning when the option is not turned on and the expected desktop Qt is not found.

This test uses an arch for Qt that is not the hardcoded "mingw81_64"; hopefully this will ensure that a passing implementation will not hardcode the architecture.

Move default_desktop_arch_dir & dir_for_version to QtRepoProperty

This also adds a lot of type hints. I refrained from changing a lot of 'str' variables to Path variables, but eventually I think they will need to become Paths

This will allow testing multiple qt installations, as required by the feature that installs the default desktop qt where required Add autodesktop test for ios

tests/test_install.py

ddalcino · 2022-08-20T16:49:11Z

I think it worth squashing commits to merge.

Ok. I've re-ordered and squashed about half of these commits, and I think it's worthwhile to preserve the remaining history. I'm willing to squash it further if you like.

We should probably discuss the default behavior of the --autodesktop flag. Right now, it is opt-in, because I was expecting this PR to go into aqt v2.3 and not 3.0. There was some discussion of this issue here: #528 (comment).

Since we are now talking about aqt v3.0, do you think we should change the flag to --no-autodesktop, and make this an opt-out feature? There are probably other ways to modify the default behavior, such as settings.ini entries.

miurahr · 2022-08-21T14:01:23Z

I'd like to merge it as-is.

ddalcino mentioned this pull request Jul 24, 2022

Android installs missing gcc_64 directory #528

Closed

ddalcino commented Jul 24, 2022

View reviewed changes

aqt/installer.py Outdated Show resolved Hide resolved

ddalcino commented Jul 24, 2022

View reviewed changes

sonatype-lift bot reviewed Jul 28, 2022

View reviewed changes

tests/test_cli.py Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Jul 30, 2022

View reviewed changes

tests/test_install.py Outdated Show resolved Hide resolved

ddalcino marked this pull request as ready for review July 31, 2022 02:51

miurahr added the enhancement New feature or request label Aug 2, 2022

ddalcino closed this Aug 2, 2022

ddalcino reopened this Aug 2, 2022

ddalcino added 13 commits August 20, 2022 07:52

Add function to choose default desktop architecture

910159f

Add function to select default installed mingw architecture

f0a4806

Add CLI option to install desktop qt automatically

ed6cf76

This change also causes aqt to emit a warning when the option is not turned on and the expected desktop Qt is not found.

Add --autodesktop to docs

687adb1

Improve unit test coverage

78fe5f3

Test --autodesktop in Azure Pipelines

5563b3e

Add a build job that uses --autodesktop on Windows

0fcf201

This test uses an arch for Qt that is not the hardcoded "mingw81_64"; hopefully this will ensure that a passing implementation will not hardcode the architecture.

Refactor: move Qt repo properties to QtRepoProperty

85eee1a

Move default_desktop_arch_dir & dir_for_version to QtRepoProperty

Fix formatting of version in paths

0a8dd41

Improve install test patch specificity

861a3d6

Fix updater interface: pass desktop_arch_path in args

94ff9f4

This also adds a lot of type hints. I refrained from changing a lot of 'str' variables to Path variables, but eventually I think they will need to become Paths

rework logic for readability

703f2d2

Refactor test_install to mock multiple Qts

bcd4e03

This will allow testing multiple qt installations, as required by the feature that installs the default desktop qt where required Add autodesktop test for ios

ddalcino force-pushed the auto-install-desktop-android-dev branch from f86bdeb to bcd4e03 Compare August 20, 2022 16:19

sonatype-lift bot reviewed Aug 20, 2022

View reviewed changes

tests/test_install.py Outdated Show resolved Hide resolved

sonatype-lift bot reviewed Aug 20, 2022

View reviewed changes

tests/test_install.py Outdated Show resolved Hide resolved

Fix type error

3bb364b

Fix test_get_autodesktop_dir_and_arch for Windows

f63e0d9

miurahr merged commit 179bea9 into miurahr:master Aug 21, 2022

ddalcino deleted the auto-install-desktop-android-dev branch August 21, 2022 18:33

ddalcino mentioned this pull request Sep 5, 2023

Error when use qmake with Qt 6 #710

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically install desktop qt when required for android/ios qt installations #540

Automatically install desktop qt when required for android/ios qt installations #540

ddalcino commented Jul 24, 2022 •

edited

ddalcino Jul 24, 2022

ddalcino commented Jul 31, 2022 •

edited

ddalcino commented Aug 2, 2022 •

edited

miurahr commented Aug 3, 2022

miurahr commented Aug 17, 2022

miurahr commented Aug 17, 2022

ddalcino commented Aug 20, 2022

miurahr commented Aug 21, 2022

Automatically install desktop qt when required for android/ios qt installations #540

Automatically install desktop qt when required for android/ios qt installations #540

Conversation

ddalcino commented Jul 24, 2022 • edited

ddalcino Jul 24, 2022

Choose a reason for hiding this comment

ddalcino commented Jul 31, 2022 • edited

ddalcino commented Aug 2, 2022 • edited

miurahr commented Aug 3, 2022

miurahr commented Aug 17, 2022

miurahr commented Aug 17, 2022

ddalcino commented Aug 20, 2022

miurahr commented Aug 21, 2022

ddalcino commented Jul 24, 2022 •

edited

ddalcino commented Jul 31, 2022 •

edited

ddalcino commented Aug 2, 2022 •

edited