Skip to content

Prepare for linking with UCRT binaries#268

Merged
eddelbuettel merged 1 commit intoTileDB-Inc:masterfrom
jeroen:master
Jun 29, 2021
Merged

Prepare for linking with UCRT binaries#268
eddelbuettel merged 1 commit intoTileDB-Inc:masterfrom
jeroen:master

Conversation

@jeroen
Copy link
Copy Markdown
Contributor

@jeroen jeroen commented Jun 29, 2021

Minimal tweak that will allow you to add the ucrt binaries in rwinlib-tiledb under lib/x64-ucrt. The trick here is that R-for-ucrt builds use Makevars.ucrt to build if it exists.

This is the same pattern we use for many other packages, eg https://github.com/jeroen/curl/blob/master/src/Makevars.ucrt with https://github.com/rwinlib/libcurl/tree/master/lib

@eddelbuettel
Copy link
Copy Markdown
Contributor

Thanks @jeroen -- can confirm that the pattern suggested here appears to work fine as e.g. seen on the RcppRedis package for which you had already sent a PR -- but note that it currently nags you about an included binary.

@jeroen
Copy link
Copy Markdown
Contributor Author

jeroen commented Jun 29, 2021

but note that it currently nags you about an included binary.

Maybe try adding windows to the rbuildignore, as you already do here:

^windows

@eddelbuettel
Copy link
Copy Markdown
Contributor

Nice catch, thanks. Will all packages relying on your tools/winlibs.R usage pattern need that?

@jeroen
Copy link
Copy Markdown
Contributor Author

jeroen commented Jun 29, 2021

Nice catch, thanks. Will all packages relying on your tools/winlibs.R usage pattern need that?

No, it's just a temporary glitch because Tomas extracts, patches and then rebuilds the source package on the ucrt server. The note will probably also disappear by itself once he removes the server patches.

Copy link
Copy Markdown
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

The PR is fine, and truly appreciated.

We contemplated leaving it open until we have a proper artifact (as our UCRT build TileDB Embedded is still in a branch) but as the Makefile.ucrt will only used by a suitable builder there will not be any side-effects.

@eddelbuettel eddelbuettel merged commit cbd9984 into TileDB-Inc:master Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants