sea-orm icon indicating copy to clipboard operation
sea-orm copied to clipboard

`sea-orm-cli generate entity` creates `mod.rs` in library crate

Open billy1624 opened this issue 2 years ago • 7 comments

Discussed in https://github.com/SeaQL/sea-orm/discussions/604

Originally posted by viktorbahr March 12, 2022

Preface

Since I'm relatively new to Rust, I'm not 100% sure if I'm missing something here. In case I did, please excuse my ignorance!

Description

When pointing the output directory of sea-orm-cli generate entity to the src directory of an entity library crate, it generates a (what I assume to be an unneeded) mod.rs file while leaving the lib.rs untouched. Using the generated entity from another crate in the workspace (i.e. the migration crate) then requires to manually copy the changes made to the mod.rs file over to the lib.rs file of the entity crate.

Steps to Reproduce

Requirements: Database access and valid DB_URL env set

  1. Create a new crate.
  2. Inside that crate, create an empty entity library crate as described in the migration docs
  3. Additionally, create a new migration crate using sea-orm-cli migrate init
  4. Add a migration to the migration crate that creates a new table in the DB
  5. Apply the migration using sea-orm-cli migrate up
  6. Generate the entity using sea-orm-cli generate entity -o entity/src

Expected Behavior

The CLI should add the pub mod declaration for the newly generated entity (and prelude, seaql_migrations) to entity/src/lib.rs.

Actual Behavior

The CLI generates a mod.rs file, containing the pub mod declarations.

Versions

│   └── sea-orm v0.6.0
│       ├── sea-orm-macros v0.6.0 (proc-macro)
│       ├── sea-query v0.21.0
│       │   ├── sea-query-derive v0.2.0 (proc-macro)
│       ├── sea-strum v0.23.0
│       │   └── sea-strum_macros v0.23.0 (proc-macro)
│   └── sea-schema v0.5.1
│       ├── sea-orm v0.6.0 (*)
│       ├── sea-query v0.21.0 (*)
│       ├── sea-schema-derive v0.1.0 (proc-macro)
├── sea-orm v0.6.0 (*)

Additional Information

To me it looks like that this is a mismatch between the behavior of the CLI and the structure proposed in the docs. To fix this, I'd propose adding a check for an existing lib.rs to the CLI, which, if triggered, would cause the changes that would otherwise be written to mod.rs to be made there. I could try to setup a PR for this in case my assumptions are validated by someone from the maintainer team.

billy1624 avatar Aug 04 '22 10:08 billy1624

What kind of checks should there be in case the lib.rs exists?

  1. Should it check if the lib.rs is empty?
  2. What if it is not empty? Should it append to the existing one?

anshulxyz avatar Aug 04 '22 16:08 anshulxyz

@HigherOrderLogic is working on this. Just to let everyone know :)

billy1624 avatar Aug 05 '22 02:08 billy1624

What kind of checks should there be in case the lib.rs exists?

  1. Should it check if the lib.rs is empty?
  2. What if it is not empty? Should it append to the existing one?

I think we could add a --lib flag to CLI. With this flag enabled the content of mod.rs will be written to lib.rs instead. I want to make the behaviour explicit and avoid confusion. I.e. always create / replace instead of append.

billy1624 avatar Aug 05 '22 02:08 billy1624

What kind of checks should there be in case the lib.rs exists?

  1. Should it check if the lib.rs is empty?
  2. What if it is not empty? Should it append to the existing one?

I think this should overwrite the lib.rs file whatever the content is since the CLI normally just overwrite the mod.rs file.

HigherOrderLogic avatar Aug 05 '22 07:08 HigherOrderLogic

Or if possible check Cargo.toml if it is a [lib] and then choose the action accordingly ....... This can also be clubbed with --lib or --bin flags in cli for non default action

giripriyadarshan avatar Sep 13 '22 18:09 giripriyadarshan

Or if possible check Cargo.toml if it is a [lib] and then choose the action accordingly ....... This can also be clubbed with --lib or --bin flags in cli for non default action

I don't think it's a good idea since [lib] isn't required to lib crate so this may lead to undesired result.

HigherOrderLogic avatar Sep 14 '22 13:09 HigherOrderLogic

Or if possible check Cargo.toml if it is a [lib] and then choose the action accordingly ....... This can also be clubbed with --lib or --bin flags in cli for non default action

Hey @giripriyadarshan, thanks for the suggestions! I feel like we should make the behaviour explicit which user have full control over the codegen behaviour. That's why this feature is decided to be opt-in, i.e. user need to supply an additional feature flag --lib to enable it.

billy1624 avatar Sep 20 '22 11:09 billy1624