https://bugzilla.redhat.com/show_bug.cgi?id=2181039
Bug ID: 2181039 Summary: Review Request: firecracker - Secure and fast microVMs for serverless computing Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: fedora.dm0@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://github.com/dm0-/copr-firecracker/raw/fedora/firecracker.spec SRPM URL: https://github.com/dm0-/copr-firecracker/raw/fedora/firecracker-1.3.1-1.fc37... Description: Firecracker is an open source virtualization technology that is purpose-built for creating and managing secure, multi-tenant container and function-based services that provide serverless operational models. Firecracker runs workloads in lightweight virtual machines, called microVMs, which combine the security and isolation properties provided by hardware virtualization technology with the speed and flexibility of containers. Fedora Account System Username: dm0
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://firecracker-microvm | |.github.io/
--- Comment #1 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5696296 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181036, which changed state.
Bug 2181036 Summary: Review Request: rust-vm-fdt - For writing Flattened Devicetree blobs https://bugzilla.redhat.com/show_bug.cgi?id=2181036
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181029, which changed state.
Bug 2181029 Summary: Review Request: rust-userfaultfd-sys - Low-level bindings for userfaultfd functionality on Linux https://bugzilla.redhat.com/show_bug.cgi?id=2181029
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181038, which changed state.
Bug 2181038 Summary: Review Request: rust-vm-superio - Emulation for legacy devices https://bugzilla.redhat.com/show_bug.cgi?id=2181038
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181033, which changed state.
Bug 2181033 Summary: Review Request: rust-versionize_derive - Implements the Versionize derive proc macro https://bugzilla.redhat.com/show_bug.cgi?id=2181033
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181022, which changed state.
Bug 2181022 Summary: Review Request: rust-device_tree - Reads and parses Linux device tree images https://bugzilla.redhat.com/show_bug.cgi?id=2181022
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NOTABUG
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181020, which changed state.
Bug 2181020 Summary: Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde https://bugzilla.redhat.com/show_bug.cgi?id=2181020
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NOTABUG
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181021, which changed state.
Bug 2181021 Summary: Review Request: rust-crc64 - CRC64 checksum implementation https://bugzilla.redhat.com/show_bug.cgi?id=2181021
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181028, which changed state.
Bug 2181028 Summary: Review Request: rust-userfaultfd - Rust bindings for the Linux userfaultfd functionality https://bugzilla.redhat.com/show_bug.cgi?id=2181028
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181030, which changed state.
Bug 2181030 Summary: Review Request: rust-versionize - Version tolerant serialization/deserialization framework https://bugzilla.redhat.com/show_bug.cgi?id=2181030
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181023, which changed state.
Bug 2181023 Summary: Review Request: rust-event-manager - Abstractions for implementing event based systems https://bugzilla.redhat.com/show_bug.cgi?id=2181023
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181025, which changed state.
Bug 2181025 Summary: Review Request: rust-linux-loader - Linux kernel image loading crate https://bugzilla.redhat.com/show_bug.cgi?id=2181025
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039 Bug 2181039 depends on bug 2181035, which changed state.
Bug 2181035 Summary: Review Request: rust-vm-allocator - Helpers for allocating resources needed during the lifetime of a VM https://bugzilla.redhat.com/show_bug.cgi?id=2181035
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? Assignee|nobody@fedoraproject.org |decathorpe@gmail.com CC| |decathorpe@gmail.com Status|NEW |ASSIGNED
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- Initial comments:
1. I don't really understand what this comment is meant to say:
""" # This should be left enabled for generating buildreqs correctly. """ %bcond_without check
Yes, "check" needs to be enabled for "dev-dependencies" to be included in generated BuildRequires, if that's what you meant to say ... but I'm not sure if that needs to be said explicitly, since this is normal for Rust packages.
2. Building for the *-musl Rust targets is not going to be supported in Fedora (we don't even have the musl toolchain targets available). All conditionals that rely on %{cargo_target} and / or %{with jailer} are noops, please remove them.
3. The Source URLs use the old and deprecated format that relies on the URL "#/fragment" hack. Please use the "new" format (which has worked for almost a decade), i.e. Source0: https://github.com/firecracker-microvm/firecracker/archive/v%%7Bversion%7D/%... instead of: Source0: https://github.com/firecracker-microvm/firecracker/archive/refs/tags/v%%7Bve... Same applies to the URLs for Source1 and Source2.
Documentation for this (both for using tarballs for git tags or for specific commits) is here: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_commit...
4. Strange names for downloads of bundled crates. I'm not sure why you're downloading kvm-bindings-e835920.tar.gz and micro-http-4b18a04.tar.gz but store them as kvm-bindings-0.6.0-1.crate and micro_http-0.1.0.crate. This is confusing and unnecessary, since you control all parts of this process, there's no need to name them as if they were upstream crates.
5. Patches are included without comments or explanations. Please include short comments about what your patches do to the spec file, and whether they are suitable for upstreaming (or not).
6. Don't ignore test failures. It's OK to ignore specific tests that don't work in our containerized build environment, but "|| :"-ing everything is not OK.
7. Include breakdown of the dependencies / licenses. The License tag looks comprehensive, but there's no indication how this list was generated (you're not using the %cargo_license or %cargo_license_summary macros in the spec file), and there's no breakdown that lists which dependency is available under which license.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #3 from fedora.dm0@gmail.com --- The spec and SRPM are updated.
(In reply to Fabio Valentini from comment #2)
- I don't really understand what this comment is meant to say:
""" # This should be left enabled for generating buildreqs correctly. """ %bcond_without check
Yes, "check" needs to be enabled for "dev-dependencies" to be included in generated BuildRequires, if that's what you meant to say ... but I'm not sure if that needs to be said explicitly, since this is normal for Rust packages.
The workspace packages share the same build.rs which seems to fail if their dev-dependencies aren't installed.
https://github.com/firecracker-microvm/firecracker/blob/v1.3.1/src/firecrack...
I've just changed %generate_buildrequires to always pass --with-check and dropped the comment.
- Building for the *-musl Rust targets is not going to be supported in
Fedora (we don't even have the musl toolchain targets available). All conditionals that rely on %{cargo_target} and / or %{with jailer} are noops, please remove them.
Will I have no discretion as maintainer to provide build options that do not conflict with the guidelines? As was mentioned in the original e-mail thread, Firecracker's security benefits (seccomp, sandboxing) currently only apply to the musl targets. If I can't support a non-default option to use it, I will have to maintain a secure version of the spec separately. I don't see a downside to allowing other developers to run "rpmbuild -D ..." to produce the full build as needed.
- The Source URLs use the old and deprecated format that relies on the URL
"#/fragment" hack. Please use the "new" format (which has worked for almost a decade), i.e. Source0: https://github.com/firecracker-microvm/firecracker/archive/v%%7Bversion%7D/ %{name}-%{version}.tar.gz instead of: Source0: https://github.com/firecracker-microvm/firecracker/archive/refs/tags/ v%{version}.tar.gz#/%{name}-%{version}.tgz Same applies to the URLs for Source1 and Source2.
Documentation for this (both for using tarballs for git tags or for specific commits) is here: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ #_commit_revision
- Strange names for downloads of bundled crates.
I'm not sure why you're downloading kvm-bindings-e835920.tar.gz and micro-http-4b18a04.tar.gz but store them as kvm-bindings-0.6.0-1.crate and micro_http-0.1.0.crate. This is confusing and unnecessary, since you control all parts of this process, there's no need to name them as if they were upstream crates.
URLs are updated.
- Patches are included without comments or explanations.
Please include short comments about what your patches do to the spec file, and whether they are suitable for upstreaming (or not).
Comments are added to the spec.
- Don't ignore test failures.
It's OK to ignore specific tests that don't work in our containerized build environment, but "|| :"-ing everything is not OK.
I did that to follow what the rust package does.
https://src.fedoraproject.org/rpms/rust/blob/7ac7a42b5e2fd46d73d2a0946c55109...
There are over 100 test failures on x86_64 alone, and the Rust test --skip flag seems broken for doc tests. (I want to use --exact instead of substring filtering since there are so many tests, and e.g. --skip=filter_cpuid doesn't match the exact test name, but the full string --skip='src/lib.rs - filter_cpuid (line 43)' causes every doc test for every package to be skipped.)
I've dropped the "|| :" and disabled tests by default instead since that seems acceptable judging by similar packages, although we won't see results in the log anymore.
- Include breakdown of the dependencies / licenses.
The License tag looks comprehensive, but there's no indication how this list was generated (you're not using the %cargo_license or %cargo_license_summary macros in the spec file), and there's no breakdown that lists which dependency is available under which license.
It was generated by copying the rust-* package dependencies out of a build log and running this (and manually removing any equivalent SPDX expressions):
dnf repoquery --queryformat='%{LICENSE}\n' $(<deps.txt) | sort -u
I've added a LICENSE.dependencies file from %cargo_license. I'm assuming that's what it's supposed to do based on how a couple packages use it--I don't see it mentioned in the guidelines, and Google has zero results for 'fedora "cargo_license"'.
I've also dropped ISC from the License field since that's only on libloading (used by bindgen, not linked into the binaries).
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- (In reply to fedora.dm0 from comment #3)
The spec and SRPM are updated.
Thanks! I left a few more comments.
(In reply to Fabio Valentini from comment #2)
- I don't really understand what this comment is meant to say:
""" # This should be left enabled for generating buildreqs correctly. """ %bcond_without check
Yes, "check" needs to be enabled for "dev-dependencies" to be included in generated BuildRequires, if that's what you meant to say ... but I'm not sure if that needs to be said explicitly, since this is normal for Rust packages.
The workspace packages share the same build.rs which seems to fail if their dev-dependencies aren't installed.
That seems strange and sounds like a bug somewhere :(
%{__cargo_to_rpm} --path Cargo.toml buildrequires --with-check
Macros that include double underscores are implementation details and shouldn't be used directly. If you really want to override this behaviour to work around the bug, just use "cargo2rpm" instead of "%__cargo_to_rpm".
https://github.com/firecracker-microvm/firecracker/blob/v1.3.1/src/ firecracker/Cargo.toml#L6
I've just changed %generate_buildrequires to always pass --with-check and dropped the comment.
- Building for the *-musl Rust targets is not going to be supported in
Fedora (we don't even have the musl toolchain targets available). All conditionals that rely on %{cargo_target} and / or %{with jailer} are noops, please remove them.
Will I have no discretion as maintainer to provide build options that do not conflict with the guidelines? As was mentioned in the original e-mail thread, Firecracker's security benefits (seccomp, sandboxing) currently only apply to the musl targets. If I can't support a non-default option to use it, I will have to maintain a secure version of the spec separately. I don't see a downside to allowing other developers to run "rpmbuild -D ..." to produce the full build as needed.
If you want to continue maintaining the conditionals even though they will not be used in Fedora, that is your decision. I based my recommendation on this rule: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility But in this case, the additions are not *too bad*, so I'm fine with you keeping them.
- The Source URLs use the old and deprecated format that relies on the URL
"#/fragment" hack. Please use the "new" format (which has worked for almost a decade), i.e. Source0: https://github.com/firecracker-microvm/firecracker/archive/v%%7Bversion%7D/ %{name}-%{version}.tar.gz instead of: Source0: https://github.com/firecracker-microvm/firecracker/archive/refs/tags/ v%{version}.tar.gz#/%{name}-%{version}.tgz Same applies to the URLs for Source1 and Source2.
Documentation for this (both for using tarballs for git tags or for specific commits) is here: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ #_commit_revision
- Strange names for downloads of bundled crates.
I'm not sure why you're downloading kvm-bindings-e835920.tar.gz and micro-http-4b18a04.tar.gz but store them as kvm-bindings-0.6.0-1.crate and micro_http-0.1.0.crate. This is confusing and unnecessary, since you control all parts of this process, there's no need to name them as if they were upstream crates.
URLs are updated.
Thanks! You could even use shortened hashes, but that's a stylistic choice - i.e. this would work fine too, and would be less verbose:
Source1: https://github.com/firecracker-microvm/kvm-bindings/archive/e835920/kvm-bind... Source2: https://github.com/firecracker-microvm/micro-http/archive/4b18a04/micro_http...
- Patches are included without comments or explanations.
Please include short comments about what your patches do to the spec file, and whether they are suitable for upstreaming (or not).
Comments are added to the spec.
Thanks! Looks good to me.
- Don't ignore test failures.
It's OK to ignore specific tests that don't work in our containerized build environment, but "|| :"-ing everything is not OK.
I did that to follow what the rust package does.
https://src.fedoraproject.org/rpms/rust/blob/ 7ac7a42b5e2fd46d73d2a0946c55109c03e22966/f/rust.spec#_874-886
There are over 100 test failures on x86_64 alone, and the Rust test --skip flag seems broken for doc tests. (I want to use --exact instead of substring filtering since there are so many tests, and e.g. --skip=filter_cpuid doesn't match the exact test name, but the full string --skip='src/lib.rs - filter_cpuid (line 43)' causes every doc test for every package to be skipped.)
I've dropped the "|| :" and disabled tests by default instead since that seems acceptable judging by similar packages, although we won't see results in the log anymore.
Makes sense to me. Running the tests but ignoring the results is usually a waste of resources IMO ... In cases like this where test results are almost entirely useless due to failures in the containerized build environment, it makes more sense to disable them by default.
- Include breakdown of the dependencies / licenses.
The License tag looks comprehensive, but there's no indication how this list was generated (you're not using the %cargo_license or %cargo_license_summary macros in the spec file), and there's no breakdown that lists which dependency is available under which license.
It was generated by copying the rust-* package dependencies out of a build log and running this (and manually removing any equivalent SPDX expressions):
dnf repoquery --queryformat='%{LICENSE}\n' $(<deps.txt) | sort -u
I've added a LICENSE.dependencies file from %cargo_license. I'm assuming that's what it's supposed to do based on how a couple packages use it--I don't see it mentioned in the guidelines, and Google has zero results for 'fedora "cargo_license"'.
I've also dropped ISC from the License field since that's only on libloading (used by bindgen, not linked into the binaries).
Yeah, as you have noticed, the "dnf repoquery" command you used for creating the list includes *too many things* (i.e. it also includes crates that are only used at build time, like [build-dependencies] and crates that only provide macros (like bindgen + its dependency tree). It also references things in a mix of old Fedora style license identifiers and new SPDX license identifiers, because not all Rust packages have been migrated yet.
The %cargo_license(_summary) macros are supposed to be solving exactly this problem - get the tree of cargo dependencies *excluding* dev-, build-, and proc-macro-only dependencies, and print the result in SPDX format *only*. It's still pretty new and I still need to document it ...
BuildRequires: rust-packaging
This is also outdated, the new package name is cargo-rpm-macros (and the %cargo_license macro is only present in rust-packaging / cargo-rpm-macros >= 23), and cargo2rpm that you use to generate BuildRequires is only present with "cargo-rpm-macros >= 24", so it should probably just be "BuildRequires: cargo-rpm-macros >= 24".
Other than these few small-ish things, the package looks pretty good to me now, thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #5 from fedora.dm0@gmail.com --- Thanks, spec and SRPM are updated. (The SRPM URL switched to fc38.)
I actually started with cargo-rpm-macros as the dependency but changed it to follow the guidelines, so I suppose this line needs to be updated:
All Rust packages MUST have BuildRequires: rust-packaging.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #6 from Fabio Valentini decathorpe@gmail.com --- (In reply to fedora.dm0 from comment #5)
Thanks, spec and SRPM are updated. (The SRPM URL switched to fc38.)
I actually started with cargo-rpm-macros as the dependency but changed it to follow the guidelines, so I suppose this line needs to be updated:
All Rust packages MUST have BuildRequires: rust-packaging.
Meh, you're right. This is outdated. I need to update the Rust packaging guidelines anyway :(
Package looks good to me now (with two small things that I'll mention below).
===
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. Note: No gcc, gcc-c++ or clang found in BuildRequires See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
(This is a false positive for Rust crates that also build C / Assembly with the "cc" crate, which pulls in gcc automatically.)
- Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 5079040 bytes in 73 files. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_documentation
(This is an actual "problem".)
===== MUST items =====
Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. Note: Documented in generated file instead. [x]: %build honors applicable compiler flags or justifies otherwise. [!]: Package contains no bundled libraries without FPC exception. Note: FPC exception is no longer required, but bundled libraries need to be declared. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [-]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. Note: Not tested. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Spec use %global instead of %define unless justified. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Package should compile and build into binary rpms on all supported architectures.
===== EXTRA items =====
Generic: [!]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Rpmlint is run on debuginfo package(s). [x]: Rpmlint is run on all installed packages. [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: firecracker-1.3.1-1.fc39.x86_64.rpm firecracker-debuginfo-1.3.1-1.fc39.x86_64.rpm firecracker-debugsource-1.3.1-1.fc39.x86_64.rpm firecracker-1.3.1-1.fc39.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmp2bhl0wgy')] checks: 31, packages: 4
firecracker.x86_64: W: package-with-huge-docs 62% firecracker.x86_64: W: no-manual-page-for-binary firecracker firecracker.x86_64: W: no-manual-page-for-binary rebase-snap firecracker.x86_64: W: no-manual-page-for-binary seccompiler-bin firecracker.src: W: invalid-license (Apache-2.0 firecracker.x86_64: W: invalid-license (Apache-2.0 firecracker-debuginfo.x86_64: W: invalid-license (Apache-2.0 firecracker-debugsource.x86_64: W: invalid-license (Apache-2.0 4 packages and 0 specfiles checked; 0 errors, 8 warnings, 0 badness; has taken 0.5 s
Rpmlint (debuginfo) ------------------- Checking: firecracker-debuginfo-1.3.1-1.fc39.x86_64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpaybupi8x')] checks: 31, packages: 1
firecracker-debuginfo.x86_64: W: invalid-license (Apache-2.0 1 packages and 0 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.2 s
Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 3
firecracker.x86_64: W: package-with-huge-docs 62% firecracker.x86_64: W: no-manual-page-for-binary firecracker firecracker.x86_64: W: no-manual-page-for-binary rebase-snap firecracker.x86_64: W: no-manual-page-for-binary seccompiler-bin firecracker-debuginfo.x86_64: W: invalid-license (Apache-2.0 firecracker-debugsource.x86_64: W: invalid-license (Apache-2.0 firecracker.x86_64: W: invalid-license (Apache-2.0 3 packages and 0 specfiles checked; 0 errors, 7 warnings, 0 badness; has taken 0.6 s
Source checksums ---------------- https://github.com/firecracker-microvm/micro-http/archive/4b18a043e997da5b5f... : CHECKSUM(SHA256) this package : 7540fd4bb5be024c2227ce12af3e5a7822389e3e63d66986c03053e8df515fe7 CHECKSUM(SHA256) upstream package : 7540fd4bb5be024c2227ce12af3e5a7822389e3e63d66986c03053e8df515fe7 https://github.com/firecracker-microvm/kvm-bindings/archive/e8359204b41d5c2e... : CHECKSUM(SHA256) this package : d7976008ea568bc82963a8ad5b3835da903d24335a010455de6dbd9ceb97179b CHECKSUM(SHA256) upstream package : d7976008ea568bc82963a8ad5b3835da903d24335a010455de6dbd9ceb97179b https://github.com/firecracker-microvm/firecracker/archive/v1.3.1/firecracke... : CHECKSUM(SHA256) this package : f9295c3738b07c503a43977fa19633dc4197a90092b3b88346cd10713ff95bed CHECKSUM(SHA256) upstream package : f9295c3738b07c503a43977fa19633dc4197a90092b3b88346cd10713ff95bed
Requires -------- firecracker (rpmlib, GLIBC filtered): libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) rtld(GNU_HASH)
firecracker-debuginfo (rpmlib, GLIBC filtered):
firecracker-debugsource (rpmlib, GLIBC filtered):
Provides -------- firecracker: firecracker firecracker(x86-64)
firecracker-debuginfo: debuginfo(build-id) firecracker-debuginfo firecracker-debuginfo(x86-64)
firecracker-debugsource: firecracker-debugsource firecracker-debugsource(x86-64)
Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23 Command line :/usr/bin/fedora-review -b 2181039 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, fonts, PHP, Python, R, SugarActivity, Ocaml, Haskell, Perl Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
===
Two minor issues remaining:
1. Bundled crates are not declared. You will need to add something like this (please fix and adapt the version strings as necessary):
# kvm-bindings: Apache-2.0 Provides: bundled(crate(kvm-bindings)) = 0.6.0^gite835920 # micro_http: Apache-2.0 Provides: bundled(crate(micro_http)) = 0^git4b18a04
2. Docs are rather large compared to the rest of the package: firecracker.x86_64: W: package-with-huge-docs 62%
The suggested action here would be to add a separate "-doc" subpackage and move large documentation there, but this is not a MUST if you want to keep these files in the main package (most likely this would be the /docs/ directory, which is ~5 MB, while the rest of the package and *all* binaries combined are only ~3 MB).
===
As far as I can tell right now, these two are the last issues, and the package is ready for approval otherwise.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #7 from fedora.dm0@gmail.com --- The spec and SRPM are updated. Most of the documentation size was from included images like alternate logos that weren't being used, so I removed unreferenced images which keeps the useful diagrams but drops enough megabytes to fix the issue.
Also I have example commands on the Copr page if you need to test running it ( https://copr.fedorainfracloud.org/coprs/dm0/Firecracker ). You can run "reboot" in the guest to kill the VM process.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #8 from Fabio Valentini decathorpe@gmail.com --- Thanks, looks good to me! Package APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #9 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/firecracker
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
fedora.dm0@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |RAWHIDE Last Closed| |2023-04-21 14:22:50
--- Comment #10 from fedora.dm0@gmail.com --- Thanks again, the package has been added.
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-dca8124d3b has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-dca8124d3b
https://bugzilla.redhat.com/show_bug.cgi?id=2181039
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-dca8124d3b 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-dca8124d3b *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-dca8124d3b
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=2181039
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-edcbcf18e0 has been pushed to the Fedora 38 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-edcbcf18e0 *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-edcbcf18e0
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
package-review@lists.fedoraproject.org