Skip to content

Conversation

@romainfrancois
Copy link
Contributor

No description provided.

@romainfrancois
Copy link
Contributor Author

This did not test on win builder because I supposed we need a new build of libarrow, #4413 followed up on #4316 and introduced some incompatibilities. https://win-builder.r-project.org/KG75rToaX3Qd/

How do we trigger new builds for https://github.com/rwinlib/arrow ? @jeroen @javierluraschi

@codecov-io
Copy link

codecov-io commented Jun 10, 2019

Codecov Report

Merging #4511 into master will decrease coverage by 13.09%.
The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4511       +/-   ##
===========================================
- Coverage   88.11%   75.01%    -13.1%     
===========================================
  Files         850       54      -796     
  Lines      105774     3114   -102660     
  Branches     1253        0     -1253     
===========================================
- Hits        93203     2336    -90867     
+ Misses      12326      778    -11548     
+ Partials      245        0      -245
Impacted Files Coverage Δ
r/src/arrowExports.cpp 72.7% <ø> (ø) ⬆️
r/src/recordbatch.cpp 82.69% <85.71%> (+0.51%) ⬆️
python/pyarrow/ipc.pxi
cpp/src/arrow/csv/chunker-test.cc
cpp/src/parquet/column_page.h
cpp/src/parquet/bloom_filter-test.cc
cpp/src/arrow/array/builder_decimal.cc
cpp/src/plasma/client.cc
cpp/src/arrow/io/test-common.h
cpp/src/arrow/util/int-util-test.cc
... and 788 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48ee38f...7052b9e. Read the comment docs.

@nealrichardson
Copy link
Member

https://issues.apache.org/jira/browse/ARROW-4845 shows lots of warnings that look to be from different lines--are you sure this is the only change needed?

@romainfrancois
Copy link
Contributor Author

I'm not sure, that's why I tried to trigger a win builder build with devtools::check_win_devel() but it ended with this:

*** arch - i386
"D:/RCompile/recent/R/bin/i386/Rscript.exe" "../tools/winlibs.R" 0.13.0.9000
Error in download.file(sprintf("https://github.com/rwinlib/arrow/archive/v%s.zip",  : 
  cannot open URL 'https://github.com/rwinlib/arrow/archive/v0.13.0.9000.zip'
In addition: Warning message:
In download.file(sprintf("https://github.com/rwinlib/arrow/archive/v%s.zip",  :
  cannot open URL 'https://github.com/rwinlib/arrow/archive/v0.13.0.9000.zip': HTTP status was '404 Not Found'
Execution halted
make: *** [winlibs] Error 1
ERROR: compilation failed for package 'arrow'
* removing 'd:/RCompile/CRANguest/R-devel/lib/arrow'

before it could show me compile errors.

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Looks like the fix would indeed silence the warning, but some comments.

@romainfrancois
Copy link
Contributor Author

Perhaps we can use -Wall on travis to get some other warnings as well.

@romainfrancois romainfrancois force-pushed the ARROW-4845/sign_compare branch from 8ec6d6a to e646f82 Compare June 12, 2019 07:17
@romainfrancois romainfrancois deleted the ARROW-4845/sign_compare branch June 12, 2019 08:30
@jeroen
Copy link
Contributor

jeroen commented Jun 12, 2019

Once libarrow 0.14 is released you can send a PR to https://github.com/r-windows/rtools-packages and https://github.com/r-windows/rtools-backports and we can upload new binaries on rwinlib.

@wesm
Copy link
Member

wesm commented Jun 12, 2019

At some point we need to document the source build process on Windows rather than relying on pre-packaged binaries so that we can run it in CI, otherwise we will eventually get slammed with a Windows-only problem post-release

https://issues.apache.org/jira/browse/ARROW-3758

@jeroen
Copy link
Contributor

jeroen commented Jun 12, 2019

The build process is documented and automated.

You can either build libarrow locally or send a PR to https://github.com/r-windows/rtools-backports and then download the binaries from appveyor, and build your R package with that.

There is nothing special about rwinlib, it's just a place where we host binaries for (stable) libraries. But you could host them anywhere else.

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.

6 participants