Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fallback for Qt downloads #2333

Merged
merged 1 commit into from
Feb 21, 2022

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Feb 2, 2022

Adds a fallback mirror for Qt on failure. Proposed by @dtinth

Short description of changes
Changes the Qt mirror on error.

Context: Fixes an issue?
Autobuild currently fails quite often

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

No
Status of this Pull Request

Waiting on Review

What is missing until this pull request can be merged?
Review

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

Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

I'm certainly in favor of the idea as a short-time fix as the flakiness is really annoying.

Looking at the diff, repeated calls with repeated hardcoded values might be missed when doing future changes (At least the version hardcoding will be gone after one of my future refactoring PRs).

Maybe using a function would make sense?

As noted in the check: At least in theory, Mac builds should be affected as well as they also use aqt-install. My subjective feeling is that they are affected less often, but maybe we should add a similar fix there?
(Android uses a bash-based downloader which already performs failovers; Linux installs Ubuntu Qt packages)

Something like this (untested):

diff --git a/autobuild/windows/autobuild_windowsinstaller_1_prepare.ps1 b/autobuild/windows/autobuild_windowsinstaller_1_prepare.ps1
index 094720e4..e7e5d158 100644
--- a/autobuild/windows/autobuild_windowsinstaller_1_prepare.ps1
+++ b/autobuild/windows/autobuild_windowsinstaller_1_prepare.ps1
@@ -15,6 +15,24 @@ param(
 # Fail early on all errors
 $ErrorActionPreference = "Stop"
 
+Function Install-Qt {
+    param(
+        [string] $QtVersion,
+        [string] $QtArch,
+    )
+    $Args = ("--outputdir", "C:\Qt", "windows", "desktop", "$QtVersion", "$QtArch")
+    aqt install-qt @Args
+    if ( !$? )
+    {
+        echo "Qt installation from primary mirror failed, trying fallback."
+        aqt install-qt @Args -b https://mirrors.ocf.berkeley.edu/qt/
+        if ( !$? )
+        {
+            throw "Qt installation with args @Arguments failed with exit code $LastExitCode"
+        }
+    }
+}
+
 ###################
 ###  PROCEDURE  ###
 ###################
@@ -28,20 +46,10 @@ if ( !$? )
 }
 
 echo "Get Qt 64 bit..."
-# intermediate solution if the main server is down: append e.g. " -b https://mirrors.ocf.berkeley.edu/qt/" to the "aqt"-line below
-aqt install-qt --outputdir C:\Qt windows desktop 5.15.2 win64_msvc2019_64
-if ( !$? )
-{
-		throw "64bit Qt installation failed with exit code $LastExitCode"
-}
+Install-Qt 5.15.2 win64_msvc2019_64
 
 echo "Get Qt 32 bit..."
-# intermediate solution if the main server is down: append e.g. " -b https://mirrors.ocf.berkeley.edu/qt/" to the "aqt"-line below
-aqt install-qt --outputdir C:\Qt windows desktop 5.15.2 win32_msvc2019
-if ( !$? )
-{
-		throw "32bit Qt installation failed with exit code $LastExitCode"
-}
+Install-Qt 5.15.2 win32_msvc2019
 
 
 #################################

@ann0see
Copy link
Member Author

ann0see commented Feb 2, 2022

You can push to my branch if you want to. It's not my code.

Having a function would be good; we might even add more mirrors as fallback then.

@ann0see
Copy link
Member Author

ann0see commented Feb 2, 2022

CI is failing for the other PRs. Let's have a look at this one ;-). Applied your suggestion.

@ann0see ann0see force-pushed the bug/addWindowsQtFallback branch from e88cb9c to ce969d7 Compare February 2, 2022 21:18
@ann0see
Copy link
Member Author

ann0see commented Feb 2, 2022

It didn't execute the fallback... Not a good way to test.

@hoffie
Copy link
Member

hoffie commented Feb 5, 2022

Needs rebasing now.
One approach would be to specify an invalid mirror url for the first call for testing (-b http://example.org/).

@ann0see ann0see force-pushed the bug/addWindowsQtFallback branch 7 times, most recently from b2b888b to d84b959 Compare February 6, 2022 19:12
@ann0see
Copy link
Member Author

ann0see commented Feb 6, 2022

https://github.com/jamulussoftware/jamulus/runs/5085211247?check_suite_focus=true#step:4:138

Strangely enough it seems to already try an alternative mirror?

Connection to 'https://localhost/invalid' failed. Retrying with fallback 'https://www.mirrorservice.org/sites/download.qt-project.org/'.

@ann0see ann0see force-pushed the bug/addWindowsQtFallback branch 5 times, most recently from c49135c to 10f36e5 Compare February 6, 2022 19:27
@ann0see ann0see requested a review from hoffie February 6, 2022 19:54
@hoffie
Copy link
Member

hoffie commented Feb 6, 2022

Strangely enough it seems to already try an alternative mirror?
Connection to 'https://localhost/invalid' failed. Retrying with fallback 'https://www.mirrorservice.org/sites/download.qt-project.org/'.

Interesting, I didn't know about that. The relevant code is here:
https://github.com/miurahr/aqtinstall/blob/3f78bd1e84ffd2cf42098826d53ef2e5ac584c54/aqt/metadata.py#L576

The list of mirrors is here:
https://github.com/miurahr/aqtinstall/blob/3f78bd1e84ffd2cf42098826d53ef2e5ac584c54/aqt/settings.ini#L19

There's one mirror which returns 404s (https://mirror.csclub.uwaterloo.ca/qtproject/archive/). And there's one mirror (the one which got chosen in your run) which doesn't seem to mirror all parts (only official_releases). It also seems like that list is outdated. I have submitted an upstream PR to update it: miurahr/aqtinstall#485

I guess we should still have our work around unless we wanted to submit an upstream patch to try at least two fallbacks...?

Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

I think we should add it, despite the retry logic within aqt and despite the plan for caching.
Minor suggestion about the output now that we know more about aqt. Commit message could be adapted accordingly as well.

autobuild/windows/autobuild_windowsinstaller_1_prepare.ps1 Outdated Show resolved Hide resolved
@ann0see
Copy link
Member Author

ann0see commented Feb 6, 2022

Commit message could be adapted

What's your suggestion?

@ann0see
Copy link
Member Author

ann0see commented Feb 6, 2022

Suggestion: Merge this and open an issue at aqt to request multiple fallbacks.

@hoffie
Copy link
Member

hoffie commented Feb 6, 2022

Commit message could be adapted

What's your suggestion?

The CI used to fail quickly on network errors. This introduces a retry of the whole aqt install-qt run on failure.

Suggestion: Merge this and open an issue at aqt to request multiple fallbacks.

Sounds good, can you do that?

Proposed by @dtinth
The CI used to fail quickly on network errors. This introduces a retry of the whole aqt install-qt run on failure.

Co-authored-by: Christian Hoffmann <christian@hoffie.info>
Co-authored-by: ann0see <ann0see@users.noreply.github.com>
@ann0see ann0see force-pushed the bug/addWindowsQtFallback branch from b234f98 to 72357da Compare February 21, 2022 21:38
@ann0see ann0see requested a review from hoffie February 21, 2022 21:39
@ann0see
Copy link
Member Author

ann0see commented Feb 21, 2022

Since I rebased this PR, a review is needed again (including the new caching logic)

@ann0see ann0see merged commit 9e11545 into jamulussoftware:master Feb 21, 2022
@ann0see ann0see deleted the bug/addWindowsQtFallback branch February 21, 2022 22:02
@ann0see
Copy link
Member Author

ann0see commented Feb 21, 2022

CHANGELOG: Internal: Make Qt downloads on Windows more reliable by re-trying downloads

@ann0see ann0see added this to the Release 3.9.0 milestone Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants