Skip to content

Make sea-orm-cli & sea-orm-migration dependencies optional #2367

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xamgore
Copy link

@xamgore xamgore commented Sep 26, 2024

Addresses #2320.

My team is working on a workspace project. To make sure generated models are always the same, we decided to make sea-orm-cli a member crate. You may find details at the discussion 1889.

The problem is, sea-orm-cli/codegen feature imports all the database drivers, even though sqlite and mysql represent zero interest for me.

codegen = ["sea-schema/sqlx-all", "sea-orm-codegen"]

libsqlite-sys + sqlx-mysql + sqlx-sqlite take quite a few seconds in total compilation time, and I believe this could be improved.

image

I've gone through dependency-hell feature-shenanigans, but could easily make a mistake. So testing and merging this PR won't be easy. I tested the code on my local environment: tweaked all the possible combinations of features to see, whether the compiler would grumble. Also, I have patched my team project with local sea-orm codebase and tested it against it.

@xamgore
Copy link
Author

xamgore commented Sep 30, 2024

@billy1624 could you allow CI checks, please?

@xamgore
Copy link
Author

xamgore commented Apr 2, 2025

I've rebased the branch, probably some issues should be gone now.

Comment on lines -445 to -447
DbBackend::MySql => MySql.query_tables(),
DbBackend::Postgres => Postgres.query_tables(),
DbBackend::Sqlite => Sqlite.query_tables(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would something like this work?

        #[cfg(feature = "sqlx-mysql")]
        DbBackend::MySql => MySql.query_tables(),
        #[cfg(feature = "sqlx-postgres")]
        DbBackend::Postgres => Postgres.query_tables(),
        #[cfg(feature = "sqlx-sqlite")]
        DbBackend::Sqlite => Sqlite.query_tables(),

if the feature is not enabled, the variant shouldn't exist

Copy link
Author

@xamgore xamgore Apr 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not, as Rust needs the arms to be present. But the direction of thought is right, thank you

@xamgore
Copy link
Author

xamgore commented Apr 13, 2025

Ok, I've started testing against these two checks from the action scripts:

cargo clippy --manifest-path sea-orm-migration/Cargo.toml -- -D warnings
cargo clippy --manifest-path sea-orm-migration/Cargo.toml --features sqlx-postgres -- -D warnings

+on my personal project which uses the patched version

[patch.crates-io]
sea-orm = { git = "https://github.com/xamgore/sea-orm.git", branch = "master" }
sea-orm-cli = { git = "https://github.com/xamgore/sea-orm.git", branch = "master" }
sea-orm-migration = { git = "https://github.com/xamgore/sea-orm.git", branch = "master" }

It works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants