Skip to content

Comments

Build: Windows: Fix Github JACK build failures & improve build speed#3206

Merged
ann0see merged 2 commits intojamulussoftware:mainfrom
hoffie:autobuild-windows-wait-for-jack-install
Dec 12, 2023
Merged

Build: Windows: Fix Github JACK build failures & improve build speed#3206
ann0see merged 2 commits intojamulussoftware:mainfrom
hoffie:autobuild-windows-wait-for-jack-install

Conversation

@hoffie
Copy link
Member

@hoffie hoffie commented Dec 9, 2023 •
edited
Loading

Short description of changes

  • Autobuild: Ensure JACK install completion
    Previously, the JACK installers were started as ordinary programs. As they are GUI applications, they detach from the console and the powershell build script (and Github Actions) continues. Therefore, once the actual build starts, the JACK installation may not have finished at all. This commit uses Powershell's Start-Process -Wait feature to wait for completion before continuing.

  • Build: Windows: Improve dependency download speed
    Invoke-WebRequest enables a progress bar by default which slows down the requests a lot. Disabling these makes the build run much faster.
    Source:

CHANGELOG: SKIP

Context: Fixes an issue?
Fixes #3035.

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

No.

Status of this Pull Request

Ready on CI green.

What is missing until this pull request can be merged?
CI.

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. (Except for MacOS Legacy which seems to be broken anyway)
  • I've filled all the content above

AUTOBUILD: Please build all targets

hoffie added 2 commits December 9, 2023 22:17
@hoffie
Build: Windows: Improve dependency download speed
3cc4318 
Invoke-WebRequest enables a progress bar by default
which slows down the requests a lot. Disabling these makes the build run
much faster.

Source:
- https://stackoverflow.com/a/43477248
- Interactive testing
@hoffie
Autobuild: Ensure JACK install completion
aeb7239 
Previously, the JACK installers were started as ordinary programs. As
they are GUI applications, they detach from the console and the
powershell build script (and Github Actions) continues.
Therefore, once the actual build starts, the JACK installation may not
have finished at all.
This commit uses Powershell's Start-Process -Wait feature to wait
for completion before continuing.

Fixes: jamulussoftware#3035
@hoffie hoffie added this to the Release 3.11.0 milestone Dec 9, 2023
@hoffie hoffie requested a review from ann0see December 9, 2023 21:42
@hoffie hoffie changed the title Autobuild windows wait for jack install Build: Windows: Fix Github JACK build failures & improve build speed Dec 9, 2023
ann0see
ann0see reviewed Dec 10, 2023
View reviewed changes
Copy link
Member

@ann0see ann0see left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for this! Strange that the progress bar introduces delay.
Will check this PR on my repo.

@hoffie hoffie mentioned this pull request Dec 10, 2023
Merged
8 tasks
@hoffie
Copy link
Member Author

hoffie commented Dec 10, 2023

Thanks for this! Strange that the progress bar introduces delay. Will check this PR on my repo.

Just to be clear: the progress bar is only an improvement, the actual fix is the -Wait.

@ann0see
Copy link
Member

ann0see commented Dec 10, 2023

Yes. I've read the stackoverflow link now.

--
For the bug:
Set the permissions to more restrictive on this repo and kicked a rerun.

image

@hoffie
Copy link
Member Author

hoffie commented Dec 10, 2023 •
edited
Loading

Set the permissions to more restrictive on this repo and kicked a rerun.

You did that for jamulussoftware/jamulus, right?
The latest run on this PR was identical, JACK still works. So it seems good to me.

Also, I think that setting should not matter for two reasons (good to have it tested nevertheless, though!):

  • It only defines defaults while our autobuild workflow sets specific values and should therefore not be affected by defaults.
  • The build did not fail in a git-related operation (and the setting should only be related to git permissions, not filesystem permissions or anything else).

@ann0see
Copy link
Member

ann0see commented Dec 10, 2023

You did that for jamulussoftware/jamulus, right?

Yes

Also, I think that setting should not matter for two reasons (good to have it tested nevertheless, though!)

I think so too. Maybe the reason is something else. But it could be some concurrency issue - which you fix here.

@ann0see ann0see merged commit 6ee0c9e into jamulussoftware:main Dec 12, 2023
@ann0see
Copy link
Member

ann0see commented Dec 12, 2023

Thanks!

@pljones pljones added the tooling Changes to the automated build system label Dec 14, 2023
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

Assignees

@hoffie hoffie

Labels

tooling Changes to the automated build system

Projects

Tracking
Status: Done

Milestone

Release 3.11.0

Development

Successfully merging this pull request may close these issues.

New JACK autobuild fails on GitHub Actions Pull Requests

3 participants

@hoffie @ann0see @pljones