Skip to content

Conversation

@jonkeane
Copy link
Member

@jonkeane jonkeane commented Aug 17, 2024

Trying to replicate the issue's on CRAN's M1 machine so that we can fix them.

@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-install-local

@github-actions
Copy link

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

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Aug 17, 2024
@github-actions
Copy link

Revision: bf2bcee416f975d7d2d97dd1842f4390a7252bdd

Submitted crossbow builds: ursacomputing/crossbow @ actions-c51734247c

Task Status
test-r-install-local GitHub Actions

@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-macos-as-cran

@github-actions
Copy link

Revision: b366ccc1a3fc0098b2ea3e3db50b1bb1bdda320a

Submitted crossbow builds: ursacomputing/crossbow @ actions-76d2411a11

Task Status
test-r-macos-as-cran GitHub Actions

@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-macos-as-cran

@github-actions
Copy link

Revision: b75dd5945183896854d40862f834975c6fbba534

Submitted crossbow builds: ursacomputing/crossbow @ actions-80cddc2e7e

Task Status
test-r-macos-as-cran GitHub Actions

@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-macos-as-cran

@github-actions
Copy link

Revision: ddc42f152179bde91a3ab34ebcf322a1c1289abb

Submitted crossbow builds: ursacomputing/crossbow @ actions-1451ffbbe2

Task Status
test-r-macos-as-cran GitHub Actions

@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-macos-as-cran

@github-actions
Copy link

Revision: 250e554c93b0eb6b58c17c45ac0453de5b05b4f8

Submitted crossbow builds: ursacomputing/crossbow @ actions-6b92fe50a6

Task Status
test-r-macos-as-cran GitHub Actions

@jonkeane
Copy link
Member Author

The R version failure is resolved in #43737

Comment on lines 4968 to 4989
Copy link
Member

Choose a reason for hiding this comment

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

Could you use "appending -Wno-error*" approach instead of "replacing a flag" approach like we did for gRPC?

set(GRPC_C_FLAGS "${EP_C_FLAGS}")
set(GRPC_CXX_FLAGS "${EP_CXX_FLAGS}")
if(NOT MSVC)
# Negate warnings that gRPC cannot build under
# See https://github.com/grpc/grpc/issues/29417
string(APPEND
GRPC_C_FLAGS
" -Wno-attributes"
" -Wno-format-security"
" -Wno-unknown-warning-option")
string(APPEND
GRPC_CXX_FLAGS
" -Wno-attributes"
" -Wno-format-security"
" -Wno-unknown-warning-option")
endif()
set(GRPC_CMAKE_ARGS
"${EP_COMMON_CMAKE_ARGS}"
"-DCMAKE_C_FLAGS=${GRPC_C_FLAGS}"
"-DCMAKE_CXX_FLAGS=${GRPC_CXX_FLAGS}"

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes! Thanks for this pointer. I tried constructing this before based on the substrait approach which is similar, but I couldn't get the cmake to work, but this did!

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 17, 2024
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 17, 2024
@jonkeane
Copy link
Member Author

@github-actions crossbow submit -g r

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Aug 17, 2024
@github-actions
Copy link

Revision: 5e80a72

Submitted crossbow builds: ursacomputing/crossbow @ actions-ca72a12061

Task Status
r-binary-packages GitHub Actions
test-r-arrow-backwards-compatibility GitHub Actions
test-r-clang-sanitizer GitHub Actions
test-r-depsource-bundled Azure
test-r-depsource-system GitHub Actions
test-r-dev-duckdb GitHub Actions
test-r-devdocs GitHub Actions
test-r-extra-packages GitHub Actions
test-r-gcc-11 GitHub Actions
test-r-gcc-12 GitHub Actions
test-r-install-local GitHub Actions
test-r-install-local-minsizerel GitHub Actions
test-r-linux-as-cran GitHub Actions
test-r-linux-rchk GitHub Actions
test-r-linux-valgrind GitHub Actions
test-r-macos-as-cran GitHub Actions
test-r-minimal-build Azure
test-r-offline-maximal GitHub Actions
test-r-offline-minimal Azure
test-r-rhub-debian-gcc-devel-lto-latest Azure
test-r-rhub-debian-gcc-release-custom-ccache Azure
test-r-rhub-ubuntu-release-latest Azure
test-r-rocker-r-ver-latest Azure
test-r-rstudio-r-base-4.1-opensuse155 Azure
test-r-rstudio-r-base-4.2-focal Azure
test-r-ubuntu-22.04 GitHub Actions
test-r-versions GitHub Actions
test-ubuntu-r-sanitizer GitHub Actions

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 17, 2024
Comment on lines 4978 to 4982
set(AWS_EP_COMMON_CMAKE_ARGS "${EP_COMMON_CMAKE_ARGS}" "-DCMAKE_C_FLAGS=${AWS_C_FLAGS}"
"-DCMAKE_CXX_FLAGS=${AWS_CXX_FLAGS}")

set(AWSSDK_COMMON_CMAKE_ARGS
${EP_COMMON_CMAKE_ARGS}
${AWS_EP_COMMON_CMAKE_ARGS}
Copy link
Member

Choose a reason for hiding this comment

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

Can we simplify this by not defining AWS_EP_COMMON_CMAKE_ARGS?

set(AWSSDK_COMMON_CMAKE_ARGS
    ${EP_COMMON_CMAKE_ARGS}
    -DCMAKE_C_FLAGS=${AWS_C_FLAGS}
    -DCMAKE_CXX_FLAGS=${AWS_CXX_FLAGS}
    ...

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Aug 18, 2024
Co-authored-by: Sutou Kouhei <[email protected]>
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 18, 2024
@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-macos-as-cran

@github-actions
Copy link

Revision: 625a847

Submitted crossbow builds: ursacomputing/crossbow @ actions-8c15b5b2e3

Task Status
test-r-macos-as-cran GitHub Actions

@jonkeane
Copy link
Member Author

The failures for Java JNI / AMD64 manylinux2014 Java JNI (pull_request) are the same on main and not related to this PR

@jonkeane jonkeane marked this pull request as ready for review August 18, 2024 12:43
@jonkeane jonkeane merged commit 1ae38d0 into apache:main Aug 18, 2024
@jonkeane jonkeane removed the awaiting change review Awaiting change review label Aug 18, 2024
jonkeane added a commit that referenced this pull request Aug 18, 2024
…3736)

Trying to replicate the issue's on CRAN's M1 machine so that we can fix them.
* GitHub Issue: #43735

Lead-authored-by: Jonathan Keane <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>
@conbench-apache-arrow
Copy link

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

There were no benchmark performance regressions. 🎉

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

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.

2 participants