rebuild for ray V2.49.0, release python 3.13 limit#224
rebuild for ray V2.49.0, release python 3.13 limit#224apmorton merged 18 commits intoconda-forge:mainfrom
Conversation
|
@conda-forge-admin, please rerender |
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( I do have some suggestions for making it better though... For recipe/meta.yaml:
This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/17370084851. Examine the logs at this URL for more detail. |
recipe/meta.yaml
Outdated
| # Avoid building openssl-1.1.1f and redis, not needed in the shipped package | ||
| - patches/0011-do-not-build-redis-openssl.patch | ||
| # Ray patches grpc, update the patch for 1.67 | ||
| - patches/0012-update-patch-for-grpc-1.67.patch |
There was a problem hiding this comment.
New patch, taken from ray-project/ray#51673. It is a shame that PR is stuck on one failing test file.
There was a problem hiding this comment.
can you make this part of the vendor 1.67 patch rather than a second patch?
recipe/meta.yaml
Outdated
| - patches/0003-remove-dependencies.patch | ||
| # This needs modification for each release | ||
| - patches/0012-update-commit-sha.patch | ||
| - patches/0010-cython-shebang.patch # [linux] |
There was a problem hiding this comment.
merged upstream, no longer needed
|
macos builds are failing to find a compiler. Did the 'compilers' package change? In arm64 I see and similarly in x86_64 I see |
|
Ahh, we can drop osx_64 (macos x86_64) builds, as upstream no longer supports macos x86_64. Is there talk on conda-forge about using arm64 macos build machines and not cross-compiling? |
Just because upstream drops them, we don't have to. There are very few differences between
We're waiting for availability of osx-arm64 agents on azure pipelines, but osx-64 agents are still available on there for a while (e.g. |
|
The failure is at this line in the templated shell file |
|
Ahh, indeed, the line before the error shows (locally) Note the $PATH is missing the conda env binary path. Edit: so how did it work previously? |
recipe/meta.yaml
Outdated
| # Avoid building openssl-1.1.1f and redis, not needed in the shipped package | ||
| - patches/0011-do-not-build-redis-openssl.patch | ||
| # Ray patches grpc, update the patch for 1.67 | ||
| - patches/0012-update-patch-for-grpc-1.67.patch |
There was a problem hiding this comment.
can you make this part of the vendor 1.67 patch rather than a second patch?
| - msgpack-python >=1.0.0,<2.0.0 | ||
| - packaging | ||
| - protobuf >=3.15.3,!=3.19.5 | ||
| - protobuf >=3.20.3 |
There was a problem hiding this comment.
fwiw, I don't know when this happened, but it seems the following requirements are orphaned:
aiosignal
frozenlist
setproctitle
There was a problem hiding this comment.
aiosignal and frozenlist were dropped in 2.45.0
`setproctitle is vendored in thirdparty/setproctitle from 2.48.0
recipe/meta.yaml
Outdated
| patches: | ||
| - patches/0001-Disable-making-entry-scripts.patch | ||
| - patches/0002-Ignore-warnings-in-event.cc-and-logging.cc.patch | ||
| - patches/0003-remove-dependencies.patch |
There was a problem hiding this comment.
I don't think we need to patch out dependencies.
Instead of using setup.py install on unix, use:
"${PYTHON}" -m pip install . -vv --no-deps --no-build-isolation
(and add those flags to windows)
We already use -m pip install on windows, so this shouldn't be a problem.
There was a problem hiding this comment.
Also, the cython build dep is out of date - it should be cython >=3.0.12.
If you switch to pip you'll also need to add it to host deps.
There was a problem hiding this comment.
Does --no-deps hold for both extras_requires and install_requires kwargs in the setup_tools.setup() call?
|
Linux builds fail in CI with: This was happening locally for me last night too, so at least I can reproduce. |
|
Fixed the dependencies and the macOS build, still todo
|
Grr. Why are they installing such an old python? I will look at this later. |
|
macos CC_FOR_BUILD doesn't work for cross compile; attempting to disable bazel sandbox, since that seems to be what is blowing up the path... |
It worked locally. Where is the CI run with the commit 61e8c3c that tried this? I see various CI runs at https://dev.azure.com/conda-forge/feedstock-builds/_build?definitionId=11419&_a=summary. The commit sha in those runs is not the one in the branch. I see two CI runs: |
I have no sudo permissions inside the docker |
That sounds familiar, I think we had this before. |
|
Hmm. The previous problem led to this comment from the rules_boost maintainer:
|
|
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipe/recipe.yaml:
This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/17382187634. Examine the logs at this URL for more detail. |
Yup, cuz the |
recipe/recipe.yaml
Outdated
| # Avoid building openssl-1.1.1f and redis, not needed in the shipped package | ||
| - patches/0011-do-not-build-redis-openssl.patch | ||
| # https://github.com/conda-forge/docker-images/issues/311 | ||
| - patches/0012-use-unzip-from-env.patch # [linux] |
There was a problem hiding this comment.
This needs conversion to if: then:
There was a problem hiding this comment.
I'm experimenting with some fixes locally; I don't think we should be patching in this way - it should be possible to point to unzip installed to $PREFIX from conda-forge.
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
recipe/recipe.yaml
Outdated
| - psutil | ||
| - python | ||
| - setproctitle >=1.2.2,<1.4 | ||
| - unzip |
There was a problem hiding this comment.
I think this needs to be m2-unzip on windows
|
Let's try to get this over the finish line without additional refactoring. There are people waiting for the release. Please do the boost refactoring after we have a successful merge of this PR. |
|
I specifically unvendored boost in hopes it would fix the osx_arm64 cross compilation builds - if you have an alternate fix I'm all ears. |
|
I bet we can drop the CC_FOR_BUILD stuff with |
|
I will try that change locally on an macsos-arm64 build. Thanks for finding the |
It turns out the Edit: for future me, the rattler-build command requires target-platform: |
you should be able to use there are also lint/rerender/smithy commands |
This reverts commit e75d35a.
| # Exchange the short exe name for the full path, used in | ||
| # # bazel_toolchain/cc_wrapper.sh |
There was a problem hiding this comment.
Bleh. I will remove it in the upcoming 2.49.1 release, which should be happening in a few days
apmorton
left a comment
There was a problem hiding this comment.
LGTM once build is green
|
Windows python 3.12 test is hung :( Given other windows versions passed, I'm going to proceed with merging in hopes its a fluke. |
|
Thanks @apmorton |
Checklist
0(if the version changed)conda-smithy(Use the phrase@conda-forge-admin, please rerenderin a comment in this PR for automated rerendering)closes (again) #222
@apmorton I couldn't resist, feel free to close this if you have a similar PR.