search

cloudsmith-io / action

Github Action that uses the cloudsmith cli to interact with the Cloudsmith API (pushes, etc)
https://github.com/marketplace/actions/cloudsmith-push
MIT License
15 stars 12 forks source link

CENG-269: Add macos runner support #64

Closed BartoszBlizniak closed 3 months ago

BartoszBlizniak commented 3 months ago

What's Changed

paddycarey commented 3 months ago

What bash features are we using that aren't in the default version? I'm concerned updating bash would have knock-on impacts for user's workflows as it's a pretty invasive thing to change.

BartoszBlizniak commented 3 months ago

@paddycarey

What bash features are we using that aren't in the default version? I'm concerned updating bash would have knock-on impacts for user's workflows as it's a pretty invasive thing to change.

Getting the following error:

/Users/runner/work/_actions/BartoszBlizniak/action/master/entrypoint.sh: line 4: declare: -A: invalid option
declare: usage: declare [-afFirtx] [-p] [name[=value] ...]

Associative arrays are a feature of Bash version 4 and above, but macOS, by default, uses an older version of Bash (version 3.2) due to licensing reasons.

paddycarey commented 3 months ago

We could definitely do the things we do in entrypoint.sh without needing to use associative arrays, and make the script more portable across platforms, but that's perhaps a bigger change that doesn't need to happen here. We should follow up here later.

Brew installs bash to a different location on Arm (/opt/homebrew/bin/bash) and not sure it'll use it by default unless you tell it to, what does it look like in testing?

Self-hosted mac runners are also worth thinking about, installing packages from brew may not be possible or desired. But again, moybe not important for this PR.

BartoszBlizniak commented 3 months ago

We could definitely do the things we do in entrypoint.sh without needing to use associative arrays, and make the script more portable across platforms, but that's perhaps a bigger change that doesn't need to happen here. We should follow up here later.

Brew installs bash to a different location on Arm (/opt/homebrew/bin/bash) and not sure it'll use it by default unless you tell it to, what does it look like in testing?

Self-hosted mac runners are also worth thinking about, installing packages from brew may not be possible or desired. But again, moybe not important for this PR.

Valid points, I think it might be worthwhile to look at deprecating this action and just create cloudsmith-cli as an action to install it and let users use that instead, in Q3. Should be easier to maintain and would work better across all platforms.

BartoszBlizniak commented 3 months ago

Tested with arm and works as well: https://github.com/BartoszBlizniak/cloudsmith-githubactions-test/actions/runs/9680379165/job/26708398557

BartoszBlizniak commented 3 months ago

@paddycarey just bumping this - is there anything else you'd suggest doing as part of this or can we release it?