Workspace changes and MSRV 1.70

[str4d]

This sets the MSRV of the libraries in this workspace to 1.70 (released June 2023), which is the MSRV required for the previously-declared dependency versions (specifically clap 4.4). This is not the actual minimum Rust version that works with the published libraries however; I was able locally to get everything working with Rust 1.64 (released September 2022) by adjusting a couple of dependency lower version bounds. Going below this would require significant dependency version downgrades, and I'm not sure is actually desirable.

For context, the Rust version packaged with current Debian stable (bookworm) is 1.63, and it looks like the next stable Debian will use 1.70. The Rust Docker images provide latest stable, as does rustup.

Declaring some lower bound is beneficial both for downstream developers to know whether or not they can use the libraries up-front, and for contributors to know what features they can rely on:

• An MSRV of 1.64 provides access to workspace inheritance, which I make use of heavily in this PR. In particular, it de-duplicates dependencies on workspace crates to ensure a consistent workspace.
• An MSRV of 1.70 has the nice side-effect of being when the sparse crate index was enabled by default, which improves local developer experience, but isn't necessary for the libraries themselves yet (you'd want to consider what language features are added since 1.64 that might be useful).
• Once MSRV is at least 1.75, the async-trait crate can be removed.

[sugyan]

Thanks for the great pull-request! I did not properly understand the role of MSRV and how to use it.
And I didn't know that workspace.dependencies could centralize versions of dependent libraries, so this change seems very meaningful and good.

...However, I think that the libraries we want to provide to developers (api, xrpc-client, etc.) and the libraries to generate them (codegen), examples, etc. have different essential roles. So it seems a bit strange to centralize these. Would it have been better if only the ones we wanted to provide to developers were grouped together in a separate repository...

Thanks also for the reference information about MSRV. I think 1.70 is a good choice at this point. When a little more time goes by and 1.75 or higher becomes more common, I would like to move up to 1.75 and remove the dependency on async-trait.

[str4d]

...However, I think that the libraries we want to provide to developers (api, xrpc-client, etc.) and the libraries to generate them (codegen), examples, etc. have different essential roles. So it seems a bit strange to centralize these. Would it have been better if only the ones we wanted to provide to developers were grouped together in a separate repository...

If you want to take this approach, we can do so; I was proposing just the minimal set of changes to modernise the workspace, but I can make a more radical PR 😈

atrium-lex, atrium-codegen, and the top-level atrium crate are essentially self-contained and only for tooling. In other repositories with this kind of structure, we've used multiple workspaces to handle this, and we could do so here as well:

crates/
    atrium-api/
        src/
        Cargo.toml
    atrium-cli/
        src/
        Cargo.toml
    atrium-xrpc/
        src/
        Cargo.toml
    atrium-xrpc-client/
        src/
        Cargo.toml
    atrium-xrpc-server/
        src/
        Cargo.toml
    Cargo.toml <-- workspace for published crates
    rust-toolchain.toml
tools/
    atrium/
        src/bin/lexgen.rs
        Cargo.toml
    atrium-codegen/
        src/
        Cargo.toml
    atrium-lex/
        src/
        Cargo.toml
    Cargo.toml <-- workspace for tooling

In this structure:

• The regular "development environment" would be in the crates/ subdirectory. You'd need to run cargo in that directory, or like cargo check --manifest-path crates/Cargo.toml.
• To do codegen, you run cargo in the tools/ directory (or path it like above).
• MSRV on tooling would only be enforced in the crates/ subdirectory; tools/ could use latest stable Rust without interfering with CI.

An alternative would be to use a similar structure, but moving crates/Cargo.toml and crates/rust-toolchain.toml into the repository root (i.e. leaving the current workspace where it is, but moving the tooling crates into tools/ and optionally the code for the published crates into crates/ for symmetry). In this structure:

• The regular "development environment" is the repository root as currently (maybe with the code under crates/, but you can still run cargo at the top level as usual).
• To do codegen, you run cargo in the tools/ directory (or path it like above).
• MSRV on tooling would inherit the rust-toolchain.toml from the root, and so would effectively be the same as for the published crates in practice (you could set an explicit override for the tools/ subdirectory, but that wouldn't be the default when people check out the repository).

I'm happy to rework this PR in this direction if you want.

[str4d]

I separately note that atrium-cli is not published. Is it intended to be published at some point, or is it for local testing purposes?

[sugyan]

Regarding the structure of the repository, I think your first suggestion sounds good: a library that we want to provide to developers as ATrium (whether or not you want to make it public), and a library as a tool to maintain those libraries. It seemed to me that it would be good to have a clear distinction by role.

In this case, is my understanding correct that the so-called examples will be included in tools?
Also, since bin/lexgen.rs was placed in root for the sole purpose of calling code generation from the CLI, it might be better to use atrium-lexgen or simply lexgen instead of atrium as a package name.

As for atrium-lex, I am a bit torn. It is indeed a tool for code generation, but it is a collection of type definitions for lexicon, so I thought it might be useful for other developers to publish it as crates.

As for atrium-cli... at first, I made it as a reference for how to use the API library instead of examples. I think it's OK to publish it, but I thought no one would use it if I publish it... so I left it as it is.

---

I am not good at English and am writing this reply with the help of machine translation. Sorry if it was not clear.

[str4d]

Regarding the structure of the repository, I think your first suggestion sounds good: a library that we want to provide to developers as ATrium (whether or not you want to make it public), and a library as a tool to maintain those libraries. It seemed to me that it would be good to have a clear distinction by role.

Sounds good!

In this case, is my understanding correct that the so-called examples will be included in tools?

If you want the examples in the repository to be updated as changes are made to the other crates (i.e. if you want the examples to reflect the development state), then they would go into the workspace in crates/ (but could still live in an examples/ folder in the repository for visibility).

If you want the examples to be standalone then it would make more sense to leave them at the top level (i.e. not part of either workspace), and then just remember to update them after releases.

Also, since bin/lexgen.rs was placed in root for the sole purpose of calling code generation from the CLI, it might be better to use atrium-lexgen or simply lexgen instead of atrium as a package name.

Yep, I can do that. The crates under tools/ that are only intended for internal usage should also have publish = false in their Cargo.toml files.

As for atrium-lex, I am a bit torn. It is indeed a tool for code generation, but it is a collection of type definitions for lexicon, so I thought it might be useful for other developers to publish it as crates.

In that case, rather than a tools/ directory we could make it a lexicon/ directory, and then atrium-lex could be published from within that while the other crates are kept internal.

I am not good at English and am writing this reply with the help of machine translation. Sorry if it was not clear.

It's perfectly clear to me! 🙂

[str4d]

Force-pushed to split the repository into two workspaces.

I actually worked out that I could add a rust-toolchain.toml file into the lexicon/ folder to have it use stable Rust, which overrides the root-level MSRV Rust. So I've implemented the smaller refactor where I just moved the codegen crates into the lexicon/ directory, and left the main crates and examples in the root repository. If you still would prefer it, I can move those into a crates/ directory.

[sugyan]

Thank you for the reconstruction!
I think it looks good, with only lexicons as a separated workspace.
Similarly, it would be nice if examples were separated as a separate workspace, but I'm not sure. rs-car and others used in the firehose example are not currently used by other atrium-* libraries, so I don't think it is necessary to include them in the root workspace.dependencies.

[str4d]

Similarly, it would be nice if examples were separated as a separate workspace, but I'm not sure.

If examples are moved to a separate workspace, they cannot depend on the atrium-* libraries in the repository, and would be required to only depend on the published versions of the crates. This is what I was asking earlier: do you want the examples to show how to use the published crates, or to show how the upcoming version of the crates will be used? These two are the same thing when looking specifically at tagged revisions of the repository, but differ when looking at the tip of the main branch.

rs-car and others used in the firehose example are not currently used by other atrium-* libraries, so I don't think it is necessary to include them in the root workspace.dependencies.

It isn't necessary to move dependencies into the workspace, just useful. So if you don't find it useful to move the non-atrium-* dependencies into the workspace and would prefer duplication there, we can leave them where they are. We can also do a hybrid: pull dependencies from the workspace that are used in atrium-* crates, and just leave in each example the dependencies that are only used there.

[str4d]

Force-pushed to remove from the workspace.dependencies dependencies that are only used in examples.

[str4d]

Pushed an additional commit to add a .gitattributes file that marks codegen output files as generated in GitHub's UI.

[sugyan]

As for what I want the examples to show, my personal intention is to "how to use the published crates", not the latest in repositories.
To take it a bit to the extreme, I hope that a beginner can copy just the directory under examples and still have it work.

Anyway, for now, the current repository structure looks very good.
Could you please just fix the default-features part of futures? I'd like to merge this after the tests passed.
Thank you very much!

[str4d]

Force-pushed to fix the futures feature flags, and move the examples out of the workspace (so they are standalone examples). I note that the firehose example is not yet completely standalone because atrium-xrpc-server has never been published, but the local path dependency does appear to keep it compiling even outside the workspace (and this can be fixed to work when someone copies the examples/ directory once atrium-xrpc-server is published).

[sugyan]

Oh... I forgot about atrium-xrpc-server.
It is true that firehose example is not yet standalone until this is published.
...but atrium-xrpc-server should have server functionality to implement PDS, and I don't think the current implementation, which only has stream message type definitions, is suitable to be released as a crate library. Maybe we could move these definitions into the firehose project for once.

Anyway, I will proceed that separately from this pull-request. I'll merge this once. Thank you!