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

fix(checkver) use Start-Job for manifest script to not to exit on break #5407

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

sedlund
Copy link

@sedlund sedlund commented Feb 27, 2023

Description

Change the method checkver runs manifest scripts to use Start-Job instead of Invoke-Command. When using Invoke-Command if the script calls a break the parent process will exit.

See: Do not use break outside of a loop, switch, or trap

Using break inside a pipeline break, such as a ForEach-Object script block, not only exits the pipeline, it potentially terminates the entire runspace.

Motivation and Context

Currently autoupdating is broken on all Scoop buckets that have a manifest that runs a break in the autoupdate script portion. Once one manifest is run that has issued a break the Github action will exit and the rest of the bucket will fail to update.

This change starts the manifest script as a separate job allowing the parent process to continue after the manifest script exits, and letting the github action to finish updating the buckets.

How Has This Been Tested?

I ran the change on my copy of the current (3579b11f99c692f6b68c1ce5119b8e5d6db9f2bb) Extras repo. I have attached the output of the update here. Compare with the current log of Excavator ran during the same period:

See where it fails on librewolf. That manifest issues a break command. My local bucket updated 84 manifests.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Invoke-Command will exit the parent process if a manifest script issues
a break command.  Use Start-Job to resolve this.

Fixes ScoopInstaller/GithubActions#32
@rashil2000
Copy link
Member

This didn't used to happen before, did something change in GitHub Actions?

/cc @niheaven

@niheaven
Copy link
Member

Nope, this part hasn't been changed.

@niheaven
Copy link
Member

This could skip break but it omits all error message in checkver.script.

A better way is NEVER use break in checkver's script field.

@sedlund
Copy link
Author

sedlund commented Feb 27, 2023

This could skip break but it omits all error message in checkver.script.

A better way is NEVER use break in checkver's script field.

I disagree. randoms should not be be able to run something in the parent namespace. security says no.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants