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

Install JACK via web request #3003

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Jan 30, 2023

Short description of changes

As the choco build doesn't seem to get updated, this is an alternative approach to install JACK via cli. The download is done via gh a web request in PowerShell and the installation is done via the normal installer.
CHANGELOG: Internal: The JACK build will now install directly from the JACK GitHub repository

Context: Fixes an issue?

Fixes: #2992

Does this change need documentation? What needs to be documented and how?
Don't think so.

Status of this Pull Request
Ready for review. Needs to be tested. Moreover we might need to implement caching Tested and works.

What is missing until this pull request can be merged?
Check if the automated update works (bump-dependencies)

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

AUTOBUILD: Please build all targets

@ann0see ann0see added this to the Release 3.10.0 milestone Jan 30, 2023
@ann0see ann0see added this to Triage in Tracking (old) via automation Jan 30, 2023
@ann0see
Copy link
Member Author

ann0see commented Jan 30, 2023

GH might not be worth the hassle. Using a simple web request is easier.

@ann0see ann0see changed the title Install JACK via gh cli Install JACK via web request Jan 31, 2023
@ann0see ann0see force-pushed the moveJacktoGH branch 9 times, most recently from d445b84 to 49b63bc Compare January 31, 2023 20:59
@ann0see ann0see moved this from Triage to Waiting on Team in Tracking (old) Jan 31, 2023
@ann0see ann0see marked this pull request as ready for review January 31, 2023 21:01
@ann0see
Copy link
Member Author

ann0see commented Jan 31, 2023

Ok. I think this is ready for review now @henkdegroot

get_upstream_version: |
curl -s -o /dev/null --location --range 0-5 --write-out '%{url_effective}' https://community.chocolatey.org/api/v2/package/jack/ |
grep -oP '.*/jack\.\K.*(?=\.nupkg)'
get_upstream_version: GH_REPO=jackaudio/jack2-releases gh release view --json tagName --jq .tagName | sed -re 's/^v//'
Copy link
Member Author

Choose a reason for hiding this comment

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

@hoffie I think this needs some verification from your end. I copied it from the other GitHub dependency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Accepting risk.

Copy link
Contributor

@henkdegroot henkdegroot left a comment

Choose a reason for hiding this comment

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

Nice work! Just two (actually only one) minor comments but twice because of the 32 and 64 install.

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

ann0see commented Feb 1, 2023

Tested on Win 11 and it seems to be all right.

@henkdegroot
Copy link
Contributor

No surprise here....I tested on Win 10 and working fine here as well.

@pljones
Copy link
Collaborator

pljones commented Feb 13, 2023

I'm tempted to say just get this merged...

@ann0see
Copy link
Member Author

ann0see commented Feb 13, 2023

@pljones feel free to review and merge.

Copy link
Collaborator

@pljones pljones left a comment

Choose a reason for hiding this comment

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

Accepting all outstanding comments.


if ( !$? )
{
throw "Download of "+ $Name + " (" + $Uri + ") failed with exit code $LastExitCode"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why mixed syntax of variable expansion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not important, resolving.

Copy link
Member Author

Choose a reason for hiding this comment

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

True. Raising an issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Unresolving them for convenience.
#3012

if ( !$? )
{
throw "64bit jack installation failed with exit code $LastExitCode"
throw "64bit JACK installer failed with exit code $LastExitCode"
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for else block around the echo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not important, resolving.

throw "32bit jack installation failed with exit code $LastExitCode"
throw "32bit JACK installer failed with exit code $LastExitCode"
} else {
echo "32bit JACK installation completed successfully"
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not important, resolving.

get_upstream_version: |
curl -s -o /dev/null --location --range 0-5 --write-out '%{url_effective}' https://community.chocolatey.org/api/v2/package/jack/ |
grep -oP '.*/jack\.\K.*(?=\.nupkg)'
get_upstream_version: GH_REPO=jackaudio/jack2-releases gh release view --json tagName --jq .tagName | sed -re 's/^v//'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Accepting risk.

@pljones pljones merged commit 2cfbf2e into jamulussoftware:main Feb 14, 2023
Tracking (old) automation moved this from Waiting on Team to Done Feb 14, 2023
@ann0see ann0see deleted the moveJacktoGH branch February 14, 2023 17:33
ann0see added a commit to ann0see/jamulus that referenced this pull request Feb 14, 2023
pljones added a commit that referenced this pull request Feb 15, 2023
Improve updated JACK autobuild (#3003)
@pljones pljones removed this from Done in Tracking (old) Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

JACK Autobuild fails during installation of JACK
3 participants