On 6/19/24 07:34, Richard Henderson wrote:
First silly question: how much of this is boiler plate that gets moved
the moment that the second rust subdirectory is added?
If my suggestion at
https://lore.kernel.org/qemu-devel/CABgObfaP7DRD8dbSKNmUzhZNyxeHWO0MztaW3_EFYt=vf6s...@mail.gmail.com/
works, we'd have only two directories that have a Cargo.toml in it. For
example it could be rust/qemu/ (common code) and rust/hw/ (code that
depends on Kconfig).
I think we can also have a rust/Cargo.toml file as in
https://doc.rust-lang.org/cargo/reference/workspaces.html#virtual-workspace,
and then the various configuration files under rust/ will be valid for
all subpackages.
+[build]
+rustflags = ["-Crelocation-model=pic", "-Ctarget-feature=+crt-static"]
It seems certain that this is not specific to pl011, and will be commot to other rust
subdirectories. Or, given the .cargo directory name, is this generated by cargo and
committed by mistake?
-Crelocation-mode should be pie. But also, I am not sure it works
because I think it's always going to be overridden by cargo_wrapper.py?
See https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags.
(I'm not sure what +crt-static is for).
I think the generate_cfg_flags() mechanism of cargo_wrapper.py has to be
rewritten from Python to Rust and moved to build.rs (using
cargo::rustc-cfg). By doing this, the cfg flags are added to whatever
is in .cargo/config.toml, rather than replaced.
diff --git a/rust/pl011/.gitignore b/rust/pl011/.gitignore
new file mode 100644
index 0000000000..28a02c847f
--- /dev/null
+++ b/rust/pl011/.gitignore
@@ -0,0 +1,2 @@
+target
+src/generated.rs.inc
Is this a symptom of generating files into the source directory and not
build directory?
If I understand correctly, Manos considered two possible ways to invoke
cargo on the Rust code:
- directly, in which case you need to copy the generated source file to
rust/pl011/src/generated.rs.inc, because cargo does not know where the
build tree
- do everything through meson, which does the right thing because
cargo_wrapper.py knows about the build tree and passes the information.
To avoid this, the first workflow could be adjusted so that cargo can
still be invoked directly, _but from the build tree_, not the source
tree. For example configure could generate a
.../build/.cargo/config.toml with
[env]
MESON_BUILD_ROOT = ".../build"
(extra advantage: -Crelocation-model=pie can be added based on
--enable-pie/--disable-pie). configure can also create symlinks in the
build tree for the source tree's rust/, Cargo.toml and Cargo.lock.
This allows rust/pl011/src/generated.rs (which probably will become
something like rust/common/src/generated.rs) to be:
include!(concat!(env!("MESON_BUILD_ROOT"), "/generated.rs.inc"));
when cargo is invoked from the build tree.
Putting everything together you'd have
build/
rust/
.cargo/
config.toml # generated by configure or meson.build
Cargo.toml # workspace generated by configure or meson.build
Cargo.lock # can be either linked to srctree or generated
qemu/ # symlink to srctree/rust/qemu
aarch64-softmmu-hw/
Cargo.toml # generated by meson.build (*)
src/ # symlink to srctree/rust/hw/
i386-softmmu-hw/
Cargo.toml # generated by meson.build
src/ # symlink to srctree/rust/hw/
generated/ # files generated by rust/generated/meson.build
(*) these have to be generated to change the package name, so
configure_file() seems like a good match for it.
This is suspiciously similar to what tests/tcg/ looks like, except that
tests/tcg/*/Makefile is just a symbolic link. I tried creating a
similar directory structure in a toy project, and it seemed to work...
Second silly question: does this really need to be committed to the
repository? It *appears* to be specific to the host+os-version of the
build. It is certainly very specific about versions and checksums...
Generally the idea is that libraries should not commit it, while
applications should commit it. The idea is that the Cargo.lock file
reproduces a working configuration, and dependencies are updated to
known-good releases when CI passes. I don't think I like this, but it
is what it is. I ascribe it to me being from the Jurassic.
But for now I would consider not committing Cargo.lock, because we don't
have the infrastructure to do that automatic dependency update (assuming
we want it). But we could generate it at release time so that it is
included in tarballs, and create the symlink from
srctree/rust/Cargo.lock into build/rust/ only if the file is present in
the source tree.
diff --git a/rust/pl011/Cargo.toml b/rust/pl011/Cargo.toml
[...]
+# bilge deps included here to include them with docs
+[dependencies]
+arbitrary-int = { version = "1.2.7" }
+bilge = { version = "0.2.0" }
+bilge-impl = { version = "0.2.0" }
This one has to be included, but it is less restrictive than it seems.
It is equivalent to
arbitrary-int = { version = ">=1.2.7, <2.0.0" }
bilge = { version = ">=0.2.0, <0.3.0" }
bilge-impl = { version = ">=0.2.0, <0.3.0" }
That is, it assumes an API breakage when the first nonzero component
changes in the version. It is also possible to put a caret in front of
the version, so that it's clearer that this is a range; but the behavior
is the same.
Paolo
