Skip to content
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.

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

Make OpenMP optional instead of default dependency #184

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

solaoi
Copy link
Contributor

@solaoi solaoi commented Nov 1, 2024

Make OpenMP optional to improve compatibility with MacOS

Background

Currently, OpenMP is specified as a default dependency in Cargo.toml. However, OpenMP is not installed by default on MacOS systems, which can cause unexpected build failures, especially in CI environments like GitHub Actions macOS runners.

Changes

  • Remove OpenMP from default features
  • Make it available as an optional dependency, similar to whisper.cpp's approach

Benefits

  • Better out-of-the-box experience on MacOS
  • No additional libomp installation required for basic builds
  • Improved CI/CD workflow, especially on macOS runners
  • More flexible dependency management for users

Note

Users who want to use OpenMP can still enable it explicitly through features.

@tazz4843
Copy link
Owner

tazz4843 commented Nov 1, 2024

Huh never even realized OpenMP was enabled by default now, not sure when that happened. Thanks for the PR :)

@tazz4843 tazz4843 enabled auto-merge November 1, 2024 01:29
@tazz4843 tazz4843 merged commit dd0c3af into tazz4843:master Nov 1, 2024
13 checks passed
@arizhih
Copy link
Contributor

arizhih commented Nov 8, 2024

Huh never even realized OpenMP was enabled by default now, not sure when that happened. Thanks for the PR :)

Sorry for late reply, i did it in #169 because it’s enabled by default in whisper.cpp too. But maybe it’s better not to enable it by default here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants