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

Refactor into a composite action #9

Merged
merged 47 commits into from
Jun 16, 2022
Merged

Refactor into a composite action #9

merged 47 commits into from
Jun 16, 2022

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Jun 14, 2022

Closes #8.
Closes #2.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
WIP.

Signed-off-by: William Woodruff <[email protected]>
@woodruffw woodruffw self-assigned this Jun 14, 2022
@woodruffw
Copy link
Member Author

This works, but some open questions:

  1. How do composite actions interact with their parent workflow? I don't have to install python or any other state here, presumably because I'm running in the ubuntu-latest instance defined in the parent workflow, but can I rely on this? Should the action opportunistically use actions/setup-python if it can't detect a modern enough Python?
  2. How does this interact with the target repository's working directory? Normally it's the $CWD after an actions/checkout, but we don't guarantee that here.
  3. How should this interact with virtual environments? Right now this action is completely unaware of them (and was before as well)

These are wrong; we don't want to override the working directory like this.

Signed-off-by: William Woodruff <[email protected]>
@woodruffw
Copy link
Member Author

  1. How do composite actions interact with their parent workflow? I don't have to install python or any other state here, presumably because I'm running in the ubuntu-latest instance defined in the parent workflow, but can I rely on this? Should the action opportunistically use actions/setup-python if it can't detect a modern enough Python?

I think the answers to this are "technically no, but yes" and "depends on the first."

In principle it's a user error to attempt to run pip-audit on a host that doesn't have Python installed, since there's no Python environments whatsoever to audit. But maybe that's too legalistic of us, since someone might want to audit a fully-resolved and hashed requirements file without worrying about the rest of the Python environment? We should probably support that case.

If the first is "no", then the second is definitely "yes."

@woodruffw
Copy link
Member Author

2. How does this interact with the target repository's working directory? Normally it's the $CWD after an actions/checkout, but we don't guarantee that here.

I'm not 100% sure about this, but I think this will end up being a non-issue. I suspect that this action will inherit the $CWD from the checkout, since that's what seems to happen implicitly with every other action in the GitHub Actions ecosystem.

@woodruffw
Copy link
Member Author

3. How should this interact with virtual environments? Right now this action is completely unaware of them (and was before as well)

For this, we should probably figure out a configuration setting. Something like venv: path-to-dir/, which we'd then...opportunistically load into the environment, right before action.py? Or something similar? Or maybe we just add ${{ inputs.venv }}/bin to the head of the $PATH and let pip to the rest?

woodruffw added 16 commits June 15, 2022 16:22
Signed-off-by: William Woodruff <[email protected]>
Can't really be null, since we unconditionally populate variables.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Conflicts with virtual environments.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
...to avoid newlines, which break ::set-output

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Sigh.

Signed-off-by: William Woodruff <[email protected]>
@woodruffw
Copy link
Member Author

woodruffw commented Jun 16, 2022

The venv self-test is blocked on a fix for pypa/pip-audit#156.

Edit: Unblocked by installing pip-audit into the venv, which hides the bug.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Not supported yet, see pypa/pip-audit#305.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
@woodruffw woodruffw marked this pull request as ready for review June 16, 2022 18:32
@woodruffw woodruffw merged commit a84a2d8 into main Jun 16, 2022
@woodruffw woodruffw deleted the ww/composite-action branch June 16, 2022 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to a "composite" action Add a debugging/verbose option
1 participant