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

Windows: Upgrade JACK dependency to 1.9.21 #2665

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Jun 22, 2022

Short description of changes
Another maintenance upgrading PR.

CHANGELOG: Windows: Upgraded JACK build to use JACK 1.9.21

Context: Fixes an issue?

No.

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

No.

Status of this Pull Request

Should be tested.

What is missing until this pull request can be merged?

Test + 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.
  • I've filled all the content above

@ann0see ann0see force-pushed the upgrade/upgradeJACKWindows branch from a7e704e to 524cc77 Compare June 22, 2022 18:40
@ann0see ann0see added this to the Release 3.9.0 milestone Jun 22, 2022
@ann0see
Copy link
Member Author

ann0see commented Jun 22, 2022

Build failed. Not sure what the problem is.

@hoffie
Copy link
Member

hoffie commented Jun 22, 2022

Culprit seems to be here:
https://github.com/jamulussoftware/jamulus/runs/7011426493?check_suite_focus=true#step:7:160

Some wild suggestions:

  • Access denial might stem from the fact the installer still runs when choco expects it to have finished. It seems like the AutoHotkey logic was updated as part of this release. Maybe the installer hangs in a non-finished step.
  • Access denial might be from Windows Defender (?)

Logs will probably help, not sure if it's worth to spend the effort here now, though?

@ann0see ann0see removed this from the Release 3.9.0 milestone Jun 23, 2022
@ann0see
Copy link
Member Author

ann0see commented Jun 23, 2022

Hmm. Yes. De-tagged. Probably worth focusing on other stuff first.

@ann0see ann0see marked this pull request as draft June 23, 2022 05:35
@henkdegroot
Copy link
Contributor

FWIW, when I try to install manually on a Win2022 server...then the autokey script which should take care of the installation of jack, does not kick in. Sounds like something is not working as expected. I posted a message on chocolatey. Hope to get a reply.

@henkdegroot
Copy link
Contributor

The autohotkey script is not working due to changes in the installer screens. Buttons names are slightly different. I have submitted a PR containing a fix to the repo of the package maintainer. Until fixed, we cannot use 1.9.21.

@hoffie
Copy link
Member

hoffie commented Jun 24, 2022

Cool, thanks!
Linking the PR here: chtof/chocolatey-packages#73

@@ -19,7 +19,7 @@ $ChocoCacheDir = 'C:\ChocoCache'
$Qt32Version = "5.15.2"
$Qt64Version = "5.15.2"
$AqtinstallVersion = "2.1.0"
$JackVersion = "1.9.17"
$JackVersion = "1.9.21"
Copy link
Contributor

@henkdegroot henkdegroot Jul 5, 2022

Choose a reason for hiding this comment

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

@ann0see, the JACK version at choco has been updated. The version to install is now: 1.9.21.20220626
I have validated this on my local system and the auto install works when using that version number.

EDIT: Also verified using the autobuild option in my own repo and that completed succesfull as well.

Suggested change
$JackVersion = "1.9.21"
$JackVersion = "1.9.21.20220626"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm...seems I selected the wrong option for submitting my comment.

@ann0see ann0see marked this pull request as ready for review July 8, 2022 15:40
@hoffie hoffie added this to the Release 3.9.1 milestone Aug 11, 2022
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.

This seems to have been forgotten somehow as it lacked a milestone? I think it's good to merge, isn't it?

@henkdegroot
Copy link
Contributor

Perhaps another (re) run of the action script? Just to confirm the auto install still works?
Otherwise it should be good to merge.

@hoffie
Copy link
Member

hoffie commented Aug 11, 2022

I don't see a way to trigger a re-run via UI.
@ann0see Can you just rebase and push?

Co-authored-by: Henk De Groot <13550012+henkdegroot@users.noreply.github.com>
@ann0see
Copy link
Member Author

ann0see commented Aug 12, 2022

Done

@hoffie
Copy link
Member

hoffie commented Aug 12, 2022

@henkdegroot Can you validate the installation?

@henkdegroot
Copy link
Contributor

I downloaded the artifact for Windows JACK and installed on my system. All fine.
My system had JACK v1.9.19 installed and Jamulus works correct. Connecting to server and audio transmits correct.
Next I upgraded my system to JACK v1.9.21 and all still working fine.

Also when JACK is not running, Jamulus detects it and alerts the user.

I would say all good to go.

@hoffie hoffie requested a review from pljones August 13, 2022 09:45
@ann0see ann0see merged commit facdd44 into jamulussoftware:master Aug 15, 2022
@ann0see ann0see deleted the upgrade/upgradeJACKWindows branch August 15, 2022 05:55
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