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

Merging kvm-ioctls and kvm-bindings GitHub repositories #291

Closed
roypat opened this issue Oct 23, 2024 · 7 comments · Fixed by #292
Closed

Merging kvm-ioctls and kvm-bindings GitHub repositories #291

roypat opened this issue Oct 23, 2024 · 7 comments · Fixed by #292

Comments

@roypat
Copy link
Collaborator

roypat commented Oct 23, 2024

Hey all,

at a recent community sync meeting, we ended up talking about rust-vmm/community#180 again, and how merging kvm-bindings and kvm-ioctls into a single repository (not crate, e.g. the repository would hold a cargo workspace with two members, kvm-ioctls and kvm-bindings) could be one of the simplifications we can make to our repository structure.

I found an old issue about doing exactly this that seemed to have turned stale (rust-vmm/community#98), so this seems to have been the plan already at some point in the past at least. Are folks still interested in doing this?

cc @rbradford tagging you explicitly since it's mostly been us commiting here recently :D

@TimePrinciple
Copy link
Contributor

TimePrinciple commented Oct 23, 2024

In my view that these two would be more natural and appropriate to be grouped closed instead of managed separately, at least like vfio-bindings and vfio-ioctls

I can help doing this :) And @stefano-garzarella had pointed me the way on how to merge different git repos

@rbradford
Copy link
Contributor

I don't have any objections to that - it seems to have worked out fine for vfio (and of course the virtio crates are all hosted in a single repository.)

@TimePrinciple
Copy link
Contributor

TimePrinciple commented Oct 28, 2024

I managed to merge kvm-bindings and kvm-ioctls into kvm repository, see https://github.com/TimePrinciple/kvm

It's done by the following command (not sure relative path would work, perhaps absolute path is needed here):

# kvm-bindings
git clone https://github.com/rust-vmm/kvm-bindings.git
pushd kvm-bindings
git-filter-repo --to-subdirectory-filter kvm-bindings
popd

# kvm-ioctls
git clone https://github.com/rust-vmm/kvm-ioctls.git
pushd kvm-ioctls
git-filter-repo --to-subdirectory-filter kvm-ioctls
popd

mkdir kvm && cd kvm
git remote add bindings ../kvm-bindings/.git
git fetch bindings main:bindings
git remote add ioctls ../kvm-ioctls/.git
git fetch ioctls main:ioctls
# create an initial commit
touch README.md
git add README.md
git commit -s -m "Initial commit"

git checkout bindings
git rebase main
git checkout ioctls
git rebase bindings
# now we treat ioctls as the "main" branch
git push origin ioctls:main

@epilys
Copy link
Member

epilys commented Oct 28, 2024

Made a draft "how it could look" PR here: rust-vmm/kvm-bindings#125

  1. We need to check whether both merged CIs get triggered on buildkite
  2. We need to decide which repository to sacrifice to move the new history into. Do we rename kvm-ioctls to kvm (by deleting rust-vmm/kvm first)? (It has more github stars :D -not being serious about that though). Do we rename kvm-bindings to kvm? Do we force push this merged history into the already existing kvm repo without deleting it first?

(2) is really minutiae, basically which repository we want to redirect to the new one (and keep the github stars from). IIRC Github supports redirecting URLs for renamed repos, so this would be useful for users of the crates.

@roypat
Copy link
Collaborator Author

roypat commented Oct 29, 2024

Made a draft "how it could look" PR here: rust-vmm/kvm-bindings#125

1. We need to check whether **both merged CIs get triggered** on buildkite

2. We need to decide **which repository to sacrifice** to move the new history into. Do we rename `kvm-ioctls` to `kvm` (by deleting `rust-vmm/kvm` first)? (It has more github stars :D -not being serious about that though). Do we rename `kvm-bindings` to `kvm`? Do we force push this merged history into the already existing `kvm` repo without deleting it first?

It should trigger them both by default, because our CI module passes --workspace to cargo test. I can't actually see any of the kvm-ioctls code when I navigate to https://github.com/epilys/kvm-bindings/tree/history-merge 🤔 I think by doing -s ours the merge might acutally have discarded all the kvm-ioctls stuff? I guess we'd need a commit on top of the base repository first that turns it into a workspace, then a commit on top of the to-be-merged repository that deletes all the duplicated files and moves the src directory and the Cargo files into a subdirectory, and then do the git merge (manually resolving all conflicts, which I hope would be very minimal at this point).

(2) is really minutiae, basically which repository we want to redirect to the new one (and keep the github stars from). IIRC Github supports redirecting URLs for renamed repos, so this would be useful for users of the crates.

Ah, I never really considered that we can just keep one of the repositories. I do agree it'd be nicer to just merge everything into kvm-ioctls to keep stars, watchers, or whatever other bookmarks people might have!

@epilys
Copy link
Member

epilys commented Oct 29, 2024

I think it's me screwing up the merge commit like you say. Will reroll a new try...

@epilys
Copy link
Member

epilys commented Oct 29, 2024

Pushed a new merge attempt, CI reports some errors on the ioctls tests: https://buildkite.com/rust-vmm/kvm-bindings-ci/builds/274#0192d7f5-66a6-4904-beea-edf43fe47cd3 I assume kvm-bindings repo is not privileged while kvm-ioctls repo is?

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 a pull request may close this issue.

4 participants