Skip to content
This repository was archived by the owner on Oct 23, 2025. It is now read-only.

Conversation

@BartoszBlizniak
Copy link
Member

@BartoszBlizniak BartoszBlizniak commented Jun 26, 2024

What's Changed

  • Add support for macOS runner by updating the version of bash that comes pre-installed (v3.2) on default GHA runners.

@BartoszBlizniak BartoszBlizniak requested a review from a team as a code owner June 26, 2024 11:33
@paddycarey
Copy link
Member

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
Copy link
Member Author

@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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

@BartoszBlizniak
Copy link
Member Author

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

@BartoszBlizniak BartoszBlizniak merged commit 8fe2c21 into cloudsmith-io:master Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants