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

Automatically build against hdf5 from rwinlib (fixes #128) #129

Merged
merged 5 commits into from Oct 4, 2019

Conversation

@jeroen
Copy link

commented Oct 3, 2019

Automatically downoad libhdf5 from rwinlib at build time.

This makes it possible for any user, build server, or CI to install the source package on Windows. You never have to worry about which version of hdf5 is installed on the server.

Also cleaned up your Makevars.win and appveyor.

jeroen added 2 commits Oct 3, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Oct 3, 2019

Codecov Report

Merging #129 into master will increase coverage by 0.44%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage    61.7%   62.15%   +0.44%     
==========================================
  Files          51       51              
  Lines       23797    23287     -510     
==========================================
- Hits        14685    14474     -211     
+ Misses       9112     8813     -299
Impacted Files Coverage Δ
R/Helper_functions.R 77.9% <0%> (-0.51%) ⬇️
src/H5ls.c 74.45% <0%> (-0.4%) ⬇️
src/Wrapper_manual_H5T.c 97.4% <0%> (-0.26%) ⬇️
R/R6Classes_H5R.R 86.29% <0%> (-0.16%) ⬇️
R/h5wrapper.R 87.64% <0%> (-0.14%) ⬇️
R/convert.R 79.26% <0%> (-0.13%) ⬇️
R/R6Classes_H5P.R 76.05% <0%> (-0.09%) ⬇️
src/Wrapper_auto_H5I.c 46.48% <0%> (-0.06%) ⬇️
R/R6Classes_H5T.R 74.17% <0%> (-0.06%) ⬇️
R/R6Classes_H5D.R 82.19% <0%> (-0.05%) ⬇️
... and 24 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 c1d7f00...3ac2b33. Read the comment docs.

jeroen added 3 commits Oct 3, 2019
@jeroen

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

OK this is done now. Passes on AppVeyor and WinBuilder:

@hhoeflin hhoeflin merged commit 55b1ce3 into hhoeflin:master Oct 4, 2019
5 checks passed
5 checks passed
codecov/patch Coverage not affected when comparing c1d7f00...3ac2b33
Details
codecov/project 62.15% (+0.44%) compared to c1d7f00
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
wercker/check_asan Wercker pipeline passed
Details
@hhoeflin

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2019

Thanks!

@jeroen

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

Yay. Could you maybe try to submit this to CRAN soon-ish? That would ease the transition to the new toolchain for us.

@hhoeflin

This comment has been minimized.

Copy link
Owner

commented Oct 4, 2019

@jeroen

This comment has been minimized.

Copy link
Author

commented Oct 5, 2019

Thanks, let me know if you need any help.

@hhoeflin

This comment has been minimized.

Copy link
Owner

commented Oct 6, 2019

I just saw you stuck with version 1.8.16 of HDF5 instead of the newer 1.10.5. Any particular reason?

@jeroen

This comment has been minimized.

Copy link
Author

commented Oct 6, 2019

I couldn't get it to work with 1.10.5, it would segfault on 32bit.

@hhoeflin

This comment has been minimized.

Copy link
Owner

commented Oct 6, 2019

ok, thanks. I will submit later today.

@jeroen

This comment has been minimized.

Copy link
Author

commented Oct 6, 2019

Thanks. If you have time we could try to debug why it segfaults with 1.10.5 on 32bit windows, but I'm not sure if that's easy to find out. So for now this will at least improve the status quo.

@jeroen

This comment has been minimized.

Copy link
Author

commented Oct 6, 2019

When you submit, it won't be auto-accepted by the bot because the UBSAN warning: https://www.stats.ox.ac.uk/pub/bdr/memtests/gcc-UBSAN/hdf5r/tests/testthat.Rout

So after you get a reply from the bot, you need to reply to the email saying that you tried to fix the UBSAN problem.

@hhoeflin

This comment has been minimized.

Copy link
Owner

commented Oct 6, 2019

I am trying to fix the UBSAN warnings right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.