vendor path #353
vendor path #353
Conversation
There was a problem hiding this comment.
@DavisVaughan: Any chance you'd consider vendoring into a different directory, perhaps by default? I think the current practice is somewhat dangerous because this makes cpp11 available for all dependants of the package it is vendored into. What's the reasoning behind choosing this path for vendoring?
R/vendor.R
Outdated
| @@ -9,13 +9,14 @@ | |||
| #' cpp11 currently installed on your machine. | |||
| #' | |||
| #' If you choose to vendor the headers you should _remove_ `LinkingTo: | |||
| #' cpp11` from your DESCRIPTION. | |||
| #' cpp11` from your DESCRIPTION. This is done automatically by this function. | |||
There was a problem hiding this comment.
yes! I added the changes for that, because I am a dummy
|
|
||
| message(paste( | ||
| "Makevars and/or Makevars.win should have a line such as", | ||
| "'PKG_CPPFLAGS = -I../inst/include'" |
There was a problem hiding this comment.
Does this now need to show the new path?
There was a problem hiding this comment.
that line is added automatically, and uses the path the user wants
There was a problem hiding this comment.
i reallize i never pushed that change, done now
|
I don't currently have the resources to spend much time on cpp11, but I don't love vendoring in general because it doesn't actually 100% insulate you from cpp11 changes. Previous versions of cpp11 relied on a global R option for the precious list, and that ends up getting shared between, say:
So they aren't fully independent, and if there are changes made to the structure of the thing that gets stored in the global option (which has happened) then whoever sets up the global option first "wins" and can cause the other package to break |
|
Interesting. Do you think problems like this are going to resurface? I've seen very weird behaviors when two cpp11 packages (one of them being duckdb) were used in the same R session. Could have been that. For duckdb, I don't see a good alternative to vendoring. But if this isn't going to be widely supported, I'm fine with hacking my own solution to move from |
Co-authored-by: Kirill Müller <[email protected]>
|
@krlmlr hi, I re-made the part that edits the DESCRIPTION |
1 similar comment
|
@krlmlr hi, I re-made the part that edits the DESCRIPTION |
|
hi @DavisVaughan @krlmlr |
|
btw, the only CI that fails is gcc 4.8 because of a bug with the compiler itself |
A tidy (as in less noise and just 1 commit) version of #340
As @krlmlr suggested, it might be better to vendor to
src/vendorinstead ofinstThis PR implements that and also a better handling and messages about Makevars