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.

Sign up for GitHub

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

Jump to bottom

Replace set-output command #1986

Merged
merged 5 commits into from
Oct 23, 2022
Merged

Replace set-output command #1986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
00c5b9e
Replace `set-output` command
parkerbxyz Oct 21, 2022
8d522c3
Consolidate redirects
parkerbxyz Oct 21, 2022
0ec4e4c
Double quote to prevent globbing and word splitting
parkerbxyz Oct 22, 2022
ede755b
Replace `set-output` in Python scripts
parkerbxyz Oct 22, 2022
9a01349
Merge remote-tracking branch 'upstream/main' into replace-set-output
parkerbxyz Oct 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .automation/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"')
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)



def reformat_markdown_tables():
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/deploy-DEV-linters.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:

prepare:
name: Prepare matrix
runs-on: ubuntu-latest
runs-on: ubuntu-latest
outputs:
tag: ${{ steps.image_tag.outputs.tag }}
steps:
Expand All @@ -40,7 +40,7 @@ jobs:
BRANCH_NAME="${GITHUB_REF##*/}"
TAG="test-${GITHUB_ACTOR}-${BRANCH_NAME}"
echo "Tag name: ${TAG}"
echo "::set-output name=tag::${TAG}"
echo "tag=${TAG}" >>"$GITHUB_OUTPUT"

build:
# Name the Job
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/deploy-DEV.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
# Get the current date #
########################
- name: Get current date
run: echo "BUILD_DATE=$(date -u +'%Y-%m-%dT%H:%M:%SZ')" >> ${GITHUB_ENV}
run: echo "BUILD_DATE=$(date -u +'%Y-%m-%dT%H:%M:%SZ')" >>"$GITHUB_ENV"

########################
# Build image tag name #
Expand All @@ -66,7 +66,7 @@ jobs:
BRANCH_NAME="${GITHUB_REF##*/}"
TAG="test-${GITHUB_ACTOR}-${BRANCH_NAME}"
echo "Tag name: ${TAG}"
echo "::set-output name=tag::${TAG}"
echo "tag=${TAG}" >>"$GITHUB_OUTPUT"

###################################
# Build image locally for testing #
Expand Down Expand Up @@ -210,7 +210,7 @@ jobs:
##############################################
# Check Docker image security with Trivy #
##############################################

- name: Run Trivy vulnerability scanner
uses: aquasecurity/trivy-action@master
with:
Expand All @@ -221,4 +221,4 @@ jobs:
security-checks: vuln
vuln-type: 'os,library'
severity: 'CRITICAL,HIGH'
timeout: 10m0s
timeout: 10m0s
12 changes: 7 additions & 5 deletions .github/workflows/deploy-RELEASE.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
runs-on: ubuntu-latest
# Only run on main repo
if: github.repository == 'oxsecurity/megalinter' && !contains(github.event.head_commit.message, 'skip deploy')
environment:
environment:
name: latest
##################
# Load all steps #
Expand Down Expand Up @@ -109,10 +109,12 @@ jobs:
- name: Get release version
id: version
run: |
echo "::set-output name=cversion::$(git describe --tags --match='v*' --abbrev=0 | cut -c2-)"
echo "::set-output name=ctag::$(git describe --tags --match='v*' --abbrev=0)"
echo "::set-output name=pversion::$(git describe --abbrev=0 --match='v*' --tags $(git rev-list --tags --skip=1 --max-count=1) | cut -c2-)"
echo "::set-output name=ptag::$(git describe --abbrev=0 --match='v*' --tags $(git rev-list --tags --skip=1 --max-count=1))"
{
echo "cversion=$(git describe --tags --match='v*' --abbrev=0 | cut -c2-)"
echo "ctag=$(git describe --tags --match='v*' --abbrev=0)"
echo "pversion=$(git describe --abbrev=0 --match='v*' --tags "$(git rev-list --tags --skip=1 --max-count=1)" | cut -c2-)"
echo "ptag=$(git describe --abbrev=0 --match='v*' --tags "$(git rev-list --tags --skip=1 --max-count=1)")"
} >>"$GITHUB_OUTPUT"
- name: Print tags
run: |
echo "prev tag ${{ steps.version.outputs.ptag }}"
Expand Down
2 changes: 1 addition & 1 deletion megalinter/MegaLinter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

Copy link
Member

@nvuillam nvuillam Oct 23, 2022 •
edited

Choose a reason for hiding this comment

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

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 ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

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")

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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 :)

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Ha ha, the meme that will never die.

if self.status == "success":
logging.info(c.green("✅ Successfully linted all files without errors"))
config.delete()
Expand Down