aqtinstall icon indicating copy to clipboard operation
aqtinstall copied to clipboard

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

Open ddalcino opened this issue 1 year ago • 3 comments

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:

  • [x] 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.
  • [x] Improve test coverage.
  • [x] 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.

ddalcino avatar Jul 24 '22 07:07 ddalcino

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: https://github.com/gforcada/flake8-isort/issues/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 avatar Jul 31 '22 00:07 ddalcino

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

  1. It should probably be squashed into fewer commits
  2. 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.
  3. 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.
  4. 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!

ddalcino avatar Aug 02 '22 14:08 ddalcino

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 avatar Aug 03 '22 03:08 miurahr

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

miurahr avatar Aug 17 '22 00:08 miurahr

I think it worth squashing commits to merge.

miurahr avatar Aug 17 '22 02:08 miurahr

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: https://github.com/miurahr/aqtinstall/issues/528#issuecomment-1193364530.

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.

ddalcino avatar Aug 20 '22 16:08 ddalcino

I'd like to merge it as-is.

miurahr avatar Aug 21 '22 14:08 miurahr