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

feat: increase range of python FFI support #870

Merged
merged 23 commits into from
Aug 8, 2024
Merged

Conversation

sxlijin
Copy link
Collaborator

@sxlijin sxlijin commented Aug 7, 2024

  • aarch64-linux now targets glibc 2.24, which should now support amazon linux 2 (which relies on glibc 2.26; we might be able to make 2.17 work but i have more concerns about that)
  • add musl builds, which will allow alpine to work

Copy link

vercel bot commented Aug 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 8, 2024 0:38am

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

The pull request focuses on rolling down the glibc version used for aarch64-linux to ensure compatibility.

  • Updated .github/workflows/build-ruby-release.reusable.yaml to include rb-sys-dock-setup for aarch64-linux.
  • Modified .github/workflows/release.yml to adjust the build matrix for aarch64-unknown-linux-gnu.
  • Ensured cargo install -f [email protected] in .github/workflows/release.yml for WASM builds.
  • Adjusted pnpm build commands in .github/workflows/primary.yml to align with the new glibc version.
  • Verified manylinux settings in .github/workflows/build-python-release.reusable.yaml for aarch64 targets.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

No major changes found since the last review.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on rolling down the glibc version used for aarch64-linux in the workflows.

  • Updated container image to ghcr.io/rust-cross/manylinux_2_28-cross:aarch64 in .github/workflows/release.yml for aarch64-unknown-linux-gnu builds.
  • Adjusted cargo_args to include -p baml-typescript-ffi -p baml-python-ffi for aarch64-unknown-linux-gnu in .github/workflows/release.yml.
  • No changes detected in .github/workflows/primary.yml.

These changes ensure compatibility with the specified glibc version for aarch64-linux builds.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

No major changes found since the last review.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for aarch64-linux builds.

  • Updated .github/workflows/build-python-release.reusable.yaml to specify manylinux: '2_17' for aarch64-unknown-linux-gnu.
  • Added rb-sys-dock-setup: ./x86-64_linux-setup.sh for x86_64-linux in .github/workflows/build-ruby-release.reusable.yaml.
  • Modified engine/language_client_ruby build steps to ensure compatibility with the new glibc version.
  • Ensured all artifacts are correctly uploaded post-build for both Python and Ruby workflows.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for aarch64-linux builds.

  • Updated .github/workflows/build-python-release.reusable.yaml to specify CFLAGS_aarch64_unknown_linux_gnu: "-D__ARM_ARCH=8" for aarch64-unknown-linux-gnu.
  • Added rb-sys-dock-setup: ./x86-64_linux-setup.sh for x86_64-linux in .github/workflows/build-ruby-release.reusable.yaml.
  • Modified engine/language_client_ruby build steps to ensure compatibility with the new glibc version.
  • Ensured all artifacts are correctly uploaded post-build for both Python and Ruby workflows.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for aarch64-linux builds.

  • Updated .github/workflows/build-python-release.reusable.yaml to specify CFLAGS_aarch64_unknown_linux_gnu: "-D__ARM_ARCH=8" for aarch64-unknown-linux-gnu.
  • Added rb-sys-dock-setup: ./x86-64_linux-setup.sh for x86_64-linux in .github/workflows/build-ruby-release.reusable.yaml.
  • Modified engine/language_client_ruby build steps to ensure compatibility with the new glibc version.
  • Ensured all artifacts are correctly uploaded post-build for both Python and Ruby workflows.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for aarch64-linux builds.

  • Updated .github/workflows/build-python-release.reusable.yaml to specify CFLAGS_aarch64_unknown_linux_gnu: "-D__ARM_ARCH=8" for aarch64-unknown-linux-gnu.
  • Added rb-sys-dock-setup: ./x86-64_linux-setup.sh for x86_64-linux in .github/workflows/build-ruby-release.reusable.yaml.
  • Modified engine/language_client_ruby build steps to ensure compatibility with the new glibc version.
  • Ensured all artifacts are correctly uploaded post-build for both Python and Ruby workflows.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for aarch64-linux builds.

  • Updated .github/workflows/build-python-release.reusable.yaml to specify CFLAGS_aarch64_unknown_linux_gnu: "-D__ARM_ARCH=8" for aarch64-unknown-linux-gnu.
  • Added rb-sys-dock-setup: ./x86-64_linux-setup.sh for x86_64-linux in .github/workflows/build-ruby-release.reusable.yaml.
  • Modified engine/language_client_ruby build steps to ensure compatibility with the new glibc version.
  • Ensured all artifacts are correctly uploaded post-build for both Python and Ruby workflows.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

No major changes found since the last review.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for aarch64-linux to ensure compatibility and stability.

  • Updated /.github/workflows/build-python-release.reusable.yaml to include CFLAGS_aarch64_unknown_linux_gnu for aarch64 builds.
  • Modified /.github/workflows/build-ruby-release.reusable.yaml to handle specific setup steps for aarch64-linux.
  • Ensured engine/language_client_python and engine/language_client_ruby directories are correctly configured for the new glibc version.
  • Added specific environment variables and setup steps to address build issues related to the glibc version downgrade.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request adds a comment to the engine/.cargo/config.toml file for better context and maintainability without changing the build configuration.

  • Added a comment referencing Rust Cargo issue #8607 in engine/.cargo/config.toml for context.
  • Configuration settings for aarch64-unknown-linux-musl target remain unchanged.
  • No functional changes to the build configuration, ensuring compatibility and proper compilation.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request removes the linker configuration for the aarch64-unknown-linux-musl target in engine/.cargo/config.toml to roll down the glibc version used for aarch64-linux.

  • Removed linker configuration from engine/.cargo/config.toml for aarch64-unknown-linux-musl target.
  • Existing rustflags setting remains unchanged.
  • Important to verify build and runtime behavior for consistency post-change.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request modifies the engine/.cargo/config.toml file to remove the linker configuration for the aarch64-unknown-linux-musl target, while keeping the existing rustflags setting unchanged.

  • Removed linker configuration from engine/.cargo/config.toml for aarch64-unknown-linux-musl target.
  • Existing rustflags setting remains unchanged.
  • Important to verify build and runtime behavior for consistency post-change.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request modifies the engine/.cargo/config.toml file to remove the linker configuration for the aarch64-unknown-linux-musl target, while keeping the existing rustflags setting unchanged.

  • Removed linker configuration from engine/.cargo/config.toml for aarch64-unknown-linux-musl target.
  • Existing rustflags setting remains unchanged.
  • Important to verify build and runtime behavior for consistency post-change.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request modifies the engine/.cargo/config.toml file to remove the linker configuration for the aarch64-unknown-linux-musl target, while keeping the existing rustflags setting unchanged.

  • Removed linker configuration from engine/.cargo/config.toml for aarch64-unknown-linux-musl target.
  • Existing rustflags setting remains unchanged.
  • Important to verify build and runtime behavior for consistency post-change.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on rolling down the glibc version for aarch64-linux and introduces several Dockerfiles and an install script for dependencies.

  • Updated .mise.toml to specify Python 3.10 for consistent environment setup.
  • Added integ-tests/python/docker-tests/aarch64-unknown-linux-gnu.Dockerfile for integration tests on aarch64 architecture.
  • Added integ-tests/python/docker-tests/aarch64-unknown-linux-musl.Dockerfile for musl-based aarch64 environment testing.
  • Added integ-tests/python/docker-tests/python-3_10.Dockerfile to set up a Python 3.10 environment for integration testing.
  • Introduced tools/install script for streamlined dependency installation (Poetry, Rustup).

Ensure compatibility and functionality across environments, especially with the new glibc version and Python 3.10.

5 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings

RUN pip3 install ./baml_py-0.53.0-cp38-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl

ENV RUST_LOG=trace
RUN baml-cli --help
Copy link

Choose a reason for hiding this comment

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

style: Add a newline at the end of the file for better formatting.

RUN pip install ./baml_py-0.53.0-cp38-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl

ENV RUST_LOG=trace
RUN baml-cli --help
Copy link

Choose a reason for hiding this comment

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

style: Add a newline at the end of the file for better formatting.

fi

if [ "${install_poetry:-0}" -eq 1 ]; then
curl -sSL https://install.python-poetry.org | python3 -
Copy link

Choose a reason for hiding this comment

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

style: Consider adding error handling for the Poetry installation command to catch and report any issues during the installation process.

fi

if [ "${install_rustup:-0}" -eq 1 ]; then
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
Copy link

Choose a reason for hiding this comment

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

style: Consider adding error handling for the Rustup installation command to catch and report any issues during the installation process.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

No major changes found since the last review.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The pull request focuses on adjusting the glibc version for the aarch64-linux platform in the build and release workflows.

  • Updated /.github/workflows/build-python-release.reusable.yaml to set manylinux to 2_17 for aarch64-unknown-linux-gnu.
  • Modified /.github/workflows/build-ruby-release.reusable.yaml to include aarch64-linux in the build matrix.
  • Adjusted /.github/workflows/release.yml to ensure compatibility with the new glibc version for aarch64-linux.
  • Ensured all relevant build steps and dependencies are aligned with the new glibc version for aarch64-linux.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

No major changes found since the last review.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This pull request focuses on rolling down the glibc version used for aarch64-linux.

  • Updated .github/workflows/build-python-release.reusable.yaml to adjust glibc version for aarch64-linux.
  • Added manylinux: 2_24 configuration for aarch64-unknown-linux-gnu target in build-python-release.reusable.yaml.
  • Modified before-script-linux to handle package installations for different Linux distributions.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@sxlijin sxlijin changed the title fix: roll down the glibc version used for aarch64-linux feat: increase range of python FFI support Aug 8, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The PR aims to roll down the glibc version used for aarch64-linux and remove the Python version specification in .mise.toml.

  • Removed Python version specification in /.mise.toml, which may lead to inconsistencies across environments.
  • Added manylinux: 2_24 configuration for aarch64-unknown-linux-gnu target in .github/workflows/build-python-release.reusable.yaml.
  • Modified before-script-linux in .github/workflows/build-python-release.reusable.yaml to handle package installations for different Linux distributions.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@sxlijin sxlijin merged commit ec9b66c into canary Aug 8, 2024
16 checks passed
@sxlijin sxlijin deleted the sam/glibc-aarch64 branch August 8, 2024 16:38
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.

1 participant