-
Notifications
You must be signed in to change notification settings - Fork 125
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
Proposal: Monorepo + workspace build for the esp-idf-*
crates
#314
Comments
I definitely think this is the right direction, we're finding a lot of success with it in bare-metal land.
Imo, this should stay separate, just because it's easy to remember if you need the template it's just
I would say yes, just from our experience - but if you can make the workspace work properly then by all means you should use it :D.
Do you not think there might be some confusion with this? I imagine this will make it very hard to find good search results via google etc. I'd suggest keeping it either esp-idf-hal, esp-idf-svc or bikeshedding some other name. Unless you have a compelling reason to call it esp-idf. |
True... even though
Would you elaborate on the benefits of a monorepo without a workspace setup for the baremetal projects? Anything I'm missing here?
Would there be confusion or hard-to-search results indeed?
|
I can just speak for myself here, but in my experience Rust tooling support for workspaces is fine. The most useful thing for me is that my crates live side by side and I can pin dependencies painlessly by defining most of them in the virtual workspace manifest. |
Heh. I would not qualify Rust tooling support as "fine" if RA is unusable. Interesting that RA is affected by the size of the workspace, as my assumption was that it is also affected by the number and size of your dependencies? Where I'm getting at is that with a workspace setup, whatever extra weight we get via workspace deps, we'll lose in the form of out-of-workspace deps, so net-net we should be at the same "weight" at the end.
That's an easy restriction to comply with.
Exactly. As I do here.
|
Yeah, it's a bit of a disappointment. I currently maintain a small/mid-sized codebase with 10kloc. It's a bit macro heavy because of I think heavy macro useage is the number one factor which can slow down RA. The problem with Since I haven't identified any macro useage in esp-idf-* crates, this shouldn't be as much of a problem. |
Just to add, I think maintaining most crates in the |
Are you sure you don't hit something very specific for your setup? As for the ESP-IDF crates - yes - no proc-macros there. |
Not sure I follow. We are (or at least we fool) ourselves that we maintain them? Or do you mean paid maintenance? |
Whoops, sorry. I meant adding them to a monorepo. It's been a long day. |
I'm not sure if it's something specific for my setup, I actually had a look at rs-matter when initializing my project. It's a reference implementation for a state of the art zero-touch/zero-trust enrollment standard. I maintain it at https://github.com/hm-edu/open-brski if you want to take a look. This could also be interesting to @MabezDev I suppose. |
I think some open question regarding the migration strategy are still not discussed. How do we manage the interleaving of the PR numbering for past PR's that are already merged - E.g #440 is something different in hal than in svc. Transferring issues - i assume we will only transfer open issues to the combined repo? That is also a step we do pretty late where we know the other stuff works. Also moved PR's are creating a new issue number i think and so bumping the numbering system of PR's / issues. Something to keep in mind. |
I think embuild and the template are relative independent, and we first should only focus on sys/hal/svc. If we want we still can include it later but the operation is already complex enough, so i would appreciate we don't do to much at the same time increasing risk of the unknown. Also embuild itself would not be a good fit for in a workspace.It also provides ldproxy that will always be build against a host-target. That is deeply problematic as a rust workspace just doesn't work with mutti target builds. |
As for
The key is to check if a single crate can be a member of multiple workspaces. If it can, then we don't have an issue. I think we need to experiment a bit rather than theorizing, so as to see what works and what not. Also, and regardless whether the above multi-workspace build works or not, we probably want all of sys/hal/svc/embuild/ldproxy/cargo-pio/template in a single monorepo. This would allow us to play with various workspace setups post-migration. And would allow us to have at least atomic commits spanning multiple crates. (UPDATE: and multiple CIs (if we end up with multiple workspaces) triggering on any commit. As in a commit in embuild would trigger the CI for sys/hal/svc. And users will stop opening issues "in the wrong repo" because very often they can't really distinguish whether an issue belongs to sys, hal, svc, template or embuild. |
We don't? Legacy PRs remain in the legacy repos which will be archived and readonly but not deleted.
I do not understand this. If you could clarify.
I don't think we need to move open PRs at all. They anyway would be totally unmergeable with the new repo. |
git-filter-repo should make joining together the projects as workspace easily. I would make an esp-idf empty repo with the workspace and a main readme, then import the full history of all the target esp-idf* repos and at the end setup the new CI. |
Overview
This proposal suggests migrating the
esp-idf-*
crates' GIT repos (and a few others, see below) to:(This change was inspired from the experience I got with the monorepo-yet-micro-crates approach of
edge-net
which works quite nicely.)What will NOT be changed by this proposal?
Crates' names and their consumption from
crates.io
.From the POV of the end user, everything remains backwards compatible:
esp-idf-sys
,esp-idf-hal
,esp-idf-svc
are separate cratesBut why?
The main reason is that the
esp-idf-*
crates are highly inter-dependent at it is just easier to develop them in a single repo, as part of a workspace setup.Major pain points of the current micro-repo approach, examples below.
Introducing changes across > 1 crate
esp-idf-hal
master
often implies a backwards-incompatible change toesp-idf-svc
master
:esp-idf-hal
.esp-idf-svc
too.Cargo.toml
ofesp-idf-svc
needs to be patched with[patch.crates-io]
to use themaster
(GIT) repo. And we need not to forget to remove this patch prior to publishing theesp-idf-svc
crateContrast this with a monorepo with a workspace setup:
esp-idf-sys
,esp-idf-hal
,esp-idf-svc
, and if necessary -embedded-svc
(if it is part of the repo), andembuild
Cargo.toml
equilibristics, as - with a workspace setup - the deps between the crates are expressed using apath
notation syntax (whichcargo publish
automatically turns on references to the published crates during publishing). It just works!Slow CI
CI is currently slow, because of the fact that the build of
esp-idf-sys
itself accounts to 80% of the total build time of any other downstream crate (hal, svc, the -template).Because we are using separate GIT repos for each micro-crate, their CIs are in fact repeating the (slow) build of
esp-idf-sys
multiple times:esp-idf-sys
itself (I mean, multiple times, but once per ESP IDF version + target + MCU triple + type of build)esp-idf-hal
esp-idf-svc
esp-idf-template
Contrast this with a workspace setup:
esp-idf-template
... as a side effect, all downstream crates will be built. And with non-incremental build only the first time. Doing
no_std
and so on re-builds will be incremental.Worse: Unreliable CI / testing changes
Because CI is so slow, and because we are duplicating what amounts to the same build pipeline across 3-4 crates, there is NO single CI which does all the builds against everything:
esp-idf-sys
builds against different versions of ESP IDF compared to hal and svcesp-idf-template
crate, against released versions of the other crates (i.e. we don't know if the code in upstream crates will break the CMake build until after we release them, which sucks)No unified examples
Each crate has its own
examples
folder. On one hand, this is nice, because some examples need only - say -esp-idf-hal
so it is somewhat nice to show that these examples can work withoutesp-idf-svc
.On the other hand, the
hal
examples cannot use the Rustlog
crate, as support for it is only inesp-idf-svc
, so they useprintln!
, which is not nice at all.Further, the reality is, NO useful end-user application of the
esp-idf-*
crates can be implemented without depending on thesvc
crate, sohal
-only examples are a solution looking for a problem really.(
Digression:
We recognized this problem several months ago, and this is one of the reasons why the
hal
crate re-exports thesys
crate underesp_idf_hal::sys
and further - thesvc
crate re-exports bothsys
andhal
.This way, the user needs - in 99% of the cases - only one dependency -
esp-idf-svc
. Via this single dependency, the user can still call intohal
andsys
as these are re-exported.Further, such a setup lifts from the shoulders of the user the problem that otherwise she has to think about what
hal
version matches itssvc
version.For niche cases, user can still depend directly or only on
sys
and/orhal
, or any combination of the crates thereof, but that's the exception rather than the rule, so we are not optimizing for it; but also not making it any harder.In retrospective - we could've renamed
esp-idf-svc
to justesp-idf
or we could've created a new aggregationr crate namedesp-idf
- if onlyesp-idf
was not taken already by somebody incrates.io
- and taken under the sub-optimalesp_idf
name, which prohibits the usage ofesp-idf
(note that dash vs underscore) by somebody else either!)
While we can move all examples to
esp-idf-svc
without introducing monorepo and workspace setup, if we move to a monorepo setup, it does make sense to move the examples to the workspace level in a commonexamples/
folder.Monorepo without a workspace setup - is it worth it?
I'm not sure, but I'm inclined to say: NO.
Because a lot of the benefits actually come from the workspace setup, not from the monorepo per-se. The workspace setup requires monorepo of course. Examples where workspace would help, but "just monorepo" won't:
Cargo.toml
for backwards-incompatible changes. Without workspace setup, we still need to do thisesp-idf-svc
, we can just buildesp-idf-svc
and not even build the sys and hal; but we still need to buildesp-idf-template
).What crates should be part of the monorepo + workspace setup:
Minimum:
esp-idf-sys
esp-idf-hal
esp-idf-svc
esp-idf-template
- note - we have to be a bit inventive here, as you cannot just "build" this crate. You need to generate a project from it first.Extra:
embedded-svc
esp-idf-*
crates, pulling it into the workspace would allow to easily implement changes that transcend both the esp-idf crates and this one. This crate can still be consumed independently from esp-hal (baremetal). nothing will be changed hereembuild
esp-idf-sys
hence the same issues apply here: if we changeembuild
in a backward incompatible way and want to use it inesp-idf-sys
, we have to patch the sys crate (and all downstream crates) so that they use the un-releasedembuild
.embuild
into a workspace setup is that it always compiles against the host target triple, not against the MCU triple. I'm unsure that - given that restriction - we can pull it into the workspace. Maybe, maybe not. We need to experiment withforce-target
in Cargo to see how that would work (if at all)What is NOT solved by this proposal?
Ability to do builds against different versions of
sdkconfig.defaults
. I.e. a BT example might need onesdkconfig.defaults
, while an Ethernet one - another.Currently, we "stuff" a single
sdkconfig.defaults
with all settings for all examples, and it works.sdkconfig.defaults
incompatible with a monorepo + workspace build? I hope not, but it is only a gut feeling. If you have examples where it would be incompatible - show themMigration strategy
The biggest pain-point and biggest risk is the GIT migration itself.
The baremetal team migrated to a single repo by just generating a bunch of patches (patch-per-commit) for all repos which were migrated to the "monorepo" one. And then one of the repos was chosen as the "monorepo" one.
Everything else (implementing the
Cargo.toml
of the workspace and so on) can be done post-factum, and I don't see major hurdles there.Possible migration steps:
esp-idf-svc
as the future monorepoesp-idf-svc
's root into a sub-folder namedesp-idf-svc
to leave room for the other crates to join the monorepomaster
of each of those repos - a bunch of patches in a directorygit apply
all these commits on the `esp-idf-svc repoesp-idf-svc
directory. See how exactly that would workesp-idf-svc
branchCargo.toml
workspace fileexamples/
folderesp-idf-svc
esp-idf-svc
GIT repo to justesp-idf
Done!
The text was updated successfully, but these errors were encountered: