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

New monorepo repository #180

Open
JonathanWoollett-Light opened this issue Mar 6, 2024 · 4 comments
Open

New monorepo repository #180

JonathanWoollett-Light opened this issue Mar 6, 2024 · 4 comments
Assignees

Comments

@JonathanWoollett-Light
Copy link
Contributor

JonathanWoollett-Light commented Mar 6, 2024

Crate Name

monorepo

Short Description

A monorepo to contain most rust-vmm code.

Why is this crate relevant to the rust-vmm project?

A monorepo design was proposed to assist in resolving/mitigating current issues regarding updates across multiple crates and maintaining consistency across multiple repositories (e.g. #177), this has been discussed in the sync1 on multiple occasions and consensus has been reached from attendants this is a useful change.

Todo

Task Assignee Status Result
Create example demonstrating layout and functionality @JonathanWoollett-Light Done https://github.com/JonathanWoollett-Light/monorepo
Create example maintaining the git history. @JonathanWoollett-Light Done https://github.com/JonathanWoollett-Light/rustvmm-monorepo
CI changes Todo
Create initial monorepo within rust-vmm with vm-superio-ser, vm-superio and vmm-sys-util Todo
Review draft monorepo within rust-vmm Todo
Archive vm-superio-ser, vm-superio and vmm-sys-util giving link to new monorepo location Todo

Footnotes

  1. The rust-vmm sync meeting is 16:00 (London, UK) Monday every other week (the next is 2024/02/05). Meeting link: https://meetpad.opendev.org/rust-vmm-sync. Meeting notes: https://meetpad.opendev.org/etherpad/p/rust-vmm-sync.

@JonathanWoollett-Light JonathanWoollett-Light self-assigned this Mar 6, 2024
@JonathanWoollett-Light JonathanWoollett-Light changed the title New monorepo repository. New monorepo repository Mar 6, 2024
@epilys
Copy link
Member

epilys commented Oct 14, 2024

(Disclaimer: wasn't part of the community call discussions about this, sorry for only voicing this now)

I'm not strongly opposed to this, this is just my two cents: It seems like we're solving a problem with nail + hammer instead of something milder e.g. glue.

A problem with updates in core crates is that if an interface changes, the PR would break everything that depends on it, potentially stalling merging until everything else is ported to the new update. This essentially makes a PR dependent on a contributor plus the affected crate maintainers, instead of breaking it down to two PRs, the initial update and the porting.

I think we could also do something less drastic such as adding a github action to automatically create PRs and merge them if they don't have conflicts. It can be triggered by a web hook or run on a schedule.

@TimePrinciple
Copy link

(Disclaimer: wasn't part of the community call discussions about this, sorry for only voicing this now)

I'm not strongly opposed to this, this is just my two cents: It seems like we're solving a problem with nail + hammer instead of something milder e.g. glue.

A problem with updates in core crates is that if an interface changes, the PR would break everything that depends on it, potentially stalling merging until everything else is ported to the new update. This essentially makes a PR dependent on a contributor plus the affected crate maintainers, instead of breaking it down to two PRs, the initial update and the porting.

I think we could also do something less drastic such as adding a github action to automatically create PRs and merge them if they don't have conflicts. It can be triggered by a web hook or run on a schedule.

I see your point, this indeed brings more job for making a change.

Say if we have a monorepo which incorporates all rust-vmm crates:
kvm-bindings and kvm-ioctls are in this monorepo, and kvm-bindings depends on kvm-ioctls. And at this time, someone make a change in kvm-bindings which breaks the build of kvm-ioctls and other crates depends on kvm-ioctls, that PR couldn't be merged until it has changes needed in other crates as well. But once that work is done and merged, the whole rust-vmm crates are in an consistent state whenever it releases its crates. This is the monorepo approach.

If we stay on the approach of having separate crates, a breaking change merged in kvm-bindings need to go through all crates it affecting. During this process, only us know if we want a working rust-vmm system, we need to stick to the version specified in kvm-ioctls (the crates immediately depends on kvm-bindings), but not ordinary users. They might just cargo add kvm-bindings which gives them the latest available and developed something with it, then cargo add kvm-ioctls which will eventually break their build, and figure it out.

By having a monorepo, we can bring all maintainers and contributors in rust-vmm together, give every issue/pr more exposure to everyone, which might increase the possibility it gets solved and speed it up. There are many monorepos out there, like firecracker, cloud-hypervisor, kata-containers, nydus and etc., and I think we can also have one :)

I appreciate @JonathanWoollett-Light 's effort of initiating this :) And I will help maintainers doing this, if rust-vmm community decides to have one.

@epilys
Copy link
Member

epilys commented Oct 15, 2024

Devil's advocate: a monorepo doesn't mean you publish/update all crates at once necessarily. It would make it more difficult to add breaking updates if all crates must be "synced" before merging because every change must be either universal or not. We all work async from different timezones and frankly, there's no burning need to update for example a vm-memory dependency for every crate at once unless there's a bug fix.

Users selecting incompatible versions in their dependencies is not an upstream problem, too. Also a downstream user mis-configuring their Cargo.toml does not seem a good enough reason for us to make changes.

I'm not generally convinced this is needed, all in all (but I am not vetoing or anything; just writing down my thoughts!).

@stefano-garzarella
Copy link
Member

(Disclaimer: wasn't part of the community call discussions about this, sorry for only voicing this now)

I'm not strongly opposed to this, this is just my two cents: It seems like we're solving a problem with nail + hammer instead of something milder e.g. glue.

I agree in part, in fact yesterday in the call we decided that for now maybe it only makes sense to group similar things together, like for example kvm-ioctls and kvm-bindings or vm-virtio and vhost.

What we'd like to avoid is that for example we release a new version of a crate. Then we find that by updating some crate that uses it, the version introduces some regression or some wrong API, and so we need to redo a new release.
Having everything together instead should make us discover these problems a little earlier, but I understand that in theory testing should prevent this.

I think we could also do something less drastic such as adding a github action to automatically create PRs and merge them if they don't have conflicts. It can be triggered by a web hook or run on a schedule.

We already do that in part with dependabot, right?

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

No branches or pull requests

4 participants