Replace `set-output` command #1986

parkerbxyz · 2022-10-21T23:18:49Z

Replaces the soon-to-be deprecated set-output command.

Reference: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Readiness Checklist

Author/Contributor

Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

Label as breaking if this is a large fundamental change
Label as either automation, bug, documentation, enhancement, infrastructure, or performance

Use `{ cmd1; cmd2; } >> file` instead of individual redirects per shellcheck ([SC2129](https://www.shellcheck.net/wiki/SC2129)).

parkerbxyz · 2022-10-22T02:06:03Z

The build no longer shows the following annotations, so it looks like these changes are working as intended! 🎉

Tests + Deploy Docker Image - DEV

The set-output command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

nvuillam · 2022-10-22T16:38:07Z

@parkerbxyz please can you make your branch up to date with main ? it will fix CI issues not related to your PR :)

codecov-commenter · 2022-10-23T04:42:48Z

Codecov Report

Merging #1986 (9a01349) into main (e957998) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1986      +/-   ##
==========================================
+ Coverage   82.68%   82.71%   +0.02%     
==========================================
  Files         157      157              
  Lines        3384     3384              
==========================================
+ Hits         2798     2799       +1     
+ Misses        586      585       -1

Impacted Files	Coverage Δ
megalinter/MegaLinter.py	`83.53% <100.00%> (ø)`
megalinter/reporters/UpdatedSourcesReporter.py	`89.74% <0.00%> (+2.56%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

nvuillam

Thanks for the catch :)
Let's hope you know what you are doing, else release CI jobs are broken :D :D

Kurt-von-Laven

@parkerbxyz, thanks for taking a pass at this.

Kurt-von-Laven · 2022-10-23T08:45:49Z

.automation/build.py

@@ -2757,7 +2757,7 @@ def manage_output_variables():
                updated_versions = 1
                break
        if updated_versions == 1:
-            print("::set-output name=has_updated_versions::1")
+            print('"has_updated_versions=1" >>"$GITHUB_OUTPUT"')


The Bash changes look correct at a glance, but I suspect printing Bash syntax from Python has no special effect. See EnricoMi/publish-unit-test-result-action#360 for an example of the correct syntax.

I'm afraid you're right... I'll rollback the PR to not break behaviour until we have a working solution ( cc @parkerbxyz )

I suspect printing Bash syntax from Python has no special effect

In that case, is it working as expected without my changes?

How about something like this?

import shlex, subprocess add_to_output_cmd = 'echo "has_updated_versions=1" >>"$GITHUB_OUTPUT"' add_to_output_args = shlex.split(add_to_output_cmd) subprocess.run(add_to_output_args)

Suggested change

print('"has_updated_versions=1" >>"$GITHUB_OUTPUT"')

add_to_output_cmd = 'echo "has_updated_versions=1" >>"$GITHUB_OUTPUT"'

add_to_output_args = shlex.split(add_to_output_cmd)

subprocess.run(add_to_output_args)

This reverts commit 2561f74.

parkerbxyz · 2022-10-23T18:48:03Z

.automation/build.py

@@ -2757,7 +2757,7 @@ def manage_output_variables():
                updated_versions = 1
                break
        if updated_versions == 1:
-            print("::set-output name=has_updated_versions::1")
+            print('"has_updated_versions=1" >>"$GITHUB_OUTPUT"')


I suspect printing Bash syntax from Python has no special effect

In that case, is it working as expected without my changes?

How about something like this?

import shlex, subprocess add_to_output_cmd = 'echo "has_updated_versions=1" >>"$GITHUB_OUTPUT"' add_to_output_args = shlex.split(add_to_output_cmd) subprocess.run(add_to_output_args)

Suggested change

print('"has_updated_versions=1" >>"$GITHUB_OUTPUT"')

add_to_output_cmd = 'echo "has_updated_versions=1" >>"$GITHUB_OUTPUT"'

add_to_output_args = shlex.split(add_to_output_cmd)

subprocess.run(add_to_output_args)

parkerbxyz · 2022-10-23T18:54:05Z

megalinter/MegaLinter.py

@@ -822,7 +822,7 @@ def display_header():
        logging.info("")

    def check_results(self):
-        print(f"::set-output name=has_updated_sources::{str(self.has_updated_sources)}")
+        print(f'"has_updated_sources={str(self.has_updated_sources)}" >>"$GITHUB_OUTPUT"')


Suggested change

print(f'"has_updated_sources={str(self.has_updated_sources)}" >>"$GITHUB_OUTPUT"')

add_to_output_cmd = f'echo "has_updated_sources={str(has_updated_sources)}" >>"$GITHUB_OUTPUT"'

add_to_output_args = shlex.split(add_to_output_cmd)

subprocess.run(add_to_output_args)

It could work, but it will update $GITHUB_OUTPUT within MegaLinter docker image
Will Github Action workflow retrieve the variable from inside the docker image ? if not we have to find another way (maybe some dummy file in artifacts that we could check for existence ...

Yes, the existing code looks correct, just deprecated. The goal here is simply to append to the file named GITHUB_OUTPUT, so we can achieve that without spawning a process.

from os import environ output_file = environ["GITHUB_OUTPUT"] with open(output_file, "a", encoding="utf-8") as output_stream: output_stream.write(f"has_updated_sources={str(self.has_updated_sources)}\n")

@Kurt-von-Laven print(), not output_stream.write() ?

The last line could be replaced with:

output_stream.write(f"has_updated_sources={str(self.has_updated_sources)}\n")

Any preference?

Just saw your previous message. To be clear, we don’t want to overwrite the environment variable GITHUB_OUTPUT, which is essentially the path to a magic dummy file that gets sourced at the beginning of each subsequent step.

I'd prefer the write option , it's more readable that a print that we expect more to be just a line in the console :)

@parkerbxyz maybe u'd like to ressuscitate your PR and update it ? :) ( ps: just been rickrolled... 😠 🤣 )

Sounds good to me. I updated the code snippet accordingly.

Ha ha, the meme that will never die.

parkerbxyz added 4 commits October 21, 2022 16:16

Replace set-output command

00c5b9e

Consolidate redirects

8d522c3

Use `{ cmd1; cmd2; } >> file` instead of individual redirects per shellcheck ([SC2129](https://www.shellcheck.net/wiki/SC2129)).

Double quote to prevent globbing and word splitting

0ec4e4c

Replace set-output in Python scripts

ede755b

parkerbxyz marked this pull request as ready for review October 22, 2022 02:06

parkerbxyz requested a review from nvuillam as a code owner October 22, 2022 02:06

Merge remote-tracking branch 'upstream/main' into replace-set-output

9a01349

nvuillam approved these changes Oct 23, 2022

View reviewed changes

nvuillam merged commit 2561f74 into oxsecurity:main Oct 23, 2022

Kurt-von-Laven reviewed Oct 23, 2022

View reviewed changes

nvuillam pushed a commit that referenced this pull request Oct 23, 2022

Revert "Replace set-output command (#1986)"

a47459f

This reverts commit 2561f74.

parkerbxyz commented Oct 23, 2022

View reviewed changes

parkerbxyz mentioned this pull request Oct 24, 2022

Replace set-output usage with GITHUB_OUTPUT #1996

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `set-output` command #1986

Replace `set-output` command #1986

parkerbxyz commented Oct 21, 2022 •

edited

parkerbxyz commented Oct 22, 2022

Tests + Deploy Docker Image - DEV

nvuillam commented Oct 22, 2022

codecov-commenter commented Oct 23, 2022

nvuillam left a comment

Kurt-von-Laven left a comment

Kurt-von-Laven Oct 23, 2022

nvuillam Oct 23, 2022

parkerbxyz Oct 23, 2022

parkerbxyz Oct 23, 2022

parkerbxyz Oct 23, 2022

nvuillam Oct 23, 2022 •

edited

Kurt-von-Laven Oct 23, 2022 •

edited

nvuillam Oct 23, 2022

Kurt-von-Laven Oct 23, 2022

nvuillam Oct 23, 2022

nvuillam Oct 23, 2022

Kurt-von-Laven Oct 23, 2022

-            print('"has_updated_versions=1" >>"$GITHUB_OUTPUT"')
+            add_to_output_cmd = 'echo "has_updated_versions=1" >>"$GITHUB_OUTPUT"'
+            add_to_output_args = shlex.split(add_to_output_cmd)
+            subprocess.run(add_to_output_args)

Replace set-output command #1986

Replace set-output command #1986

Conversation

parkerbxyz commented Oct 21, 2022 • edited

Readiness Checklist

Author/Contributor

Reviewing Maintainer

parkerbxyz commented Oct 22, 2022

nvuillam commented Oct 22, 2022

codecov-commenter commented Oct 23, 2022

Codecov Report

nvuillam left a comment

Choose a reason for hiding this comment

Kurt-von-Laven left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvuillam Oct 23, 2022 • edited

Choose a reason for hiding this comment

Kurt-von-Laven Oct 23, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Replace `set-output` command #1986

Replace `set-output` command #1986

parkerbxyz commented Oct 21, 2022 •

edited

nvuillam Oct 23, 2022 •

edited

Kurt-von-Laven Oct 23, 2022 •

edited