Skip to content
This repository has been archived by the owner on Feb 15, 2025. It is now read-only.

chore: mac deployments #507

Closed
wants to merge 11 commits into from

Conversation

alekst23
Copy link
Contributor

@alekst23 alekst23 commented May 14, 2024

This PR adds support for building and deploy packages on Macs.

The issue arrises because the current build process is configured to build the packages on the local machine, and then copy the package for installation to a Docker image. Doing this on Mac causes the packages to be built for the local Mac architecture, and these file versions do not work in the Docker Linux container.

We address the issue in this PR by moving the build process inside Docker:

  • Makefile now copies all required files to the package folder first
  • Dockerfile for each package builds the package dependencies and installs the package itself inside the builder layer
  • We add an optional ARCH parameter for the make command, which defaults to amd64, and can be set to arm64 for building on Mac
  • We add an optional REG_PORT parameter to make, to allow the registry container to change port if 5000 is already taken on the local system

Additional Notes:

  • We change lfaiui to leapfrogai-ui in the ui package

@alekst23 alekst23 linked an issue May 14, 2024 that may be closed by this pull request
Copy link

netlify bot commented May 14, 2024

Deploy Preview for leapfrogai-docs canceled.

Name Link
🔨 Latest commit 4554081
🔍 Latest deploy log https://app.netlify.com/sites/leapfrogai-docs/deploys/6657398b520f080008621648

@alekst23 alekst23 marked this pull request as ready for review May 17, 2024 21:47
@alekst23 alekst23 requested a review from a team as a code owner May 17, 2024 21:47
@alekst23
Copy link
Contributor Author

I need to test this locally outside docker, but at this point the build and uds deploy should be working on mac.

alekst23 added 2 commits May 17, 2024 18:16
…b.com:defenseunicorns/leapfrogai into 491-chore-add-support-to-build-on-mac-silicon
Copy link
Member

@YrrepNoj YrrepNoj left a comment

Choose a reason for hiding this comment

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

One small change request, but other than that this PR is looking good to me!

@YrrepNoj
Copy link
Member

@alekst23 Can you update the vllm Maketarget and Dockerfile so that it will conform with the rest of our builds?

I know vllm isn't MacOS compatible so it wasn't in the original scope of what you were doing here, but I think it's important enough to keep our builds consistent.

-rm packages/llama-cpp-python/build/*.whl

## The external link is needed to pull a pre-compiled cpu wheel for llama-cpp-python
python -m pip wheel packages/llama-cpp-python -w packages/llama-cpp-python/build --find-links=${SDK_DEST}
Copy link
Member

Choose a reason for hiding this comment

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

The llama-cpp-python Dockerfile was taking a 'shortcut' to see if a model existed in the build/ directory to try to bypass the model-download script. If the build/ directory doesn't exist than our Dockerfile will run into an error when trying to build.

The Dockerfile is trying to do the copy here and the workflow is currently failing because the build/ directory doesn't exist.

I think we can get away with removing this shortcut, but I wouldn't be opposed to just adding another conditional check here either.

@alekst23 alekst23 changed the title chore: add support for build process on mac silicon chore: mac deployments May 29, 2024
Copy link
Contributor

@justinthelaw justinthelaw left a comment

Choose a reason for hiding this comment

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

Everything, with the exception of @YrrepNoj comments, looks good to me!

Future reference: the copying over of src into the build context's directory is an interesting move to get rid of the relative import within the pyproject.toml. I think a clean-up of these, not just a .gitignore, would be a nice touch to the Makefile. (understanding that this isn't your original pattern, but an already present one)

Thanks for all the testing and improvements!

@gphorvath
Copy link

Closing in favor of #659

@gphorvath gphorvath closed this Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore: Add support to build on Mac silicon
4 participants