Skip to content

Conversation

@KSemenenko
Copy link
Member

Summary

  • make the macOS staging steps resilient when the mlx.metallib asset is absent
  • ensure subsequent copy operations emit warnings instead of failing the workflow

Testing

  • dotnet build src/MLXSharp/MLXSharp.csproj

https://chatgpt.com/codex/tasks/task_e_68fc9f13b5b4832685fca3d771f143dd

Copilot AI review requested due to automatic review settings October 25, 2025 10:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the CI workflow to gracefully handle scenarios where the mlx.metallib Metal shader library is missing from macOS native artifacts. Instead of failing the build, the workflow now emits warnings and continues execution.

  • Adds conditional checks before copying mlx.metallib files
  • Emits GitHub Actions warnings when the Metal library is absent
  • Adds bash strict mode (set -euo pipefail) for better error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

set -euo pipefail
mkdir -p src/MLXSharp/runtimes/osx-arm64/native
cp artifacts/native/osx-arm64/libmlxsharp.dylib src/MLXSharp/runtimes/osx-arm64/native/
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

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

Adding set -euo pipefail at line 175 makes the script more strict, but the existing cp command at line 178 will still fail if libmlxsharp.dylib is missing. Consider adding a similar conditional check for the required libmlxsharp.dylib file to maintain consistency with the optional mlx.metallib handling, or document why this file's absence should cause a hard failure while mlx.metallib's absence should not.

Suggested change
cp artifacts/native/osx-arm64/libmlxsharp.dylib src/MLXSharp/runtimes/osx-arm64/native/
if [ -f artifacts/native/osx-arm64/libmlxsharp.dylib ]; then
cp artifacts/native/osx-arm64/libmlxsharp.dylib src/MLXSharp/runtimes/osx-arm64/native/
else
echo "::error::libmlxsharp.dylib not found in macOS native artifact; cannot continue"
exit 1
fi

Copilot uses AI. Check for mistakes.
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

cp artifacts/native/osx-arm64/libmlxsharp.dylib artifacts/native/
cp artifacts/native/osx-arm64/mlx.metallib artifacts/native/
cp artifacts/native/linux-x64/libmlxsharp.so artifacts/native/

P1 Badge Guard later packaging steps when metallib is missing

The earlier changes add optional handling for mlx.metallib, but the packaging stage still unconditionally copies the file and later dotnet pack commands pass it as a required parameter. When mlx.metallib is absent, the workflow will still exit with cp: cannot stat here (and the subsequent pack/verification steps would also fail), so the job remains non‑resilient to a missing metallib. Consider skipping these copies and pack arguments when the file is not present, or failing early with a clear message if the asset is required.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@KSemenenko KSemenenko merged commit 53cc881 into main Oct 25, 2025
3 of 4 checks passed
@KSemenenko KSemenenko deleted the codex/integrate-mlx-lm-with-.net-framework branch October 25, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants