Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "tighten fmt in libmamba" #629

Merged
merged 1 commit into from
Jan 26, 2024
Merged

Conversation

h-vetinari
Copy link
Member

fmt 10.2 accidentally dropped a symbol that lead to widespread breakage on windows, including all of our windows builds (because it affected mamba itself). After #628 stopped the bleeding, @saraedum went digging and rootcaused+fixed this upstream in fmtlib/fmt#3786, which already made it to conda-forge: conda-forge/fmt-feedstock#42

This means we should be able to drop the pin again, as over time, more and more packages will now build with fmt 10.2 and thus get a fmt >=10.2,<11.0a0 run-export, that would be incompatible with the patched constraints for mamba from #628.

An alternative would be to just rebuild mamba once for 10.2, and leave the older builds constrained to 10.1, though I don't really see the need for this if the ABI is fixed.

CC @jaimergp

@h-vetinari h-vetinari requested a review from a team as a code owner January 3, 2024 21:55
@conda-forge-webservices
Copy link
Contributor

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.

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Jan 3, 2024

I believe the offending build was marked as broken in conda-forge/admin-requests#902 is that correct?

@SylvainCorlay
Copy link
Member

I believe the offending build was marked as broken in conda-forge/admin-requests#902 is that correct?

Yes, it seems so. Do we need to wait for CDNs to synchronize before merging ?

@jakirkham
Copy link
Member

The patches were regenerated in this CI job. May take a bit for the CDN to pick that up

Though got the sense this change might be less urgent. This based on the fact Axel asked Jaime to review (who is likely done for the day) and presented options for discussion. Also got the sense that other fixes were already in place (this is just relaxing some of them)

Please let me know if I'm missing something though. Haven't caught up on all the related threads yet

@h-vetinari
Copy link
Member Author

The patches were regenerated in this CI job. May take a bit for the CDN to pick that up

The patches from #628 were picked up quickly yesterday, which is what "unbroke" our windows pipelines. Now that a bugfix was identified and backported, we are able to undo that workaround (if we want).

Though got the sense this change might be less urgent. This based on the fact Axel asked Jaime to review (who is likely done for the day) and presented options for discussion. Also got the sense that other fixes were already in place (this is just relaxing some of them)

💯

Copy link
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ryanvolz
Copy link

Sorry for the ping, but I'd love to see this merged to resolve a conflict I'm seeing between libmamba and recent builds of a few packages that depend on fmt. (This is in the context of updating an installer built with conda constructor, where I haven't transitioned yet to having the installer create extra environments besides the base env.)

@h-vetinari h-vetinari merged commit 5fd35aa into conda-forge:main Jan 26, 2024
4 checks passed
@h-vetinari h-vetinari deleted the fmt branch January 26, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants