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

New JACK autobuild fails on GitHub Actions Pull Requests #3035

Closed
ann0see opened this issue Mar 24, 2023 · 10 comments · Fixed by #3206
Closed

New JACK autobuild fails on GitHub Actions Pull Requests #3035

ann0see opened this issue Mar 24, 2023 · 10 comments · Fixed by #3206
Assignees
Labels
bug Something isn't working tooling Changes to the automated build system

Comments

@ann0see
Copy link
Member

ann0see commented Mar 24, 2023

Describe the bug

The JACK build fails on GitHub Actions PRs if Workflow Permissions are set to "Read repository contents and packages permissions". No fail can be seen on PRs by me/if "Read and write permissions" are set.
To Reproduce
#3031 with "Read repository contents and packages permission" set doesn't fail but #3011 does.
This is a successful run while write permissions were enabled: https://github.com/jamulussoftware/jamulus/actions/runs/4515448402/jobs/7952904802

Expected behavior

No matter what permissions are set the JACK build succeeds for GitHub actions and other contributors.

Screenshots

/
Operating system

Windows on GH
Version of Jamulus

/
Additional context

I strongly believe this is a permission error we miss a setting or Autobuild permission somewhere. Something related to GH_TOKEN maybe?

@ann0see ann0see added the bug Something isn't working label Mar 24, 2023
@ann0see
Copy link
Member Author

ann0see commented Apr 26, 2023

Currently we have set the permissions to read and write.

@pljones
Copy link
Collaborator

pljones commented May 21, 2023

Should we make 3.10 dependent upon this or leave until 3.11?

@pljones pljones added the tooling Changes to the automated build system label May 21, 2023
@ann0see
Copy link
Member Author

ann0see commented May 21, 2023

I think it's not that important to get correct before the next release.

@pljones pljones added this to the Release 3.11.0 milestone May 21, 2023
@pljones pljones added this to Tracking Jun 17, 2023
@github-project-automation github-project-automation bot moved this to Triage in Tracking Jun 17, 2023
@pljones pljones moved this from Triage to Backlog in Tracking Jun 17, 2023
@pljones
Copy link
Collaborator

pljones commented Aug 1, 2023

How easy is this to test? Can a list of steps be included to demonstrate failure and success cases?

If we know it's down to a permission issue (e.g. unzip needs write permissions - which is perfectly sane), do we have reason to restrict the permissions? Can we restrict permissions so that only the unzip step has write permission on a specific target directory (e.g. by granting write on that target from a "higher power" / elevated privilege state)?

@ann0see
Copy link
Member Author

ann0see commented Aug 1, 2023

As I said above, just change the permissions in the GitHub repository and run two same builds. Once with and once without write permissions. They can be changed here: https://github.com/jamulussoftware/jamulus/settings/actions

Once the build fails, once it goes through. I'm unsure if this is still the case, but I'm currently working on Weblate.

@pljones
Copy link
Collaborator

pljones commented Aug 2, 2023

image
Could that be used to grant the required permission just to the step that needs it? unzip will always need to write...

@ann0see
Copy link
Member Author

ann0see commented Aug 2, 2023

Is unzip the problem? Especially I thought write means that we write to the Repo or something on GitHub- not in the runner.

@pljones
Copy link
Collaborator

pljones commented Aug 3, 2023

I thought it was the unzip that was failing? Maybe I've missed something.

@ann0see
Copy link
Member Author

ann0see commented Aug 3, 2023

I would need to check it again and look at the logs.

@pljones pljones moved this from Backlog to Triage in Tracking Sep 27, 2023
@hoffie hoffie self-assigned this Dec 9, 2023
@hoffie
Copy link
Member

hoffie commented Dec 9, 2023

Hi there.

after some hours of debugging, I believe this is related to timing.

& $JACKInstallPath $JACKInstallParms

We are invoking the JACK installers, but as they are GUI applications (even if we pass parameters to have them run unattended), they will detach from the console and just launch in the background. The build continues nevertheless, which means that by the time that qmake Jamulus.pro runs it is more or less luck if JACK installation has already completed or not.
Waiting for the installers to complete seems to fix this. Will submit a PR.

@hoffie hoffie moved this from Triage to In Progress in Tracking Dec 9, 2023
hoffie added a commit to hoffie/jamulus that referenced this issue Dec 9, 2023
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
@github-project-automation github-project-automation bot moved this from In Progress to Done in Tracking Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tooling Changes to the automated build system
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants