forgejo-release.sh: Improve support for different Linux bases #95

Synchro · 2025-09-30T22:12:25Z

Synchro commented

2025-09-30 22:12:25 +00:00

First-time contributor

Currently if you run this action and aren't using a Debian based runner you need to install all tools ahead of time or the Action fails.

Calls to dpkg and apt are replaced with Distro-agnostic equivalents. Additionally the use of which has also been swapped out with command -v for use in environments where which isn't available.
This should improve the behaviour so that it will attempt to install all dependencies on a best effort basis, and at the very least provide some information to the console.

Currently if you run this action and aren't using a Debian based runner you need to install all tools ahead of time or the Action fails. Calls to dpkg and apt are replaced with Distro-agnostic equivalents. Additionally the use of `which` has also been swapped out with `command -v` for use in environments where `which` isn't available. This should improve the behaviour so that it will attempt to install all dependencies on a best effort basis, and at the very least provide some information to the console.

earl-warren requested changes

2025-10-01 07:59:33 +00:00

forgejo-release.sh Outdated

					
				@ -172,0 +191,4 @@

				    echo "jq and/or curl missing, attempting to install..." >&2

				    if command -v apt-get >/dev/null 2>&1; then

these all need to be tested

I've tested this with all the listed package managers locally and they all work properly.

Do you want me to write a test in CI to do this? Adding a unit test is also something that could be done if wanted.

I've tested this with all the listed package managers locally and they all work properly. Do you want me to write a test in CI to do this? Adding a unit test is also something that could be done if wanted.

Also: looking at this again, yum is fairly niche and could probably be removed.
I have a version of CentOS 7 that I run locally when an archaic build environment is required.

Also: looking at this again, yum is fairly niche and could probably be removed. I have a version of CentOS 7 that I run locally when an archaic build environment is required.

@Synchro wrote in #95 (comment):

Do you want me to write a test in CI to do this? Adding a unit test is also something that could be done if wanted.

Yes, this is what will guard this against future regressions. Untested code, however small and simple, tend to break. And without CI, fixing it will require a contributor to manually re-created the problem which is going to be non trivial.

@Synchro wrote in https://code.forgejo.org/actions/forgejo-release/pulls/95#issuecomment-62570: > Do you want me to write a test in CI to do this? Adding a unit test is also something that could be done if wanted. Yes, this is what will guard this against future regressions. Untested code, however small and simple, tend to break. And without CI, fixing it will require a contributor to manually re-created the problem which is going to be non trivial.

I'll make some changes to the test.yml workflow. Due to the actions/checkout action being used, images with Node would be required. I will remove everything except the apt and apkinstall lines, the fallback message is an improvement over how it currently functions now.

Testing on Alpine and Debian should be enough; seeing as the release action already assumes you're in a Debian environment.

I'll make some changes to the test.yml workflow. Due to the actions/checkout action being used, images with Node would be required. I will remove everything except the `apt` and `apk`install lines, the fallback message is an improvement over how it currently functions now. Testing on Alpine and Debian should be enough; seeing as the release action already assumes you're in a Debian environment.

Synchro marked this conversation as resolved

Synchro force-pushed distro-agnostic from a91ae74f37

/ integration (pull_request) Has been cancelled

Details

/ tests (pull_request) Has been cancelled

Details

to 854ee4ce4f

/ integration (pull_request) Has been cancelled

Details

/ tests (lts-alpine) (pull_request) Has been cancelled

Details

/ tests (lts) (pull_request) Has been cancelled

Details

2025-10-07 08:56:12 +00:00

Compare

Synchro commented

2025-10-07 08:58:55 +00:00

First-time contributor

I have updated the tests.yml workflow so that it runs on a Debian and Alpine base image.

If jq and curl aren't found packages will attempt to be installed. On other distros that we can't test for here, an error message is displayed.

I have updated the tests.yml workflow so that it runs on a Debian and Alpine base image. If jq and curl aren't found packages will attempt to be installed. On other distros that we can't test for here, an error message is displayed.

earl-warren requested changes

2025-10-07 14:10:39 +00:00

.forgejo/workflows/tests.yml Outdated

					
				@ -4,1 +6,4 @@

				        tag: [lts, lts-alpine]

				    runs-on: docker

				    container:

				      image: "node:${{ matrix.tag }}"

on: [ pull_request, push ]
jobs:
  tests:
    strategy:
      matrix:
        image: 
          - code.forgejo.org/oci/node:22-trixie
          - code.forgejo.org/oci/node:22-alpine
    runs-on: docker
    container:
      image: ${{ matrix.image }}

Is needed because:

hub.docker.io will impose severe rate limiting, using mirrored images avoid that
renovate will take care of proposing upgrades when needed. It may not match exactly LTS but it will be in sync with what Forgejo uses

Also lts and lts-alpine are not mirrored at the moment 😁

```yaml on: [ pull_request, push ] jobs: tests: strategy: matrix: image: - code.forgejo.org/oci/node:22-trixie - code.forgejo.org/oci/node:22-alpine runs-on: docker container: image: ${{ matrix.image }} ``` Is needed because: - hub.docker.io will impose severe rate limiting, using mirrored images avoid that - renovate will take care of proposing upgrades when needed. It may not match exactly LTS but it will be in sync with what Forgejo uses Also lts and lts-alpine are not mirrored at the moment 😁

Sweet that makes things easy 😄

Didn't realise you guys had local mirrors, I'll swap those out and then hopefully all is good.

Sweet that makes things easy 😄 Didn't realise you guys had local mirrors, I'll swap those out and then hopefully all is good.

This should be in place now as of 1d79e86315

Let me know if anything else needs to be adjusted.

This should be in place now as of https://code.forgejo.org/actions/forgejo-release/commit/1d79e86315db6951e9ea35afd461f8bce5a1fd62 Let me know if anything else needs to be adjusted.

Synchro marked this conversation as resolved

Synchro force-pushed distro-agnostic from 854ee4ce4f

/ integration (pull_request) Has been cancelled

Details

/ tests (lts-alpine) (pull_request) Has been cancelled

Details

/ tests (lts) (pull_request) Has been cancelled

Details

to 1d79e86315

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 38s

Details

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Failing after 6s

Details

/ integration (pull_request) Successful in 19m51s

Details

2025-10-07 20:54:51 +00:00

Compare

earl-warren commented

2025-10-08 07:43:25 +00:00

https://code.forgejo.org/actions/forgejo-release/actions/runs/846/jobs/0/attempt/2

the checkout action fails on 22-alpine 🤔

https://code.forgejo.org/actions/forgejo-release/actions/runs/846/jobs/0/attempt/2 the checkout action fails on 22-alpine 🤔

earl-warren requested changes

2025-10-08 09:58:30 +00:00

.forgejo/workflows/tests.yml

you need to install bash for alpline before running the tests, that's why it fails

earl-warren commented

2025-10-08 09:59:53 +00:00

@earl-warren wrote in #95 (comment):

https://code.forgejo.org/actions/forgejo-release/actions/runs/846/jobs/0/attempt/2

the checkout action fails on 22-alpine 🤔

It works, it is bash missing, of course. It is worth a mention in the action.yml / README

@earl-warren wrote in https://code.forgejo.org/actions/forgejo-release/pulls/95#issuecomment-62751: > https://code.forgejo.org/actions/forgejo-release/actions/runs/846/jobs/0/attempt/2 > > the checkout action fails on 22-alpine :thinking: It works, it is bash missing, of course. It is worth a mention in the action.yml / README

Synchro commented

2025-10-08 18:40:32 +00:00

First-time contributor

@earl-warren wrote in #95 (comment):

@earl-warren wrote in #95 (comment):

https://code.forgejo.org/actions/forgejo-release/actions/runs/846/jobs/0/attempt/2
the checkout action fails on 22-alpine 🤔

It works, it is bash missing, of course. It is worth a mention in the action.yml / README

Ah that's a good point. Do you have any issues with dropping some bash-isms in the script(s) and using POSIX sh instead?

Functionality should be identical, just some minor syntax adjustments should be needed. That should solve any issues for images that lack bash

@earl-warren wrote in https://code.forgejo.org/actions/forgejo-release/pulls/95#issuecomment-62762: > @earl-warren wrote in #95 (comment): > > > https://code.forgejo.org/actions/forgejo-release/actions/runs/846/jobs/0/attempt/2 > > the checkout action fails on 22-alpine :thinking: > > It works, it is bash missing, of course. It is worth a mention in the action.yml / README Ah that's a good point. Do you have any issues with dropping some bash-isms in the script(s) and using POSIX sh instead? Functionality should be identical, just some minor syntax adjustments should be needed. That should solve any issues for images that lack `bash`

Synchro added 1 commit

2025-10-09 20:37:36 +00:00

Refactor bash-isms and use POSIX sh where applicable

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Failing after 10s

Details

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 12s

Details

/ integration (pull_request) Successful in 17m48s

Details

324005b6c0

Synchro added 1 commit

2025-10-09 20:53:22 +00:00

forgejo-release.sh: Handle dependency globally

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Failing after 10s

Details

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 15s

Details

/ integration (pull_request) Successful in 17m56s

Details

8cfa95b448

This ensures checks are consistent  where needed. setup_api was retained in case of changes that might need to be made in the future.

Synchro added 1 commit

2025-10-09 21:00:10 +00:00

forgejo-release.sh: Allow arguments to be checked in check_dependencies

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Failing after 12s

Details

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 14s

Details

/ integration (pull_request) Successful in 18m24s

Details

fc4e870d23

This allows us to pass in extra dependencies in testing scenarios. In this case sign_release requires gpg to run.

Synchro added 1 commit

2025-10-09 21:06:24 +00:00

forgejo-release.sh: gpg-agent is also required for signing releases.

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Successful in 13s

Details

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 14s

Details

/ integration (pull_request) Successful in 18m30s

Details

603d197551

Synchro commented

2025-10-09 21:09:37 +00:00

First-time contributor

Okay, it looks like I have everything working according to the tests!

Let me know if you'd like any changes to the implementation, or if there is anything new that needs to be added to the integration.yml workflow.

Okay, it looks like I have everything working according to the tests! Let me know if you'd like any changes to the implementation, or if there is anything new that needs to be added to the `integration.yml` workflow.

mahlzahn requested changes

2025-10-09 22:35:54 +00:00

Dismissed

.forgejo/workflows/tests.yml

					
				@ -4,0 +4,4 @@

				    strategy:

				      matrix:

				        image: 

				          - code.forgejo.org/oci/node:22-trixie

not data.forgejo.org? @earl-warren ?

that's ok for now, it lands on code.forgejo.org anyways. Only actions are on a separate server.

Synchro marked this conversation as resolved

forgejo-release.sh Outdated

					
				@ -29,0 +43,4 @@

				}

				check_dependencies() {

				    # Always required for API parsing and downloads

Concerning my comment below, don’t check always for these default deps, but make them all extra arguments.

Good point! I was tempted to go that route initially and I guess I should have in hindsight 😄

I can make that adjustment, setup_api can pass in curl and jq as arguments. One limitation that might be good to adjust in the future when it becomes a problem is that the binary names that are searched for with command -v may not match the package name.

As of now however it seems like the package name matches the binaries that they provide (for the packages that this action needs)

Good point! I was tempted to go that route initially and I guess I should have in hindsight 😄 I can make that adjustment, setup_api can pass in curl and jq as arguments. One limitation that might be good to adjust in the future when it becomes a problem is that the binary names that are searched for with `command -v` may not match the package name. As of now however it seems like the package name matches the binaries that they provide (for the packages that this action needs)

Fixed with !95 (commit 5d73a41e6a)

Fixed with https://code.forgejo.org/actions/forgejo-release/pulls/95/commits/5d73a41e6aa01cec0ff1401740ecec68edd854d2

Synchro marked this conversation as resolved

forgejo-release.sh Outdated

					
				@ -128,3 +175,2 @@

				sign_release() {

				    local passphrase

				    if test -s "$GPG_PASSPHRASE"; then

				    check_dependencies gpg gpg-agent

After this latest edit it seems to me that check_dependencies should always be called with input arguments, e.g., sign_release does not need jq and curl.

After this latest edit it seems to me that `check_dependencies` should always be called with input arguments, e.g., `sign_release` does not need jq and curl.

Fixed with !95 (commit 5d73a41e6a)

Fixed with https://code.forgejo.org/actions/forgejo-release/pulls/95/commits/5d73a41e6aa01cec0ff1401740ecec68edd854d2

Synchro marked this conversation as resolved

mahlzahn commented

2025-10-09 22:36:36 +00:00

First-time contributor

@Synchro wrote in #95 (comment):

Ah that's a good point. Do you have any issues with dropping some bash-isms in the script(s) and using POSIX sh instead?

Functionality should be identical, just some minor syntax adjustments should be needed. That should solve any issues for images that lack bash

Looks good, in my opinion!

@Synchro wrote in https://code.forgejo.org/actions/forgejo-release/pulls/95#issuecomment-62801: > Ah that's a good point. Do you have any issues with dropping some bash-isms in the script(s) and using POSIX sh instead? > > Functionality should be identical, just some minor syntax adjustments should be needed. That should solve any issues for images that lack `bash` Looks good, in my opinion!

👍 1

mahlzahn reviewed

2025-10-09 22:40:59 +00:00

.forgejo/workflows/tests.yml

					
				@ -4,0 +5,4 @@

				      matrix:

				        image: 

				          - code.forgejo.org/oci/node:22-trixie

				          - code.forgejo.org/oci/node:22-alpine

why not node:24?

why not `node:24`?

This was to make sure that the LTS release is targeted, there is no LTS tag that is mirrored.

See below for context:
#95 (comment)

This was to make sure that the LTS release is targeted, there is no `LTS` tag that is mirrored. See below for context: https://code.forgejo.org/actions/forgejo-release/pulls/95#issuecomment-62669

OK, thanks for the explanation.
@earl-warren Why can't I resolve threads here?

OK, thanks for the explanation. @earl-warren Why can't I resolve threads here?

Synchro marked this conversation as resolved

Synchro added 1 commit

2025-10-09 23:16:53 +00:00

forgejo-release.sh: check_dependencies should only act on passed arguments.

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 9s

Details

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Successful in 12s

Details

/ integration (pull_request) Successful in 17m48s

Details

5d73a41e6a

Removed the default curl and jq option, and passed it in when needed.

mahlzahn approved these changes

2025-10-10 07:39:01 +00:00

earl-warren commented

2025-10-10 10:16:30 +00:00

that changes the scope of the pull request significantly. It is excellent the tests pass but it will take time to review it thoroughly.

Could you split this refactor in a different pull request? It is fine with me on principle.

that changes the scope of the pull request significantly. It is excellent the tests pass but it will take time to review it thoroughly. Could you split this refactor in a different pull request? It is fine with me on principle.

mahlzahn commented

2025-10-10 10:20:50 +00:00

First-time contributor

@earl-warren wrote in #95 (comment):

that changes the scope of the pull request significantly. It is excellent the tests pass but it will take time to review it thoroughly.

Could you split this refactor in a different pull request? It is fine with me on principle.

I found reviewing via the new single-commit review with split view pleasant for this PR. But, up to you to decide.

@earl-warren wrote in https://code.forgejo.org/actions/forgejo-release/pulls/95#issuecomment-62983: > that changes the scope of the pull request significantly. It is excellent the tests pass but it will take time to review it thoroughly. > > Could you split this refactor in a different pull request? It is fine with me on principle. I found reviewing via the new single-commit review with split view pleasant for this PR. But, up to you to decide.

earl-warren commented

2025-10-10 10:32:21 +00:00

I agree this pull request is very pleasant to work with. There are two very different topics (refactor + alpine support) and I do prefer they are in two separate pull requests to help with discovery in the future.

👍 1

Synchro commented

2025-10-18 11:24:57 +00:00

First-time contributor

Apologies I got swamped with work, so just got to looking at this now.

I can go ahead and split this PR into two parts, but in order for Alpine support to function correctly without first installing bash the refactor is also needed. It feels a bit weird to require bash for this PR when in reality a minor refactor is needed to avoid it entirely.

If I do split this PR in two parts the testing will fail when run on an Alpine container. So I can either disable Alpine tests or omit it from this PR entirely or I can rephrase this PR to be a POSIX sh refactor + improvements to dependency checking.

If reordering the commits is what you'd like to make it easier to review (or make more sense overall) let me know.

The refactor only really does some minor syntax changes (dealing with arrays, debugging adjustments) other functionality is untouched.

Apologies I got swamped with work, so just got to looking at this now. I can go ahead and split this PR into two parts, but in order for Alpine support to function correctly without first installing bash the refactor is also needed. It feels a bit weird to require bash for this PR when in reality a minor refactor is needed to avoid it entirely. If I do split this PR in two parts the testing will fail when run on an Alpine container. So I can either disable Alpine tests or omit it from this PR entirely or I can rephrase this PR to be a POSIX sh refactor + improvements to dependency checking. If reordering the commits is what you'd like to make it easier to review (or make more sense overall) let me know. The refactor only really does some minor syntax changes (dealing with arrays, debugging adjustments) other functionality is untouched.

crueter requested changes

2025-11-12 02:30:56 +00:00

forgejo-release.sh

					
				@ -29,0 +63,4 @@

				            return 1

				        fi

				    fi

				}

I would also add dnf and pacman. Do note that this will not work on bare-metal (dockerless) runners.

I would also add `dnf` and `pacman`. Do note that this will not work on bare-metal (dockerless) runners.

0xlogn referenced this pull request

2025-12-31 05:45:44 +00:00

fix: properly check for bash in $PATH #111

/ tests (code.forgejo.org/oci/node:22-trixie) (pull_request) Successful in 9s

Details

/ tests (code.forgejo.org/oci/node:22-alpine) (pull_request) Successful in 12s

Details

/ integration (pull_request) Successful in 17m48s

Required

Details

/ tests (pull_request)

Required

This pull request has changes conflicting with the target branch.

.forgejo/workflows/tests.yml
forgejo-release.sh

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u distro-agnostic:Synchro-distro-agnostic

git switch Synchro-distro-agnostic

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

4 participants

Notifications

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

actions/forgejo-release!95

Rows
Columns