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

GH-44314: [Packaging][Python] Use macOS 12 as deployment target to have macOS 12 pyarrow wheels #44315

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Oct 7, 2024

Rationale for this change

We did just bump MACOSX_DEPLOYMENT_TARGET from 10 to 12 for pyarrow wheels on 18.0.0, we probably should wait a little before dropping.

What changes are included in this PR?

Revert moving macOS deployment target to 12

Are these changes tested?

Will trigger wheel jobs on archery

Are there any user-facing changes?

Yes, wheels will be available for macOS 12.

Copy link

github-actions bot commented Oct 7, 2024

⚠️ GitHub issue #44314 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Oct 7, 2024
@raulcd
Copy link
Member Author

raulcd commented Oct 7, 2024

@github-actions crossbow submit wheel-macos-monterey-*-amd64

This comment was marked as outdated.

@jorisvandenbossche
Copy link
Member

Thanks @raulcd!

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Oct 7, 2024
@github-actions github-actions bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Oct 7, 2024
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 7, 2024
@raulcd
Copy link
Member Author

raulcd commented Oct 7, 2024

@github-actions crossbow submit wheel-macos-monterey-*-amd64

Copy link

github-actions bot commented Oct 7, 2024

Revision: f5f407a

Submitted crossbow builds: ursacomputing/crossbow @ actions-bc85ef5cec

Task Status
wheel-macos-monterey-cp310-cp310-amd64 GitHub Actions
wheel-macos-monterey-cp311-cp311-amd64 GitHub Actions
wheel-macos-monterey-cp312-cp312-amd64 GitHub Actions
wheel-macos-monterey-cp313-cp313-amd64 GitHub Actions
wheel-macos-monterey-cp313-cp313t-amd64 GitHub Actions
wheel-macos-monterey-cp39-cp39-amd64 GitHub Actions

@pitrou
Copy link
Member

pitrou commented Oct 7, 2024

Is there someone with a macOS 12 machine that can test (one of) the generated wheels?

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 8, 2024
@raulcd
Copy link
Member Author

raulcd commented Oct 8, 2024

Is there someone with a macOS 12 machine that can test (one of) the generated wheels?

Maybe @assignUser can spin up an AWS macOS 12 and just run the tests with one of those wheels? I don't have free access to an AWS account at the moment

@raulcd
Copy link
Member Author

raulcd commented Oct 9, 2024

We should merge this to 18.0.0, would be nice if we can get a test @amoeba @assignUser maybe?

@jorisvandenbossche
Copy link
Member

Ideally we can indeed test this, but FWIW I think many projects build with an older deployment target than what they are testing with (given the limitations of free CI).

We of course have the history of the macos-11 target actually failing in practice .. (#41696), because we were also not testing with it. But given it was working recently on CI, I assume running pyarrow on macos-12 is still fine.

@amoeba
Copy link
Member

amoeba commented Oct 10, 2024

I don't have an amd64 macOS 12 machine laying around but I can probably find one. It sounds like it's not critical for this issue but I'll give it a try and report back.

@assignUser
Copy link
Member

I was able to install the wheel on one of the deprecated macos-12 runners:

bash-3.2$ pip install wheel/pyarrow-18.0.0.dev426-cp312-cp312-macosx_12_0_x86_64.whl
Processing ./wheel/pyarrow-18.0.0.dev426-cp312-cp312-macosx_12_0_x86_64.whl
Collecting numpy>=1.16.6 (from pyarrow==18.0.0.dev426)
  Downloading numpy-2.1.2-cp312-cp312-macosx_10_13_x86_64.whl.metadata (60 kB)
Downloading numpy-2.1.2-cp312-cp312-macosx_10_13_x86_64.whl (20.8 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 20.8/20.8 MB 139.7 MB/s eta 0:00:00
Installing collected packages: numpy, pyarrow
Successfully installed numpy-2.1.2 pyarrow-18.0.0.dev426

@jorisvandenbossche
Copy link
Member

FWIW installing is not enough to actually test it (installing just depends on the tag in the wheel name), you will have to at least import pyarrow in a python session (for macos-11 importing was sufficient to trigger a segfault (#41696), but if there is a problem in one of the submodules, even importing is not enough)

@assignUser
Copy link
Member

🤦 of course ... sorry I'll do that soon

@assignUser
Copy link
Member

I successfully tested reading and writing parquet locally and reading from s3 and imported and used compute. So it should be fine. I do think in general it's ok to keep the deployment target at 12 until we get user reports that it doesn't work.

We should probably note somewhere that eol macos support is best effort and can't be guaranteed due to the lack of testing infra. (support policy?)

@kou
Copy link
Member

kou commented Oct 11, 2024

Thanks for testing this.
I merge this.

@kou kou merged commit deee9ac into apache:main Oct 11, 2024
8 checks passed
@kou kou removed the awaiting merge Awaiting merge label Oct 11, 2024
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit deee9ac.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 10 possible false positives for unstable benchmarks that are known to sometimes produce them.

raulcd added a commit that referenced this pull request Oct 11, 2024
…ve macOS 12 pyarrow wheels (#44315)

### Rationale for this change

We did just bump MACOSX_DEPLOYMENT_TARGET from 10 to 12 for pyarrow wheels on 18.0.0, we probably should wait a little before dropping.

### What changes are included in this PR?

Revert moving macOS deployment target to 12

### Are these changes tested?

Will trigger wheel jobs on archery

### Are there any user-facing changes?

Yes, wheels will be available for macOS 12.
* GitHub Issue: #44314

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
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.

6 participants