Skip to content

Comments

Autobuild: Reduce Qt download size & bump aqtinstall to 2.0.6#2498

Merged
hoffie merged 4 commits intojamulussoftware:masterfrom
hoffie:aqt-reduce-downloads
Mar 13, 2022
Merged

Autobuild: Reduce Qt download size & bump aqtinstall to 2.0.6#2498
hoffie merged 4 commits intojamulussoftware:masterfrom
hoffie:aqt-reduce-downloads

Conversation

@hoffie
Copy link
Member

@hoffie hoffie commented Mar 13, 2022 •
edited by ann0see
Loading

Short description of changes

  • Autobuild: Bump aqtinstall to 2.0.6

  • Autobuild: Add proper quoting around aqt invocations

  • Autobuild: Download required Qt archives only

  • Autobuild: Shorten aqt install-qt invocation on Windows

CHANGELOG: Autobuild: Reduced Qt download size and updated aqtinstall to 2.0.6

Context: Fixes an issue?

  • Saves some time (download/install now takes 11.2s vs. 25.5s), bandwidth, storage and cache save/restore volume (62MB vs. 98MB).
  • Decreases artifact size, most notably on Windows (75.3MB -> 61.8MB) -> Needs verification of correctness

Does this change need documentation? What needs to be documented and how?

No.

Status of this Pull Request

Ready.

What is missing until this pull request can be merged?

  • All builds green
  • Smoke test Windows (needs special consideration as artifacts are ~20% smaller, maybe due to bundling of less DLLs)
  • Smoke test Mac
  • Smoke test iOS

I believe that all possible errors which might be introduced by this change should either show up during build or during binary launch.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@hoffie hoffie added this to the Release 3.9.0 milestone Mar 13, 2022
@hoffie hoffie requested a review from ann0see March 13, 2022 00:07
@hoffie hoffie force-pushed the aqt-reduce-downloads branch from 2179a74 to 2e4c9a4 Compare March 13, 2022 01:37
@hoffie hoffie force-pushed the aqt-reduce-downloads branch from 2e4c9a4 to c716b3f Compare March 13, 2022 01:41
@ann0see
Copy link
Member

ann0see commented Mar 13, 2022

Anything specific to look for? Missing translations? Missing multi-threading support?

@ann0see
Copy link
Member

ann0see commented Mar 13, 2022 •
edited
Loading

Windows ASIO build works same with server (tested audio and translation).
iOS works as well.

ann0see
ann0see approved these changes Mar 13, 2022
View reviewed changes
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless macOS shows any errors (which I can't test at the moment since my VM doesn't start up) I think it's OK. Do we need more tests

@ann0see ann0see requested a review from softins March 13, 2022 07:45
@ann0see
Copy link
Member

ann0see commented Mar 13, 2022

Also tested both macOS clients and they worked (audio via black hole)

@hoffie
Copy link
Member Author

hoffie commented Mar 13, 2022 •
edited
Loading

Anything specific to look for? Missing translations? Missing multi-threading support?

Hmm, some missing modules cause failures (e.g. macextras), but other's don't (winextras, qttranslations):
https://github.com/hoffie/jamulus/actions/runs/1975744507

This PR essentially does what you did for Android here:
https://github.com/jamulussoftware/jamulus/pull/2394/files

(Except that we've never specified dependencies at all which made us download everything)

I'm not sure if or what for winextras is needed, but I'd rather keep it as part of this PR.
I suspect dropping qttranslations will drop support for Qt-provided translations for default items like "OK": https://github.com/qt/qttranslations/tree/dev/translations

It's hard to say what to look for. The still listed Qt archives are those which are not dropped (nothing to check as there's no change). The more interesting part is seeing whether any other archive might be needed and might fail silently.

At the same time, if it does break something, Android has probably been broken since #2394 anyway. ;)

This is the full list for Mac: qt3d qtbase qtcanvas3d qtconnectivity qtdeclarative qtgamepad qtgraphicaleffects qtimageformats qtlocation qtmacextras qtmultimedia qtquickcontrols qtquickcontrols2 qtremoteobjects qtscxml qtsensors qtserialbus qtserialport qtspeech qtsvg qttools qttranslations qtwebchannel qtwebsockets qtwebview qtxmlpatterns.

I think the most valuable testing is the generic "3.9.0 beta" phase.

@hoffie hoffie merged commit 880b6ff into jamulussoftware:master Mar 13, 2022
@ann0see
Copy link
Member

ann0see commented Mar 13, 2022 •
edited
Loading

Interesting side fact: I can rename d3dcompiler_47.dll libEGL.dll libGLESv2.dll and Jamulus still starts without error. To me this seems as if we can remove OpenGL on Windows.

@hoffie
Copy link
Member Author

hoffie commented Mar 13, 2022

Interesting side fact: I can rename d3dcompiler_47.dll libEGL.dll libGLESv2.dll and Jamulus still starts without error. To me this seems as if we can remove OpenGL on Windows.

What artifact did you use for this test -- this PR or very recent master?
I would have assumed that after this PR d3* would no longer be distributed?

@hoffie hoffie deleted the aqt-reduce-downloads branch March 13, 2022 22:00
@ann0see
Copy link
Member

ann0see commented Mar 13, 2022

Good question. I'll retry with the current master.

@ann0see
Copy link
Member

ann0see commented Mar 13, 2022

Yes, the build from master still has these files.

@hoffie
Copy link
Member Author

hoffie commented Mar 13, 2022

Hmm, I've no idea what adds them. When manually running the relevant aqt command, I don't see these added, but maybe I'm missing something.

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

Reviewers

@pljones pljones pljones approved these changes

@ann0see ann0see ann0see approved these changes

@softins softins Awaiting requested review from softins

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Release 3.9.0

Development

Successfully merging this pull request may close these issues.

3 participants

@hoffie @ann0see @pljones