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

WIP: Build on MacOS. #740

Closed
wants to merge 2 commits into from
Closed

Conversation

dblock
Copy link
Member

@dblock dblock commented Oct 13, 2021

Signed-off-by: dblock dblock@amazon.com

Description

With this change build.sh manfests/1.1.0/opensarch-1.1.0.yml works up until k-nn, which is #737.

Current scripts for OpenSearch hard-code linux when they see -a x64. Our options are:

  1. This change. Quick but dirty.
  2. Reuse -a (or replace it) and change it from "architecture" to "target", so -a x64 to -a darwin-x64 and -a linux-x64.
  3. Add another option for passing on system, e.g. -m darwin.
  4. Remove system and arch options and use current platform in the build scripts.

Thoughts? /cc:@peternied @bbarani

Issues Resolved

Part of #38

Check List

  • Commits are signed per the DCO using --signoff

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.

Signed-off-by: dblock <dblock@amazon.com>
@peternied peternied self-requested a review October 14, 2021 01:30
Signed-off-by: dblock <dblock@amazon.com>
@peternied
Copy link
Member

Rather than building natively for mac (or a given platform), what about making it easier to use the docker image in the CI system? Sayali spend a considerable amount of time adding changes to support child process detection in the test workflow and I feel like that was the tip of the iceberg.

If our build system could detect if you are are docker or not, and then automatically launch the container that seems like it would be a compelling feature. This would also make transitioning to windows builds easier to manage because everything would be consistent.

@dblock
Copy link
Member Author

dblock commented Oct 14, 2021

Rather than building natively for mac (or a given platform), what about making it easier to use the docker image in the CI system? Sayali spend a considerable amount of time adding changes to support child process detection in the test workflow and I feel like that was the tip of the iceberg.

If our build system could detect if you are are docker or not, and then automatically launch the container that seems like it would be a compelling feature. This would also make transitioning to windows builds easier to manage because everything would be consistent.

Are you saying that Mac (or Windows) users would need to run Docker to run OpenSearch, or are you saying we can build using a docker container running MacOS (or Windows) on Linux?

@peternied
Copy link
Member

Build using docker, effectively changing the requirement of this build system to require docker, or to be run inside of docker.

@dblock
Copy link
Member Author

dblock commented Oct 14, 2021

Isn't running an OSX VM inside docker just another way to providing mac "hardware"?
https://serverfault.com/questions/607443/can-mac-os-x-be-run-inside-docker

@peternied
Copy link
Member

Ah, I am proposing that we do not support mac at all, but instead support running the CI CentOS docker image on a mac, windows, or linux to produce build artifacts. If we need platform specific packaging system, eg producing MSI for windows or DMGs for mac, we would separate that into another step with a separate docker image to minimize the variability in our build system.

image
Chart Src

@dblock
Copy link
Member Author

dblock commented Oct 14, 2021

@peternied The ask in #38 and #33 is to have native support for Mac OS and Windows. On MacOS I'd want a DMG or support for brew services, on Windows I want a native Windows service and bat files, etc. We also have plugins that need native bits such as k-nn. Since we need native bits now, what's the benefit for doing what you propose?

@peternied
Copy link
Member

Reversing my stance after read up on those issues and quickly synced offline - Yup we need to support building on the mac, we have gaps around docker images and we will need to mitigate those so we can support top down building for linux, mac and windows.

@dblock
Copy link
Member Author

dblock commented Oct 14, 2021

@peternied Could I please have your opinion on the options in the body of this PR before I finalize it?

@peternied
Copy link
Member

peternied commented Oct 14, 2021

Current scripts for OpenSearch hard-code linux when they see -a x64. Our options are:

  1. This change. Quick but dirty.
  2. Reuse -a (or replace it) and change it from "architecture" to "target", so -a x64 to -a darwin-x64 and -a linux-x64.
  3. Add another option for passing on system, e.g. -m darwin.
  4. Remove system and arch options and use current platform in the build scripts.

In order of preference

  1. Not blocking contributions is important, so I would weight this highly, however if we have cycles option 3 is preferred

  2. I think is this a good north star, it allows for flexibility in systems where we have common behavior or to diverge intelligently. Might require a lot of param plumbing

  3. Not a fan because it conflates architecture and platform into a single parameter, this will get merged/unmerged and when we have a noarch/multi-arch target it goes off the rails.

  4. This defers the problem into the internals of the build system, which could work, but I feel like this would lead to some components building for arm, arm64, aarch64, etc.. if we have to take their word for it it means we can have deviation in terminology which doesn't make as much sense when we will be creating a distribution that should have a single platform/architecture associated with it.

@dblock
Copy link
Member Author

dblock commented Oct 14, 2021

I went after the north star in #748.

@dblock dblock closed this Oct 14, 2021
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.

2 participants