https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Bug ID: 2124697 Summary: Review Request: libblkio - Block device I/O library Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: rjones@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-0.1-1.20220906gitc8f5ed9... Description: Block device I/O library Fedora Account System Username: rjones
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |stefanha@redhat.com Flags| |needinfo?(stefanha@redhat.c | |om)
--- Comment #1 from Richard W.M. Jones rjones@redhat.com --- This is not quite ready for review yet! I have some questions of the package author to be resolved first.
Stefan:
- Shouldn't this depend on liburing-devel? I added that as a BR, but I'm not sure if that is correct.
- Cargo doesn't build the *.so.* files, so I had to manually link them which does not seem ideal:
pushd %{buildroot}%{_libdir} ln -s libblkio.so libblkio.so.0 ln -s libblkio.so.0 libblkio.so.0.0 popd
- I suspect this will not actually build in Koji because it's going to try to pull in a whole load of cargo dependencies which are not listed as BRs. Will try a scratch build soon and post the results.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #2 from Richard W.M. Jones rjones@redhat.com --- As expected the build attempts to download stuff, and that doesn't end well:
https://koji.fedoraproject.org/koji/taskinfo?taskID=91712209
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Yanko Kaneti yaneti@declera.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |yaneti@declera.com
--- Comment #3 from Yanko Kaneti yaneti@declera.com --- librsvg2 is a C lib with parts rust
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #4 from Richard W.M. Jones rjones@redhat.com --- In librsvg2:
$ du -sh vendor/ 278M vendor/
After running 'cargo vendor' in libblkio sources:
$ du -sh vendor/ 123M vendor/
Wow.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Kalev Lember klember@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |klember@redhat.com
--- Comment #5 from Kalev Lember klember@redhat.com --- There are broadly two ways to build rust packages such as this:
1) Package all the rust crates that it needs in Fedora instead of downloading from the internet.
2) Use vendoring (another word for bundling in rust land) to include all the required rust crates in the tarball.
Right now we are using option 2 in librsvg2, but over the last few days I've been trying to get the missing rust crates that it requires (there are hundreds of crates required for building librsvg2, but less than 10 are missing in Fedora right now) packaged up in Fedora proper. It's actually surprising simple to package a crate: we have a generator (rust2rpm) that spits out a ready spec file to go and it's so good that it usually doesn't need any fixes at all afterwards.
So coming from the perspective of someone who has historically much preferred option 2, I think option 1 is actually very viable these days because it's so easy to package rust dependencies and because we already have a large amount of them pre-packaged.
There's a rust BuildRequires generator that makes all the dependency generation automatic. See %generate_buildrequires section (currently %if'd out) in librsvg2 spec file for details :)
https://src.fedoraproject.org/rpms/librsvg2/blob/rawhide/f/librsvg2.spec
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #6 from Richard W.M. Jones rjones@redhat.com --- The ones needed for libblkio are, with "*" meaning NOT present in Fedora, "?" meaning unsure:
autocfg bitflags cc ? cfg-if * concat-idents const-cstr * io-uring lazy_static libc * memfd memmap memoffset nix proc-macro2 quote ? syn ? unicode-xid virtio-bindings * winapi * winapi-i686-pc-windows-gnu * winapi-x86_64-pc-windows-gnu
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #7 from Richard W.M. Jones rjones@redhat.com --- https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/...
Looks like we should use cargo-c to fix the missing symlinks.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com
--- Comment #8 from Fabio Valentini decathorpe@gmail.com --- (In reply to Richard W.M. Jones from comment #6)
The ones needed for libblkio are, with "*" meaning NOT present in Fedora, "?" meaning unsure:
What does "unsure" mean in this context?
? cfg-if
https://src.fedoraproject.org/rpms/rust-cfg-if
- io-uring
I tried to package this a few months ago (for tokio-uring and actix-rt), but frequent breakage of io_uring kernel APIs made it very painful. I can try again to see if this has improved.
- memfd
Was packaged but retired: https://src.fedoraproject.org/rpms/rust-memfd
? syn
https://src.fedoraproject.org/rpms/rust-syn
? unicode-xid
https://src.fedoraproject.org/rpms/rust-unicode-xid
- winapi
- winapi-i686-pc-windows-gnu
- winapi-x86_64-pc-windows-gnu
These three *MUST NOT BE PACKAGED*, they are bindings for Windows APIs.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #9 from Richard W.M. Jones rjones@redhat.com ---
What does "unsure" mean in this context?
Just meant I had problems finding them initially in Fedora, but it turns out they are in fact all present. I've just uploaded my latest attempt at packaging (which still doesn't work) so hit Shift + Reload on the spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #10 from Fabio Valentini decathorpe@gmail.com --- I can package the three remaining missing crates, if that helps you (concat-idents, io-uring, memfd).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #11 from Richard W.M. Jones rjones@redhat.com --- This is a bit better:
https://koji.fedoraproject.org/koji/taskinfo?taskID=91736622
I'll chase upstream about the i686 failure (if we care about that).
This is done by "vendoring" the 6 missing dependencies:
cat >> Cargo.toml <<EOF [patch.crates-io] concat-idents = { path = 'vendor/concat-idents' } io-uring = { path = 'vendor/io-uring' } memfd = { path = 'vendor/memfd' } winapi = { path = 'vendor/winapi' } winapi-i686-pc-windows-gnu = { path = 'vendor/winapi-i686-pc-windows-gnu' } winapi-x86_64-pc-windows-gnu = { path = 'vendor/winapi-x86_64-pc-windows-gnu' } EOF %cargo_prep sed -e 's/--locked/--offline/' -i src/cargo-build.sh
Hopefully the winapi ones are not actually used, but they appear to be necessary. The other 3 should likely be added to Fedora.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #12 from Richard W.M. Jones rjones@redhat.com --- (In reply to Fabio Valentini from comment #10)
I can package the three remaining missing crates, if that helps you (concat-idents, io-uring, memfd).
Yes that would help. Cc me on the package reviews if you need a reviewer.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(stefanha@redhat.c | |om) |
--- Comment #13 from Stefan Hajnoczi stefanha@redhat.com --- Hi Rich, Thank you for this work. libblkio 1.0 has now been released: https://gitlab.com/libblkio/libblkio/-/tree/v1.0.0
The C API is now stable.
The licenses have been changed : 1. libblkio - MIT OR Apache-2.0 2. blkio - MIT OR Apache-2.0 3. virtio-driver - (MIT OR Apache-2.0) AND BSD-3-Clause
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #14 from Stefan Hajnoczi stefanha@redhat.com --- (In reply to Richard W.M. Jones from comment #1)
Stefan:
- Shouldn't this depend on liburing-devel? I added that as a BR, but I'm not sure if that is correct.
No. The io-uring crate directly invokes the io_uring system calls instead of going through the liburing library.
- Cargo doesn't build the *.so.* files, so I had to manually link them which does not seem ideal:
pushd %{buildroot}%{_libdir} ln -s libblkio.so libblkio.so.0 ln -s libblkio.so.0 libblkio.so.0.0 popd
I'm not sure about cargo's capabilities for sonames. libblkio currently doesn't use any special versioning configuration (e.g. symbol versioning, explicitly setting soname, etc).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #15 from Stefan Hajnoczi stefanha@redhat.com ---
I'll chase upstream about the i686 failure (if we care about that).
Alberto fixed the i686 build. libblkio 1.0.0 should build successfully now.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(stefanha@redhat.c | |om)
--- Comment #16 from Richard W.M. Jones rjones@redhat.com --- Just a couple more clarifications needed:
- Should cargo-c be used? Apparently (I have not checked) it supports creating .so symlinks and handling .pc files.
- Will libblkio & blkio live in the same upstream repository or be split out upstream? I think this may have implications for how we package in Fedora, although TBH either should be workable.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #17 from Richard W.M. Jones rjones@redhat.com --- (In reply to Richard W.M. Jones from comment #16)
- Will libblkio & blkio live in the same upstream repository or be split out upstream? I think this may have implications for how we package in Fedora, although TBH either should be workable.
& same question about virtio-driver
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(stefanha@redhat.c | |om) |
--- Comment #18 from Stefan Hajnoczi stefanha@redhat.com --- (In reply to Richard W.M. Jones from comment #16)
Just a couple more clarifications needed:
- Should cargo-c be used? Apparently (I have not checked) it supports creating .so symlinks and handling .pc files.
libblkio currently uses meson for the library installation and pkg-config file. It does not create .so symlinks.
I'm open to changing the build, if necessary. What would you recommend?
- Will libblkio & blkio live in the same upstream repository or be split out upstream? I think this may have implications for how we package in Fedora, although TBH either should be workable.
For the time being they live in the same git repo but blkio is published on crates.io while libblkio is not (it's a C library, not a Rust crate).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #19 from Richard W.M. Jones rjones@redhat.com --- It was really me pointing out that possibility, what you do is for upstream to decide :-)
I think my question is are we going to go ahead with this package right now (with bundling), OR shall we wait for the dependencies to be built, OR shall we wait until you split upstream into blkio + libblkio (which would mean in turn that we're really doing two packages here and will need an extra review)? Let me know how you want to proceed on this one.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(rjones@redhat.com | |)
--- Comment #20 from Stefan Hajnoczi stefanha@redhat.com --- (In reply to Richard W.M. Jones from comment #19)
It was really me pointing out that possibility, what you do is for upstream to decide :-)
I think my question is are we going to go ahead with this package right now (with bundling), OR shall we wait for the dependencies to be built, OR shall we wait until you split upstream into blkio + libblkio (which would mean in turn that we're really doing two packages here and will need an extra review)? Let me know how you want to proceed on this one.
The blkio and virtio-driver crates are being developed in libblkio.git for the time being. Changing that is not a priority, so we can work with the current layout.
Bundling blkio and virtio-driver is fine by me. At some point another Rust program may be packaged by Fedora that also uses blkio or virtio-driver. At that point unbundling would be natural.
Right now the only program I'm aware of that uses blkio and virtio-driver is qsd-rs (https://gitlab.com/hreitz/qsd-rs), an experimental codebase that isn't close to be packaged.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #21 from Stefan Hajnoczi stefanha@redhat.com --- Hi Rich, libblkio 1.1 has been released: https://gitlab.com/libblkio/libblkio/-/commits/v1.1.0/
It adds a virtio-blk-vfio-pci driver.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(rjones@redhat.com | |) |
--- Comment #22 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.0-1.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=93464214
I made several changes:
- Update libblkio to version 1.1.0
- New bundled deps: * vendor/num-traits * vendor/pci-driver
- Add "Provides: bundled(rust-...)" for bundled deps.
- Remove hand-generated libblkio.so.* symlinks.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #23 from Fabio Valentini decathorpe@gmail.com --- I wonder why you're bundling num-traits? It's up-to-date in Fedora.
I also started to look at packaging the missing dependencies, and it looks like the dependency on memfd is quite old (v0.4), upstream is at v0.7 now - is there a reason for that? If I go through package review for memfd again, I'd rather have it not be for a version that can't even be used for libblkio.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(stefanha@redhat.c | |om)
--- Comment #25 from Richard W.M. Jones rjones@redhat.com --- (In reply to Fabio Valentini from comment #23)
I wonder why you're bundling num-traits? It's up-to-date in Fedora.
I must have missed it when I was looking at the new dependencies. I'll do an updated package that fixes this & the other things.
I also started to look at packaging the missing dependencies, and it looks like the dependency on memfd is quite old (v0.4), upstream is at v0.7 now - is there a reason for that? If I go through package review for memfd again, I'd rather have it not be for a version that can't even be used for libblkio.
Stefan? ^
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #26 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.0-2.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=93518039
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #27 from Fabio Valentini decathorpe@gmail.com --- Review request for the io-uring crate (with some changes to ensure it works correctly on all architectures): https://bugzilla.redhat.com/show_bug.cgi?id=2138378
Can you check whether the packages which are still missing are:
- concat-idents - memfd (v0.4 or latest v0.7?) - pci-driver
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #28 from Richard W.M. Jones rjones@redhat.com --- Yes, just those 3 left once io-uring gets in.
I don't know the answer to the question about memfd version, hopefully Stefan will reply to the needinfo with that information.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2138378
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2138378 [Bug 2138378] Review Request: rust-io-uring - Low-level io_uring userspace interface for Rust
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #29 from Richard W.M. Jones rjones@redhat.com --- Stefan, as well as the memfd version question, same question about io-uring 0.5.6 vs 0.5.8.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #30 from Fabio Valentini decathorpe@gmail.com --- Review requests:
- rust-concat-idents: https://bugzilla.redhat.com/show_bug.cgi?id=2138409 - rust-pci-driver: https://bugzilla.redhat.com/show_bug.cgi?id=2138410
I'll wait with the re-review / unretirement of rust-memfd until I know which version we need.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2138410
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2138410 [Bug 2138410] Review Request: rust-pci-driver - PCI/PCIe driver development library
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2138409
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2138409 [Bug 2138409] Review Request: rust-concat-idents - Allows concatenating multiple identifiers and using them everywhere
https://bugzilla.redhat.com/show_bug.cgi?id=2124697 Bug 2124697 depends on bug 2138378, which changed state.
Bug 2138378 Summary: Review Request: rust-io-uring - Low-level io_uring userspace interface for Rust https://bugzilla.redhat.com/show_bug.cgi?id=2138378
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=2124697 Bug 2124697 depends on bug 2138409, which changed state.
Bug 2138409 Summary: Review Request: rust-concat-idents - Allows concatenating multiple identifiers and using them everywhere https://bugzilla.redhat.com/show_bug.cgi?id=2138409
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=2124697 Bug 2124697 depends on bug 2138410, which changed state.
Bug 2138410 Summary: Review Request: rust-pci-driver - PCI/PCIe driver development library https://bugzilla.redhat.com/show_bug.cgi?id=2138410
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #31 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.0-3.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Updated package:
- I patched blkio to use io-uring 0.5.8
- I removed the bundled concat-idents & pci-driver & io-uring
The only bundled packages now are memfd & the winapi-* stuff.
Weirdly this builds fine in Koji but *not* locally (fails with a strange io-uring error which I don't understand).
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=93622342
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #32 from Fabio Valentini decathorpe@gmail.com --- I wonder what still pulls in the winapi crates? memfd doesn't pull them in, and what we have packaged for Fedora has the Windows-specific dependencies stripped from it.
Weirdly this builds fine in Koji but *not* locally (fails with a strange io-uring error which I don't understand).
Huh, koji / local environments are not 100% identical - when I encountered similar problems, either seccomp filtering or slightly different network configuration was to blame. And since you mention io_uring, I kind of assume the former.
BTW, do you have a reviewer for this package yet, or do you want me to take a look?
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #33 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.0-4.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
This one completely removes winapi* bundles. As it turns out those dependencies were not needed after all.
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=93625623
Here's the io-uring local build failure: http://oirase.annexia.org/reviews/libblkio/io-uring-fail.txt (Note that I'm patching libblkio to change the io-uring version. The failure is related to this patch. This does not happen with the upstream version / io-uring 0.5.6)
Yes, it'd be great if you could review this!
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |decathorpe@gmail.com
--- Comment #34 from Fabio Valentini decathorpe@gmail.com ---
This one completely removes winapi* bundles. As it turns out those dependencies were not needed after all.
Great! Yeah, I assumed that they were leftovers, probably they had been pulled in by one of the other, previously vendored dependencies.
Here's the io-uring local build failure: http://oirase.annexia.org/reviews/libblkio/io-uring-fail.txt
That's interesting. Looks like the io_uring struct definitions in the kernel headers have changed *again* ... Is the version of the kernel-headers package in koji different than in your local build environment?
Yes, it'd be great if you could review this!
I'll get to it, though I can't promise whether I'll have time for the first pass today or tomorrow.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #35 from Richard W.M. Jones rjones@redhat.com --- Oh I didn't think this would be related to C / kernel headers, but yes you're right. I previously had kernel-headers-5.19.0-0.rc3.git0.1.fc37.x86_64, but upgrading to 6.1.0-0.rc2.git0.1.fc38 fixes the local build!
Which is annoying .. I hope we don't have to keep chasing endless kernel changes. But here we are.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #36 from Richard W.M. Jones rjones@redhat.com --- Also means backporting this package to RHEL is going to be painful, but luckily that's somebody else's problem.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(stefanha@redhat.c | |om) |
--- Comment #37 from Stefan Hajnoczi stefanha@redhat.com --- Hi Fabio, Apologies for the late reply. memfd can be updated though the dependencies have grown in recent memfd releases due to the switch from the libc crate to rustix (see Cargo.lock diff below).
I have confirmed that libblkio builds and passes tests with memfd 0.6.1. I've sent a libblkio pull request to update the memfd version here: https://gitlab.com/libblkio/libblkio/-/merge_requests/143
diff --git a/Cargo.lock b/Cargo.lock index 222c7d7..c20375a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -57,6 +57,33 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed3d0b5ff30645a68f35ece8cea4556ca14ef8a1651455f789a099a0513532a6"
+[[package]] +name = "errno" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f639046355ee4f37944e44f60642c6f3a7efa3cf6b78c78a0d989a8ce6c396a1" +dependencies = [ + "errno-dragonfly", + "libc", + "winapi", +] + +[[package]] +name = "errno-dragonfly" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa68f1b12764fab894d2755d2518754e71b4fd80ecfb822714a1206c2aab39bf" +dependencies = [ + "cc", + "libc", +] + +[[package]] +name = "io-lifetimes" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ce5ef949d49ee85593fc4d3f3f95ad61657076395cbbce23e2121fc5542074" + [[package]] name = "io-uring" version = "0.5.6" @@ -85,17 +112,23 @@ dependencies = [
[[package]] name = "libc" -version = "0.2.132" +version = "0.2.137" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8371e4e5341c3a96db127eb2465ac681ced4c433e01dd0e938adbef26ba93ba5" +checksum = "fc7fcc620a3bff7cdd7a365be3376c97191aeaccc2a603e600951e452615bf89" + +[[package]] +name = "linux-raw-sys" +version = "0.0.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4d2456c373231a208ad294c33dc5bff30051eafd954cd4caae83a712b12854d"
[[package]] name = "memfd" -version = "0.4.1" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6627dc657574b49d6ad27105ed671822be56e0d2547d413bfbf3e8d8fa92e7a" +checksum = "480b5a5de855d11ff13195950bdc8b98b5e942ef47afc447f6615cdcc4e15d80" dependencies = [ - "libc", + "rustix", ]
[[package]] @@ -166,6 +199,20 @@ dependencies = [ "proc-macro2", ]
+[[package]] +name = "rustix" +version = "0.35.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "727a1a6d65f786ec22df8a81ca3121107f235970dc1705ed681d3e6e8b9cd5f9" +dependencies = [ + "bitflags", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "syn" version = "1.0.99" @@ -223,3 +270,60 @@ name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + +[[package]] +name = "windows-sys" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a3e1820f08b8513f676f7ab6c1f99ff312fb97b553d30ff4dd86f9f15728aa7" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41d2aa71f6f0cbe00ae5167d90ef3cfe66527d6f613ca78ac8024c3ccab9a19e" + +[[package]] +name = "windows_aarch64_msvc" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd0f252f5a35cac83d6311b2e795981f5ee6e67eb1f9a7f64eb4500fbc4dcdb4" + +[[package]] +name = "windows_i686_gnu" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fbeae19f6716841636c28d695375df17562ca208b2b7d0dc47635a50ae6c5de7" + +[[package]] +name = "windows_i686_msvc" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "84c12f65daa39dd2babe6e442988fc329d6243fdce47d7d2d155b8d874862246" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf7b1b21b5362cbc318f686150e5bcea75ecedc74dd157d874d754a2ca44b0ed" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09d525d2ba30eeb3297665bd434a54297e4170c7f1a44cad4ef58095b4cd2028" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f40009d85759725a34da6d89a94e63d7bdc50a862acf0dbc7c8e488f1edcb6f5"
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #38 from Richard W.M. Jones rjones@redhat.com --- Looks like we have: - https://src.fedoraproject.org/rpms/rust-errno - https://src.fedoraproject.org/rpms/rust-io-lifetimes - https://src.fedoraproject.org/rpms/rust-linux-raw-sys - https://src.fedoraproject.org/rpms/rust-rustix
and we are missing: - errno-dragonfly
(I ignored the windows ones)
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #39 from Richard W.M. Jones rjones@redhat.com --- (and obviously memfd still too)
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #40 from Fabio Valentini decathorpe@gmail.com --- Great, thanks for confirming that memfd 0.6 is fine as well.
I've submitted a re-review request for it here: https://bugzilla.redhat.com/show_bug.cgi?id=2140674
We already have rustix and all its dependencies packaged and up-to-date in Fedora (except for things that are blocked by the F37 Freeze). So there should be no missing dependencies other than memfd (errno-dragonfly is a dependency that is specific to, uhm, well, DragonflyBSD, and not relevant to us).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2140674
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2140674 [Bug 2140674] Review Request: rust-memfd - Pure-Rust library to work with Linux memfd and sealing
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #41 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.0-5.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Removed the memfd dependency (see bug 2140674).
No scratch build because that requires memfd to appear in Fedora first.
This contains two downstream patches to adjust the version numbers of dependencies. Hopefully the need for these should disappear: http://oirase.annexia.org/reviews/libblkio/0001-Use-io-uring-0.5.8.patch http://oirase.annexia.org/reviews/libblkio/0002-virtio-driver-upgrade-to-mem...
https://bugzilla.redhat.com/show_bug.cgi?id=2124697 Bug 2124697 depends on bug 2140674, which changed state.
Bug 2140674 Summary: Review Request: rust-memfd - Pure-Rust library to work with Linux memfd and sealing https://bugzilla.redhat.com/show_bug.cgi?id=2140674
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(stefanha@redhat.c | |om)
--- Comment #42 from Richard W.M. Jones rjones@redhat.com --- Well ... https://gitlab.com/libblkio/libblkio/-/commit/500b4fdc9b95f8e75f2a98bb1861e2... drops the dependency on memfd! Apparently nix provides access to equivalent functionality.
Dependencies upstream now are:
autocfg (ok) bitflags (ok) cc (ok) cfg-if (ok) concat-idents (ok) const-cstr (ok) io-uring (ok - also updated to 0.5.8) lazy_static (ok) libc (ok) memmap2 (ok - NEW) nix (ok) num-traits (ok) pci-driver (ok) proc-macro2 (ok) quote (ok) syn (ok) unicode-ident (ok) virtio-bindings (ok)
Removed:
memfd memmap memoffset winapi*
Since there's quite a large change here I'm going to wait until upstream tag a new release.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(stefanha@redhat.c | |om) |
--- Comment #43 from Stefan Hajnoczi stefanha@redhat.com --- Hi Rich, The libblkio 1.1.1 release is now available without the memfd crate dependency: https://gitlab.com/libblkio/libblkio/-/tree/v1.1.1
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #44 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.1-6.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build FAILS for unclear reasons. It builds fine locally. https://koji.fedoraproject.org/koji/taskinfo?taskID=94425117
Update to 1.1.1
- Remove downstream patches. - Remove memfd, memmap, memoffset dependencies. - Add memmap2 dependency. - Use SPDX code for license. - Remove all remaining bundles.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #45 from Richard W.M. Jones rjones@redhat.com --- It builds fine against the f37 target: https://koji.fedoraproject.org/koji/taskinfo?taskID=94425359
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #46 from Stefan Hajnoczi stefanha@redhat.com --- meson 0.64.0 added a new optimization level called "plain" that means "don't set explicit compiler optimization options" (apparently nix needs this because it controls the compiler options). libblkio's src/cargo-build.sh script parses the meson_optimization option and doesn't know how to deal with "plain".
I am investigating further to see how to solve this.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #47 from Alberto Faria afaria@redhat.com --- (In reply to Richard W.M. Jones from comment #44)
Scratch build FAILS for unclear reasons. It builds fine locally. https://koji.fedoraproject.org/koji/taskinfo?taskID=94425117
Looks like a newer libc declaring read() and write() with __attribute__((warn_unused_result)). This should fix it: https://gitlab.com/libblkio/libblkio/-/merge_requests/148
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #48 from Alberto Faria afaria@redhat.com --- (In reply to Alberto Faria from comment #47)
(In reply to Richard W.M. Jones from comment #44)
Scratch build FAILS for unclear reasons. It builds fine locally. https://koji.fedoraproject.org/koji/taskinfo?taskID=94425117
Looks like a newer libc declaring read() and write() with __attribute__((warn_unused_result)). This should fix it: https://gitlab.com/libblkio/libblkio/-/merge_requests/148
Nevermind, we aren't using -Werror.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #49 from Stefan Hajnoczi stefanha@redhat.com --- Hi Rich, Is it possible to add the following to the rpm spec file?
export CARGO_NET_OFFLINE=true # avoid the need to patch src/cargo-build.sh
# Explicitly enable debuginfo and release build optimization level for now. # In the future this can be dropped when rpm macros set RUSTFLAGS # themselves like they set CFLAGS, CXXFLAGS, etc. {%meson} -Ddebug=true -Doptimization=3
I think this will work on both f37 and f38.
For f38 please see this libblkio merge request: https://gitlab.com/libblkio/libblkio/-/merge_requests/149
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #50 from Richard W.M. Jones rjones@redhat.com --- (In reply to Stefan Hajnoczi from comment #49)
Hi Rich, Is it possible to add the following to the rpm spec file?
export CARGO_NET_OFFLINE=true # avoid the need to patch src/cargo-build.sh
The above failed with the error:
error: the lock file /home/rjones/rpmbuild/BUILD/libblkio-v1.1.1/Cargo.lock needs to be updated but --locked was passed to prevent this
(The original code which edited cargo-build.sh was designed to prevent that)
# Explicitly enable debuginfo and release build optimization level for now. # In the future this can be dropped when rpm macros set RUSTFLAGS # themselves like they set CFLAGS, CXXFLAGS, etc. {%meson} -Ddebug=true -Doptimization=3
Making just this change, here are new scratch builds:
f37: https://koji.fedoraproject.org/koji/taskinfo?taskID=94429101 f38: https://koji.fedoraproject.org/koji/taskinfo?taskID=94429097
I think this will work on both f37 and f38.
For f38 please see this libblkio merge request: https://gitlab.com/libblkio/libblkio/-/merge_requests/149
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #51 from Richard W.M. Jones rjones@redhat.com --- Both builds whinged about:
+ /usr/bin/meson --buildtype=plain --prefix=/usr --libdir=/usr/lib64 --libexecdir=/usr/libexec --bindir=/usr/bin --sbindir=/usr/sbin --includedir=/usr/include --datadir=/usr/share --mandir=/usr/share/man --infodir=/usr/share/info --localedir=/usr/share/locale --sysconfdir=/etc --localstatedir=/var --sharedstatedir=/var/lib --wrap-mode=nodownload --auto-features=enabled . redhat-linux-build -Ddebug=true -Doptimization=3 WARNING: Recommend using either -Dbuildtype or -Doptimization + -Ddebug. Using both is redundant since they override each other. See: https://mesonbuild.com/Builtin-options.html#build-type-options
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #52 from Stefan Hajnoczi stefanha@redhat.com --- (In reply to Richard W.M. Jones from comment #51)
Both builds whinged about:
- /usr/bin/meson --buildtype=plain --prefix=/usr --libdir=/usr/lib64
--libexecdir=/usr/libexec --bindir=/usr/bin --sbindir=/usr/sbin --includedir=/usr/include --datadir=/usr/share --mandir=/usr/share/man --infodir=/usr/share/info --localedir=/usr/share/locale --sysconfdir=/etc --localstatedir=/var --sharedstatedir=/var/lib --wrap-mode=nodownload --auto-features=enabled . redhat-linux-build -Ddebug=true -Doptimization=3 WARNING: Recommend using either -Dbuildtype or -Doptimization + -Ddebug. Using both is redundant since they override each other. See: https://mesonbuild.com/Builtin-options.html#build-type-options
It's a warning, not an error. The build should still proceed.
I think we have to accept the warning for now, unless we're willing to switch away from the %meson macro entirely.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #53 from Stefan Hajnoczi stefanha@redhat.com --- (In reply to Richard W.M. Jones from comment #50)
(In reply to Stefan Hajnoczi from comment #49)
Hi Rich, Is it possible to add the following to the rpm spec file?
export CARGO_NET_OFFLINE=true # avoid the need to patch src/cargo-build.sh
The above failed with the error:
error: the lock file /home/rjones/rpmbuild/BUILD/libblkio-v1.1.1/Cargo.lock needs to be updated but --locked was passed to prevent this
(The original code which edited cargo-build.sh was designed to prevent that)
# Explicitly enable debuginfo and release build optimization level for now. # In the future this can be dropped when rpm macros set RUSTFLAGS # themselves like they set CFLAGS, CXXFLAGS, etc. {%meson} -Ddebug=true -Doptimization=3
Making just this change, here are new scratch builds:
f37: https://koji.fedoraproject.org/koji/taskinfo?taskID=94429101 f38: https://koji.fedoraproject.org/koji/taskinfo?taskID=94429097
Cool, the builds are succeeding!
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #54 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.1-7.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch builds: f37 https://koji.fedoraproject.org/koji/taskinfo?taskID=94452911 f38 https://koji.fedoraproject.org/koji/taskinfo?taskID=94452912
I think we may be ready for review now.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #55 from Fabio Valentini decathorpe@gmail.com --- Note that using BuildRequires on Rust crates like this:
BuildRequires: crate(cc/default)
Might or might not do what you want it to do, because you didn't include version requirements. Not specifying a version will always resolve to the *latest* version, which might not be the one you need.
Specifying the acceptable version range like this
BuildRequires: (crate(cc/default) >= 1.0.44 with crate(cc/default) < 2.0.0~)
(or whatever minimum version of the crate is specified) is much safer, and will always result in the correct version of the crate getting installed.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(stefanha@redhat.c | |om)
--- Comment #56 from Richard W.M. Jones rjones@redhat.com --- I could certainly encode the information from the package into the spec file, but this seems to duplicate the information. Do we have any way to keep the two in synch automatically?
Stefan ^^ do you have any preference here?
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #57 from Fabio Valentini decathorpe@gmail.com ---
Do we have any way to keep the two in synch automatically?
Yeah, that's what dynamic BuildRequires are for. The macro that does that for cargo doesn't support workspaces / path dependencies yet, though :(
But until I have time to implement and test that, you can use a workaround like this one here: https://src.fedoraproject.org/rpms/system76-keyboard-configurator/blob/rawhi...
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(stefanha@redhat.c | |om) |
--- Comment #58 from Stefan Hajnoczi stefanha@redhat.com --- (In reply to Richard W.M. Jones from comment #56)
I could certainly encode the information from the package into the spec file, but this seems to duplicate the information. Do we have any way to keep the two in synch automatically?
Stefan ^^ do you have any preference here?
Preventing the major version number from changing sounds good. Other than that, libblkio doesn't have specific version requirements for cc/default.
Some of the crate dependencies have pre-1.0 version numbers where breaking API changes can occur in any new version (even minor or patch versions, according to semver). In those cases it would be best to hardcode a specific package version? Either way, it may be necessary to update the spec file as new crate versions are packaged for Fedora :/.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #59 from Fabio Valentini decathorpe@gmail.com --- No, cc was just an example, I was making a general statement about all Rust dependencies that are listed as BuildRequires. The automatically generated BuildRequires already include version specifiers that are compatible with Cargo metadata.
Additionally, Cargo-SemVer actually provides stronger stability guarantees than plain SemVer - for example, 0.6.x and 0.6.y should be API-compatible with each other. So specifying *exact* version requirements (i.e. something like "= 0.6.5") actually a very bad idea (and unnecessary).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #60 from Richard W.M. Jones rjones@redhat.com --- (In reply to Fabio Valentini from comment #55)
BuildRequires: (crate(cc/default) >= 1.0.44 with crate(cc/default) <2.0.0~)
Does ~ here have a meaning or was it a typo?
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #61 from Fabio Valentini decathorpe@gmail.com --- It's not a typo, it's necessary to prevent the expression from matching 2.0.0 pre-releases. Using just "< 2.0.0" would include versions like 2.0.0~beta.1, which is not what "^1.0.44" means.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #62 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.1-8.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=94485089
I didn't fully understand the script posted in comment 57, so I just added some boolean version dependencies. For dependencies with major version >= 1, I added the requirement that the major version doesn't change. For major version < 1, I added a simple minimum requirement.
We would want to switch to using %generate_buildrequires at some time in the future.
Also unicode-xid -> unicode-ident (this changed in an earlier version but we didn't notice).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #63 from Fabio Valentini decathorpe@gmail.com --- Why didn't you do the same for pre-1.0 versions? The same restrictions apply there.
For example, the "nix" crate will likely be updated to version 0.25.0 soon, and using the >= 0.24.2 requirement you have now, doing that would break this package (as it would start to pull in rust-nix-0.25, making the build of this package fail with missing dependency on nix v0.24).
Instead, use "(crate(nix/default) >= 0.24.2 with crate(nix/default) < 0.25~)" so that this package will continue pulling in the nix v0.24 compat package once the main nix package is updated to v0.25.
As I mentioned, cargo-semver provides stronger compatibility guarantees than plain semver in this regard (for pre-1.0-versions), as patch releases in the 0.24.x series are supposed to be compatible with each other - but minor releases of pre-1.0 versions are treated just like "major" version changes (i.e. something like v0.24 -> v0.25).
The only range of versions where "exact" requirements are fine are for pre-0.1-versions like "0.0.x", where a change from "0.0.x" to "0.0.x+1" conveys a breaking change, as well (if you discount pre-releases).
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #64 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.1-9.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=94487848
Enforce same minor version for 0.x dependencies.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(decathorpe@gmail. | |com)
--- Comment #65 from Stefan Hajnoczi stefanha@redhat.com --- Hi Fabio, Is there anything I can do to help this make progress?
Thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(decathorpe@gmail. | |com) |
--- Comment #66 from Fabio Valentini decathorpe@gmail.com --- Hi! Sorry for the radio silence. I didn't remember that I had assigned this bug to myself ... I don't really have much time to spend on Fedora these days, so I cannot promise a formal review of this package. If somebody else wants to take a stab at it before I can come back to this, feel free to un-assign me from this bug and take over.
For what it's worth, I think the Rust related things in this package look good now, with one exception: The way cargo is invoked in src/cargo-build.sh is not compatible with Fedora packaging guidelines, as it makes cargo ignore the default compiler flags in Fedora, which is explicitly forbidden.
So if at all possible, the cargo-build.sh script should probably not be used during the build, but it should be possible to replicate the only two lines in it that matter, i.e.
""" ( set -x && RUSTFLAGS="${rustflags[*]}" cargo build "${args[@]}" --color always \ --locked ) cp "src/target/${profile}/libblkio.so" "src/libblkio.so.${version}" """
with calls to the %cargo_build macro (making the sed "s/--locked/--offline/g" also unnecessary) and the cp call in %build. Not sure how that change could be integrated in meson, though - but does it even need to be? I.e. would it be possible to make %build something like
""" %build %meson %cargo_build cp "src/target/${profile}/libblkio.so" "src/libblkio.so.${version}" %meson_build """
And patch meson.build to not run cargo-build.sh at all?
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #67 from Stefan Hajnoczi stefanha@redhat.com --- Thanks, Fabio!
The rpm could invoke cargo directly, although the build won't be 100% the same as tested upstream. I think it's preferable to use meson instead of bypassing it.
I think the build is almost correct already because rpm invokes meson with buildtype "plain". That tells meson not to set optimization flags and cargo-build.sh honors that (see case "${meson_optimization}"). They are inherited from the environment.
However, cargo-build.sh currently overrides RUSTFLAGS. Instead, it should use RUSTFLAGS="$RUSTFLAGS ${rustflags[*]}". That way rpm can pass any options via RUSTFLAGS while non-rpm builds can still set compiler flags in cargo-build.sh.
How does that sound?
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #68 from Fabio Valentini decathorpe@gmail.com --- The problem is that RUSTFLAGS are not set automatically, and are not exposed directly, but passed as environment variables to the cargo command in the %cargo_build macro. So there's currently no way to "just set RUSTFLAGS", as that's not really a supported use case right now. So even if your meson setup doesn't override the flags, there's nothing to inherit from the environment - because they are not *set* in the environment.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #69 from Fabio Valentini decathorpe@gmail.com --- It might be possible to partially work around this by writing the value of the `%build_rustflags` macro to the RUSTFLAGS environment variable, with something like:
export RUSTFLAGS="%build_rustflags"
(untested)
And looking at the difference between the values of "%cargo_build"
"/usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 RUSTFLAGS='-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Clink-arg=-Wl,-z,relro -Clink-arg=-Wl,-z,now --cap-lints=warn' /usr/bin/cargo build -j16 -Z avoid-dev-deps --release"
and "%build_rustflags"
"-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Clink-arg=-Wl,-z,relro -Clink-arg=-Wl,-z,now --cap-lints=warn"
that *should* be enough, since the script is already passing `--release`.
We'll still need to verify that the flags actually end up being passed through correctly cargo / rustc - especially `-Cdebuginfo=2` - otherwise builds of the package won't get valid debug information.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(rjones@redhat.com | |)
--- Comment #70 from Stefan Hajnoczi stefanha@redhat.com --- Thanks for the %build_rustflags suggestion! My cargo-build.sh patch has been merged upstream, so it's now possible to pass RUSTFLAGS.
Rich: Is it possible to incorporate "export RUSTFLAGS="%build_rustflags"" as suggested by Fabio into the spec file and include the following libblkio patch? https://gitlab.com/libblkio/libblkio/-/commit/44652d6c7fe9c61ac417db9dd05ec2...
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(rjones@redhat.com | |) |
--- Comment #71 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.1.1-10.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=95326385
I wasn't very clear on exactly where I should add the new export, so I guessed.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #72 from Richard W.M. Jones rjones@redhat.com --- error: failed to select a version for the requirement `io-uring = ">=0.5.6, <=0.5.8"` candidate versions found which didn't match: 0.5.9
This build failure seems to be a problem with libblkio. I don't see any upstream patch to handle the new version of io-uring in Fedora.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #73 from Fabio Valentini decathorpe@gmail.com ---
">=0.5.6, <=0.5.8"
Hum, that's kind of a very weird version restriction. I updated io-uring to version 0.5.9 yesterday, so that explains why things are broken.
Usually you'd want to do just "0.5.6" or "^0.5.6", which are both equivalent to ">= 0.5.6, < 0.6.0". I freely update Rust crates in Fedora from 0.x.y to 0.x.y+1, because these are always supposed to be backwards compatible. Unusual / non-standard restrictions like this one are very annoying to deal with, as they kind of circumvent guarantees of SemVer.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #74 from Fabio Valentini decathorpe@gmail.com ---
I wasn't very clear on exactly where I should add the new export, so I guessed.
Yeah, adding it in %build before calling %meson and %meson_build should do it. I'll verify that the build flags are applied correctly once it builds again.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #76 from Alberto Faria afaria@redhat.com --- (In reply to Richard W.M. Jones from comment #72)
error: failed to select a version for the requirement `io-uring = ">=0.5.6, <=0.5.8"` candidate versions found which didn't match: 0.5.9
This build failure seems to be a problem with libblkio. I don't see any upstream patch to handle the new version of io-uring in Fedora.
Upstream libblkio is trying to stay compatible with stable Debian, which is still on Rust 1.48, but io-uring 0.5.9 requires Rust 1.51.
I sent https://github.com/tokio-rs/io-uring/pull/161 to fix that, but we'll have to wait for io-uring 0.5.10 to be released and packaged, then update libblkio's blkio/Cargo.toml to allow io-uring 0.5.10.
(In reply to Fabio Valentini from comment #73)
">=0.5.6, <=0.5.8"
Hum, that's kind of a very weird version restriction. I updated io-uring to version 0.5.9 yesterday, so that explains why things are broken.
Usually you'd want to do just "0.5.6" or "^0.5.6", which are both equivalent to ">= 0.5.6, < 0.6.0". I freely update Rust crates in Fedora from 0.x.y to 0.x.y+1, because these are always supposed to be backwards compatible. Unusual / non-standard restrictions like this one are very annoying to deal with, as they kind of circumvent guarantees of SemVer.
Unfortunately we can't depend on io-uring "0.5" since we rely on unstable features.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #77 from Fabio Valentini decathorpe@gmail.com ---
Upstream libblkio is trying to stay compatible with stable Debian, which is still on Rust 1.48, but io-uring 0.5.9 requires Rust 1.51.
I don't see how this affects the range of supported versions? Assuming debian has packaged io-uring 0.5.8, and Fedora has packaged io-uring 0.5.9, both are compatible with something like ">=0.5.6, <0.5.10", unless you lock the version explicitly to either 0.5.8 or 0.5.9? But the Fedora build already drops the "--locked" flag for cargo, so that doesn't apply here, I think.
Unfortunately we can't depend on io-uring "0.5" since we rely on unstable features.
That's unfortunate, and will be very annoying to deal with. I guess I'll have to put io-uring on a list of "special" crates that need to be handled differently from everything else (and it would be the only crate on that list, out of over 2000 that we have packaged for Fedora ...)
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #78 from Alberto Faria afaria@redhat.com --- (In reply to Fabio Valentini from comment #77)
Upstream libblkio is trying to stay compatible with stable Debian, which is still on Rust 1.48, but io-uring 0.5.9 requires Rust 1.51.
I don't see how this affects the range of supported versions? Assuming debian has packaged io-uring 0.5.8, and Fedora has packaged io-uring 0.5.9, both are compatible with something like ">=0.5.6, <0.5.10", unless you lock the version explicitly to either 0.5.8 or 0.5.9? But the Fedora build already drops the "--locked" flag for cargo, so that doesn't apply here, I think.
More precisely, we're keeping the crates.io-based build of libblkio working on stable Debian, so users can install it from source. In fact, there doesn't seem to be a Debian package for the io-uring crate.
Unfortunately we can't depend on io-uring "0.5" since we rely on unstable features.
That's unfortunate, and will be very annoying to deal with. I guess I'll have to put io-uring on a list of "special" crates that need to be handled differently from everything else (and it would be the only crate on that list, out of over 2000 that we have packaged for Fedora ...)
I sent https://github.com/tokio-rs/io-uring/pull/162 proposing the removal of the "unstable" feature. It does seem redundant to have such a feature on a pre-1.0 crate.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #79 from Alberto Faria afaria@redhat.com --- io-uring 0.5.10 has been released. It restores compatibility with Rust 1.48 and also makes the entire public API follow the usual SemVer rules, making the "unstable" feature have no effect.
libblkio 1.2.0 will be released today, adding some new functionality and updating the io-uring dependency version specifier to "0.5.10".
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Stefan Hajnoczi stefanha@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(rjones@redhat.com | |)
--- Comment #80 from Stefan Hajnoczi stefanha@redhat.com --- libblkio 1.2.0 has been released. It has the updated io-uring crate dependency.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #81 from Fabio Valentini decathorpe@gmail.com --- Great, thanks for submitting that upstream. I've already updated the rust-io-uring package in Fedora to 0.5.10.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(rjones@redhat.com | |) |
--- Comment #82 from Richard W.M. Jones rjones@redhat.com --- (In reply to Fabio Valentini from comment #75)
sed -e 's/--locked/--offline/' -i src/cargo-build.sh
I'm also pretty sure you should be able to just drop "--locked" and not add "--offline", as that flag should be a no-op assuming the cargo configuration is injected correctly by %cargo_prep.
If I remove this line then I get:
error: the lock file /home/rjones/rpmbuild/BUILD/libblkio-v1.2.1/Cargo.lock needs to be updated but --locked was passed to prevent this
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #83 from Richard W.M. Jones rjones@redhat.com --- Oh I see, you said to drop --locked, but not add --offline. That works too ...
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #84 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.2.1-11.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=95585044
Version 1.2.1
- Remove patches which are all upstream. - Update io-uring dependency minimum to 0.5.10. - Don't add --offline to cargo-build.sh
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #85 from Stefan Hajnoczi stefanha@redhat.com --- libblkio v1.2.2 has been released. It only fixes a version number inconsistency, so there's no need to upgrade unless you noticed you're getting 1.2.0 version numbers instead of 1.2.1.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Richard W.M. Jones rjones@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(decathorpe@gmail. | |com)
--- Comment #86 from Richard W.M. Jones rjones@redhat.com --- Hi Fabio, do you still want to take this package for review, or shall I look for someone else to review it? Fine for me either way.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | |needinfo?(decathorpe@gmail. | |com) | Assignee|decathorpe@gmail.com |nobody@fedoraproject.org Status|ASSIGNED |NEW
--- Comment #87 from Fabio Valentini decathorpe@gmail.com --- Sorry, I didn't know whether you wanted to update to 1.2.2 before the review, and then dropped the ball over the holidays.
I don't really have the bandwidth to deal with this right now, so I'm fine with you finding somebody else to finish the review. All I can say is that that Rust side of things appears to be looking good now, but I haven't looked at any other aspect of this package yet.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ngompa13@gmail.com Assignee|nobody@fedoraproject.org |ngompa13@gmail.com Flags| |fedora-review?
--- Comment #88 from Neal Gompa ngompa13@gmail.com --- Taking this review.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #89 from Neal Gompa ngompa13@gmail.com --- Reviewing the spec, I see only one issue left:
%{_libdir}/libblkio.so.1*
This is unnecessarily permissive. Change this to "%{_libdir}/libblkio.so.1{,.*}" so that we ensure that we're talking about "libblkio.so.1", rather than potentially "libblkio.so.10" or some other crazy permutation.
Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_l...
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #90 from Richard W.M. Jones rjones@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/libblkio/libblkio.spec SRPM URL: http://oirase.annexia.org/reviews/libblkio/libblkio-1.2.2-12.fc37.src.rpm Description: Block device I/O library Fedora Account System Username: rjones
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=96003601
I fixed the %files problem with the library. I suspect this affects other packages I manage, never really thought about it before.
I also updated to the new upstream version 1.2.2.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #91 from Richard W.M. Jones rjones@redhat.com --- Argh, the %changelog version is wrong - will fix in my local copy.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #92 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5216289 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #93 from Richard W.M. Jones rjones@redhat.com ---
[!]: Reviewer should test that the package builds in mock.
- I scratch-built the package in Koji which should cover this.
libblkio.x86_64: W: incoherent-version-in-changelog 1.2.1-12 ['1.2.2-12.fc38', '1.2.2-12'] 4 packages and 0 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.3 s
This is true - fixed locally.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |POST Flags|fedora-review? |fedora-review+
--- Comment #94 from Neal Gompa ngompa13@gmail.com --- Review notes:
- Package is named appropriately - Package (mostly) following Rust packaging guidelines - Package builds and installs - No serious issues from rpmlint
PACKAGE APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #95 from Richard W.M. Jones rjones@redhat.com --- https://pagure.io/releng/fedora-scm-requests/issue/50385
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #96 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/libblkio
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
--- Comment #97 from Richard W.M. Jones rjones@redhat.com --- https://pagure.io/releng/fedora-scm-requests/issue/50386 (separate request for f37 branch)
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #98 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-a4a9a1ca5c has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-a4a9a1ca5c
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #99 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-a4a9a1ca5c has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-a4a9a1ca5c *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-a4a9a1ca5c
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2124697
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |ERRATA Status|ON_QA |CLOSED Last Closed| |2023-01-21 03:30:23
--- Comment #100 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-a4a9a1ca5c has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com