julia-actions / cache

A shortcut action to cache Julia artifacts, packages, and registries.
MIT License
38 stars 9 forks source link

Remove deprecated set-output command #32

Closed SaschaMann closed 1 year ago

SaschaMann commented 1 year ago

See https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/ & https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-output-parameter

rikhuijzer commented 1 year ago

Sascha, do you also know the core reason why GitHub keeps moving things to files instead of ENV vars and logs? My guess is that file access in Linux is easier to restrict than log, ENV and the list of running processes access.

giordano commented 1 year ago

I can't answer the question, but honestly, the syntax of set-output was awful, I'm glad they changed it. This is now a plain var=value list in a dedicated file, easier to read and parse than grabbing some specific text in the log.

SaschaMann commented 1 year ago

When they moved add-path and set-env, it was due to a security issue: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/

The explanation given in the blog post sounds similar

To avoid untrusted logged data to use set-stateand set-output workflow commands without the intention of the workflow author we have introduced a new set of environment files to manage state and output.

I'm guessing the whole idea of printing things to stdout as a way to run commands didn't work out because it's very hard to sanitize everything that's being printed in a CI log on potentially untrusted PRs and such?

Fully agree that the syntax was awful and this is much better.

SaschaMann commented 1 year ago

Ah good call, yes that should be updated as well. I only checked action.yml

Do you want to open a PR?

Apart from that the API remains the same, right? For the end-user. If yes and tests pass then all looks good to me 👍

Yes, I think so. Unfortunately we'll have to tag the 2nd release today and spam people who use Dependabot a bit because I had already tagged a release for #31

rikhuijzer commented 1 year ago

Do you want to open a PR?

Easier if you do it, merge and tag all in one go I think.