-
Notifications
You must be signed in to change notification settings - Fork 238
Improve deploy_windows.ps1 fetching of dependencies #3612
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
base: main
Are you sure you want to change the base?
Improve deploy_windows.ps1 fetching of dependencies #3612
Conversation
Updated Install-Dependency to create a unique empty directory for unpacking the fetched zip file into, for ASIO and NSIS. This avoids having to know the top-level directory name used within the archive, which for ASIO is not consistent between versions.
| @@ -11,9 +11,7 @@ param ( | |||
| # | |||
| # The following version pinnings are semi-automatically checked for | |||
| # updates. Verify .github/workflows/bump-dependencies.yaml when changing those manually: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will bump-dependencies be affected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think so, but just double-checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs #3607 to be applied to bump-dependencies, but that is all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I applied that to a test branch, and ran the check-dependencies action. It ran fine: https://github.com/jamulussoftware/jamulus/actions/runs/21530391149
and created a good PR (although that also included the commits from this PR and for bump-dependencies). #3615
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've got three PRs to get this working? Can we have just one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make more sense as they're all about sorting out the build for the Windows dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, all done, and I've updated the description for this PR accordingly.
Make asiosdk or ASIO-SDK part of the version, so that URLs are correct.
Short description of changes
In
deploy-windows.ps1, updatedInstall-Dependencyto create a unique empty directory for unpacking the fetched zip file into, for ASIO and NSIS.Also updated the regex for matching the ASIO file version in
bump-dependencies.yml.CHANGELOG: Build: Improve dependency fetching for Windows builds
Context: Fixes an issue?
Fixes #3560
Replaces #3607
This avoids having to know the top-level directory name used within the archive, which for ASIO is not consistent between versions, and could not be updated by
bump-dependencies.ymlDoes this change need documentation? What needs to be documented and how?
I don't think so. It just makes the PRs generated for updating ASIO and NSIS build properly.
Status of this Pull Request
Ready for review
What is missing until this pull request can be merged?
Check CI works
Checklist