Enable Cargo features by default

Enable (almost) all framework crate features by default, to make the
user experience when developing binaries (not libraries) much easier.

Users that develop a library should use `default-features = false`, but
that's also probably something that they only want to do later in the
development process (and should be evaluated on a case-by-case basis).

Fixes #627

This also makes examples much nicer to run, it's just
`cargo run --example $name`, no need to configure all the required
features.

Author: madsmtm

.github/workflows/ci.yml

@@ -35,6 +35,8 @@ unsafe_op_in_unsafe_fn = "deny"
 [workspace.lints.clippy]
 cargo = { level = "warn", priority = -1 } # Because of `lint_groups_priority`
 ptr_as_ptr = "warn"
+# We can't change the name of `objc2-ad-support`.
+redundant_feature_names = "allow"
 
 [profile.assembly-tests]
 inherits = "release"
@@ -48,7 +50,6 @@ push-remote = "0origin"
 shared-version = true # Framework crates share a version number
 tag-prefix = "icrate"
 tag-name = "{{prefix}}-{{version}}"
-enable-features = ["all"]
 owners = ["madsmtm", "simlay"]
 
 # TODO: Check for typos in CI

Cargo.toml

@@ -50,10 +50,11 @@ unstable-coerce-pointee = []
 [dependencies]
 objc2 = { path = "../objc2", version = "0.5.2", default-features = false, features = ["std"] }
 
-[dev-dependencies.objc2-foundation]
-path = "../../framework-crates/objc2-foundation"
-default-features = false
-features = ["std", "NSError"]
+[dev-dependencies]
+objc2-foundation = { path = "../../framework-crates/objc2-foundation", default-features = false, features = [
+    "std",
+    "NSError",
+] }
 
 [package.metadata.docs.rs]
 default-target = "aarch64-apple-darwin"
@@ -73,4 +74,3 @@ targets = [
 [package.metadata.release]
 shared-version = false
 tag-prefix = "block"
-enable-features = []

crates/block2/Cargo.toml

@@ -1,6 +1,6 @@
 framework = "block"
 crate = "block2"
-required-crates = []
+required-crates = ["objc2"]
 link = false
 skipped = true
 is-library = true

crates/block2/translation-config.toml

@@ -11,4 +11,3 @@ static_assertions = "1.1.0"
 [package.metadata.release]
 shared-version = false
 tag-prefix = "dispatch"
-enable-features = []

crates/dispatch2/Cargo.modified.toml

@@ -24,7 +24,6 @@ objc2 = { path = "../objc2", version = "0.5.2", default-features = false, option
 
 [package.metadata.docs.rs]
 default-target = "aarch64-apple-darwin"
-features = ["all"]
 rustc-args = ["--cfg", "docsrs"] # Fix cross-crate link to objc2::topics
 targets = [
     "aarch64-apple-darwin",
@@ -39,23 +38,21 @@ targets = [
 ]
 
 [features]
-default = ["std"]
+default = [
+    "std",
+    "block2",
+    "libc",
+    "objc2",
+]
 std = ["alloc"]
 alloc = []
 block2 = ["dep:block2"]
 libc = ["dep:libc"]
 objc2 = ["dep:objc2"]
 
-all = [
-    "block2",
-    "libc",
-    "objc2",
-]
-
 [package.metadata.release]
 shared-version = false
 tag-prefix = "dispatch"
-enable-features = []
 
 [dev-dependencies]
 static_assertions = "1.1.0"

crates/dispatch2/Cargo.toml

@@ -35,10 +35,12 @@ fn get_features(cargo_toml: &Path) -> Result<Vec<String>, Box<dyn Error>> {
     let cargo_toml = fs::read_to_string(cargo_toml)?;
     let CargoToml { features } = basic_toml::from_str(&cargo_toml)?;
 
-    // Skip GNUStep-related and "all" features
+    // Skip GNUStep-related and default/std/alloc features
     Ok(features
         .into_keys()
-        .filter(|feature| !feature.contains("gnustep") && feature != "all")
+        .filter(|feature| {
+            !feature.contains("gnustep") && !matches!(&**feature, "default" | "std" | "alloc")
+        })
         .collect())
 }
 
@@ -53,6 +55,8 @@ fn test_feature_sets<'a>(
         cmd.arg("check");
         cmd.arg("--package");
         cmd.arg(package);
+        cmd.arg("--no-default-features");
+        cmd.arg("--features=std");
         cmd.arg("--features");
         cmd.arg(features.join(","));
 
@@ -128,11 +132,13 @@ fn main() -> Result<(), Box<dyn Error>> {
     let feature_sets = features.iter().map(|feature| vec![&**feature]);
     test_feature_sets(&mut success, workspace_dir, feature_sets, "objc2-metal")?;
 
-    println!("Testing building each framework with `--features=all` and only their own features");
+    println!(
+        "Testing building each framework with `--features=default` and only their own features"
+    );
     for dir in workspace_dir.join("framework-crates").read_dir().unwrap() {
         let dir = dir.unwrap();
         if dir.file_type().unwrap().is_dir() {
-            let feature_sets = [vec!["all"]];
+            let feature_sets = [vec!["default"]];
             // println!("Testing all {dir:?} features");
             // let features = get_features(&dir.path().join("Cargo.toml"))?;
             // let feature_sets = features.iter().map(|feature| {

crates/header-translator/src/bin/check_framework_features.rs

@@ -239,6 +239,48 @@ pub struct LibraryConfig {
     pub typedef_data: HashMap<String, TypedefData>,
 }
 
+impl LibraryConfig {
+    // TODO: Merge this with `Availability` somehow.
+    pub(crate) fn can_safely_depend_on(&self, other: &Self) -> bool {
+        fn inner(
+            ours: &Option<semver::Version>,
+            other: &Option<semver::Version>,
+            rust_min: semver::Version,
+        ) -> bool {
+            match (ours, other) {
+                // If both libraries have a platform version, then ensure that
+                // ours is within the minimum of the other, OR that Rust's
+                // default min version is high enough that it won't matter.
+                (Some(ours), Some(other)) => other <= ours || *other <= rust_min,
+                // If only we have support for a platform, then we will emit a
+                // cfg-guarded [dependencies] table (done elsewhere), and thus
+                // it won't affect whether we can safely depend on it.
+                (Some(_), None) => true,
+                // If only the other library has support for platform, then
+                // that's fine.
+                (None, Some(_)) => true,
+                // If neither library support the platform, that's also fine.
+                (None, None) => true,
+            }
+        }
+
+        inner(&self.macos, &other.macos, semver::Version::new(10, 12, 0))
+            && inner(
+                &self.maccatalyst,
+                &other.maccatalyst,
+                semver::Version::new(13, 1, 0),
+            )
+            && inner(&self.ios, &other.ios, semver::Version::new(10, 0, 0))
+            && inner(&self.tvos, &other.tvos, semver::Version::new(10, 0, 0))
+            && inner(&self.watchos, &other.watchos, semver::Version::new(5, 0, 0))
+            && inner(
+                &self.visionos,
+                &other.visionos,
+                semver::Version::new(1, 0, 0),
+            )
+    }
+}
+
 #[derive(Deserialize, Debug, Default, Clone, PartialEq, Eq)]
 #[serde(deny_unknown_fields)]
 pub struct Example {

crates/header-translator/src/config.rs

@@ -19,7 +19,6 @@ workspace = true
 
 [package.metadata.docs.rs]
 default-target = "UNSET"
-features = ["all"]
 rustc-args = ["--cfg", "docsrs"] # Fix cross-crate link to objc2::topics
 targets = [
 ]

crates/header-translator/src/default_cargo.toml

@@ -5,6 +5,7 @@ use std::fmt;
 use std::fs;
 use std::io::ErrorKind;
 use std::io::Write;
+use std::iter;
 use std::path::Path;
 
 use toml_edit::InlineTable;
@@ -389,33 +390,76 @@ see that for related crates.", self.data.krate)?;
         }
 
         let mut emitted_features = self.emitted_features(config);
-        let _ = emitted_features.insert(
-            "all".to_string(),
-            emitted_features.keys().cloned().collect::<BTreeSet<_>>(),
+
+        // All features are enabled by default, except for frameworks that
+        // would bump the minimum version of the library.
+        //
+        // The reasoning is that default features are meant for ease of use,
+        // especially so in end-user binaries and hobby projects. But we also
+        // don't want to make the user's binary incompatible with older OSes
+        // if they didn't explicitly opt-in to that.
+        //
+        // End result: The "only" cost is compilation time (vs. wasted
+        // developer time in finding each feature gate, or an unintentionally
+        // raised minimum OS version).
+        //
+        // And yes, libraries that use these crates _will_ want to disable
+        // default features, but that's the name of the game.
+        //
+        // We _could_ technically try to do something fancy to avoid e.g.
+        // `objc2-app-kit` pulling in `objc2-core-data`, since that is rarely
+        // needed, but where do we draw the line? And besides, that just masks
+        // the problem, library developers _should_ also disable the file
+        // features that they don't use if they really care about compilation
+        // time.
+        //
+        // See also https://github.com/madsmtm/objc2/issues/627.
+        let is_default_feature = |feature| {
+            if let Some(lib) = config.try_library_from_crate(feature) {
+                // Dependency feature
+                self.data.can_safely_depend_on(lib) || !lib.link
+            } else {
+                // File feature
+                true
+            }
+        };
+        cargo_toml["features"]["default"] = array_with_newlines(
+            iter::once("std".to_string()).chain(
+                emitted_features
+                    .keys()
+                    .filter(|feature| is_default_feature(feature))
+                    .cloned(),
+            ),
         );
 
-        // Emit crates first.
+        // Enable non-default features when building docs.
+        let non_default_features: Vec<_> = emitted_features
+            .keys()
+            .filter(|feature| !is_default_feature(feature))
+            .cloned()
+            .collect();
+        if !non_default_features.is_empty() {
+            cargo_toml["package"]["metadata"]["docs"]["rs"]["features"] =
+                array_with_newlines(non_default_features);
+        }
+
+        // Emit crate features first (the "default" feature overrides in
+        // `default_cargo.toml`).
         for (feature, _) in emitted_features.clone().iter() {
             if config.try_library_from_crate(feature).is_none() {
                 continue;
             }
             let enabled_features = emitted_features.remove(feature).unwrap();
-            let array: Array = enabled_features.iter().collect();
-            cargo_toml["features"][feature] = value(array);
+            cargo_toml["features"][feature] = array_with_newlines(enabled_features);
         }
-        add_newline_at_end(&mut cargo_toml["features"]);
+
         // And then the rest of the features.
+        if !emitted_features.is_empty() {
+            add_newline_at_end(&mut cargo_toml["features"]);
+        }
         for (feature, enabled_features) in emitted_features {
-            let mut array: Array = enabled_features.into_iter().collect();
-            if 1 < array.len() {
-                for item in array.iter_mut() {
-                    item.decor_mut().set_prefix("\n    ");
-                }
-                array.set_trailing("\n");
-                array.set_trailing_comma(true);
-            }
             if cargo_toml["features"].get(&feature).is_none() {
-                cargo_toml["features"][feature] = value(array);
+                cargo_toml["features"][feature] = array_with_newlines(enabled_features);
             }
         }
 
@@ -546,6 +590,18 @@ fn add_newline_at_end(item: &mut Item) {
         .set_suffix("\n");
 }
 
+fn array_with_newlines(features: impl IntoIterator<Item = String>) -> Item {
+    let mut array: Array = features.into_iter().collect();
+    if 1 < array.len() {
+        for item in array.iter_mut() {
+            item.decor_mut().set_prefix("\n    ");
+        }
+        array.set_trailing("\n");
+        array.set_trailing_comma(true);
+    }
+    value(array)
+}
+
 pub trait EntryExt<'a> {
     fn implicit_table(self) -> &'a mut Table;
 }

crates/header-translator/src/library.rs

@@ -567,22 +567,32 @@ fn update_ci(workspace_dir: &Path, config: &Config) -> io::Result<()> {
         Ok(())
     }
 
+    // HACK: Linking `objc2-avf-audio` on older systems is not possible
+    // without an SDK that's new enough.
+    let uses_avf_audio = |lib: &LibraryConfig| {
+        matches!(
+            &*lib.krate,
+            "objc2-avf-audio"
+                | "objc2-av-foundation"
+                | "objc2-av-kit"
+                | "objc2-media-player"
+                | "objc2-photos"
+                | "objc2-photos-ui"
+                | "objc2-sprite-kit"
+                | "objc2-scene-kit"
+        )
+    };
     writer(&mut ci, config, "FRAMEWORKS_MACOS_10_12", |lib| {
         lib.macos
             .as_ref()
             .is_some_and(|v| VersionReq::parse("<=10.12").unwrap().matches(v))
-            // HACK: These depend on `objc2-uniform-type-identifiers` or
-            // `objc2-core-ml`, which is not available on macOS 10.12, but
-            // will be enabled by `"all"`.
-            && !["objc2-app-kit", "objc2-file-provider", "objc2-health-kit", "objc2-photos", "objc2-core-image"].contains(&&*lib.krate)
+            && !uses_avf_audio(lib)
     })?;
     writer(&mut ci, config, "FRAMEWORKS_MACOS_10_13", |lib| {
         lib.macos
             .as_ref()
             .is_some_and(|v| VersionReq::parse("<=10.13").unwrap().matches(v))
-            // HACK: These depend on `objc2-uniform-type-identifiers`, which
-            // is not available on macOS 10.13, but will be enabled by `"all"`
-            && !["objc2-app-kit", "objc2-file-provider", "objc2-health-kit", "objc2-photos"].contains(&&*lib.krate)
+            && !uses_avf_audio(lib)
     })?;
     writer(&mut ci, config, "FRAMEWORKS_MACOS_11", |lib| {
         lib.macos
@@ -705,14 +715,15 @@ fn update_test_metadata<'a>(
 
     let mut features = toml_edit::Array::new();
     for lib in libraries.clone() {
-        features.push(format!("dep:{}", lib.krate));
-        features.push(format!("{}/all", lib.krate));
+        // Add feature per crate.
+        //
+        // This is required for some reason for `cargo run --example` to work
+        // nicely in our workspace.
+        cargo_toml["features"][&lib.krate] =
+            toml_edit::Array::from_iter([format!("dep:{}", lib.krate)]).into();
+
+        features.push(lib.krate.to_string());
         // Inserting into array removes decor, so set it afterwards
-        features
-            .get_mut(features.len() - 2)
-            .unwrap()
-            .decor_mut()
-            .set_prefix("\n    ");
         features
             .get_mut(features.len() - 1)
             .unwrap()

crates/header-translator/src/main.rs

@@ -42,4 +42,3 @@ targets = [
 [package.metadata.release]
 shared-version = false
 tag-prefix = "objc-encode"
-enable-features = []

crates/objc2-encode/Cargo.toml

@@ -58,4 +58,3 @@ targets = [
 [package.metadata.release]
 shared-version = false
 tag-prefix = "objc2-exception-helper"
-enable-features = []

crates/objc2-exception-helper/Cargo.toml

@@ -21,8 +21,6 @@ proc-macro = true
 workspace = true
 
 [features]
-default = []
-
 # Runtime selection (for future compatibility)
 gnustep-1-7 = []
 gnustep-1-8 = ["gnustep-1-7"]
@@ -47,4 +45,3 @@ targets = [
 [package.metadata.release]
 shared-version = false
 tag-prefix = "objc-proc-macros"
-enable-features = []

crates/objc2-proc-macros/Cargo.toml

@@ -179,4 +179,3 @@ targets = [
 [package.metadata.release]
 shared-version = false
 tag-prefix = "objc"
-enable-features = []