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

Backport/backport 2397 to 2.x #2409

Merged

Conversation

sam-herman
Copy link
Contributor

Description

Backport of #2397

This change fixes the build issues for the C++ portion of the codebase on MacOSX for both nmslib by upgrading to the latest version and for faiss by setting the correct flags and using the right dependencies and directory paths
Ideally with this fix we will not need to ask developers to manually run text replacement commands like there are currently in the DEVELOPER.md file

Check List

  • [x ] New functionality includes testing.
  • [ x] New functionality has been documented.
  • [ x] API changes companion pull request created.
  • [ x] Commits are signed per the DCO using --signoff.
  • [ x] Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@navneet1v
Copy link
Collaborator

@sam-herman this doesn't seem correct to me. Can you please rebase your code and raise the PR again.

@naveentatikonda naveentatikonda changed the base branch from main to 2.x January 21, 2025 20:56
* fix paths for libomp on newer homebrew versions
auto load java home path on macos
update to latest nmslib revision

Signed-off-by: Samuel Herman <[email protected]>
@sam-herman sam-herman force-pushed the backport/backport-2397-to-2.x branch from 9cd8b6f to fc14d3c Compare January 23, 2025 20:17
@sam-herman
Copy link
Contributor Author

@sam-herman this doesn't seem correct to me. Can you please rebase your code and raise the PR again.

yeah, DCO was messed up. hopefully it should be fixed now.

navneet1v
navneet1v previously approved these changes Jan 23, 2025
@navneet1v
Copy link
Collaborator

@sam-herman seems like conflicts with CHANGELog file. Please fix it

@sam-herman
Copy link
Contributor Author

@sam-herman seems like conflicts with CHANGELog file. Please fix it

done.

@sam-herman
Copy link
Contributor Author

@jmazanec15 @vamshin can one of you approve as well? I need 2 approvals for merge.

@jmazanec15 jmazanec15 merged commit 8ec1c7e into opensearch-project:2.x Jan 24, 2025
99 checks passed
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