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

Request for new version release of autogenerated frameworks #656

Open
complexspaces opened this issue Sep 29, 2024 · 2 comments
Open

Request for new version release of autogenerated frameworks #656

complexspaces opened this issue Sep 29, 2024 · 2 comments
Labels
A-framework Affects the framework crates and the translator for them bug Something isn't working

Comments

@complexspaces
Copy link

Hello,

I was updating objc2-app-kit in preperation for adding objc2-ui-kit to arboard for iOS support and ran into a snag. Due to a longstanding cargo bug, something in the new 0.2.2 versions of these framework crates has started to pull unnecessary frameworks (such as objc2-cloud-kit) into the lockfile, even though they aren't built. I believe this is because of the weak dependencies used by the std feature.

To work around this I was disabling the std feature in my imports (so not even a weak dependency exists) but compiling objc2-ui-kit with this configuration fails because of a hardcoded import of std::os::raw::c_ulong. This was fixed by ec058b3 a few weeks ago, but hasn't made its way to crates.io yet. Would it be possible to make a release with these changes if time permits?

FWIW this seems to have added objc2-metal and objc2-quartz-core to objc2-app-kit's lockfile contents unconditionally. It might be because of of the bitflags weak feature interacting with objc2-foundation, maybe? None of the NSPasteboard, NSPasteboardItem, or NSImage features directly use them but this TOML results in them appearing in the lockfile (uncompiled) nontheless:

objc2-app-kit = { version = "0.2.2", default-features = false, features = ["NSPasteboard", "NSPasteboardItem", "NSImage"] }
@madsmtm madsmtm added bug Something isn't working A-framework Affects the framework crates and the translator for them labels Sep 29, 2024
@madsmtm
Copy link
Owner

madsmtm commented Sep 29, 2024

I was updating objc2-app-kit in preperation for adding objc2-ui-kit to arboard for iOS support and ran into a snag. Due to a longstanding cargo bug, something in the new 0.2.2 versions of these framework crates has started to pull unnecessary frameworks (such as objc2-cloud-kit) into the lockfile, even though they aren't built. I believe this is because of the weak dependencies used by the std feature.

Huh, I was wondering about that, thanks for investigating! Do you know if there's a bug on Cargo's issue tracker for it?

To work around this I was disabling the std feature in my imports (so not even a weak dependency exists) but compiling objc2-ui-kit with this configuration fails because of a hardcoded import of std::os::raw::c_ulong. This was fixed by ec058b3 a few weeks ago, but hasn't made its way to crates.io yet. Would it be possible to make a release with these changes if time permits?

Hmm, the problem with that is that it'll bump MSRV in a patch version, which I'm not really sure is something that's a good idea? See also #203.

In general, I will argue (and feel free to disagree here) that this is a small enough bug (the effect is that the lockfile is a bit larger and Cargo downloads a bit more) that it doesn't really warrant a new release? Though it is a problem I'd like to resolve in future releases (I don't dare give you a timeline on that, though).

FWIW this seems to have added objc2-metal and objc2-quartz-core to objc2-app-kit's lockfile contents unconditionally. It might be because of of the bitflags weak feature interacting with objc2-foundation, maybe?

Hmm... I'm a bit conflicted on how to solve this, because on one hand it seems to be the norm in the ecosystem to auto-enable same-named features on dependencies, but on the other hand, perhaps it is better that we don't auto-enable such unneeded features? Like, I could imagine a user wanting objc2 with std, and objc2-app-kit with bitflags, but objc2-foundation with neither.

Related to #627.

Let me stew on it a bit.

@complexspaces
Copy link
Author

Do you know if there's a bug on Cargo's issue tracker for it?

I believe this is rust-lang/cargo#10801.

Hmm, the problem with that is that it'll bump MSRV in a patch version, which I'm not really sure is something that's a good idea?

If these crates were meant to be widely compiled on Linux, I might be more inclined to agree but even the macOS package managers keep rustc up-to-date so there are no distros lagging behind and holding back the ecosystem. Even with that, 1.71 is over a year old so you would be matching the "12 months timeframe" proposed originally, and I don't think you would break anyone.

I don't personally consider MSRV to be a breaking change so I tend to just make sure the odds of people noticing it are low to be polite (in crates on 1.x.x I will release a minor version as a "polite marker", but that option isn't present here). In the end, this is your crate though!

In general, I will argue (and feel free to disagree here) that this is a small enough bug (the effect is that the lockfile is a bit larger and Cargo downloads a bit more) that it doesn't really warrant a new release?

For the most part I agree. I think the weird no-std support is strange but this isn't a dealbreaker for me; I can internally explain that the crates aren't actually being compiled so they don't warrant much thought or scrutiny.

Let me stew on it a bit.

Of course 👍

@madsmtm madsmtm added this to the objc2 v0.6 / frameworks v0.3 milestone Oct 27, 2024
madsmtm added a commit that referenced this issue Dec 25, 2024
madsmtm added a commit that referenced this issue Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants