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

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

Merged

Conversation

hoffie
Copy link
Member

@hoffie hoffie commented Dec 9, 2023

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

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
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
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.

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

@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

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
11 of 12 checks passed
@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
Labels
tooling Changes to the automated build system
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

New JACK autobuild fails on GitHub Actions Pull Requests
3 participants