You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2025/09/25 20:21:08 UTC
[PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
paleolimbot opened a new pull request, #152:
URL: https://github.com/apache/sedona-db/pull/152
We're building in R Universe now! https://paleolimbot.r-universe.dev/sedonadb
We have some WARNINGs from missing declared dependencies (my fault) and a build failure for Windows (path length related to the CMake builder for the AWS dependency) and WebR (which I don't attempt to solve here).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "Kontinuation (via GitHub)" <gi...@apache.org>.
Kontinuation commented on code in PR #152:
URL: https://github.com/apache/sedona-db/pull/152#discussion_r2380663283
##########
Cargo.toml:
##########
@@ -116,6 +116,17 @@ tokio = { version = "1.44" }
url = "2.5.4"
[patch.crates-io]
+# For R package build on Windows (can be removed when we upgrade to DataFusion main/51.0.0)
+datafusion = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion" }
+datafusion-catalog = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-catalog" }
+datafusion-common = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-common" }
+datafusion-common-runtime = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-common-runtime" }
+datafusion-execution = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-execution" }
+datafusion-expr = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-expr" }
+datafusion-ffi = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-ffi" }
+datafusion-physical-expr = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-physical-expr" }
+datafusion-physical-plan = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-physical-plan" }
Review Comment:
Looks like https://github.com/apache/datafusion/pull/17688 is the fix required for the R Windows build. I'm fine with this since the other forked dependencies also take time to be cleared. We can target removing all forked dependencies once DataFusion 51 is released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "yutannihilation (via GitHub)" <gi...@apache.org>.
yutannihilation commented on PR #152:
URL: https://github.com/apache/sedona-db/pull/152#issuecomment-3336888386
(Just for notice)
It seems this didn't fix the build on R-universe. Probably there's some difference that we need to figure out.
https://github.com/r-universe/paleolimbot/actions/runs/18026582294/job/51295664262#step:5:762
```
-- Check if the system is big endian
-- Searching 16 bit integer
-- Looking for sys/types.h
-- Looking for sys/types.h - not found
-- Looking for stdint.h
-- Looking for stdint.h - not found
-- Looking for stddef.h
-- Looking for stddef.h - not found
-- Check size of unsigned short
-- Check size of unsigned short - failed
-- Check size of unsigned int
-- Check size of unsigned int - failed
-- Check size of unsigned long
-- Check size of unsigned long - failed
-- Configuring incomplete, errors occurred!
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #152:
URL: https://github.com/apache/sedona-db/pull/152#discussion_r2380276089
##########
r/sedonadb/R/zzz.R:
##########
@@ -36,8 +36,10 @@ check_interrupts <- function() {
}, interrupt = function(...) TRUE, error = function(...) TRUE)
}
-# Permissively licensed (unlicense) from
-# https://github.com/r-lib/vctrs/blob/b63a233bd8b8d6511cf3f7d9182c359a3b8c3cab/R/register-s3.R
+# From the `vctrs` package (this function is intended to be copied
+# without attribution or license requirements to avoid a hard dependency on
+# vctrs:
+# https://github.com/r-lib/vctrs/blob/c2a7710fe55e3a2249c4fdfe75bbccbafcf38804/R/register-s3.R#L25-L31
Review Comment:
This reverts what I had with a self-contained version that doesn't depend on rlang
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #152:
URL: https://github.com/apache/sedona-db/pull/152#discussion_r2380632812
##########
Cargo.toml:
##########
@@ -116,6 +116,17 @@ tokio = { version = "1.44" }
url = "2.5.4"
[patch.crates-io]
+# For R package build on Windows (can be removed when we upgrade to DataFusion main/51.0.0)
+datafusion = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion" }
+datafusion-catalog = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-catalog" }
+datafusion-common = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-common" }
+datafusion-common-runtime = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-common-runtime" }
+datafusion-execution = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-execution" }
+datafusion-expr = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-expr" }
+datafusion-ffi = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-ffi" }
+datafusion-physical-expr = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-physical-expr" }
+datafusion-physical-plan = { git = "https://github.com/paleolimbot/datafusion.git", branch = "local-49-with-patch", package = "datafusion-physical-plan" }
Review Comment:
I'm not actually sure this is a good idea given that we're in the process of removing the other patches. If we leave them in we get R binaries for Windows via R Universe...we can remove them when we update to DataFusion 51, which I think we'd like to do soon. I'm also not offended if this is too much for right now (I can just comment out the Windows CI again).
##########
.github/workflows/r-extended.yml:
##########
@@ -0,0 +1,76 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: r-extended
+
+on:
+ push:
+ branches:
+ - main
+ pull_request:
+ branches:
+ - main
+ paths:
+ - 'r/**'
+jobs:
+
+ test:
+ runs-on: ${{ matrix.config.os }}
+ name: ${{ matrix.config.os }} (${{ matrix.config.r }})
+ defaults:
+ run:
+ shell: bash
+
+ strategy:
+ fail-fast: false
+ matrix:
+ config:
+ - {os: ubuntu-latest, r: 'release'}
+
+ env:
+ GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+ R_KEEP_PKG_SOURCE: yes
+
+ steps:
+ - uses: actions/checkout@v4
+ - uses: r-lib/actions/setup-pandoc@v2
+ - uses: r-lib/actions/setup-r@v2
+ with:
+ r-version: ${{ matrix.config.r }}
+ http-user-agent: ${{ matrix.config.http-user-agent }}
+ use-public-rspm: true
+
+ - name: Use stable Rust
+ id: rust
+ run: |
+ rustup toolchain install stable --no-self-update
+ rustup default stable
+
+ - name: Install dependencies
+ if: matrix.config.os == 'ubuntu-latest'
+ run: sudo apt-get update && sudo apt-get install -y libgeos-dev
+
+ - uses: r-lib/actions/setup-r-dependencies@v2
+ with:
+ extra-packages: any::rcmdcheck
+ needs: check
+ working-directory: r/sedonadb
+
+ - uses: r-lib/actions/check-r-package@v2
+ with:
+ working-directory: r/sedonadb
+ error-on: '"error"'
Review Comment:
This is the primary reason for this workflow: `R CMD check` runs some extra tests beyond just "the tests" to make sure examples run, documentation is correct, and some others. Unfortunately we generate some WARNINGs still for things that CRAN cares about but we don't (perhaps just speaking for myself)...it would be nice to ignore specific warnings so that other warnings don't go unnoticed (but perhaps a battle for a different PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #152:
URL: https://github.com/apache/sedona-db/pull/152
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "yutannihilation (via GitHub)" <gi...@apache.org>.
yutannihilation commented on PR #152:
URL: https://github.com/apache/sedona-db/pull/152#issuecomment-3336170086
Probably you need to patch datafusion-doc as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(r/sedonadb): Increase CMAKE path length allowed on Windows and fix CMD check WARNINGs [sedona-db]
Posted by "yutannihilation (via GitHub)" <gi...@apache.org>.
yutannihilation commented on code in PR #152:
URL: https://github.com/apache/sedona-db/pull/152#discussion_r2380474263
##########
r/sedonadb/bootstrap.R:
##########
@@ -29,7 +29,6 @@ writeLines(
cargo_toml
)
-dir.create("src/dep_crates/")
Review Comment:
Aw, thanks for catching!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org