pin_run_as_build for boost, boost-cpp #58
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
| @@ -46,6 +46,10 @@ zip_keys: | |||
| pin_run_as_build: | |||
| arpack: | |||
| max_pin: x.x | |||
| boost: | |||
| max_pin: x.x.x | |||
There was a problem hiding this comment.
What about Python/C++ binding code that uses Boost?
There was a problem hiding this comment.
This is why we have split up the boost package. We have libboost for explicitly compiling things against boost. We have py-boost for python usage. Ideally, I think we'd also have a boost-includes or something like that for strictly header-only components.
With current conda-forge recipes, I believe it is safest to use the setting you have here, but I would encourage everyone to move towards packages that are only used one way (linking, or python, or only headers).
There was a problem hiding this comment.
I though boost python was a C++ library and not a python package.
There was a problem hiding this comment.
You are right. We split it up so that we only build the core libraries once, then compile only the python part by itself. So yes, you are correct to have pin_run_as_build for boost and boost-cpp, and we would need it for libboost and py-boost.
|
cc @conda-forge/core @conda-forge/boost-cpp @conda-forge/boost |
recipe/conda_build_config.yaml
Outdated
| @@ -46,6 +46,10 @@ zip_keys: | |||
| pin_run_as_build: | |||
| arpack: | |||
| max_pin: x.x | |||
| boost: | |||
| max_pin: x.x.x | |||
| boost_cpp: | |||
There was a problem hiding this comment.
Isn't the package called "boost-cpp"?
There was a problem hiding this comment.
Yeah, but yaml doesn't allow that (I think?) so it's translated. Also see r_base.
There was a problem hiding this comment.
I thought you could do something like:
"boost-cpp":
max_pin: x.x.x
There was a problem hiding this comment.
Hmm, maybe, and that would be nicer. Not sure how that works, can investigate later....
There was a problem hiding this comment.
Think we have to use _. The parser got messed up with - for me previously.
There was a problem hiding this comment.
oof that is annoying. That means that - and _ should always be interchangeable, then
There was a problem hiding this comment.
I think conda-build when trying to re-render a feedstock.
TBH don't think this is that bad in practice. Don't think we have packages that differ only by -/_ and if we do we are bound to run into tons of issues from our users, which is incentive enough to steer clear.
There was a problem hiding this comment.
@isuruf described it correctly. - and _ are treated as equivalent for the purposes of matching package names to variable names (the foundation of what gets looped over).
If this is not desirable, you can make the key in CBC.yaml any arbitrary string, and then use it explicitly wherever you need it.
There was a problem hiding this comment.
Looks like @scopatz was right. If it's a top level key you need _, but for pin_run_as_build you need the exact package name. Very confusing. https://github.com/conda-forge/postgis-feedstock/pull/8/files/#r187479333
There was a problem hiding this comment.
That is an annoying detail that grew out of pin_run_as_build being implemented completely differently (and in a different part of the source) from run_exports and the jinja2 functions. PRs to fix it would be welcome. Sorry you guys have been the pin_run_as_build guinea pigs. That's a feature that we just haven't really used much in defaults. With r-base, it was easy enough for us to hack around. With all of your packages, it is a much bigger problem.
|
Ultimately, I don't think it makes sense to have to split up packages. To me, a lot of the value of conda comes from having packages that can serve multiple interfaces |
|
Yeah I'm less clear on why |
|
boost libraries are small enough that it doesn't really matter. What this is all about is helping conda-build get your dependencies right for you. Does it take one thing in the recipe to have your dependencies right, or two? This is really all about run_exports, and the ugliness of needing ignore_run_exports. Yes, it works. No, you are not encoding as much information as you could be about how you're using boost, and thus conda-build won't be able to help you get your dependencies right. With multi-purpose packages and ignore_run_exports being the approach, at worst, you'll end up with unnecessary dependencies, which are a concern for size and for introducing more constraints that may make some environments hard or impossible to create due to version conflicts. If you split the package, it takes more work upstream, but less work downstream. If a library is used many times, the balance is in favor of the split packages. |
Can you please explain exactly what you mean by this? Users don't care, if you install Forcing people to install two or three almost exact copies of the same core boost libraries because they use two or three different py-boosts is not a good thing to do. Also the 'almost exact' is problematic. You introduce more DSOs and more DSOs is more things to go wrong and reduced usage and testing of what should be the exact same DSOs. |
|
And at present, without split packages, installing boost forces python to be installed and that's not good for people who use e.g. R and have no interest in python, now they can install |
|
@mingwandroid, your explanation is for |
|
Boost is frequently used in a headers-only way and we should take care that unnecessary pinned boost dependency does not get forced onto packages that do not need it. This should be obvious though, and to me you are arguing against common sense. |
Can you please explain to me the difference between the following? requirements:
host:
- boost-cpp
run:requirements:
host:
- boost-headers
run:
Please don't insinuate that I don't have common sense. |
|
The former should use run_exports to ensure binary compatibility. It implies that you are linking against some library. The latter should not use run_exports, because it is clear that it is header-only. |
|
I thought we agreed not to use |
|
We intend to use run_exports in our recipes. You are right, though, that without boost-cpp in both host and run, it is equivalent. We feel that having run_exports will be more explicit and more clear than whether or not there is a boost-cpp run dep. |
Does conda-forge get a chance to say how we feel or is this set in stone and conda-forge has to follow defaults? |
|
We can put it up for a vote. You are not conda-forge, and Anaconda is not conda-forge. I will say that I have much less intention to maintain pin_run_as_build since we don't like it much, so you may want to factor some of your time picking that up into any decision you make to depend on pin_run_as_build in the future. |
|
Could someone please add this to the next agenda (possibly creating one if doesn't exist)? |
|
I don't understand why you shouldn't user run_exports, they are one of the most useful and powerful conda build 3 features. Can you explain? |
I did not in any way insinuate this, I am explicitly stating that I consider not using run_exports as a nonsensical thing to contemplate because they are such a powerful feature, you should not use this as an opportunity to take offence. |
This reverts commit 707d607.
Revert "REF remove binutils constraint (conda-forge#58)"
Boost breaks the ABI with minor versions.
I think this is the right behavior: we don't want
run_exportsbecause you don't want to require it atrunif it's used header-only, but this allows people to list just- boostin therunreqs rather than having to do{{ pin_compatible('boost', max_pin='x.x.x') }}(and look up how manyxs you need).