conda-forge / libcxx-feedstock

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

Build different variants for macOS system ABIs (before/after macOS 12.0) #154

Closed h-vetinari closed 3 months ago

h-vetinari commented 3 months ago

With the bump of the default CI image for macOS to 12.0, we're now running into #77 again (see #148). We intentionally want to match the system ABI (for rationale see here). Build for both ABIs.

Since we're relying on the system libcxx for the downstream tests, we need to manually override the image selection. Until the deprecated macOS-11 image is removed (end of June!), this looks like a reasonable effort to make. After that, I don't know if there's still a way to test against the old system ABI.

Closes #148 Closes #149

conda-forge-webservices[bot] commented 3 months 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.

beckermr commented 3 months ago

We need to patch older builds before and after the Abi patch with the same constraints?

h-vetinari commented 3 months ago

We need to patch older builds before and after the Abi patch with the same constraints?

So the good thing is that generally we have no caps anywhere for libcxx, meaning that all environments should pull in the fixed builds (after this PR). Given that this issue appeared ~3 years ago (#77), and given further how extremely wide-spread libcxx is, and how little we've heard about this, I'm cautiously tending towards not patching.

If we do repo-patch, we'd IMO have to apply the same split builds to all relevant major version branches, say 14, 15, 16, otherwise there wouldn't be any installable builds for those libcxx versions on __osx>=12.

h-vetinari commented 3 months ago

FYI: the macOS-11 images will be deleted end of June. If we care to test #155 for the old ABI before release, we need to merge this PR soon-ish...

isuruf commented 3 months ago

Since it's been a few years since macos-12 came up and we didn't see any issues, how about we put sys_abi < 12 package in conda-forge/label/libcxx-macos-lt-12 or something like that and remove __osx constraint?

After thinking about consequences, I'm just not sure if it's worth introducing a __osx virtual package constraint on libcxx

h-vetinari commented 3 months ago

I don't really mind either way. I'm trying to implement what you had suggested in the other thread.

I'm also a bit surprised how little we've heard of this, but since the ABI break is clearly observable in our tests (which are meant to test relevant scenarios), it's possible that this is either really rare in practice, or people just don't bother to report it.

From the POV of technical/philosophical correctness, I'd prefer to build for both ABIs (and publish to main), but then, I don't know what issues you had in mind with:

After thinking about consequences, I'm just not sure if it's worth introducing a __osx virtual package constraint on libcxx

isuruf commented 3 months ago

which are meant to test relevant scenarios),

The test is a little convoluted in that it uses -Wl,-rpath, but not -L.

I don't know what issues you had in mind with:

With things like miniforge, we have to be careful to package it with sys_abi<12 or otherwise we'll be installing a package that has wrong deps in the base environment. Not sure what the consequences of that are.

h-vetinari commented 3 months ago

Ok, so takeaway is we believe this problem is basically impossible to hit in normal use, and we are fine to find out the consequences (if any) of breaking the ABI for <12 users by publishing the old ABI to a label only. Are you in agreement with that, or could you correct my understanding if not?

isuruf commented 3 months ago

Ok, so takeaway is we believe this problem is basically impossible to hit in normal use, and we are fine to find out the consequences (if any) of breaking the ABI for <12 users by publishing the old ABI to a label only. Are you in agreement with that

Yes. If there are any consequences, people have a escape hatch with the label.