Skip to content

Commit

Permalink
Enable Cargo features by default
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
madsmtm committed Jan 16, 2025
1 parent 729925d commit 97c6d75
Show file tree
Hide file tree
Showing 143 changed files with 7,641 additions and 7,566 deletions.
42 changes: 19 additions & 23 deletions .github/workflows/ci.yml

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions crates/block2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -73,4 +74,3 @@ targets = [
[package.metadata.release]
shared-version = false
tag-prefix = "block"
enable-features = []
2 changes: 1 addition & 1 deletion crates/block2/translation-config.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
framework = "block"
crate = "block2"
required-crates = []
required-crates = ["objc2"]
link = false
skipped = true
is-library = true
Expand Down
1 change: 0 additions & 1 deletion crates/dispatch2/Cargo.modified.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ static_assertions = "1.1.0"
[package.metadata.release]
shared-version = false
tag-prefix = "dispatch"
enable-features = []
15 changes: 6 additions & 9 deletions crates/dispatch2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"
14 changes: 10 additions & 4 deletions crates/header-translator/src/bin/check_framework_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand All @@ -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(","));

Expand Down Expand Up @@ -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| {
Expand Down
42 changes: 42 additions & 0 deletions crates/header-translator/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion crates/header-translator/src/default_cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
]
Expand Down
88 changes: 72 additions & 16 deletions crates/header-translator/src/library.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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;
}
Expand Down
39 changes: 25 additions & 14 deletions crates/header-translator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion crates/objc2-encode/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,3 @@ targets = [
[package.metadata.release]
shared-version = false
tag-prefix = "objc-encode"
enable-features = []
1 change: 0 additions & 1 deletion crates/objc2-exception-helper/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,3 @@ targets = [
[package.metadata.release]
shared-version = false
tag-prefix = "objc2-exception-helper"
enable-features = []
3 changes: 0 additions & 3 deletions crates/objc2-proc-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -47,4 +45,3 @@ targets = [
[package.metadata.release]
shared-version = false
tag-prefix = "objc-proc-macros"
enable-features = []
1 change: 0 additions & 1 deletion crates/objc2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,3 @@ targets = [
[package.metadata.release]
shared-version = false
tag-prefix = "objc"
enable-features = []
Loading

0 comments on commit 97c6d75

Please sign in to comment.