Unvendor Eigen, Sundials, and Boost on *nix

conda-forge / cmdstan-feedstock

A conda-smithy repository for cmdstan.

BSD 3-Clause "New" or "Revised" License

1 stars 6 forks source link

Unvendor Eigen, Sundials, and Boost on *nix #39

Closed WardBrian closed 1 year ago

WardBrian commented 1 year ago

Checklist

[x] Used a personal fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[x] Ensured the license file is being packaged.

Closes #30

WardBrian commented 1 year ago

@conda-forge-admin, please rerender

conda-forge-webservices[bot] commented 1 year ago

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 (recipe) and found it was in an excellent condition.

github-actions[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/cmdstan-feedstock/actions/runs/4872754029.

maresb commented 1 year ago

Wow, very nice!!! It must have been a fair amount of of work to achieve this. As usual I don't really have any expertise to provide any serious feedback, but congrats on the green!

WardBrian commented 1 year ago

A lot of the heavy lifting was some upstream tweaks by other people.

I am on the fence whether or not this is really a great idea - for header-only things like Eigen, there's no real harm in having multiple of them in one environment (e.g. one vendored inside CmdStan and one in the normal conda place), but this would force you to have the game Eigen version for the entire environment, since I have it pinned to exactly the versions Stan uses.

maresb commented 1 year ago

Right, very good point. This makes it a lot harder to use as a library.

maresb commented 1 year ago

Are the vendored versions still present for fallback, or completely removed?

WardBrian commented 1 year ago

I delete them as part of the build script here, but that isn't a necessary step I suppose. I don't see the benefit of leaving them if we do take this approach however

maresb commented 1 year ago

Maybe we could do multiple outputs, one with vendored deps and one with non-vendored.

maresb commented 1 year ago

Is there much point to unvendoring them if the pins are exact?

WardBrian commented 1 year ago

Likely not much benefit for boost or Eigen. For sundials yes, since you'll potentially be benefiting from improved builds of the released 'version'. In principle it's also just less building for us to maintain, leads to a smaller package download, etc. This, also, is a bigger deal for Sundials than for the header-only dependencies

I can also look into how much looser these bounds could be. I know that (as of this release, at least) Stan can still be built against Eigen 3.3, for example.

A longer-term goal of mine is to get cmdstan building with clang-on-Windows targetting MSVC, in which case we'd also want to use vendored TBB and Sundials on Windows to avoid having to build those ourselves.

WardBrian commented 1 year ago

Closing for now, may reconsider in future