search

conda-forge / spdlog-feedstock

A conda-smithy repository for spdlog.
BSD 3-Clause "New" or "Revised" License
3 stars 19 forks source link

1.10.0_1 build broke ABI w.r.t. to 1.10.0_0 builds #53

Closed traversaro closed 1 year ago

traversaro commented 2 years ago

See discussion in https://github.com/conda-forge/spdlog-feedstock/pull/50 .

@bluescarni while we discuss how to properly do the de-vendoring, I think that we can mark these packages as broken?

traversaro commented 2 years ago

@bluescarni while we discuss how to properly do the de-vendoring, I think that we can mark these packages as broken?

In case @conda-forge/spdlog agrees, the corresponding PR is ready to be merged at https://github.com/conda-forge/admin-requests/pull/501 . Personally, I think the most important thing at the moment is fix the break that was induced by this change, then when everything is working we can think of how to properly de-vendor fmt.

traversaro commented 2 years ago

Hi @cav71, I may be wrong but it seems that this is more a comment on the de-vendoring in general? In that case, perhaps it could make sense to have a dedicated issue for the de-vendoring of fmt? I am asking as this issue is dedicated just to the ABI breakage of 1.10.0_1 , and I think we can close it once we solve it, i.e. for example by merging https://github.com/conda-forge/admin-requests/pull/501 .

cav71 commented 2 years ago

My bad, sorry: I pasted it the text in the wrong thread.

bluescarni commented 2 years ago

@traversaro I guess this is the only viable solution at this time.

It really sucks because the status quo (before this breakage) is such that it is impossible to use the current conda pinned fmt version (9.x) together with spdlog in the same package (or within a dependency chain). I guess I'll have to go and migrate back a bunch of packages to fmt pinned to 8.x :(

traversaro commented 2 years ago

Perhaps while we wait for an spdlog release we could think of doing a variant of this package with non-vendored fmt, but with a lower priority w.r.t. to the abi compatible vendored fmt spdlog . I would need to think about it, but perhaps we can find a solution before the new spdlog release.

bluescarni commented 2 years ago

@traversaro if such a strategy worked it would be a life saver... anything I can do to help? It sounds a bit beyond my conda-fu...

traversaro commented 2 years ago

@traversaro if such a strategy worked it would be a life saver... anything I can do to help? It sounds a bit beyond my conda-fu...

@bluescarni can you point me to one of the packages in which you want to install spdlog 1.10 and fmt 9.0 at the same time? So that I can try to get a clear idea on this.

kkraus14 commented 2 years ago

+1 to a variant with a track feature to weigh it down now and then consider moving to external fmt for 1.11 where we can more safely have an ABI break

traversaro commented 2 years ago

+1 to a variant with a track feature to weigh it down now and then consider moving to external fmt for 1.11 where we can more safely have an ABI break

The only missing piece of the puzzle for me is: how does the downstream package explictly depend on the custom variant? It may be obvious but I am not enough experienced with that.

bluescarni commented 2 years ago

Hi @traversaro thanks a lot for helping out...

So I have this package, heyoka, which depends on both spdlog and fmt:

https://github.com/conda-forge/heyoka-feedstock/blob/main/recipe/meta.yaml

As you can see from the recipe, I pinned spdlog to 1.10 and fmt to 8.1 in order to prevent ABI incompatibilities. So far so good.

The issue for the next version of heyoka if that one of its other dependencies, called mp++, in its latest version depends on fmt:

https://github.com/conda-forge/mppp-feedstock/blob/main/recipe/meta.yaml

The fmt version in this recipe is not pinned, so it currently depends on fmt 9.

This means that, as far as I have understood, it won't be possible to release the next version of heyoka until fmt is unbundled from spdlog, so that heyoka, spdlog and mp++ can all depend on the same version of fmt (i.e., the one globally pinned by conda-forge).

kkraus14 commented 2 years ago

The only missing piece of the puzzle for me is: how does the downstream package explictly depend on the custom variant? It may be obvious but I am not enough experienced with that.

I've seen two different approaches to it:

  1. You have a selector package, i.e. something like the arrow-cpp-proc package that has cpu and cuda variants defined via build strings. The arrow-cpp package then has a run_constrained entry on that package, i.e. https://github.com/conda-forge/arrow-cpp-feedstock/blob/main/recipe/meta.yaml#L117
  2. You just use build strings, i.e. here we could have bundledfmt and extfmt in the build string and a downstream package can then have a specification like spdlog=1.10=*extfmt*
h-vetinari commented 2 years ago

Bear is an example that doesn't depend on fmt, but breaks.

bluescarni commented 2 years ago

The only missing piece of the puzzle for me is: how does the downstream package explictly depend on the custom variant? It may be obvious but I am not enough experienced with that.

I've seen two different approaches to it:

1. You have a selector package, i.e. something like the `arrow-cpp-proc` package that has `cpu` and `cuda` variants defined via build strings. The `arrow-cpp` package then has a `run_constrained` entry on that package, i.e. https://github.com/conda-forge/arrow-cpp-feedstock/blob/main/recipe/meta.yaml#L117

2. You just use build strings, i.e. here we could have `bundledfmt` and `extfmt` in the build string and a downstream package can then have a specification like `spdlog=1.10=*extfmt*`

Would it make sense to create a temporary package called sdplog-unbundled (or something like that), to be removed/marked as broken after the new (hopefully unbundled) release of spdlog?

h-vetinari commented 2 years ago

New release is not far out, which means we could then do a clean break and re-migrate (it would be more work if it's only a patch version increment, but still feasible with some repodata patches).

h-vetinari commented 2 years ago

Heads up @conda-forge/spdlog

Upstream just tagged 1.11, and this repo is currently set to automerge. Since #50 was not reverted on main, that should be fine (we'll enter the brave new world as of 1.11), but JFYI

traversaro commented 2 years ago

Heads up @conda-forge/spdlog

Upstream just tagged 1.11, and this repo is currently set to automerge. Since #50 was not reverted on main, that should be fine (we'll enter the brave new world as of 1.11), but JFYI

Perhaps we want to disable automerge until https://github.com/conda-forge/spdlog-feedstock/pull/51 is fixed? @bluescarni

h-vetinari commented 2 years ago

I agree that the Debian patch would be good to have. The good thing is that we don't paint ourselves into a corner resolver-wise even if the automerge runs through. We can just add those patches afterwards, and it'll be pulled into the newer spdlog-as-runtime-dep.

bluescarni commented 2 years ago

Perhaps we want to disable automerge until #51 is fixed? @bluescarni

How does one disable automerge?

h-vetinari commented 2 years ago

How does one disable automerge?

Remove https://github.com/conda-forge/spdlog-feedstock/blob/main/conda-forge.yml#L1

bluescarni commented 2 years ago

How does one disable automerge?

Remove https://github.com/conda-forge/spdlog-feedstock/blob/main/conda-forge.yml#L1

Cheers, should be done now.

kkraus14 commented 1 year ago

This is now resolved

cav71 commented 1 year ago

Thanks!