ayourtch / vpp-api-gen

Parse VPP API .api.json files and generate low-level Rust binding package based on that. For an example, see https://github.com/ayourtch/latest-vpp-api
2 stars 2 forks source link

merge related vpp-api-gen crates into workspace repo #25

Open pepinns opened 2 years ago

pepinns commented 2 years ago

How do you feel about merging all these dependent repos into 1 and using cargo workspaces to build them? https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html

You can still reference individual crates at different versions if you want, but in practice you need all these at the same version almost always, and it makes submitting cross-crate PRs a 1 step process for testing and reviewing.

You should be able to use a git subtree to merge the existing repos and save commit history as well. Happy to take a stab at it in this or a fresh repo if you agree.

Example is tokio. https://github.com/tokio-rs/tokio/blob/master/Cargo.toml All of the crates are just sub-directories of the tokio-rs repo.

ayourtch commented 2 years ago

Hi Jim,

yeah this is an interesting idea, my only uncertainty is with the generated code… see e.g. “latest-vpp-api” repo which is rebuilt on a daily basis and aims to be tracking the latest changes: https://github.com/ayourtch/latest-vpp-api

It refers to individual crates via their git urls - will it be possible to retain that ? (I assume just specifying path one level deeper will work?)

My idea was that one can have several versions of the api bindings done against different VPP versions and potentially use them in parallel… (via a higher level abstraction crate that is TBD)

Will that still work ?

If yes - I have made early on a “vpp-api” placeholder repo which we could use as a place to consolidate the other ones.

pepinns commented 2 years ago

Yes. All that still works.

Example of using the tracing lib with multiple crates in one repo:

tracing = { git = "https://github.com/tokio-rs/tracing.git", branch = "master" 
tracing-subscriber = { git = "https://github.com/tokio-rs/tracing.git", branch = "master" }
tracing-tower = { git = "https://github.com/tokio-rs/tracing.git", branch = "master"}
tracing-core = { git = "https://github.com/tokio-rs/tracing.git", branch = "master" }

cargo is smart enough to know to look for tracing-tower at tracing.git/tracing-tower, at the branch you specify.

latest-vpp-api seems like it should still be separate as that tracks the VPP version, and not really the version of these crates. There is no real coupling between the 'master' version of vpp-api-gen, and the 'master' version of latest-vpp-api like there is between vpp-api-gen and vpp-api-transport for example.

The generated messages are ephemeral in the sense that they need to be re-generated when you update your version of the codegen software, or when you update your version of VPP. Putting it up on a crate is nice because it makes it easier for first-timers to get coding right away on the latest stuff, but anyone using this for a project is likely to generate their own messages from their version(s) of VPP, possibly even including those generated messages into their own project like they would with protobuf or openapi generated code. When they update these libs, they'll likely also re-generate the messages for their vetted VPP version.

Since its generated it doesn't have the same development workflow overhead either. ie. no PRs to review/test across multiple versions. Based on what I see with the github actions workflow, its basically hands off and just updates itself every day.

I think the main things you get are

And you can still pick/choose which versions you use separately if you want, but inside the workspace repo you can punt all that by referencing local paths like so: https://github.com/tokio-rs/tracing/blob/master/tracing-tower/Cargo.toml#L28 so all your crates are always tested against the one single version of the group, and they all move forward together.

Practically though I can't see anyone wanting to mix and match versions of these, unless they have custom changes to one that they don't want to push upstream for some reason.

RE: higher level api I think thats a great idea, and I see no reason that couldn't also live in the same workspace repo.

Would you like me to setup a PR then against vpp-api?

ayourtch commented 2 years ago

cool! so yeah i think this is precisely the kind of usage i thought for vpp-api repo for, so i will be happy for your PR! thanks ! :)

ayourtch commented 2 years ago

(and thank you so much for such a detailed comment btw - I am terse because roaming with iphone).

ayourtch commented 2 years ago

by the way - i tried a somewhat involved api use in https://github.com/ayourtch/vpp-bonjour and it feels like i made compiler quite unhappy…. so i think there’s a few more kinks that will need ironing out… and docs… i will probably take a first stab at it when we merge it all into the project repo…

pepinns commented 2 years ago

https://github.com/ayourtch/vpp-api/pull/1 The files changed is huge, but if you look at the most recent commits you'll see its just git subtree add --prefix vpp-api-encoding git@github.com:ayourtch/vpp-api-encoding.git main for each of the repos, and then modify the main Cargo.toml to include all the directories.

ayourtch commented 2 years ago

yeah looks reasonable! i merged it, thanks for taking care of it! i must say it does look even aesthetically pleasing :)