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

Split icrate into multiple crates #537

Closed
madsmtm opened this issue Dec 3, 2023 · 6 comments · Fixed by #592
Closed

Split icrate into multiple crates #537

madsmtm opened this issue Dec 3, 2023 · 6 comments · Fixed by #592
Labels
A-framework Affects the framework crates and the translator for them bug Something isn't working

Comments

@madsmtm
Copy link
Owner

madsmtm commented Dec 3, 2023

We're currently hitting crates.io's limit of 300 features, we should do something about that.

Maybe split the crate into multiple, e.g. icrate-appkit, icrate-foundation? Would also decrease the download size. Difficult because of coherence though.

@madsmtm madsmtm added bug Something isn't working A-framework Affects the framework crates and the translator for them labels Dec 3, 2023
@madsmtm madsmtm mentioned this issue Dec 3, 2023
6 tasks
@madsmtm madsmtm changed the title Reduce number of features in one crate Reduce number of features in icrate Dec 3, 2023
@madsmtm
Copy link
Owner Author

madsmtm commented Dec 5, 2023

I've requested an increase in the interim, though we should still solve this in a better way!

@madsmtm
Copy link
Owner Author

madsmtm commented Dec 23, 2023

icrate v0.1.0 is now released, keeping this issue open to track reducing number of features

@madsmtm
Copy link
Owner Author

madsmtm commented Dec 23, 2023

We could of course just go back to having one cfg per framework, and then live with the worse compile-times until we figure something better out.

@madsmtm madsmtm pinned this issue Dec 25, 2023
@madsmtm
Copy link
Owner Author

madsmtm commented Feb 2, 2024

Regarding splitting icrate into multiple smaller crates:

I think the coherence-problem is solvable, by properly introducing categories (in Swift, these are called "extensions"). In most cases, we will just emit categories as implementations directly on the type, as we're doing now. But if we encounter a category that is implemented on a type outside of the current library/framework, then we will emit a trait, and implement the trait for the type. This is a bit ugly, but we have to support categories if we want to support external crates anyhow.

Another problem is code-organization, as I'd still like to keep the generated files outside the main repository. Do we just use symlinks? I recall these being troublesome on Windows, will have to check how bad it is, but honestly, not a big issue if it's a bit more difficult for Windows users to clone the repository. The crates themselves won't have any symlinks, pretty sure Cargo prevents that, but will also have to verify that.

@madsmtm
Copy link
Owner Author

madsmtm commented Feb 2, 2024

Yeah, symlinks are a bit of trouble on Windows, but it's solvable. Cargo does copy files when running cargo publish/cargo package, but on Windows that of course also depends on if the repo is correctly checked out.

@madsmtm madsmtm changed the title Reduce number of features in icrate Split icrate into multiple crates Feb 6, 2024
@madsmtm
Copy link
Owner Author

madsmtm commented Feb 7, 2024

Other problems:

  1. Frameworks sometimes reference each other in cyclic ways. E.g. AppKit references Foundation, while Foundation references NSImage from AppKit. I think these are rare enough that they can be resolved by hand.

  2. Frameworks often reference a class by just typing @class, which doesn't include any information about where a class comes from (see also Fix feature flags for externally defined classes #402). To get this information, we must also compile the library where the class lives.

  3. I'm unsure how to emit feature flags for methods that require something from an external library. E.g. -[NSApplicationDelegate applicationWillFinishLaunching:] requires NSNotification to be available, but we can't write a cfg-guard like #[cfg(feature = "objc2-foundation/NSNotification")].

    • One solution would be to require the feature on NSApplicationDelegate itself, though that may be a bit inefficient as that'd require a lot of types to be available that may not actually be used. EDIT: This is what I ended up doing in Split icrate into one crate per framework #592.
    • Another would be to emit extra feature flags like "Foundation-NSNotification" in the objc2-appkit crate, but that requires the user to enable the flag, which is cumbersome.
    • An extension to the above would be to default-enable the Foundation features that are commonly used in AppKit

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

Successfully merging a pull request may close this issue.

1 participant