Skip to content

maint: add a comment to help point to a solution when docker build on macOS #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Meng-Heng wants to merge 1 commit into from
+1 −0
Closed

maint: add a comment to help point to a solution when docker build on macOS #88

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions _common/builder.inc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2300,6 +2300,7 @@ builder_is_linux() {
#
_builder_get_operating_system() {
declare -g BUILDER_OS
# Note: on macOS, bash version 4 or above is needed for `declare -g`.
Copy link
Contributor

@darcywong00 darcywong00 Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder instead of putting the comment here, if we add a note in the README.md file to mention/link to the Shared Dependencies in the main repo.

Did you have to use HomeBrew to upgrade Bash?

Copy link
Collaborator Author

@Meng-Heng Meng-Heng Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used HomeBrew. Linking to the docs would be better.

Copy link
Member

@mcdurdin mcdurdin Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as Darcy wrote, this is already documented at:
https://github.com/keymanapp/keyman/blob/2721c74a9188295320094da5237b8066a7387d08/docs/build/macos.md#L87

We could make this more visible in README.md for each of the sites.

I don't think we should add dependency comments into builder.inc.sh -- this is too buried to be useful.

Also, changes to builder.inc.sh need to be made in the keyman repo, because that's the primary version, but changes require builds on all platforms, and then ripple across to websites too, so best to avoid making changes unless really needed.

Copy link
Collaborator Author

@Meng-Heng Meng-Heng Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this is not the best place for it. I'm glad I was able to found out the reason behind my error and this will be a note for me to remember. Since there's already a document for this, I'll go ahead and close this PR. I'm still unsure on what to add for the README file so I won't do that now.


if builder_is_macos; then
BUILDER_OS=mac
Expand Down