Bug 2181039 - Review Request: firecracker - Secure and fast microVMs for serverless computing
Summary: Review Request: firecracker - Secure and fast microVMs for serverless computing
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://firecracker-microvm.github.io/
Whiteboard:
Depends On: 2181020 2181021 2181022 2181023 2181025 2181028 2181029 2181030 2181033 2181035 2181036 2181038
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-03-22 21:59 UTC by fedora.dm0
Modified: 2023-04-22 01:41 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-04-21 14:22:50 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Description fedora.dm0 2023-03-22 21:59:40 UTC
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.fc38.src.rpm
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

Comment 1 Jakub Kadlčík 2023-03-22 22:03:34 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5696296
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2181039-firecracker/fedora-rawhide-x86_64/05696296-firecracker/builder-live.log.gz

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.

Comment 2 Fabio Valentini 2023-04-07 15:26:49 UTC
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%{version}/%{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

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.

Comment 3 fedora.dm0 2023-04-07 22:45:37 UTC
The spec and SRPM are updated.

(In reply to Fabio Valentini from comment #2)
> 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.

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/firecracker/Cargo.toml#L6

I've just changed %generate_buildrequires to always pass --with-check and dropped the comment.

> 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.

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.

> 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%{version}/
> %{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
> 
> 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.

URLs are updated.

> 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).

Comments are added to the spec.

> 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.

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.

> 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.

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).

Comment 4 Fabio Valentini 2023-04-19 20:32:49 UTC
(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)
> > 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.
> 
> 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.
> 
> > 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.
> 
> 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.

> > 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%{version}/
> > %{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
> > 
> > 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.
> 
> 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-bindings-e835920.tar.gz
Source2:        https://github.com/firecracker-microvm/micro-http/archive/4b18a04/micro_http-4b18a04.tar.gz

> > 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).
> 
> Comments are added to the spec.

Thanks! Looks good to me.

> > 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.
> 
> 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.

> > 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.
> 
> 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!

Comment 5 fedora.dm0 2023-04-19 22:13:49 UTC
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.

Comment 6 Fabio Valentini 2023-04-20 21:11:11 UTC
(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/4b18a043e997da5b5f679e3defc279fec908753e/micro_http-4b18a04.tar.gz :
  CHECKSUM(SHA256) this package     : 7540fd4bb5be024c2227ce12af3e5a7822389e3e63d66986c03053e8df515fe7
  CHECKSUM(SHA256) upstream package : 7540fd4bb5be024c2227ce12af3e5a7822389e3e63d66986c03053e8df515fe7
https://github.com/firecracker-microvm/kvm-bindings/archive/e8359204b41d5c2e7c5af9ae5c26283b62337740/kvm-bindings-e835920.tar.gz :
  CHECKSUM(SHA256) this package     : d7976008ea568bc82963a8ad5b3835da903d24335a010455de6dbd9ceb97179b
  CHECKSUM(SHA256) upstream package : d7976008ea568bc82963a8ad5b3835da903d24335a010455de6dbd9ceb97179b
https://github.com/firecracker-microvm/firecracker/archive/v1.3.1/firecracker-1.3.1.tar.gz :
  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.

Comment 7 fedora.dm0 2023-04-21 00:24:00 UTC
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.

Comment 8 Fabio Valentini 2023-04-21 12:43:29 UTC
Thanks, looks good to me!
Package APPROVED.

Comment 9 Fedora Admin user for bugzilla script actions 2023-04-21 13:30:28 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/firecracker

Comment 10 fedora.dm0 2023-04-21 14:22:50 UTC
Thanks again, the package has been added.

Comment 11 Fedora Update System 2023-04-21 14:39:54 UTC
FEDORA-2023-dca8124d3b has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-dca8124d3b

Comment 12 Fedora Update System 2023-04-21 14:39:54 UTC
FEDORA-2023-edcbcf18e0 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-edcbcf18e0

Comment 13 Fedora Update System 2023-04-22 01:39:43 UTC
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.

Comment 14 Fedora Update System 2023-04-22 01:41:27 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.