Message ID | 20210907121943.3498701-14-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | Rust binding for QAPI and qemu-ga QMP handler examples | expand |
marcandre.lureau@redhat.com writes ("[RFC v3 13/32] rust: use vendored-sources"): > Most likely, QEMU will want tighter control over the sources, rather > than relying on crates.io downloading, use a git submodule with all the > dependencies. However, cargo --offline was added in 1.36. Hi. pm215 pointed me at this, as I have some background in Rust. I definitely approve of having Rust in Qemu. I don't have an opinion about whether the sources should be vendored this way. But, I tried to build this, and error: failed to select a version for the requirement `cc = "=1.0.70"` candidate versions found which didn't match: 1.0.69 location searched: directory source `/volatile/rustcargo/Rustup/Qemu/qemu.pwt/rust/vendored` (which is replacing registry `crates-io`) required by package `nix v0.20.1` ... which is depended on by `qga v0.1.0 (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/qga)` perhaps a crate was updated and forgotten to be re-vendored? As a reminder, you're using offline mode (--offline) which can sometimes cause surprising resolution failures, if this error is too confusing you may wish to retry without the offline flag. I think the most important part here is to get the general APIs, presented to general Rust code in Qemu, right. So I wanted to review those via the output from rustdoc. I tried commenting out the `replace-with` in .cargo/config.toml but evidently the systme isn't intended to be used that way. Ian.
Hi On Wed, Sep 8, 2021 at 7:40 PM Ian Jackson <iwj@xenproject.org> wrote: > marcandre.lureau@redhat.com writes ("[RFC v3 13/32] rust: use > vendored-sources"): > > Most likely, QEMU will want tighter control over the sources, rather > > than relying on crates.io downloading, use a git submodule with all the > > dependencies. However, cargo --offline was added in 1.36. > > Hi. > > pm215 pointed me at this, as I have some background in Rust. > I definitely approve of having Rust in Qemu. I don't have an opinion > about whether the sources should be vendored this way. > > But, I tried to build this, and > > error: failed to select a version for the requirement `cc = "=1.0.70"` > candidate versions found which didn't match: 1.0.69 > location searched: directory source > `/volatile/rustcargo/Rustup/Qemu/qemu.pwt/rust/vendored` (which is > replacing registry `crates-io`) > required by package `nix v0.20.1` > ... which is depended on by `qga v0.1.0 > (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/qga)` > perhaps a crate was updated and forgotten to be re-vendored? > As a reminder, you're using offline mode (--offline) which can > sometimes cause surprising resolution failures, if this error is too > confusing you may wish to retry without the offline flag. > > I think the most important part here is to get the general APIs, > presented to general Rust code in Qemu, right. So I wanted to review > those via the output from rustdoc. > > I tried commenting out the `replace-with` in .cargo/config.toml > but evidently the systme isn't intended to be used that way. > > Ian. > > Hmm, I do "cargo vendor --versioned-dirs ../rust/vendored" to vendor crates. It seems cc was updated, and I didn't update the submodule accordingly. For reference, this is the dependency tree that WFM: $ cargo tree -p qga qga v0.1.0 (/home/elmarco/src/qemu/qga) ├── common v0.1.0 (/home/elmarco/src/qemu/rust/common) │ ├── libc v0.2.101 │ └── nix v0.20.1 │ ├── bitflags v1.2.1 │ ├── cfg-if v1.0.0 │ ├── libc v0.2.101 │ └── memoffset v0.6.4 │ [build-dependencies] │ └── autocfg v1.0.1 ├── hostname v0.3.1 │ ├── libc v0.2.101 │ └── match_cfg v0.1.0 └── nix v0.20.1 (*)
Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > Hmm, I do "cargo vendor --versioned-dirs ../rust/vendored" to vendor crates. > > It seems cc was updated, and I didn't update the submodule accordingly. For > reference, this is the dependency tree that WFM: git submodules are just awful IMO. > $ cargo tree -p qga > qga v0.1.0 (/home/elmarco/src/qemu/qga) > ├── common v0.1.0 (/home/elmarco/src/qemu/rust/common) > │ ├── libc v0.2.101 > │ └── nix v0.20.1 > │ ├── bitflags v1.2.1 > │ ├── cfg-if v1.0.0 > │ ├── libc v0.2.101 > │ └── memoffset v0.6.4 > │ [build-dependencies] > │ └── autocfg v1.0.1 > ├── hostname v0.3.1 > │ ├── libc v0.2.101 > │ └── match_cfg v0.1.0 > └── nix v0.20.1 (*) With the .config/cargo.toml "replace-with" commented out, I see this: rustcargo@zealot:~/Rustup/Qemu/qemu.pwt/build$ cargo tree -p qga qga v0.1.0 (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/qga) ├── common v0.1.0 (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/rust/common) │ ├── libc v0.2.101 │ └── nix v0.20.1 │ ├── bitflags v1.2.1 │ ├── cfg-if v1.0.0 │ ├── libc v0.2.101 │ └── memoffset v0.6.4 │ [build-dependencies] │ └── autocfg v1.0.1 ├── hostname v0.3.1 │ ├── libc v0.2.101 │ └── match_cfg v0.1.0 └── nix v0.20.1 (*) rustcargo@zealot:~/Rustup/Qemu/qemu.pwt/build$ Which is the same as yours. Although "cargo build" doesn't work build, guessed from the messagese that perhaps this was the automatic codegen hadn't run. I'm now trying "make" and and it seems to be running. With the "replace-with" uncommented, cargo tree bombs out. I'm afraid I haven't used cargo vendor so I'm not sure if I am going in the right direction with this workaround. Hopefully it will finish the build. Would it be possible to have a configure option to use unvendored upstream Rust libraries from crates.io ? Ian.
Hi On Wed, Sep 8, 2021 at 7:55 PM Ian Jackson <iwj@xenproject.org> wrote: > Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > > Hmm, I do "cargo vendor --versioned-dirs ../rust/vendored" to vendor > crates. > > > > It seems cc was updated, and I didn't update the submodule accordingly. > For > > reference, this is the dependency tree that WFM: > > git submodules are just awful IMO. > Yes, but it's often (always?) the user fault. CI should help, when it will check Rust code. > > $ cargo tree -p qga > > qga v0.1.0 (/home/elmarco/src/qemu/qga) > > ├── common v0.1.0 (/home/elmarco/src/qemu/rust/common) > > │ ├── libc v0.2.101 > > │ └── nix v0.20.1 > > │ ├── bitflags v1.2.1 > > │ ├── cfg-if v1.0.0 > > │ ├── libc v0.2.101 > > │ └── memoffset v0.6.4 > > │ [build-dependencies] > > │ └── autocfg v1.0.1 > > ├── hostname v0.3.1 > > │ ├── libc v0.2.101 > > │ └── match_cfg v0.1.0 > > └── nix v0.20.1 (*) > > With the .config/cargo.toml "replace-with" commented out, I see this: > > rustcargo@zealot:~/Rustup/Qemu/qemu.pwt/build$ cargo tree -p qga > qga v0.1.0 (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/qga) > ├── common v0.1.0 (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/rust/common) > │ ├── libc v0.2.101 > │ └── nix v0.20.1 > │ ├── bitflags v1.2.1 > │ ├── cfg-if v1.0.0 > │ ├── libc v0.2.101 > │ └── memoffset v0.6.4 > │ [build-dependencies] > │ └── autocfg v1.0.1 > ├── hostname v0.3.1 > │ ├── libc v0.2.101 > │ └── match_cfg v0.1.0 > └── nix v0.20.1 (*) > rustcargo@zealot:~/Rustup/Qemu/qemu.pwt/build$ > > Which is the same as yours. Although "cargo build" doesn't work > build, guessed from the messagese that perhaps this was the automatic > codegen hadn't run. I'm now trying "make" and and it seems to be > running. > > With the "replace-with" uncommented, cargo tree bombs out. I'm afraid > I haven't used cargo vendor so I'm not sure if I am going in the right > direction with this workaround. Hopefully it will finish the build. > > Would it be possible to have a configure option to use unvendored > upstream Rust libraries from crates.io ? > Not easily, but we could have a --disable-rust-offline configure option. Whether this is desirable, I am not sure.
Hi On Wed, Sep 8, 2021 at 7:40 PM Ian Jackson <iwj@xenproject.org> wrote: > marcandre.lureau@redhat.com writes ("[RFC v3 13/32] rust: use > vendored-sources"): > > Most likely, QEMU will want tighter control over the sources, rather > > than relying on crates.io downloading, use a git submodule with all the > > dependencies. However, cargo --offline was added in 1.36. > > Hi. > > pm215 pointed me at this, as I have some background in Rust. > I definitely approve of having Rust in Qemu. I don't have an opinion > about whether the sources should be vendored this way. > > But, I tried to build this, and > > error: failed to select a version for the requirement `cc = "=1.0.70"` > candidate versions found which didn't match: 1.0.69 > location searched: directory source > `/volatile/rustcargo/Rustup/Qemu/qemu.pwt/rust/vendored` (which is > replacing registry `crates-io`) > required by package `nix v0.20.1` > ... which is depended on by `qga v0.1.0 > (/volatile/rustcargo/Rustup/Qemu/qemu.pwt/qga)` > perhaps a crate was updated and forgotten to be re-vendored? > As a reminder, you're using offline mode (--offline) which can > sometimes cause surprising resolution failures, if this error is too > confusing you may wish to retry without the offline flag. > > I think the most important part here is to get the general APIs, > presented to general Rust code in Qemu, right. So I wanted to review > those via the output from rustdoc. > You can start by reading `cargo doc -p common --open`. The generated code needs some environment variables set, so `cargo doc -p qga` will fail unless you set the environment variable MESON_BUILD_ROOT=`pwd` cargo doc -p qga --open --document-private-items works, but the QAPI types aren't documented, so this is a bit useless at this point. I wonder if I could put the schema doc, hmm... > I tried commenting out the `replace-with` in .cargo/config.toml > but evidently the systme isn't intended to be used that way. > > Ian. > >
On Wed, 8 Sept 2021 at 17:17, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > Hi > > On Wed, Sep 8, 2021 at 7:55 PM Ian Jackson <iwj@xenproject.org> wrote: >> >> Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): >> > Hmm, I do "cargo vendor --versioned-dirs ../rust/vendored" to vendor crates. >> > >> > It seems cc was updated, and I didn't update the submodule accordingly. For >> > reference, this is the dependency tree that WFM: >> >> git submodules are just awful IMO. > > > Yes, but it's often (always?) the user fault. I tend to agree with Ian -- submodules are badly designed, and have lots of sharp edges that it's easy to cut yourself on. Yes, you can say "well, the user should have held it by the other handle because that one isn't fitted with the spring-loaded razorblades", but I would argue that fault is better placed at the door of the designer in that kind of situation... -- PMM
Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > On Wed, Sep 8, 2021 at 7:55 PM Ian Jackson <iwj@xenproject.org> wrote: > > git submodules are just awful IMO. > > Yes, but it's often (always?) the user fault. I must disagree in the strongest possible terms. I don't think I can express my feelings on this in a way that would be appropriate in this context. Anyway... My trickery as described above (run configure, edit the "replace-with" out of .cargo/config.toml, run make) did produce a build. But to review the internal API I want the rustdoc output. How can I run rustdoc in a way that will work ? I tried "cargo doc" and it complained about a lack of "MESON_BUILD_ROOT". I guessed and ran MESON_BUILD_ROOT=$PWD cargo doc which seemed to produce some output and complete but I can't find the results anywhere. Can you please give me the set of runes to type view the rustdoc-built API documentation for the qemu-internal Rust APIs ? Thanks, Ian.
Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > You can start by reading `cargo doc -p common --open`. The generated > code needs some environment variables set, so `cargo doc -p qga` > will fail unless you set the environment variable > > MESON_BUILD_ROOT=`pwd` cargo doc -p qga --open --document-private-items Thanks. I did this (and your rune from bofere) and I have the docs open. I wasn't quite sure where to start. I didn't see where the entrypoints were. I did find .../target/doc/qga/qmp/fn.qmp_guest_set_vcpus.html which err, doesn't look like the kind of safe api I was hoping to find. Ian.
Hi On Wed, Sep 8, 2021 at 8:29 PM Ian Jackson <iwj@xenproject.org> wrote: > Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > > You can start by reading `cargo doc -p common --open`. The generated > > code needs some environment variables set, so `cargo doc -p qga` > > will fail unless you set the environment variable > > > > MESON_BUILD_ROOT=`pwd` cargo doc -p qga --open --document-private-items > > Thanks. I did this (and your rune from bofere) and I have the docs > open. > > I wasn't quite sure where to start. I didn't see where the > entrypoints were. I did find > > .../target/doc/qga/qmp/fn.qmp_guest_set_vcpus.html > > which err, doesn't look like the kind of safe api I was hoping to > find. > Yes, this is the shim to provide a C ABI QMP handler from Rust. This is where all the FFI<->Rust conversion takes place. The "safe" code is qga/qmp/vcpus.rs. However, there is no documentation there, since it's not meant to be the public interface. It's documented with the QAPI schema.
Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > Yes, this is the shim to provide a C ABI QMP handler from Rust. This is where > all the FFI<->Rust conversion takes place. > > The "safe" code is qga/qmp/vcpus.rs. However, there is no > documentation there, since it's not meant to be the public > interface. It's documented with the QAPI schema. Right, thanks. That does look like a PoC of a Rust API. I wanted the rustdoc output because I find it provides a very uniform and readable presentation even of an API with no doc comments. I think maybe a thing I am missing is how you expect this to be used. Which parts of the system are going to be in Rust. etc. And that would help explain what "public" means. I think the answer is probably in this example: https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/20210907121943.3498701-30-marcandre.lureau@redhat.com/ but although my C and Rust are both fine, I don't understand qemu well enough to make sense of it. ... wait, qga is "qemu guest agent" ? I think I am sort of seeing this use case now. But presuambly there are other use cases for this QMP/QAPI type bridge stuff. Sorry to be asking such stupid questions. Thanks, Ian.
Hi On Wed, Sep 8, 2021 at 8:51 PM Ian Jackson <iwj@xenproject.org> wrote: > Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > > Yes, this is the shim to provide a C ABI QMP handler from Rust. This is > where > > all the FFI<->Rust conversion takes place. > > > > The "safe" code is qga/qmp/vcpus.rs. However, there is no > > documentation there, since it's not meant to be the public > > interface. It's documented with the QAPI schema. > > Right, thanks. That does look like a PoC of a Rust API. I wanted the > rustdoc output because I find it provides a very uniform and readable > presentation even of an API with no doc comments. > > I think maybe a thing I am missing is how you expect this to be used. > Which parts of the system are going to be in Rust. etc. > And that would help explain what "public" means. > > I think the answer is probably in this example: > > > https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/20210907121943.3498701-30-marcandre.lureau@redhat.com/ > > but although my C and Rust are both fine, I don't understand qemu well > enough to make sense of it. > > ... wait, qga is "qemu guest agent" ? > > I think I am sort of seeing this use case now. But presuambly there > are other use cases for this QMP/QAPI type bridge stuff. > > Sorry to be asking such stupid questions. > There is no magic wand to introduce Rust code in an existing C code base. You need to glue some C ABI to/from Rust. It's a lot of manual work to properly bind a C API to Rust (it's a project on its own I would say). Typically, FFI bindings can be automated from headers, and high-level Rust bindings are done by hand. Then you want high-level bindings to take advantage of Rust, for idiomatic and safe code. Various internal QEMU API will have to be bound by hand to start using them from Rust. An isolated unit (say a parser, a function) could be rewritten in Rust and a C ABI be provided without much hassle. But in general, code is quickly interdependent, or the amount of stuff to rewrite in one go is large and risky to do it that way. In the glib/gobject world, the ABI are annotated, and you can automate much of the high-level binding process (which is amazing, given the complexity of the APIs, with objects, async methods, signals, properties, .. various concepts that don't match easily in Rust). To help with this process, they introduced conversion traits (the ToQemu/FromQemu adapted here), common interfaces, to help automate and compose complex types (and their own binding generator). (Unfortunately) QEMU doesn't use gobject, but it relies heavily on two type systems of its own: QAPI and QOM. QAPI is actually more of an IDL, which translates C from/to JSON/QMP and has commands and signals (and is the protocol used to communicate with qemu or qemu-ga etc). A large part of QEMU are direct users of the QAPI generated types and functions. It is thus a good target to generate bindings automatically. As demonstrated at the end, it allows writing QMP handlers in idiomatic Rust. Since qemu-guest-agent doesn't have a complex internal state (most commands are really independent, they could be different programs..!), I started rewriting some handlers there. It feels relatively straightforward to rewrite in Rust, and we could imagine a complete rewrite of qemu-ga... However, it is less of a waste to focus on critical parts or newly added code instead, imho! Furthermore, the Rust doesn't cover all targets C can currently target, so we must have some care when deciding a language. (you can imagine I would encourage anyone to do it in Rust!) QOM is the QEMU object system and is spread throughout the code. If there is enough interest for this Rust effort, I plan to look at binding it next (based on my experience working on GObject bindings). I am afraid we are not going to have as friendly bindings as what the GNOME team achieved, but since QOM is a bit simpler, we may find acceptable compromises. With QOM & QAPI, and manual bindings of internal APIs, I think we could start writing interesting code in Rust, simple devices, external interfaces etc. Hope that helps clarify a bit the current goals
On Tue, 7 Sept 2021 at 13:32, <marcandre.lureau@redhat.com> wrote: > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Most likely, QEMU will want tighter control over the sources, rather > than relying on crates.io downloading, use a git submodule with all the > dependencies. However, cargo --offline was added in 1.36. > > "cargo vendor" helps gathering and updating the dependencies. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > configure | 8 ++++++++ > meson.build | 7 ++++++- > .cargo/config.toml.in | 5 +++++ > .cargo/meson.build | 5 +++++ > .gitmodules | 4 ++++ > rust/vendored | 1 + > scripts/archive-source.sh | 2 +- > scripts/cargo_wrapper.py | 1 + > 8 files changed, 31 insertions(+), 2 deletions(-) > create mode 100644 .cargo/config.toml.in > create mode 100644 .cargo/meson.build > create mode 160000 rust/vendored So, this is a lot of extra code in a submodule. Historically we've found that submodules are a colossal pain, and so I think we should think about whether we really want to have all our rust dependencies in a submodule forever. I am definitely only at the beginner stage with Rust, but I think we should have a discussion about what the different alternative options are here, and what we want to achieve, so that we know why we're doing this and what we're gaining from the pain... For instance, could we instead commit Cargo.lock in git and use that to nail down specific versions of the dependencies ? FWIW, the "why submodules" for the C dependencies we ship like that is basically * C doesn't have a package manager, so if we need a dependency that distros don't ship then we need to wrap it up and provide it ourselves * where we ship binary blobs (guest BIOS etc) we want to also ship the source code for those blobs I think for Rust dependencies those don't really apply. Overall, I think that to the extent that we can look like a "normal" user of Rust, that's a good plan. Distros may well want to be able to do "build against our packaged rust stuff rather than downloading from crates.io" but I imagine they have machinery for that already; if we act like most other Rust programs we have better chances of not breaking that machinery. We do already effectively do "download code when QEMU is built" -- the makefile invokes scripts/git-submodule-update which pulls down submodule code. (Thanks to Ian for pointing out this framing of the question.) (I'm not personally a fan of the "download everything from crates.io" Rust ecosystem, but it is what it is, and wishing the Rust world worked more like a trad Linux-distro-provides-all-your-dependencies isn't, alas, going to make it so :-)) thanks -- PMM
Hi On Thu, Sep 9, 2021 at 8:04 PM Peter Maydell <peter.maydell@linaro.org> wrote: > On Tue, 7 Sept 2021 at 13:32, <marcandre.lureau@redhat.com> wrote: > > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > Most likely, QEMU will want tighter control over the sources, rather > > than relying on crates.io downloading, use a git submodule with all the > > dependencies. However, cargo --offline was added in 1.36. > > > > "cargo vendor" helps gathering and updating the dependencies. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > configure | 8 ++++++++ > > meson.build | 7 ++++++- > > .cargo/config.toml.in | 5 +++++ > > .cargo/meson.build | 5 +++++ > > .gitmodules | 4 ++++ > > rust/vendored | 1 + > > scripts/archive-source.sh | 2 +- > > scripts/cargo_wrapper.py | 1 + > > 8 files changed, 31 insertions(+), 2 deletions(-) > > create mode 100644 .cargo/config.toml.in > > create mode 100644 .cargo/meson.build > > create mode 160000 rust/vendored > > So, this is a lot of extra code in a submodule. Historically we've > found that submodules are a colossal pain, and so I think we should > think about whether we really want to have all our rust dependencies > in a submodule forever. > > I am definitely only at the beginner stage with Rust, but I think > we should have a discussion about what the different alternative > options are here, and what we want to achieve, so that we know > why we're doing this and what we're gaining from the pain... > > For instance, could we instead commit Cargo.lock in git and > use that to nail down specific versions of the dependencies ? > > Yes, that's the main reason this file exists I think. FWIW, the "why submodules" for the C dependencies we ship > like that is basically > * C doesn't have a package manager, so if we need a dependency that > distros don't ship then we need to wrap it up and provide it ourselves > Have we considered meson wrap? I never really looked at it in detail, not sure if that would work for us. https://mesonbuild.com/Wrap-dependency-system-manual.html * where we ship binary blobs (guest BIOS etc) we want to also ship > the source code for those blobs > I think for Rust dependencies those don't really apply. > And we mirror all those dependencies at the same location. > Overall, I think that to the extent that we can look like a "normal" > user of Rust, that's a good plan. Distros may well want to be able > to do "build against our packaged rust stuff rather than downloading > from crates.io" but I imagine they have machinery for that already; > if we act like most other Rust programs we have better chances of > not breaking that machinery. > True, at least on Fedora, there is machinery to package "regular" Rust programs/crates in an automated way. Vendoring dependencies should work equally, but may not conform with distro policies, so they have extra work eventually (it seems vendoring is more and more common though, with go projects for example) > We do already effectively do "download code when QEMU is built" -- > the makefile invokes scripts/git-submodule-update which pulls > down submodule code. (Thanks to Ian for pointing out this framing > of the question.) > > (I'm not personally a fan of the "download everything from crates.io" > Rust ecosystem, but it is what it is, and wishing the Rust world > worked more like a trad Linux-distro-provides-all-your-dependencies > isn't, alas, going to make it so :-)) > > A nice alternative to vendoring that could work well for QEMU is to mirror the Rust crate we use, so we have similar control and guarantee over them as submodules, and use `[patch.crates-io]` to point at qemu-project locations.
On Thu, Sep 09, 2021 at 05:02:01PM +0100, Peter Maydell wrote: > On Tue, 7 Sept 2021 at 13:32, <marcandre.lureau@redhat.com> wrote: > > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > Most likely, QEMU will want tighter control over the sources, rather > > than relying on crates.io downloading, use a git submodule with all the > > dependencies. However, cargo --offline was added in 1.36. > > > > "cargo vendor" helps gathering and updating the dependencies. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > configure | 8 ++++++++ > > meson.build | 7 ++++++- > > .cargo/config.toml.in | 5 +++++ > > .cargo/meson.build | 5 +++++ > > .gitmodules | 4 ++++ > > rust/vendored | 1 + > > scripts/archive-source.sh | 2 +- > > scripts/cargo_wrapper.py | 1 + > > 8 files changed, 31 insertions(+), 2 deletions(-) > > create mode 100644 .cargo/config.toml.in > > create mode 100644 .cargo/meson.build > > create mode 160000 rust/vendored > > So, this is a lot of extra code in a submodule. Historically we've > found that submodules are a colossal pain, and so I think we should > think about whether we really want to have all our rust dependencies > in a submodule forever. > > I am definitely only at the beginner stage with Rust, but I think > we should have a discussion about what the different alternative > options are here, and what we want to achieve, so that we know > why we're doing this and what we're gaining from the pain... > > For instance, could we instead commit Cargo.lock in git and > use that to nail down specific versions of the dependencies ? > > FWIW, the "why submodules" for the C dependencies we ship > like that is basically > * C doesn't have a package manager, so if we need a dependency that > distros don't ship then we need to wrap it up and provide it ourselves > * where we ship binary blobs (guest BIOS etc) we want to also ship > the source code for those blobs > I think for Rust dependencies those don't really apply. Even for our existing non-rust usage of submodules, it is very borderline questionable whether the benefit outweighs the pain they are causing us. I frequently wish we would just go "cold turkey" and drop all our submodules and spin off all the bundled blobs into separate downloads, despite the disruption it would cause in the short term. > Overall, I think that to the extent that we can look like a "normal" > user of Rust, that's a good plan. Distros may well want to be able > to do "build against our packaged rust stuff rather than downloading > from crates.io" but I imagine they have machinery for that already; > if we act like most other Rust programs we have better chances of > not breaking that machinery. Yes, distros do have machinery for this, although it is often hard to fit in with it when you have a mixed language project. Their machinery typically assumes pure single language project, so would work nicer if any QEMU rust pieces were separately released from the rest of QEMU. Obviously this is easier said than done since QEMU tends towards a monolothic repo approach historically. > We do already effectively do "download code when QEMU is built" -- > the makefile invokes scripts/git-submodule-update which pulls > down submodule code. (Thanks to Ian for pointing out this framing > of the question.) > > (I'm not personally a fan of the "download everything from crates.io" > Rust ecosystem, but it is what it is, and wishing the Rust world > worked more like a trad Linux-distro-provides-all-your-dependencies > isn't, alas, going to make it so :-)) Yes, I'm inclined to agree here. For better or worse the battle is over and "download everything from <repo> on the fly" is the accepted approach for pretty much all modern languages. The language specific repo essentially is the OS distro from their POV. Regards, Daniel
On Thu, Sep 09, 2021 at 08:29:58PM +0400, Marc-André Lureau wrote: > Hi > > On Thu, Sep 9, 2021 at 8:04 PM Peter Maydell <peter.maydell@linaro.org> > wrote: > > > On Tue, 7 Sept 2021 at 13:32, <marcandre.lureau@redhat.com> wrote: > > > > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > > > Most likely, QEMU will want tighter control over the sources, rather > > > than relying on crates.io downloading, use a git submodule with all the > > > dependencies. However, cargo --offline was added in 1.36. > > > > > > "cargo vendor" helps gathering and updating the dependencies. > > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > --- > > > configure | 8 ++++++++ > > > meson.build | 7 ++++++- > > > .cargo/config.toml.in | 5 +++++ > > > .cargo/meson.build | 5 +++++ > > > .gitmodules | 4 ++++ > > > rust/vendored | 1 + > > > scripts/archive-source.sh | 2 +- > > > scripts/cargo_wrapper.py | 1 + > > > 8 files changed, 31 insertions(+), 2 deletions(-) > > > create mode 100644 .cargo/config.toml.in > > > create mode 100644 .cargo/meson.build > > > create mode 160000 rust/vendored > > Overall, I think that to the extent that we can look like a "normal" > > user of Rust, that's a good plan. Distros may well want to be able > > to do "build against our packaged rust stuff rather than downloading > > from crates.io" but I imagine they have machinery for that already; > > if we act like most other Rust programs we have better chances of > > not breaking that machinery. > > > > True, at least on Fedora, there is machinery to package "regular" Rust > programs/crates in an automated way. Vendoring dependencies should work > equally, but may not conform with distro policies, so they have extra work > eventually (it seems vendoring is more and more common though, with go > projects for example) I wouldn't assume that we're going to be able to use that RPM support for rust, if we bundle our rust code inside the QEMU tarball and hidden behind meson. It generally only works well in single language projects using the preferred build tool exclusively (Cargo in this case). > > (I'm not personally a fan of the "download everything from crates.io" > > Rust ecosystem, but it is what it is, and wishing the Rust world > > worked more like a trad Linux-distro-provides-all-your-dependencies > > isn't, alas, going to make it so :-)) > > > > > A nice alternative to vendoring that could work well for QEMU is to mirror > the Rust crate we use, so we have similar control and guarantee over them > as submodules, and use `[patch.crates-io]` to point at qemu-project > locations. The Cargo metadata specifies the versions we'll get so we have full control over what deps are pulled in. All the mirroring our do is to cope with occassions where the main crate download website is inaccessible for some reason. I'm not convinced that's enough to justify creating extra work for ourselves through mirroring. Regards, Daniel
Daniel P. Berrangé writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > Yes, distros do have machinery for this, although it is often > hard to fit in with it when you have a mixed language project. > Their machinery typically assumes pure single language project, > so would work nicer if any QEMU rust pieces were separately > released from the rest of QEMU. Obviously this is easier said > than done since QEMU tends towards a monolothic repo approach > historically. Right. However, for a project that has Rust dependencies, the distros will (or will soon need) machinery to divert the langage-specific-package-manager downloads to their own repo. For example, the Debian Rust team provide a .cargo/config.toml to replace crates.io with the local Debian Rust packages, which the Debian package management system has provided via the (translated) build-dependencies. Debian certainly wouldn't want to use any vendored crates bundled with Qemu. Indeed Debian people hate vendoring more than they hate language specific package managers. At least with the LSPM you can usually nobble it in one place - ie many of the problems can be solved automatically. Vendoring typically involves playing whack-a-mole with compatibility problems, actually-modified versions, etc. - much human work (and quite annoying work too!) (Of course this is less of an issue if you don't actually modify the vendored code, but anyone who knows Rust and sees a vendored Rust crate will think it's been modified.) > > (I'm not personally a fan of the "download everything from crates.io" > > Rust ecosystem, but it is what it is, and wishing the Rust world > > worked more like a trad Linux-distro-provides-all-your-dependencies > > isn't, alas, going to make it so :-)) > > Yes, I'm inclined to agree here. For better or worse the battle is > over and "download everything from <repo> on the fly" is the accepted > approach for pretty much all modern languages. The language specific > repo essentially is the OS distro from their POV. To be honest, I am of the same mind as you about cargo and crates.io (and things like it) But I think the ship has sailed. At least, committing the Cargo.lock file will ensure that the same versions of the dependencies are used - at least by people who don't know anything much about Rust. Marc-André Lureau writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > A nice alternative to vendoring that could work well for QEMU is to > mirror the Rust crate we use, so we have similar control and > guarantee over them as submodules, and use `[patch.crates-io]` to > point at qemu-project locations. This is a very reasonable suggestion. In my experience crates.io is very reliable - but (as a test system onwer myself) I know how much you want to reduce the number of different sites whose upness your CI depends on, no matter how good their communities think they are :-). I think there should be a documented config option to disable this. People who know enough Rust to run `cargo update` etc. will need it, and having them hand-hack the config files is not really desirable. Ian.
Daniel P. Berrangé writes ("Re: [RFC v3 13/32] rust: use vendored-sources"): > On Thu, Sep 09, 2021 at 08:29:58PM +0400, Marc-André Lureau wrote: > > True, at least on Fedora, there is machinery to package "regular" Rust > > programs/crates in an automated way. Vendoring dependencies should work > > equally, but may not conform with distro policies, so they have extra work > > eventually (it seems vendoring is more and more common though, with go > > projects for example) > > I wouldn't assume that we're going to be able to use that RPM support > for rust, if we bundle our rust code inside the QEMU tarball and hidden > behind meson. It generally only works well in single language projects > using the preferred build tool exclusively (Cargo in this case). I can't speak to RPM, but Debian's arrangements for avoiding crates.io downloads at packag-ebuild-time will work just fine even if it the cargo calls are buried inside meson. All that would be needed is a way to disable any source replacement done by qemu (see previous mail) so that Debian's source replacement takes effect. Ian.
On 09/09/21 18:29, Marc-André Lureau wrote: > > * C doesn't have a package manager, so if we need a dependency that > distros don't ship then we need to wrap it up and provide it > ourselves > > Have we considered meson wrap? I never really looked at it in detail, > not sure if that would work for us. > https://mesonbuild.com/Wrap-dependency-system-manual.html > <https://mesonbuild.com/Wrap-dependency-system-manual.html> Sure, it would be possible to use wrap with meson 0.56.x or newer (due to https://github.com/mesonbuild/meson/pull/7740). It would all be hidden behind configure/Makefile, which would invoke "meson subprojects download" in addition to "git submodule update". Paolo
diff --git a/configure b/configure index 470b90543f..82a94ab93b 100755 --- a/configure +++ b/configure @@ -2068,6 +2068,14 @@ if test -z "$ninja"; then fi fi +case "$with_rust" in + auto|enabled) + if test -e "${source_path}/.git"; then + git_submodules="${git_submodules} rust/vendored" + fi + ;; +esac + # Check that the C compiler works. Doing this here before testing # the host CPU ensures that we had a valid CC to autodetect the # $cpu var (and we should bail right here if that's not the case). diff --git a/meson.build b/meson.build index a21c70d77f..29d218d35a 100644 --- a/meson.build +++ b/meson.build @@ -108,7 +108,11 @@ endif bzip2 = find_program('bzip2', required: install_edk2_blobs) -cargo = find_program('cargo', required: get_option('with_rust')) +cargo = find_program('cargo', + required: get_option('with_rust'), + # require --offline support (1.36), but fixed (1.42) + version: '>= 1.42') + with_rust = cargo.found() cargo_wrapper = [ find_program('scripts/cargo_wrapper.py'), @@ -126,6 +130,7 @@ if with_rust error('cross-compiling, but no Rust target-triple defined.') endif endif + subdir('.cargo') endif config_host_data.set('CONFIG_WITH_RUST', with_rust) diff --git a/.cargo/config.toml.in b/.cargo/config.toml.in new file mode 100644 index 0000000000..d1531aa52a --- /dev/null +++ b/.cargo/config.toml.in @@ -0,0 +1,5 @@ +[source.crates-io] +replace-with = "vendored-sources" + +[source.vendored-sources] +directory = "@vendored_directory@" diff --git a/.cargo/meson.build b/.cargo/meson.build new file mode 100644 index 0000000000..4e7c296ab0 --- /dev/null +++ b/.cargo/meson.build @@ -0,0 +1,5 @@ +configure_file(output: 'config.toml', + input: 'config.toml.in', + configuration: { + 'vendored_directory': meson.source_root() / 'rust/vendored' + }) diff --git a/.gitmodules b/.gitmodules index 08b1b48a09..f767a4f386 100644 --- a/.gitmodules +++ b/.gitmodules @@ -64,3 +64,7 @@ [submodule "roms/vbootrom"] path = roms/vbootrom url = https://gitlab.com/qemu-project/vbootrom.git +[submodule "rust/vendored"] + path = rust/vendored + #url = https://gitlab.com/qemu-project/qemu-rust-vendored.git + url = https://github.com/elmarco/qemu-rust-vendored.git diff --git a/rust/vendored b/rust/vendored new file mode 160000 index 0000000000..7077bbbd11 --- /dev/null +++ b/rust/vendored @@ -0,0 +1 @@ +Subproject commit 7077bbbd11a67d60062a9483f996113a349a4ca1 diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh index c6169db69f..03afcee8b9 100755 --- a/scripts/archive-source.sh +++ b/scripts/archive-source.sh @@ -26,7 +26,7 @@ sub_file="${sub_tdir}/submodule.tar" # independent of what the developer currently has initialized # in their checkout, because the build environment is completely # different to the host OS. -submodules="dtc slirp meson ui/keycodemapdb" +submodules="dtc slirp meson ui/keycodemapdb rust/vendored" submodules="$submodules tests/fp/berkeley-softfloat-3 tests/fp/berkeley-testfloat-3" sub_deinit="" diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py index 75518e8c02..c07f51494b 100644 --- a/scripts/cargo_wrapper.py +++ b/scripts/cargo_wrapper.py @@ -78,6 +78,7 @@ def get_cargo_rustc( target_dir, "--manifest-path", manifest_path, + "--offline", ] cargo_cmd += cargo_rustc_args if args.target_triple: