objc2 icon indicating copy to clipboard operation
objc2 copied to clipboard

Cargo features user experience

Open AlexanderEkdahl opened this issue 1 year ago • 5 comments

As I mentioned earlier, excellent work on these libraries and generators!

That said, I find it frustrating to have to specify the feature for every class I want to use (objc2-metal has 61 features). Should features be enabled by default and libraries that want to reduce code only specify the functionality they need?

I understand that this might be necessary when one crate extends the functionality of another.

Is there a Cargo feature to enable every feature of a crate?

AlexanderEkdahl avatar Jun 09 '24 10:06 AlexanderEkdahl

That said, I find it frustrating to have to specify the feature for every class I want to use (objc2-metal has 61 features). Should features be enabled by default and libraries that want to reduce code only specify the functionality they need?

Hmm, I guess they could all be enabled by default, though it really is a lot of code that's being compiled (objc2-foundation takes 4.5s and objc2-app-kit 9s on my fairly new Mac, a lot more on my older machine),

Is there a Cargo feature to enable every feature of a crate?

Yes there is, it's called all, and is documented in here. Please tell me how to improve the documentation such that you didn't have to ask this question (not that I mind, it's just unfortunate that you had to spend time doing so ;) ). Though now that I'm saying it, a tutorial that shows using "all" to begin with, and then narrows it down later on, would probably do the trick.

madsmtm avatar Jun 09 '24 13:06 madsmtm

Oh, I had no idea about the "magic string" "all". That certainly solves the problem.

Personally I don't mind waiting 9 seconds for something to compile if it saves me 30 minutes of debugging/enabling features. However, I can see how having the capability to turn features on/off is beneficial (especially for libraries). I would expect the linker to remove the unused code but that may not be enabled by default?

Please tell me how to improve the documentation such that you didn't have to ask this question

I was copy-pasting the objc2-metal example to get up-and-running. I'm not sure where it would be natural to introduce the "all" feature as I do believe it is a good starting point for someone who wants to get up and running with an Apple library from Rust.

Maybe a new section in the main README.md that introduces the objc-* crates or in the minimal Readme found on the crates.io documentation?

AlexanderEkdahl avatar Jun 10 '24 09:06 AlexanderEkdahl

I've been thinking about this again, and perhaps it would indeed make sense to make all features be enabled by default, and allowing users to opt out with default-features = false. I find in the few projects that I use the libraries in myself, that I end up always enabling the "all" feature, it's only in winit that I tune it down.

madsmtm avatar Sep 17 '24 23:09 madsmtm

With the "all" feature I find the developer experience to be perfectly acceptable.

I only wish it was more prominently visible in the README so that when I add a crate I can configure it correctly. I started a new project this morning and I had to find this thread 😃

I worry that if everything is enabled by default users (specifically users who depend on objc2-* transitively) will complain about long compile times.

AlexanderEkdahl avatar Sep 29 '24 05:09 AlexanderEkdahl

I only wish it was more prominently visible in the README so that when I add a crate I can configure it correctly. I started a new project this morning and I had to find this thread 😃

Yeah, that's part of why I've left this issue open, I still want to improve the docs on this!

I worry that if everything is enabled by default users (specifically users who depend on objc2-* transitively) will complain about long compile times.

That's also a worry of mine, and I know that some people reflexively use default-features = false everywhere for partly this reason. Idk., still have to think about it.

madsmtm avatar Sep 29 '24 09:09 madsmtm

I have moved the items in the "all" feature to the "default" feature in 9cb02689c3cdb77305f539e69bf376268962a796, with the motivation: Easy default for binary projects, though a bit harder for libraries that want to minimize compile-times.

It is somewhat unfortunate that I didn't do this originally, since once I do the release, I'll have to go file PRs to all the projects that currently use objc2 and add default-features = false, but such is life.

I'll also note that there was a slight complication here, namely that we don't want objc2-app-kit to implicitly enable objc2-uniform-type-identifiers, as the latter contains a #[link(name = "UniformTypeIdentifiers", kind = "framework")], and that would cause the dynamic linker to fail on versions lower than macOS 11.0 where UniformTypeIdentifiers was introduced. I fixed this by just not including such dependencies in the default features table.

madsmtm avatar Jan 16 '25 11:01 madsmtm

Okay, so I'm gonna file PRs against the popular crates that use objc2 (taken from https://github.com/madsmtm/objc2/issues/174), and make them use default-features = false. Tracking progress here (will tick the checkbox once a PR is filed, not necessarily merged):

  • [x] copypasta: https://github.com/alacritty/copypasta/pull/77
  • [x] webbrowser: https://github.com/amodm/webbrowser-rs/pull/94
  • [x] trash: https://github.com/Byron/trash-rs/pull/135
  • [x] window_clipboard: https://github.com/hecrj/window_clipboard/pull/29
  • [x] dark-light: https://github.com/rust-dark-light/dark-light/pull/64
  • [x] btleplug: https://github.com/deviceplug/btleplug/pull/420
  • [x] alacritty: Already done in https://github.com/alacritty/alacritty/pull/8312
  • [x] slint: https://github.com/slint-ui/slint/pull/7422
  • [x] neovide: https://github.com/neovide/neovide/pull/2980
  • [x] eframe: https://github.com/emilk/egui/pull/5624
  • [x] gpu-allocator: Already done when introduced.
  • [x] muda: https://github.com/tauri-apps/muda/pull/264
  • [x] window-vibrancy: https://github.com/tauri-apps/window-vibrancy/pull/164
  • [x] global-hotkey: https://github.com/tauri-apps/global-hotkey/pull/117
  • [x] tauri: https://github.com/tauri-apps/tauri/pull/12468
  • [x] tauri plugins: https://github.com/tauri-apps/plugins-workspace/pull/2343
  • [x] blade: https://github.com/kvark/blade/pull/250
  • [x] enigo: https://github.com/enigo-rs/enigo/pull/382

Just to note, I'm not normally gonna fix other people's code like this, e.g. I'll expect them to figure out how to upgrade themselves, but I'm doing it now because I feel like it'll be my fault otherwise ;)

Crates where I will file a PR with the dependency upgrade itself (as well as using new framework crates):

  • [x] winit: https://github.com/rust-windowing/winit/pull/4092
  • [x] glutin: https://github.com/rust-windowing/glutin/pull/1726
  • [x] softbuffer: https://github.com/rust-windowing/softbuffer/pull/254
  • [x] arboard: https://github.com/1Password/arboard/pull/173
  • [x] rfd: https://github.com/PolyMeilex/rfd/pull/236
  • [x] wry: https://github.com/tauri-apps/wry/pull/1468
  • [x] raw-window-metal: https://github.com/rust-windowing/raw-window-metal/pull/27
  • [x] tray-icon: https://github.com/tauri-apps/tray-icon/pull/226

madsmtm avatar Jan 21 '25 20:01 madsmtm

Wouldn't it make sense to create a set of feature to enable functionality needed? Like if metal mandates 61 feature, maybe have a feature for that? Like enabling all will likely make them not being disabled in some dependency in the graph and blow up.

Like even for said metal, like all was also a fine solution.

I'd also said that win32 has a common problem, but doesn't try to enable everything by default.

kchibisov avatar Jan 21 '25 20:01 kchibisov

Yes, and we had that before, the all feature, but it's not very discoverable.

In my own use of the objc2 crates, including the porting work I've done to migrate other crates, I've found that in every single project, it makes sense to start out with enabling everything, since it can be really difficult to figure out from inside your editor (i.e. with rust-analyzer) what features to enable for a thing to be available (NSView requires both NSView and NSResponder, for example), and that massively hampers the development workflow (as you'd have to look it up on docs.rs instead).

Basically, enabling all features should be the default, since that's always what you want when starting out. Yes, this will cause some crates to enable too much, but that's a cost we'll have to live with.


If need be, I guess I could analyse crates.io for uses of default-features = true once in a while, to ensure that the most popular crates keep their compile-times low.

Python script to do so
import json
import urllib.request
import sys

API_URL = "https://crates.io/api/v1/crates"

crate_name = sys.argv[1]

page = 1
while True:
    print(f"page {page}")
    with urllib.request.urlopen(f"{API_URL}/{crate_name}/reverse_dependencies?per_page=100&page={page}") as f:
        data = json.load(f)
    page += 1

    if len(data['versions']) == 0:
        break

    versions = {x['id']: x for x in data['versions']}

    for dep in data['dependencies']:
        req = dep['req']
        if req == "^0.2.2" or req == "^0.2.1" or req == "^0.2.0" or req == "^0.2":
            continue
        crate = versions[dep['version_id']]
        if dep['default_features']:
            print(f"{crate['crate']} ({req}) had `default-features = true`")

madsmtm avatar Jan 21 '25 21:01 madsmtm