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

Support running inside windows self-hosted runner #43

Merged
merged 2 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions action.py
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 had encoding issues with emojis, that is why i added that new line for the emojis. I took as reference this post https://stackoverflow.com/questions/3597480/how-to-make-python-3-print-utf8

Copy link
Member

Choose a reason for hiding this comment

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

Rather than fiddling with sys.stdout.buffer directly, could you try using sys.stdout.reconfigure? For example, calling this once somewhere early in the program should be sufficient:

sys.stdout.reconfigure(encoding='utf-8')

...and then we can continue using print as normal everywhere else 🙂

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 have tested a couple of things to avoid the encoding issue but have no luck. I have just tested what you mentioned but got the error again. "UnicodeEncodeError: 'charmap' codec can't encode character '\u274c' in position 0: character maps to <undefined>". Maybe i need to think about something else to try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After, testing a little bit more i realized that i was missing encoding="utf-8" where GITHUB_STEP_SUMMARY and GITHUB_OUTPUT were opened. Hopefully, it won't break anything. I updated the code.

Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ def _pip_audit(*args):


def _fatal_help(msg):
print(f"::error::❌ {msg}")
msg = f"::error::❌ {msg}"
print(sys.stdout.buffer.write(msg.encode("utf-8")))
sys.exit(1)


Expand Down Expand Up @@ -131,9 +132,11 @@ def _fatal_help(msg):
_debug(status.stdout)

if status.returncode == 0:
_summary("🎉 pip-audit exited successfully")
msg = "🎉 pip-audit exited successfully"
_summary(sys.stdout.buffer.write(msg.encode("utf-8")))
else:
_summary("❌ pip-audit found one or more problems")
msg = "❌ pip-audit found one or more problems"
_summary(sys.stdout.buffer.write(msg.encode("utf-8")))

output = "⚠️ pip-audit did not return any output"
try:
Expand Down
6 changes: 5 additions & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ runs:
# NOTE: Sourced, not executed as a script.
source "${{ github.action_path }}/setup/venv.bash"

${{ github.action_path }}/action.py "${{ inputs.inputs }}"
if [[ "${{runner.os}}" == "Windows" ]]; then
python "${{ github.action_path }}/action.py" "${{ inputs.inputs }}"
else
${{ github.action_path }}/action.py "${{ inputs.inputs }}"
fi
Copy link
Member

Choose a reason for hiding this comment

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

This Windows form should work well in both cases, so maybe skip the conditional entirely?

Suggested change
if [[ "${{runner.os}}" == "Windows" ]]; then
python "${{ github.action_path }}/action.py" "${{ inputs.inputs }}"
else
${{ github.action_path }}/action.py "${{ inputs.inputs }}"
fi
python "${{ github.action_path }}/action.py" "${{ inputs.inputs }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh that would be fine then. I just thought about not changing the existing lines to not break anything since i don't have a Linux environment to test.

Copy link
Contributor Author

@AngelMF AngelMF Oct 27, 2023

Choose a reason for hiding this comment

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

I updated the code and left the "python" line.

env:
GHA_PIP_AUDIT_SUMMARY: "${{ inputs.summary }}"
GHA_PIP_AUDIT_NO_DEPS: "${{ inputs.no-deps }}"
Expand Down
7 changes: 6 additions & 1 deletion setup/venv.bash
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ fi
# `python -m pip install ...` invocation might happen to choose.
if [[ -n "${GHA_PIP_AUDIT_VIRTUAL_ENVIRONMENT}" ]] ; then
if [[ -d "${GHA_PIP_AUDIT_VIRTUAL_ENVIRONMENT}" ]]; then
source "${GHA_PIP_AUDIT_VIRTUAL_ENVIRONMENT}/bin/activate"
if [[ "$OSTYPE" == "msys" || "$(uname)" == MSYS_NT* || "$(uname)" == MINGW* ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Where is OSTYPE coming from?

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 think it is coming from Bash. If you think it could break the Linux code i can remove it. Anyways i put two more validations to identify if OS is Windows. It will be ok if i have to remove it.

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 updated the code and removed $OSTYPE.

# execute in windows
source "${GHA_PIP_AUDIT_VIRTUAL_ENVIRONMENT}/scripts/activate"
else
source "${GHA_PIP_AUDIT_VIRTUAL_ENVIRONMENT}/bin/activate"
fi
else
die "Fatal: virtual environment is not a directory"
fi
Expand Down