madsmtm / objc2

Bindings to Apple's frameworks in Rust
https://docs.rs/objc2/
MIT License
363 stars 40 forks source link

Request for new version release of autogenerated frameworks #656

Open complexspaces opened 3 weeks ago

complexspaces commented 3 weeks ago

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 https://github.com/madsmtm/objc2/commit/ec058b32cee53e5f321a3f6f3e35c27332655469 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 commented 3 weeks ago

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 https://github.com/madsmtm/objc2/issues/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 https://github.com/madsmtm/objc2/issues/627.

Let me stew on it a bit.

complexspaces commented 3 weeks ago

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

I believe this is https://github.com/rust-lang/cargo/issues/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 👍