Feature: Added an action type for the action.yaml that uses sh #141

Merged

earl-warren merged 8 commits from slatian/act:action-using-sh into main 2025-06-14 15:31:33 +00:00

Conversation 20 Commits 8 Files changed 8 +89 -8

slatian commented 2025-06-07 23:58:34 +00:00

Contributor

Copy link

Currently the only way to get pre and post actions is to go through the nodejs mechanism, which is quite wasteful when all one wants to do is run a couple of shell commands, I'm trying to get around this with this patch.

It works similar to the node* actions in that it supports pre, main and post.

It is different in that these strings are passed to the system shell using sh -c and execute similar to the composite run action with the shell set to sh.

Example action to make use of this patch: https://codeberg.org/slatian/test-action/src/branch/main/action.yaml

References

GitHub Actions on the topic of pre / post support for composite actions https://github.com/actions/runner/issues/1478

Currently, three Action types are supported: Docker, JavaScript and Composite (see https://docs.github.com/en/actions/creating-actions/about-custom-actions#types-of-actions). However, features pre, pref-if, post and post-if are only is supported in JavaScript Actions only (see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions). Therefore, users writing workflows/actions using some scripting language such as Python are forced to wrap the steps in JavaScript in order to register pre and post steps. See, for example, https://github.com/pyTooling/Actions/tree/main/with-post-step:

Currently the only way to get pre and post actions is to go through the nodejs mechanism, which is quite wasteful when all one wants to do is run a couple of shell commands, I'm trying to get around this with this patch. It works similar to the node* actions in that it supports `pre`, `main` and `post`. It is different in that these strings are passed to the system shell using `sh -c` and execute similar to the composite run action with the shell set to `sh`. Example action to make use of this patch: https://codeberg.org/slatian/test-action/src/branch/main/action.yaml ### References GitHub Actions on the topic of pre / post support for composite actions https://github.com/actions/runner/issues/1478 > Currently, three Action types are supported: Docker, JavaScript and Composite (see https://docs.github.com/en/actions/creating-actions/about-custom-actions#types-of-actions). However, features `pre`, `pref-if`, `post` and `post-if` are only is supported in JavaScript Actions only (see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions). Therefore, users writing workflows/actions using some scripting language such as Python are forced to wrap the steps in JavaScript in order to register pre and post steps. See, for example, https://github.com/pyTooling/Actions/tree/main/with-post-step:

cascading-pr referenced this pull request from forgejo/runner 2025-06-07 23:59:33 +00:00

cascading-pr from https://code.forgejo.org/forgejo/act refs/pull/141/head to forgejo/act-141 #594

cascading-pr commented 2025-06-07 23:59:34 +00:00

Contributor

Copy link

cascading-pr updated at forgejo/runner#594

cascading-pr updated at https://code.forgejo.org/forgejo/runner/pulls/594

earl-warren changed title from Feature: Added an action type for the action.yaml that uses sh to Feature: Added an action type for the action.yaml that uses sh [skip cascade] 2025-06-08 09:31:34 +00:00

earl-warren commented 2025-06-08 09:34:57 +00:00

Contributor

Copy link

I use composite quite often but never used pre/post. Does it work with composite as well?

I use composite quite often but never used pre/post. Does it work with composite as well?

slatian commented 2025-06-08 10:41:21 +00:00

Author

Contributor

Copy link

You should be able to use the sh actions inside other composite actions without any problems as it is using the same interface as all the others (will test) but I've actively decided against touching composite actions themselves as that would have added more complexity in a place where it easily gets into the way of keeping up to date with upstream. This way it's also easier to draw a line between upstream and forgejo-flavored.

I'll also have a look at the failed test.

You should be able to use the `sh` actions inside other composite actions without any problems as it is using the same interface as all the others (will test) but I've actively decided against touching composite actions themselves as that would have added more complexity in a place where it easily gets into the way of keeping up to date with upstream. This way it's also easier to draw a line between upstream and forgejo-flavored. I'll also have a look at the failed test.

slatian commented 2025-06-08 12:45:29 +00:00

Author

Contributor

Copy link

Update: I've just tested it being used as part of a composite action and it works as expected.

Any changes you want me to apply?

For tests I might need a little pointer on how to write them.

Update: I've just tested it being used as part of a [composite action](https://codeberg.org/slatian/test-composite-action/src/branch/main/action.yaml) and it works as expected. Any changes you want me to apply? For tests I might need a little pointer on how to write them.

earl-warren commented 2025-06-09 06:47:12 +00:00

Contributor

Copy link

For tests I might need a little pointer on how to write them.

I suggest you look for tests covering the other type of pre/post and mimic what they do. I don't know where they are from the top of my head. Once you find them, also make sure they are actually run the current suite only has some (because some of the tests are GitHub runner specific).

> For tests I might need a little pointer on how to write them. I suggest you look for tests covering the other type of pre/post and mimic what they do. I don't know where they are from the top of my head. Once you find them, also make sure they are actually run [the current suite](https://code.forgejo.org/forgejo/act/src/branch/main/.forgejo/workflows/test.yml) only has some (because some of the tests are GitHub runner specific).

slatian commented 2025-06-12 21:10:49 +00:00

Author

Contributor

Copy link

I've pushed a fist attempt at writing a test that should execute all three commands.

Unfortunately docker being hardcoded for the tests makes testing, more difficult than necessary, which is why I haven't tested the test yet. I pushed it in the hoes that the CI here would run it, but I haven't found it in the logs.

In addition I've deployed a forgejo-runner with this version of act to my own runners, seems to work.

I've pushed a fist attempt at writing a test that should execute all three commands. Unfortunately docker being hardcoded for the tests makes testing, more difficult than necessary, which is why I haven't tested the test yet. I pushed it in the hoes that the CI here would run it, but I haven't found it in the logs. In addition I've deployed a `forgejo-runner` with this version of act to my own runners, seems to work.

earl-warren commented 2025-06-14 09:59:46 +00:00

Contributor

Copy link

@slatian wrote in #141 (comment):

in the hoes that the CI here would run it, but I haven't found it in the logs.

It is not trivial indeed. Can I suggest you introduce a failure deliberately to verify it runs at all? That would be proof enough.

@slatian wrote in https://code.forgejo.org/forgejo/act/pulls/141#issuecomment-42322: > in the hoes that the CI here would run it, but I haven't found it in the logs. It is not trivial indeed. Can I suggest you introduce a failure deliberately to verify it runs at all? That would be proof enough.

slatian commented 2025-06-14 11:05:32 +00:00

Author

Contributor

Copy link

Bad news, it doesn't run and it fails, along with a whole bunch of other tests. To run it:

go test -run=TestRunEventHostEnvironment -v ./pkg/runner

Bad news, it doesn't run and it fails, along with a whole bunch of other tests. To run it: ```sh go test -run=TestRunEventHostEnvironment -v ./pkg/runner ```

earl-warren commented 2025-06-14 11:34:54 +00:00

Contributor

Copy link

I tried running those in the not-too-distant past on my laptop but they fail. I suspect that's because they assume the test environment is setup in a way that only exists in GitHub Action. It is not a container but a virtual machine and has zillions of things pre-installed.

https://code.forgejo.org/forgejo/act/src/branch/main/.forgejo/workflows/test.yml

The test are running out of an application container which limits what they can install (or makes it needlessly complicated as in using DIND for instance).

Note that further down there are tests running on lxc-bookworm which is a LXC container where you can install pretty much anything you want.

To move forward, I suggest you create a new job for the sole purpose of running ./pkg/runner, out of an lxc-bookworm container. And there you can install the requirements.

I tried running those in the not-too-distant past on my laptop but they fail. I suspect that's because they assume the test environment is setup in a way that only exists in GitHub Action. It is not a container but a virtual machine and has zillions of things pre-installed. https://code.forgejo.org/forgejo/act/src/branch/main/.forgejo/workflows/test.yml The test are running out of an application container which limits what they can install (or makes it needlessly complicated as in using DIND for instance). Note that further down there are tests running on `lxc-bookworm` which is a LXC container where you can install pretty much anything you want. To move forward, I suggest you create a new job for the sole purpose of running ./pkg/runner, out of an lxc-bookworm container. And there you can install the requirements.

slatian force-pushed action-using-sh from 23a4c636fe to 16c401ea8b 2025-06-14 11:42:22 +00:00 Compare

slatian force-pushed action-using-sh from 0b2d004464 to ced8021692 2025-06-14 12:19:19 +00:00 Compare

slatian commented 2025-06-14 13:02:22 +00:00

Author

Contributor

Copy link

Okay besides the pre action, which doesn't get tested because local actions don't execute the pre-stage for intransparent github reasons my tests and code work now.

Do you want to get rid of this limitation?

My tests can be ran by themselves using:

• go test -run=TestRunEventHostEnvironment/uses-sh -v ./pkg/runner
• go test -run=TestRunEventHostEnvironment/uses-sh-test-action-path -v ./pkg/runner

Okay besides the `pre` action, which doesn't get tested because local actions don't execute the pre-stage for [intransparent github reasons](https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#runspre) my tests and code work now. Do you want to get rid of this limitation? My tests can be ran by themselves using: * `go test -run=TestRunEventHostEnvironment/uses-sh -v ./pkg/runner` * `go test -run=TestRunEventHostEnvironment/uses-sh-test-action-path -v ./pkg/runner`

earl-warren commented 2025-06-14 14:14:11 +00:00

Contributor

Copy link

@slatian wrote in #141 (comment):

Okay besides the pre action, which doesn't get tested because local actions don't execute the pre-stage for intransparent github reasons my tests and code work now.

Great!

Do you want to get rid of this limitation?

My tests can be ran by themselves using:

* `go test -run=TestRunEventHostEnvironment/uses-sh -v ./pkg/runner`

* `go test -run=TestRunEventHostEnvironment/uses-sh-test-action-path -v ./pkg/runner`

Can you add those two lines here?

.forgejo/workflows/test.yml

Lines 61 to 64 in 9348a70

	- name: unit test
	run: |
	go test -short -v ./pkg/container
	go test -v ./pkg/jobparser ./pkg/model ./pkg/exprparser

That will be perfect and ready to merge then 🚀

@slatian wrote in https://code.forgejo.org/forgejo/act/pulls/141#issuecomment-42547: > Okay besides the `pre` action, which doesn't get tested because local actions don't execute the pre-stage for [intransparent github reasons](https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#runspre) my tests and code work now. Great! > Do you want to get rid of this limitation? > > My tests can be ran by themselves using: > > * `go test -run=TestRunEventHostEnvironment/uses-sh -v ./pkg/runner` > > * `go test -run=TestRunEventHostEnvironment/uses-sh-test-action-path -v ./pkg/runner` Can you add those two lines here? https://code.forgejo.org/forgejo/act/src/commit/9348a70192bd4b8d75c3ac258b9a04321a8924ad/.forgejo/workflows/test.yml#L61-L64 That will be perfect and ready to merge then 🚀

slatian commented 2025-06-14 14:22:11 +00:00

Author

Contributor

Copy link

Can you add those two lines here?

Done!

> Can you add those two lines here? Done!

earl-warren changed title from Feature: Added an action type for the action.yaml that uses sh [skip cascade] to Feature: Added an action type for the action.yaml that uses sh 2025-06-14 14:55:55 +00:00

earl-warren approved these changes 2025-06-14 14:57:17 +00:00

earl-warren left a comment

Contributor

Checked to pass.

https://code.forgejo.org/forgejo/act/actions/runs/800#jobstep-10-1205

Checked to pass. https://code.forgejo.org/forgejo/act/actions/runs/800#jobstep-10-1205

earl-warren force-pushed action-using-sh from 677c5a93f5 to 225f4127f3 2025-06-14 14:57:33 +00:00 Compare

earl-warren scheduled this pull request to auto merge when all checks succeed 2025-06-14 14:57:58 +00:00

cascading-pr referenced this pull request from forgejo/runner 2025-06-14 14:58:47 +00:00

cascading-pr from https://code.forgejo.org/forgejo/act refs/pull/141/head to forgejo/act-141 #614

cascading-pr commented 2025-06-14 14:58:48 +00:00

Contributor

Copy link

cascading-pr updated at forgejo/runner#614

cascading-pr updated at https://code.forgejo.org/forgejo/runner/pulls/614

earl-warren merged commit 0ca07b8dd7 into main 2025-06-14 15:31:33 +00:00

earl-warren deleted branch action-using-sh 2025-06-14 15:31:33 +00:00

earl-warren referenced this pull request from a commit 2025-06-14 15:31:34 +00:00

Feature: Added an action type for the action.yaml that uses sh (#141)

earl-warren added the

Kind/Feature

label 2025-07-11 12:49:49 +00:00

earl-warren referenced this pull request from forgejo/runner 2025-07-11 12:50:12 +00:00

feat: fallback to sh if the shell is not set and bash is not available #150

earl-warren commented 2025-07-11 13:26:34 +00:00

Contributor

Copy link

Added to the description for the record:

---

GitHub Actions on the topic of pre / post support for composite actions https://github.com/actions/runner/issues/1478

Currently, three Action types are supported: Docker, JavaScript and Composite (see https://docs.github.com/en/actions/creating-actions/about-custom-actions#types-of-actions). However, features pre, pref-if, post and post-if are only is supported in JavaScript Actions only (see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions). Therefore, users writing workflows/actions using some scripting language such as Python are forced to wrap the steps in JavaScript in order to register pre and post steps. See, for example, https://github.com/pyTooling/Actions/tree/main/with-post-step:

Added to the description for the record: --- GitHub Actions on the topic of pre / post support for composite actions https://github.com/actions/runner/issues/1478 > Currently, three Action types are supported: Docker, JavaScript and Composite (see https://docs.github.com/en/actions/creating-actions/about-custom-actions#types-of-actions). However, features `pre`, `pref-if`, `post` and `post-if` are only is supported in JavaScript Actions only (see https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs-for-javascript-actions). Therefore, users writing workflows/actions using some scripting language such as Python are forced to wrap the steps in JavaScript in order to register pre and post steps. See, for example, https://github.com/pyTooling/Actions/tree/main/with-post-step:

earl-warren commented 2025-07-11 14:12:40 +00:00

Contributor

Copy link

@slatian I tried to add an end-to-end test at forgejo/end-to-end#787 but the pre step does not run. Did I do something obviously wrong or does this deserve a bug report?

@slatian I tried to add an end-to-end test at https://code.forgejo.org/forgejo/end-to-end/pulls/787 but the pre step does not run. Did I do something obviously wrong or does this deserve a bug report?

earl-warren referenced this pull request from forgejo/end-to-end 2025-07-11 14:13:00 +00:00

WIP: local-action-sh #787

slatian commented 2025-07-11 14:49:18 +00:00

Author

Contributor

Copy link

The syntax for IO is wrong, but after tinkering for a while I'm pretty sure my code is wrong too. I'll follow up with an explanation and another pull request.

The syntax for IO is wrong, but after tinkering for a while I'm pretty sure my code is wrong too. I'll follow up with an explanation and another pull request.

slatian commented 2025-07-11 15:10:56 +00:00

Author

Contributor

Copy link

Outputs are working.

Okay besides the pre action, which doesn't get tested because local actions don't execute the pre-stage for intransparent github reasons my tests and code work now.

-- Me, earlier in this thread …

Yes, I fell on my nose there too, the pre action won't run for local actions and now I know why … dependency problem, local actions come from the workspace which has to be populated at which point it is too late to run the pre stage of course … They could have explained that instead of saying it just doesn't work.

Which explains why where is no pre-stage.

For populating the outputs the correct syntax is the same as with javascript actions.

outputs:
     key:
        description: "some description"
 
runs:
  using: "sh"
  main: echo "key=something" >> $GITHUB_OUTPUT

And the test for the post command will fail simply because the post command hasn't run at that point in time, I'm also not sure if outputs are supported there at all because the post stage is for cleaning things up.

What I messed up though are the inputs, they should show up as INPUT_… environment variables, but they don't.

Outputs are working. > Okay besides the `pre` action, which doesn't get tested because local actions don't execute the pre-stage for intransparent github reasons my tests and code work now. > > -- Me, earlier in this thread … Yes, I fell on my nose there too, the `pre` action won't run for local actions and now I know why … dependency problem, local actions come from the workspace which has to be populated at which point it is too late to run the `pre` stage of course … They could have explained that instead of saying it just doesn't work. Which explains why where is no pre-stage. For populating the outputs the correct syntax is the same as with javascript actions. ```yaml outputs: key: description: "some description" runs: using: "sh" main: echo "key=something" >> $GITHUB_OUTPUT ``` And the test for the post command will fail simply because the post command hasn't run at that point in time, I'm also not sure if outputs are supported there at all because the post stage is for cleaning things up. What I messed up though are the inputs, they should show up as `INPUT_…` environment variables, but they don't.

slatian commented 2025-07-11 15:31:46 +00:00

Author

Contributor

Copy link

What I messed up though are the inputs, they should show up as INPUT_… environment variables, but they don't.

Not even that, for some reason the regex that should make sure only valid characters end up in the environment variable name lets minuses pass which aren't valid environment variables keys at some point (tbh. I have never seen a environment variable with a minus in it)

pkg/runner/action.go

Line 470 in d77afd3

envKey := regexp.MustCompile("[^A-Z0-9-]").ReplaceAllString(strings.ToUpper(inputID), "_")

To me this looks like a typo, but I haven't validated it yet.

Update: Minus characters in environment variable names are POSIX compliant, they're filtred out somewhere by mistake.

If you change the variable name to not include a minus it will work. foo becomes INPUT_FOO

> What I messed up though are the inputs, they should show up as INPUT_… environment variables, but they don't. Not even that, for some reason the regex that should make sure only valid characters end up in the environment variable name lets minuses pass which aren't valid environment variables keys at some point (tbh. I have never seen a environment variable with a minus in it) https://code.forgejo.org/forgejo/act/src/commit/d77afd36037d906a45d9d441636f282b23130f9e/pkg/runner/action.go#L470 ~~To me this looks like a typo, but I haven't validated it yet.~~ Update: Minus characters in environment variable names are [POSIX compliant](https://pubs.opengroup.org/onlinepubs/9799919799/), they're filtred out somewhere by mistake. If you change the variable name to not include a minus it will work. `foo` becomes `INPUT_FOO`

slatian commented 2025-07-11 16:43:42 +00:00

Author

Contributor

Copy link

@earl-warren Okay, that's all from me. I hope the information helps.

I'm unable to find where the inputs with minuses get lost right now.

@earl-warren Okay, that's all from me. I hope the information helps. I'm unable to find where the inputs with minuses get lost right now.

earl-warren referenced this pull request from a commit 2025-07-28 17:10:29 +00:00

Feature: Added an action type for the action.yaml that uses sh (#141)

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

earl-warren

Labels

Clear labels   

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind

Chore

Housekeeping, cleanup and chores

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

No labels

Compat/Breaking

Kind/Bug

Kind

Chore

Kind/Documentation

Kind/Enhancement

Kind/Feature

Kind/Security

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Status

Abandoned

Status

Blocked

Status

Need More Info

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo/act!141

No description provided.