Skip to content

BLD: Remove redundant manylinux1 related flag in .bazelrc#53549

Merged
aslonnie merged 2 commits intoray-project:masterfrom
czgdp1807:manylinux_2014
Jun 10, 2025
Merged

BLD: Remove redundant manylinux1 related flag in .bazelrc#53549
aslonnie merged 2 commits intoray-project:masterfrom
czgdp1807:manylinux_2014

Conversation

@czgdp1807
Copy link
Copy Markdown
Contributor

@czgdp1807 czgdp1807 commented Jun 4, 2025 •
edited
Loading

Why are these changes needed?

Related issue number

Closes #53548

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 :(

@czgdp1807
Copy link
Copy Markdown
Contributor Author

czgdp1807 commented Jun 4, 2025

@aslonnie The tests seem to pass. Does the diff contain intended changes?

@czgdp1807 czgdp1807 marked this pull request as ready for review June 4, 2025 15:34
@czgdp1807 czgdp1807 requested a review from aslonnie June 5, 2025 12:12
@aslonnie
Copy link
Copy Markdown
Collaborator

aslonnie commented Jun 5, 2025

@aslonnie The tests seem to pass. Does the diff contain intended changes?

hi, so, on the CI, we use manylinux1 related images as build environments to build the wheels.

this doc describes how one can build it manually currently:
https://github.com/ray-project/ray/blob/master/python/README-building-wheels.md

actually ray wheels are already 2014.. my bad memory..

what is the next newer version in line?

@aslonnie aslonnie requested review from edoakes and jjyao June 5, 2025 20:36
aslonnie
aslonnie reviewed Jun 5, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

looping in core team to have a look.

the macro GPR_MANYLINUX1 controls some part of grpc compiling to be manylinux1 compatible or not. we have stopped building for manylinux1 for quite some time.

.bazelrc Outdated
Copy link
Copy Markdown
Collaborator

@aslonnie aslonnie Jun 5, 2025

Choose a reason for hiding this comment

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

seems that this -DGPR_MANYLINUX* macro flag can be just removed..

@czgdp1807
Copy link
Copy Markdown
Contributor Author

czgdp1807 commented Jun 5, 2025

actually ray wheels are already 2014.. my bad memory..

Yes. That's why I asked whether the only change my PR is truly intended? Anyways so I think I would change, build:linux --per_file_copt="-\\.(asm|S)$,external/com_github_grpc_grpc/.*@-DGPR_MANYLINUX1" to build:linux --per_file_copt="-\\.(asm|S)$,external/com_github_grpc_grpc/.*@". Would that be correct?

@aslonnie
Copy link
Copy Markdown
Collaborator

aslonnie commented Jun 6, 2025

based on the syntax of the --per_file_copt:
https://bazel.build/reference/command-line-reference#flag--per_file_copt

the entire line can be removed.

parts before @ is the file filter. if there is nothing after @, the option flag does nothing and is redundant.

@czgdp1807
Copy link
Copy Markdown
Contributor Author

czgdp1807 commented Jun 7, 2025

what is the next newer version in line?

AFAIK, the latest version is manylinux_2_34. See, https://github.com/pypa/manylinux.

czgdp1807 added 2 commits June 7, 2025 19:31
@czgdp1807
BLD: Shift to manylinux2014 from manylinux1
3c38a52 
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
@czgdp1807
BLD: Remove redundant line in .bazelrc
2cca0af 
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
@czgdp1807 czgdp1807 changed the title BLD: Shift to manylinux2014 from manylinux1 BLD: Remove redundant manylinux1 related flag in .bazelrc Jun 7, 2025
@aslonnie aslonnie self-requested a review June 8, 2025 15:36
@rgommers
Copy link
Copy Markdown

rgommers commented Jun 9, 2025

what is the next newer version in line?

AFAIK, the latest version is manylinux_2_34. See, https://github.com/pypa/manylinux.

The next version to use is manylinux_2_28. NumPy 2.3.0 (released yesterday) just moved from manylinux2014 to manylinux_2_28. It's also the new default in cibuildwheel 3.0.0, which will be released soon. You probably shouldn't use anything more recent than that for quite a while.

@czgdp1807
Copy link
Copy Markdown
Contributor Author

czgdp1807 commented Jun 9, 2025 •
edited
Loading

Yes. I would suggest to stay with manylinux2014 like if we are able to use latest version of dependencies on manylinux2014. If manylinux_2_28 is needed for this reason, then I can do the migration.

By the way, NumPy migrating to manylinux_2_28 is a good assurance that we can do the migration here as well. Just my personal opinion.

cc: @peytondmurray @mattip Would be great if you can chime in on this. :-).

@mattip
Copy link
Copy Markdown
Contributor

mattip commented Jun 9, 2025

As ray may still want to run on amazon linux 2, which only supports manylinux2024 but not manylinux_2_28, perhaps it is too early still to drop support for manylinux2014.

@peytondmurray
Copy link
Copy Markdown
Contributor

peytondmurray commented Jun 9, 2025

Let's stick with manylinux2014 for now. Later on we can upgrade to manylinux_2_28 if that makes sense to do so.

@aslonnie aslonnie added alpha Alpha release features go add ONLY when ready to merge, run all tests and removed alpha Alpha release features labels Jun 10, 2025
@aslonnie aslonnie merged commit ec1f2a4 into ray-project:master Jun 10, 2025
6 checks passed
elliot-barn pushed a commit that referenced this pull request Jun 18, 2025
@czgdp1807 @elliot-barn
BLD: Remove redundant manylinux1 related flag in .bazelrc (#53549)
c32df60 
the option is only used for `manylinux1`. ray is not using manylinux1 to build things any more.

---------

Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
@czgdp1807 @elliot-barn
BLD: Remove redundant manylinux1 related flag in .bazelrc (#53549)
758099d 
the option is only used for `manylinux1`. ray is not using manylinux1 to build things any more.

---------

Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjyao jjyao Awaiting requested review from jjyao

@edoakes edoakes Awaiting requested review from edoakes

@aslonnie aslonnie Awaiting requested review from aslonnie

Assignees

No one assigned

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Migrating from manylinux1 to manylinux2014

5 participants

@czgdp1807 @aslonnie @rgommers @mattip @peytondmurray