Change ${{ github.action_path }} to ${GITHUB_ACTION_PATH}

[sroet]

implementing the proposed workaround for ${{ github.action_path }} not working inside containers.
actions/runner#2185 (comment)

should close #300

[webknjaz]

🫶 Thanks, that's an interesting discovery!

🤔 In #302 (comment), I've outlined some of my thoughts on whether this should be done. I'm kinda mentally prepared to declare it out of the scope since it implies (in people's minds) that it's okay to use workflows insecurely, and I don't want to reinforce that belief. Additionally, declaring arbitrary runtimes supported without a way to test them / integrate into CI etc. is not really sustainable. These two points are why I've been vocal about the job separation and only supporting Linux runners since the creation of the action. Previously, it didn't even cross my mind that somebody would run this inside runtimes other than ubuntu-latest.

🤷‍♂️ While merging this patch is an easy thing to do, the maintenance cost would probably be overwhelming. I feel like I need to document the expectations for “things supported / intended to function / bad if they break” and “hope-driven / lucky if works / not possible to test sustainably”...

[nikaro]

that it's okay to use workflows insecurely

I don't see it this way, for my use case it is just a workaround (for this bug actions/runner#2185 (comment)) to allow running this Action inside a container. Which could be arguably more secure than running it on the bare runner.

But you have a fair point about maintenance cost, it may open the box for a bunch of "exotics" use cases. It is up to you.

In the meantime i switched to separate jobs based on GitHub artifacts as suggested by @sroet, and i'm fine with it 🤷 But then may be it should be documented somewhere that containerised jobs are not supported.

[sroet]

In #302 (comment), I've outlined some of my thoughts on whether this should be done. I'm kinda mentally prepared to declare it out of the scope since it implies (in people's minds) that it's okay to use workflows insecurely, and I don't want to reinforce that belief. Additionally, declaring arbitrary runtimes supported without a way to test them / integrate into CI etc. is not really sustainable. These two points are why I've been vocal about the job separation and only supporting Linux runners since the creation of the action. Previously, it didn't even cross my mind that somebody would run this inside runtimes other than ubuntu-latest

I understand the maintenance burden, and am fully happy for you to close this one if you decide it will be to much.

However, I would like to plead the case for this specific issue(which only solves the running inside containers (not composite actions in general like #302)):

1. This is a know runner bug and could be reverted once that is fixed
2. It should be testable by copying the smoke-test job and adding container: ubuntu:latest under runs-on: ubuntu-latest (ref) (I can add the test on this PR if you want)
3. As @nikaro mentions, running inside containers even on github runners can be considered more safe (and repeatable) and is in my opinion currently the only way of running actions safely on self-hosted (non-cloud) runners.

By adding the test mentioned in part 2, you would not support more OS configs as ubuntu-latest runners run the ubuntu:latest image anyway if I remember correctly.

Again, I totally understand not wanting scope increase for a project like this and would totally understand if you declared this out of scope. Thanks anyway for this very useful action and help so far tracking this down 🫶

[webknjaz]

Thanks for all your inputs! I'd like to merge this, but still document the case as unsupported.
And I'd like to try adding the CI check, but not as a copy of the job — I'd make a matrix and would move building the test dists out. I'll think about it more. Meanwhile, I tried documenting the support observations @ 7252a9a.

[webknjaz]

@sroet I experimented with the CI on your branch (the commits will need to be cleaned up later). I tried both python:3.12-slim and ubuntu-latest. But it appears that it's either not enough or the patch is incomplete.
If you'd like to invest some time into trying to get this across the finish line — please add more commits on top (we'll clean them up later once there's something that resembles a working CI job). But note that each push needs a CI run approval in the PR, so you might be better off experimenting in a separate repo.

[webknjaz]

Sadly, I can't proceed with working on this myself, as I have FOSS responsibilities in many other projects.

[sroet]

Sadly, I can't proceed with working on this myself, as I have FOSS responsibilities in many other projects.

Very understandable, I unfortunately also have other commitments for the rest of this year, but might be able to pick this up in January! Thanks for the effort so far!

[webknjaz]

No problem! Let's keep this open, then.