[Misc] Make `compile_llm_requirements.sh` runnable on macos by lk-chen · Pull Request #55664 · ray-project/ray

lk-chen · 2025-08-15T19:54:32Z

Why are these changes needed?

Improve dev experience, for trivial changes I don't need to ssh to linux machine.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(
- Run ci/compile_llm_requirements.sh on macos

Signed-off-by: Linkun <github@lkchen.net>

gemini-code-assist

Code Review

This pull request makes the compile_llm_requirements.sh script runnable on macOS by handling differences in the sed command and ensuring a consistent Python platform for dependency compilation. The changes are logical and achieve the goal. I've provided a suggestion to further improve the script's portability by using a sed pattern that avoids introducing a new dependency (gsed) for macOS users, which should enhance the developer experience.

gemini-code-assist · 2025-08-15T19:55:57Z

ci/compile_llm_requirements.sh

+SED_CMD=sed
+# On macos, use gnu-sed because 'sed -i' may not be used with stdin
+if [[ "$(uname)" == "Darwin" ]]; then
+	# check if gsed is available
+	if ! command -v gsed &> /dev/null; then
+		echo "--- gsed is not installed. Install via `brew install gnu-sed`"
+		exit 1
+	fi
+	SED_CMD=gsed
+fi


While using gsed on macOS is a common workaround, it introduces a new dependency for developers. A more portable approach that works with both BSD sed (on macOS) and GNU sed (on Linux) without any extra dependencies is to use a temporary file for in-place editing. This would improve the developer experience by not requiring them to install gnu-sed.

With the alternative I'm suggesting for the sed commands, this entire block for SED_CMD logic can be removed.

Also, the comment on line 6 is slightly inaccurate. The issue with sed -i on macOS isn't about stdin, but that it requires an argument for the backup file extension, unlike GNU sed.

Pending other reviewers' opinion

Mmmm while I don't think it's a huge ask to have folks gnu-sed there might be some additional overhead we'd take in making sure CI machines had it.... Which also isn't huge, but it's noise. I think I'm content to listen to the robot on this one (provided it works)

Got it, I've updated according to Gemini suggestion

gemini-code-assist · 2025-08-15T19:55:57Z

ci/compile_llm_requirements.sh

+	${SED_CMD} -i '/^--extra-index-url /d' /tmp/ray-deps/requirements_compiled.txt
+	${SED_CMD} -i '/^--find-links /d' /tmp/ray-deps/requirements_compiled.txt


To avoid the portability issues with sed -i and remove the need for gsed on macOS, you can use a temporary file. This approach is portable across different sed versions. It also allows combining both sed operations into a single command, which is slightly more efficient.

This change should be coupled with the removal of the SED_CMD logic at the beginning of the script.

Suggested change

${SED_CMD} -i '/^--extra-index-url /d' /tmp/ray-deps/requirements_compiled.txt

${SED_CMD} -i '/^--find-links /d' /tmp/ray-deps/requirements_compiled.txt

sed -e '/^--extra-index-url /d' -e '/^--find-links /d' /tmp/ray-deps/requirements_compiled.txt > /tmp/ray-deps/requirements_compiled.txt.tmp

mv /tmp/ray-deps/requirements_compiled.txt.tmp /tmp/ray-deps/requirements_compiled.txt

Signed-off-by: Linkun <github@lkchen.net>

aslonnie · 2025-08-18T17:08:42Z

ci/raydepsets/rayllm.depsets.yaml

    - --extra-index-url https://download.pytorch.org/whl/${CUDA_CODE}
  append_flags:
    - --python-version=3.11
+    - --python-platform=x86_64-manylinux_2_28


this should be manylinux2014 rather than 2_28?

or does generic linux work? which is the default target when running on linux?

the manylinux variation should not really matter here if we are not compiling any (non-python, binary) things from source.

I did hit error

╰─▶ Because nixl==0.3.1 has no wheels with a matching platform tag (e.g., `manylinux_2_17_x86_64`) and you require nixl==0.3.1, we can conclude that your requirements are unsatisfiable. hint: Wheels are available for `nixl` (v0.3.1) on the following platform: `manylinux_2_28_x86_64````

have you tried "linux"? not sure if we want to pin to a specific manylinux or architecture right now yet.

linux works, updated

and for my own reference astral-sh/uv#14300 uv is aliasing linux to 2_28

aslonnie · 2025-08-18T17:11:46Z

ci/compile_llm_requirements.sh

+sed -e '/^--extra-index-url /d' -e '/^--find-links /d' /tmp/ray-deps/requirements_compiled.txt > /tmp/ray-deps/requirements_compiled.txt.tmp
+mv /tmp/ray-deps/requirements_compiled.txt.tmp /tmp/ray-deps/requirements_compiled.txt


this is just because mac's sed does not have a -i?

maybe convert this into a python script (or py_binary) instead?

TBH python is less readable than sed, I used temporary file due to #55664 (comment)

Signed-off-by: Linkun <github@lkchen.net>

lk-chen · 2025-08-19T18:38:49Z

Shall we merge or enable auto-merge?

…oject#55664) apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net> Signed-off-by: Andrew Grosser <dioptre@gmail.com>

…oject#55664) apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net>

…oject#55664) apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net> Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>

apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>

…oject#55664) apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net>

on mac

27394e3

Signed-off-by: Linkun <github@lkchen.net>

lk-chen requested a review from aslonnie August 15, 2025 19:54

gemini-code-assist bot reviewed Aug 15, 2025

View reviewed changes

lk-chen requested review from elliot-barn and khluu August 15, 2025 19:59

lk-chen added 2 commits August 15, 2025 13:03

Merge branch 'master' into ci_compile_x_platform

7e248db

adapt master

aaad1d1

Signed-off-by: Linkun <github@lkchen.net>

lk-chen mentioned this pull request Aug 15, 2025

[LLM] Bump to nixl==0.4.1 #55671

Merged

8 tasks

s/uv/uv_file

b2a3165

Signed-off-by: Linkun <github@lkchen.net>

lk-chen force-pushed the ci_compile_x_platform branch from 3683856 to b2a3165 Compare August 15, 2025 22:17

lk-chen requested a review from ZacAttack August 15, 2025 23:16

ray-gardener bot added the devprod label Aug 16, 2025

aslonnie reviewed Aug 18, 2025

View reviewed changes

lk-chen requested a review from aslonnie August 18, 2025 18:38

lk-chen added 2 commits August 18, 2025 16:01

s/2_28/linux

b5109d5

Signed-off-by: Linkun <github@lkchen.net>

Merge branch 'master' into ci_compile_x_platform

6cb4032

aslonnie approved these changes Aug 19, 2025

View reviewed changes

lk-chen added the go add ONLY when ready to merge, run all tests label Aug 19, 2025

elliot-barn approved these changes Aug 19, 2025

View reviewed changes

Merge branch 'master' into ci_compile_x_platform

d1f744b

lk-chen mentioned this pull request Aug 20, 2025

[llm] bump vllm to 0.10.1.1 #55766

Closed

8 tasks

aslonnie merged commit 5c31311 into ray-project:master Aug 20, 2025
5 checks passed

lk-chen deleted the ci_compile_x_platform branch August 20, 2025 07:10

alimaazamat pushed a commit to alimaazamat/ray that referenced this pull request Aug 21, 2025

[depset] make compile_llm_requirements.sh runnable on macos (ray-pr…

6dca175

…oject#55664) apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net>

dstrodtman pushed a commit that referenced this pull request Oct 6, 2025

[depset] make compile_llm_requirements.sh runnable on macos (#55664)

3870ecc

apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>

landscapepainter pushed a commit to landscapepainter/ray that referenced this pull request Nov 17, 2025

[depset] make compile_llm_requirements.sh runnable on macos (ray-pr…

adfaed5

…oject#55664) apple silicon macbooks only though. --------- Signed-off-by: Linkun <github@lkchen.net>

		${SED_CMD} -i '/^--extra-index-url /d' /tmp/ray-deps/requirements_compiled.txt
		${SED_CMD} -i '/^--find-links /d' /tmp/ray-deps/requirements_compiled.txt

		sed -e '/^--extra-index-url /d' -e '/^--find-links /d' /tmp/ray-deps/requirements_compiled.txt > /tmp/ray-deps/requirements_compiled.txt.tmp
		mv /tmp/ray-deps/requirements_compiled.txt.tmp /tmp/ray-deps/requirements_compiled.txt

Conversation

lk-chen commented Aug 15, 2025

Why are these changes needed?

Related issue number

Checks

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lk-chen commented Aug 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants