actions/forgejo-release
2
1
Fork 4
Code Issues 4 Pull requests Actions Packages Projects Releases Wiki Activity

add arch option (specify runner architecture) #11

Merged
earl-warren merged 3 commits from leana8959/forgejo-release:add_arch_option into main 2024-05-12 11:02:44 +00:00
Conversation 2 Commits 3 Files changed 1 +2 -1
leana8959 commented 2024-03-04 18:29:24 +00:00
Contributor
Copy link

This would close #10 by adding the architecture option.

Should I add integration tests for this? It's fairly simple, and I don't know how to test it.

This would close #10 by adding the `architecture` option. Should I add integration tests for this? It's fairly simple, and I don't know how to test it.
leana8959 added 2 commits 2024-03-04 18:29:24 +00:00
Some checks failed
/ integration (pull_request) Failing after 5m19s
Details
cc259da0cb
update README
earl-warren reviewed 2024-03-05 08:01:37 +00:00
forgejo-release.sh Outdated
@ -21,3 +21,3 @@
setup_tea() {
if ! test -f $BIN_DIR/tea ; then
curl -sL https://dl.gitea.io/tea/$TEA_VERSION/tea-$TEA_VERSION-linux-amd64 > $BIN_DIR/tea
curl -sL https://dl.gitea.io/tea/$TEA_VERSION/tea-$TEA_VERSION-linux-$ARCH > $BIN_DIR/tea
earl-warren commented 2024-03-05 08:01:37 +00:00
Owner
Copy link

Could it be inferred from the information of the machine running the action? It would be easier if that was transparent to the user.

Could it be inferred from the information of the machine running the action? It would be easier if that was transparent to the user.
leana8959 commented 2024-03-05 09:11:23 +00:00
Author
Contributor
Copy link

I'm thinking about adding a case statement to parse outputs of uname -m. Would this help?
In this case, maybe we can reconsider if we need to expose this option to the end user, since it's inferred from the runner automatically.

I'm thinking about adding a `case` statement to parse outputs of `uname -m`. Would this help? In this case, maybe we can reconsider if we need to expose this option to the end user, since it's inferred from the runner automatically.
earl-warren commented 2024-03-05 09:18:19 +00:00
Owner
Copy link

I think that would be fine. Automated detection is best.

I think that would be fine. Automated detection is best.
leana8959 commented 2024-03-05 09:26:59 +00:00
Author
Contributor
Copy link

In this case, what should I put for the default case? i.e. how should the script fail when the platform is not in the list of cases I defined?

In this case, what should I put for the default case? i.e. how should the script fail when the platform is not in the list of cases I defined?
leana8959 commented 2024-03-05 09:28:38 +00:00
Author
Contributor
Copy link

Here's the draft I have in mind at the moment

if [ -z $ARCH ]; then
	platform=$(uname -s | tr '[:upper:]' '[:lower:]')
	arch=$(case $(uname -m) in
	arm)
		echo "arm"
		;;
	arm64 | aarch64_be | aarch64 | armv6l | armv7l | armv8b | armv8l)
		echo "arm64"
		;;
	i386 | i686)
		echo "386"
		;;
	x86_64)
		echo "amd64"
		;;
	*)
		echo "???" #FIXME: what's the default case
		;;
	esac)
	ARCH=$platform-$arch
fi
Here's the draft I have in mind at the moment ```bash if [ -z $ARCH ]; then platform=$(uname -s | tr '[:upper:]' '[:lower:]') arch=$(case $(uname -m) in arm) echo "arm" ;; arm64 | aarch64_be | aarch64 | armv6l | armv7l | armv8b | armv8l) echo "arm64" ;; i386 | i686) echo "386" ;; x86_64) echo "amd64" ;; *) echo "???" #FIXME: what's the default case ;; esac) ARCH=$platform-$arch fi ```
earl-warren commented 2024-03-05 14:52:33 +00:00
Owner
Copy link

Falling back to amd64 would ensure backward compatibility: if a case was missed it won't break on a setup that previously worked. Does that sound right?

Falling back to amd64 would ensure backward compatibility: if a case was missed it won't break on a setup that previously worked. Does that sound right?
Seltsamsel commented 2024-05-11 05:06:59 +00:00
First-time contributor
Copy link

I just stumbled upon the problem, that forgejo-release does not work on ARM devices. So I would be very happy, if this PR could be revived.

To just give another option for this specific issue, dpkg --print-architecture gives the architecture directly without the need to be parsed. As we assume a Debian based system anyways by using apt-get, this should not break any current setups.

I just stumbled upon the problem, that forgejo-release does not work on ARM devices. So I would be very happy, if this PR could be revived. To just give another option for this specific issue, `dpkg --print-architecture` gives the architecture directly without the need to be parsed. As we assume a Debian based system anyways by using `apt-get`, this should not break any current setups.
👍 2
leana8959 added 1 commit 2024-05-11 22:26:26 +00:00
All checks were successful
/ integration (pull_request) Successful in 5m21s
Details
1f75309ffd
use dpkg --print-architecture
leana8959 commented 2024-05-11 22:29:04 +00:00
Author
Contributor
Copy link

@Seltsamsel I have just switch to dpkg to detect the architecture, thanks for the tip !

@earl-warren Is there something I need to do to test this change ? Thanks.

@Seltsamsel I have just switch to `dpkg` to detect the architecture, thanks for the tip ! @earl-warren Is there something I need to do to test this change ? Thanks.
earl-warren merged commit eb0fcc44a1 into main 2024-05-12 11:02:44 +00:00
earl-warren deleted branch add_arch_option 2024-05-12 11:02:44 +00:00
earl-warren commented 2024-05-12 11:03:45 +00:00
Owner
Copy link

the tests show that what you did does not break anything: this is enough to be merged 👍

the tests show that what you did does not break anything: this is enough to be merged 👍
👍 1
earl-warren approved these changes 2024-05-12 11:03:53 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
earl-warren
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 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#11
No description provided.
Delete branch "leana8959/forgejo-release:add_arch_option"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?