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-37923: [R] Move macOS build system to nixlibs.R #37684

Merged
merged 78 commits into from
Oct 5, 2023

Conversation

assignUser
Copy link
Member

@assignUser assignUser commented Sep 13, 2023

This PR modifies the build system of the R package to no longer rely on auto/homebrew. Instead this PR adds the infrastructure and code paths to use the same type of pre-compiled libarrow binaries as we use for Linux. The main difference is the use of the binaries even on CRAN (as we previously also used binaries in form of brew bottles).

The addition of the new artifacts to tasks.yml should ensure that they get uploaded to the nightly repo as well as to the artifactory during the release (@kou please confirm).

A summary of the changes in this PR:

Follow up issues:

@assignUser

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@assignUser

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@kou
Copy link
Member

kou commented Sep 14, 2023

#37716 and #37717 will be helpful for this.

@assignUser
Copy link
Member Author

@github-actions crossbow submit r-binary-packages

@github-actions
Copy link

Revision: 25ae44e

Submitted crossbow builds: ursacomputing/crossbow @ actions-95e0198aa1

Task Status
r-binary-packages Github Actions

@assignUser
Copy link
Member Author

assignUser commented Sep 20, 2023

@github-actions crossbow submit r-binary-packages

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@assignUser
Copy link
Member Author

assignUser commented Sep 20, 2023

@github-actions crossbow submit r-binary-packages

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@assignUser
Copy link
Member Author

@github-actions crossbow submit r-binary-packages

This should be it... test program compilation correctly detects openssl and chooses the right binary... 🤞

@github-actions
Copy link

Revision: a30f3cd

Submitted crossbow builds: ursacomputing/crossbow @ actions-d657866664

Task Status
r-binary-packages Github Actions

@kou
Copy link
Member

kou commented Sep 20, 2023

@github-actions crossbow submit r-binary-packages

@github-actions
Copy link

Revision: 4327943

Submitted crossbow builds: ursacomputing/crossbow @ actions-05b816ca9b

Task Status
r-binary-packages Github Actions

@kou
Copy link
Member

kou commented Sep 20, 2023

@github-actions crossbow submit r-binary-packages

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 4, 2023
@assignUser
Copy link
Member Author

@github-actions crossbow submit r-binary-packages

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

Revision: 99cfe8a

Submitted crossbow builds: ursacomputing/crossbow @ actions-a57a53186a

Task Status
r-binary-packages Github Actions

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 5, 2023
Copy link
Member

@paleolimbot paleolimbot left a comment

Choose a reason for hiding this comment

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

It seems there's one CI failure on MacOS (which seems relevant given the MacOS-nature of this PR), but I'm not familiar with that set of tests to know if it is related.

I think there are probably some further changes that will be required; however, I do think it's important to merge this sooner than later so that there is time for the nightlies to run before the code freeze + release.

I'll defer to @kou to approve any changes to the cpp/ build system since I don't have expertise there!

@github-actions github-actions bot added awaiting merge Awaiting merge awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting merge Awaiting merge labels Oct 5, 2023
@assignUser
Copy link
Member Author

assignUser commented Oct 5, 2023

Thanks! As all checks pass now it seems that it was a flakey test

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

I have only minor comments.

cpp/cmake_modules/SetupCxxFlags.cmake Outdated Show resolved Hide resolved
dev/tasks/r/github.packages.yml Outdated Show resolved Hide resolved
dev/tasks/r/github.packages.yml Outdated Show resolved Hide resolved
dev/tasks/r/github.packages.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels Oct 5, 2023
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@kou
Copy link
Member

kou commented Oct 5, 2023

@github-actions crossbow submit r-binary-packages

@github-actions
Copy link

github-actions bot commented Oct 5, 2023

Revision: eca2134

Submitted crossbow builds: ursacomputing/crossbow @ actions-09ced6cc0b

Task Status
r-binary-packages Github Actions

@paleolimbot paleolimbot merged commit dd8734d into apache:main Oct 5, 2023
31 checks passed
@paleolimbot paleolimbot removed the awaiting merge Awaiting merge label Oct 5, 2023
@conbench-apache-arrow
Copy link

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

There were 3 benchmark results indicating a performance regression:

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

@assignUser assignUser deleted the r-macos-prebuild branch October 6, 2023 01:59
JerAguilon pushed a commit to JerAguilon/arrow that referenced this pull request Oct 23, 2023
This PR modifies the build system of the R package to no longer rely on auto/homebrew. Instead this PR adds the infrastructure and code paths to use the same type of  pre-compiled libarrow binaries as we use for Linux. The main difference is the use of the binaries even on CRAN (as we previously also used binaries in form of brew bottles).

The addition of the new artifacts to tasks.yml should ensure that they get uploaded to the nightly repo as well as to the artifactory during the release (@ kou please confirm). 

A summary of the changes in this PR:
- update `r/configure` and `r/tools/nixlibs.R` to enable the source build on macOS and usage of precompiled binaries using the existing mechanism to test compile a program to detect the exisitng openssl version
- added tests for the changes in nixlibs.R
- update the binary allow-list
- Add the build jobs for libarrow binaries for arm64 and x86_64 macos with openssl 1.1 and 3.0 to the `r-binary-packages` job
- Use the binaries to build the nightly packages  
- bump snappy version to 1.1.10 (and patch it on 10.13) due to build issues with the current version. This also touches on a number of issues in regards to a sanitizer issue we have had for a long time: apache#32562 apache#31766
- Disable the centos binary test step: apache#37922

Follow up issues:
- apache#37921
- apache#37941 
- apache#37945
* Closes: apache#37923

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Dewey Dunnington <dewey@voltrondata.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
This PR modifies the build system of the R package to no longer rely on auto/homebrew. Instead this PR adds the infrastructure and code paths to use the same type of  pre-compiled libarrow binaries as we use for Linux. The main difference is the use of the binaries even on CRAN (as we previously also used binaries in form of brew bottles).

The addition of the new artifacts to tasks.yml should ensure that they get uploaded to the nightly repo as well as to the artifactory during the release (@ kou please confirm). 

A summary of the changes in this PR:
- update `r/configure` and `r/tools/nixlibs.R` to enable the source build on macOS and usage of precompiled binaries using the existing mechanism to test compile a program to detect the exisitng openssl version
- added tests for the changes in nixlibs.R
- update the binary allow-list
- Add the build jobs for libarrow binaries for arm64 and x86_64 macos with openssl 1.1 and 3.0 to the `r-binary-packages` job
- Use the binaries to build the nightly packages  
- bump snappy version to 1.1.10 (and patch it on 10.13) due to build issues with the current version. This also touches on a number of issues in regards to a sanitizer issue we have had for a long time: apache#32562 apache#31766
- Disable the centos binary test step: apache#37922

Follow up issues:
- apache#37921
- apache#37941 
- apache#37945
* Closes: apache#37923

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Dewey Dunnington <dewey@voltrondata.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
This PR modifies the build system of the R package to no longer rely on auto/homebrew. Instead this PR adds the infrastructure and code paths to use the same type of  pre-compiled libarrow binaries as we use for Linux. The main difference is the use of the binaries even on CRAN (as we previously also used binaries in form of brew bottles).

The addition of the new artifacts to tasks.yml should ensure that they get uploaded to the nightly repo as well as to the artifactory during the release (@ kou please confirm). 

A summary of the changes in this PR:
- update `r/configure` and `r/tools/nixlibs.R` to enable the source build on macOS and usage of precompiled binaries using the existing mechanism to test compile a program to detect the exisitng openssl version
- added tests for the changes in nixlibs.R
- update the binary allow-list
- Add the build jobs for libarrow binaries for arm64 and x86_64 macos with openssl 1.1 and 3.0 to the `r-binary-packages` job
- Use the binaries to build the nightly packages  
- bump snappy version to 1.1.10 (and patch it on 10.13) due to build issues with the current version. This also touches on a number of issues in regards to a sanitizer issue we have had for a long time: apache#32562 apache#31766
- Disable the centos binary test step: apache#37922

Follow up issues:
- apache#37921
- apache#37941 
- apache#37945
* Closes: apache#37923

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Dewey Dunnington <dewey@voltrondata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[R] Move macOS build system to nixlibs.R
7 participants