search

conda-forge / or-tools-feedstock

A conda-smithy repository for or-tools.
BSD 3-Clause "New" or "Revised" License
2 stars 8 forks source link

Rebuild for abseil / protobuf #24

Closed h-vetinari closed 1 year ago

h-vetinari commented 1 year ago

For some reason this feedstock isn't being picked up by the abseil migrator.

Rebuild for the most recent packages, and add myself to maintainers.

conda-forge-linter 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.

BastianZim commented 1 year ago

Do you mean this one? https://github.com/conda-forge/or-tools-feedstock/pull/22

h-vetinari commented 1 year ago

Do you mean this one? #22

What I meant is that it isn't listed in https://conda-forge.org/status/#abseil_cpp202206230, but that's perhaps because it's the second go-around of that migration... 🤔

BastianZim commented 1 year ago

Ahh hmm haven’t checked. Maybe?

h-vetinari commented 1 year ago

OK, finally figured this one out, it was the pins in the recipe itself. Not sure why coin-or-clp was pinned to a particular patch version, but since the global pinning only sets 1.17, I'm assuming this should be fine.

PTAL @conda-forge/or-tools

h-vetinari commented 1 year ago

@BastianZim Could you give this a look over & push the button? 🙃

hmaarrfk commented 1 year ago

I think we messed up the migrations a while back.

BastianZim commented 1 year ago

Thanks for the PR @h-vetinari but I haven’t been able to review this yet and it changes more than the migrations. Some of the pins were there on purpose so I’m not sure if this is a good idea. I would appreciate it if you could roll back and break this up so we can review it properly.

hmaarrfk commented 1 year ago

sorry about that. I merged too quickly. I'll rollback.

h-vetinari commented 1 year ago

Some of the pins were there on purpose so I’m not sure if this is a good idea.

So the only pins this lifted was going from zlib 1.2.11 -> 1.2.12 (which was blocking resolution of everything, and we should really let the global pinning take care of this), as well as coin-or-clp from 1.17.4 -> 1.17.7 (again, the global pinning only goes to the minor version).

I feel these are pretty minor things, but I see @hmaarrfk will roll back. I'll excuse myself from this while you investigate what you need to do, and let you reinstate the migration.

hmaarrfk commented 1 year ago

I somewhat agree with h-vetinari about the unpinnings, which is why i merged. zlib is quite widely used, and if it was a build problem it would have showed up.

Same with coin-or-clp, at runtime, it would be unpinned too.

I understand that it narrows your compatibility with some other private packages you might maintain.

hmaarrfk commented 1 year ago

Well for one thing the build number was never bumped.

h-vetinari commented 1 year ago

Well for one thing the build number was never bumped.

Yes, I realized that when I touched the version bump PRs. Sorry for that. 😑

BastianZim commented 1 year ago

Thanks, everyone. It's not that I oppose this and I understand the arguments, I just wanted to check a couple of things first as or-tools is quite finnicky. Generally, we also check with upstream as not everything is documented which I would like to do before as well and they have some specific tests for their dependencies that should be added before they are unpinned.