Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/dorinda/vhost-device-sound/rust-vhost-device-sound.git/plain/rust-vhost-device-sound.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/dorinda/vhost-device-sound/fedora-rawhide-x86_64/07437489-rust-vhost-device-sound/rust-vhost-device-sound-0.1.0-1.fc41.src.rpm Description: Vhost-user SOUND backend device Fedora Account System Username: dorinda
I would like someone from RUST team to review as well, but here's my review: - Add issues you find to the list of issues on top. If there isn't such a list, create one. - Add your own remarks to the template checks. - Add new lines marked [!] or [?] when you discover new things not listed by fedora-review. - Change or remove any text in the template which is plain wrong. In this case you could also file a bug against fedora-review - Remove the "[ ] Manual check required", you will not have any such lines in what you paste. - Remove attachments which you deem not really useful (the rpmlint ones are mandatory, though) - Remove this text Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Package contains duplicates in %files! Note: warning: File listed twice: /usr/share/cargo/registry/vhost-device- sound-0.1.0/CHANGELOG.md See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_duplicate_files - Man page is shipped in devel package instead of in the main package. - See [!] comments below. - I think there's some packaging issue with devel packages for the backends. I see for example: ``` $ rpm -qlp rust-vhost-device-sound+pw-devel-0.1.0-1.fc41.x86_64.rpm /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml $ rpm -qlp rust-vhost-device-sound+alsa-backend-devel-0.1.0-1.fc41.x86_64.rpm /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml $ rpm -qlp rust-vhost-device-sound-devel-0.1.0-1.fc41.x86_64.rpm /usr/bin/vhost-device-sound /usr/lib/.build-id /usr/lib/.build-id/90/3d5dba274233482f13c18992668a913053a95d /usr/share/cargo/registry/vhost-device-sound-0.1.0 /usr/share/cargo/registry/vhost-device-sound-0.1.0/.cargo-checksum.json /usr/share/cargo/registry/vhost-device-sound-0.1.0/CHANGELOG.md /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml /usr/share/cargo/registry/vhost-device-sound-0.1.0/LICENSE-APACHE /usr/share/cargo/registry/vhost-device-sound-0.1.0/LICENSE-BSD-3-Clause /usr/share/cargo/registry/vhost-device-sound-0.1.0/LICENSE.dependencies /usr/share/cargo/registry/vhost-device-sound-0.1.0/README.md /usr/share/cargo/registry/vhost-device-sound-0.1.0/rustfmt.toml /usr/share/cargo/registry/vhost-device-sound-0.1.0/src /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa/test_utils.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/null.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/pipewire /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/pipewire.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/pipewire/test_utils.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/device.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/lib.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/main.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/stream.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/virtio_sound.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/vhost-device-sound.1 /usr/share/man/man1/vhost-device-sound.1.gz ``` I would have expected different content there. ===== 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. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* Apache License 2.0", "BSD 3-Clause License", "*No copyright* Apache License (v2.0) or bsd_-3-Clause_clause". 8 files have unknown license. Detailed output of licensecheck in licensecheck.txt [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. [x]: Package requires other packages for directories it uses. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: 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. Note: package is requiring an ExcludeArch and provides justification for it [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]: The License field must be a valid SPDX expression. [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]: 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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 1221 bytes in 2 files. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [x]: 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). [?]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in vhost- device-sound , rust-vhost-device-sound-devel , rust-vhost-device- sound+default-devel , rust-vhost-device-sound+alsa-backend-devel , rust-vhost-device-sound+pw-devel , rust-vhost-device-sound+pw-backend- devel , rust-vhost-device-sound+xen-devel [x]: Package functions as described. [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. Note: gpgverify is not used. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. Note: in %install you're installing a man page, just add -p to the install command to preserve the timestamp. [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]: 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]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: vhost-device-sound-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound-devel-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound+default-devel-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound+alsa-backend-devel-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound+pw-devel-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound+pw-backend-devel-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound+xen-devel-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound-debugsource-0.1.0-1.fc41.x86_64.rpm rust-vhost-device-sound-0.1.0-1.fc41.src.rpm =========================================================================== rpmlint session starts ========================================================================== rpmlint: 2.5.0 configuration: /usr/lib/python3.12/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/tmplj91uhk_')] checks: 32, packages: 9 rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound.src: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound.src: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') vhost-device-sound.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') vhost-device-sound.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound.spec:124: W: setup-not-quiet vhost-device-sound.x86_64: W: no-manual-page-for-binary vhost-device-sound rust-vhost-device-sound+alsa-backend-devel.x86_64: W: no-documentation rust-vhost-device-sound+default-devel.x86_64: W: no-documentation rust-vhost-device-sound+pw-backend-devel.x86_64: W: no-documentation rust-vhost-device-sound+pw-devel.x86_64: W: no-documentation rust-vhost-device-sound+xen-devel.x86_64: W: no-documentation rust-vhost-device-sound.spec: W: %ifarch-applied-patch Patch2: build-fix-for-i386.patch ==================================== 9 packages and 0 specfiles checked; 16 errors, 8 warnings, 44 filtered, 16 badness; has taken 0.8 s ==================================== Rpmlint (debuginfo) ------------------- Checking: rust-vhost-device-sound-devel-debuginfo-0.1.0-1.fc41.x86_64.rpm =========================================================================== rpmlint session starts ========================================================================== rpmlint: 2.5.0 configuration: /usr/lib/python3.12/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/tmp2j90pp_6')] checks: 32, packages: 1 ====================================== 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 5 filtered, 0 badness; has taken 0.2 s ===================================== Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.12/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: 32, packages: 9 rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') vhost-device-sound.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') vhost-device-sound.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') rust-vhost-device-sound-devel.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') rust-vhost-device-sound-devel.x86_64: E: spelling-error ('virtio', '%description -l en_US virtio -> virtuoso') vhost-device-sound.x86_64: W: no-manual-page-for-binary vhost-device-sound rust-vhost-device-sound+pw-devel.x86_64: W: no-documentation rust-vhost-device-sound+xen-devel.x86_64: W: no-documentation rust-vhost-device-sound+default-devel.x86_64: W: no-documentation rust-vhost-device-sound+alsa-backend-devel.x86_64: W: no-documentation rust-vhost-device-sound+pw-backend-devel.x86_64: W: no-documentation 9 packages and 0 specfiles checked; 14 errors, 6 warnings, 46 filtered, 14 badness; has taken 0.6 s Source checksums ---------------- https://crates.io/api/v1/crates/vhost-device-sound/0.1.0/download#/vhost-device-sound-0.1.0.crate : CHECKSUM(SHA256) this package : 8d89731cfe36eb3b57b6eec55b7ed0f79e34b34274fd4f672035b548417c7f09 CHECKSUM(SHA256) upstream package : 8d89731cfe36eb3b57b6eec55b7ed0f79e34b34274fd4f672035b548417c7f09 Requires -------- vhost-device-sound (rpmlib, GLIBC filtered): ld-linux-x86-64.so.2()(64bit) libasound.so.2()(64bit) libasound.so.2(ALSA_0.9)(64bit) libasound.so.2(ALSA_0.9.0rc4)(64bit) 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) libpipewire-0.3.so.0()(64bit) rtld(GNU_HASH) rust-vhost-device-sound-devel (rpmlib, GLIBC filtered): (crate(clap/default) >= 4.4.0 with crate(clap/default) < 5.0.0~) (crate(clap/derive) >= 4.4.0 with crate(clap/derive) < 5.0.0~) (crate(env_logger/default) >= 0.11.0 with crate(env_logger/default) < 0.12.0~) (crate(log/default) >= 0.4.0 with crate(log/default) < 0.5.0~) (crate(thiserror/default) >= 1.0.0 with crate(thiserror/default) < 2.0.0~) (crate(vhost-user-backend/default) >= 0.13.0 with crate(vhost-user-backend/default) < 0.14.0~) (crate(vhost/default) >= 0.10.0 with crate(vhost/default) < 0.11.0~) (crate(vhost/vhost-user-backend) >= 0.10.0 with crate(vhost/vhost-user-backend) < 0.11.0~) (crate(virtio-bindings/default) >= 0.2.1 with crate(virtio-bindings/default) < 0.3.0~) (crate(virtio-queue/default) >= 0.11.0 with crate(virtio-queue/default) < 0.12.0~) (crate(vm-memory/default) >= 0.14.0 with crate(vm-memory/default) < 0.15.0~) (crate(vmm-sys-util/default) >= 0.12.0 with crate(vmm-sys-util/default) < 0.13.0~) cargo ld-linux-x86-64.so.2()(64bit) libasound.so.2()(64bit) libasound.so.2(ALSA_0.9)(64bit) libasound.so.2(ALSA_0.9.0rc4)(64bit) 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) libpipewire-0.3.so.0()(64bit) rtld(GNU_HASH) rust-vhost-device-sound+default-devel (rpmlib, GLIBC filtered): cargo crate(vhost-device-sound) crate(vhost-device-sound/alsa-backend) crate(vhost-device-sound/pw-backend) rust-vhost-device-sound+alsa-backend-devel (rpmlib, GLIBC filtered): (crate(alsa/default) >= 0.8.0 with crate(alsa/default) < 0.9.0~) cargo crate(vhost-device-sound) rust-vhost-device-sound+pw-devel (rpmlib, GLIBC filtered): (crate(pipewire/default) >= 0.8.0 with crate(pipewire/default) < 0.9.0~) cargo crate(vhost-device-sound) rust-vhost-device-sound+pw-backend-devel (rpmlib, GLIBC filtered): cargo crate(vhost-device-sound) crate(vhost-device-sound/pw) rust-vhost-device-sound+xen-devel (rpmlib, GLIBC filtered): (crate(vhost-user-backend/xen) >= 0.13.0 with crate(vhost-user-backend/xen) < 0.14.0~) (crate(vhost/xen) >= 0.10.0 with crate(vhost/xen) < 0.11.0~) (crate(vm-memory/xen) >= 0.14.0 with crate(vm-memory/xen) < 0.15.0~) cargo crate(vhost-device-sound) rust-vhost-device-sound-debugsource (rpmlib, GLIBC filtered): Provides -------- vhost-device-sound: vhost-device-sound vhost-device-sound(x86-64) rust-vhost-device-sound-devel: crate(vhost-device-sound) rust-vhost-device-sound-devel rust-vhost-device-sound-devel(x86-64) rust-vhost-device-sound+default-devel: crate(vhost-device-sound/default) rust-vhost-device-sound+default-devel rust-vhost-device-sound+default-devel(x86-64) rust-vhost-device-sound+alsa-backend-devel: crate(vhost-device-sound/alsa-backend) rust-vhost-device-sound+alsa-backend-devel rust-vhost-device-sound+alsa-backend-devel(x86-64) rust-vhost-device-sound+pw-devel: crate(vhost-device-sound/pw) rust-vhost-device-sound+pw-devel rust-vhost-device-sound+pw-devel(x86-64) rust-vhost-device-sound+pw-backend-devel: crate(vhost-device-sound/pw-backend) rust-vhost-device-sound+pw-backend-devel rust-vhost-device-sound+pw-backend-devel(x86-64) rust-vhost-device-sound+xen-devel: crate(vhost-device-sound/xen) rust-vhost-device-sound+xen-devel rust-vhost-device-sound+xen-devel(x86-64) rust-vhost-device-sound-debugsource: rust-vhost-device-sound-debugsource rust-vhost-device-sound-debugsource(x86-64) Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -b 2274028 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic Disabled plugins: Java, Python, Perl, fonts, R, Ruby, SugarActivity, PHP, Haskell, Ocaml, C/C++ Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
I took a quick peek and have the following comments: * Both patches ("ignore-pw-server-test.patch" and "build-fix-for-alsa-test.patch") simply add the `ignore` attribute to some tests. Therefore I see them as redundant and would just skip them in the `%check` section like you did with `result::tests::async_seq_panic`. * The patch for `i386` should be called "build-fix-for-i686", since this is what we actually build in Fedora. `i386` is only used in copr. * Is the `i386` patch forwarded to upstream? I think it might be useful to them too. * Why do all packages have "BuildArch: noarch"? * The man page should not be added by using a patch. Use `Source` and copy the man page in the `%install` section. Also this can / should be forwarded to upstream. * The comment "# FIXME: paste output of %%cargo_license_summary here" should be removed since you wrote down the output of `%cargo_license_summary`. * Please add the output of "%cargo_license_summary" as a comment in the spec file above `License`. We do this for all Rust packages and it makes it easier to double check the license tags. * Is there a specific reason for including the binary file and the man page in the devel subpackage? I think this was added by mistake since the binary package does not contain the man page. > I think there's some packaging issue with devel packages for the backends. I see for example: > I would have expected different content there. The output looks correct to me. What content would you have expected?
(In reply to Sandro Bonazzola from comment #1) > I would like someone from RUST team to review as well, but here's my review: > > > - Add issues you find to the list of issues on top. If there isn't such > a list, create one. > - Add your own remarks to the template checks. > - Add new lines marked [!] or [?] when you discover new things not > listed by fedora-review. > - Change or remove any text in the template which is plain wrong. In this > case you could also file a bug against fedora-review > - Remove the "[ ] Manual check required", you will not have any such lines > in what you paste. > - Remove attachments which you deem not really useful (the rpmlint > ones are mandatory, though) > - Remove this text > > > > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > [ ] = Manual review needed > > > Issues: > ======= > - Package contains duplicates in %files! > Note: warning: File listed twice: /usr/share/cargo/registry/vhost-device- > sound-0.1.0/CHANGELOG.md > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/#_duplicate_files > - Man page is shipped in devel package instead of in the main package. > - See [!] comments below. > - I think there's some packaging issue with devel packages for the backends. > I see for example: > ``` > $ rpm -qlp rust-vhost-device-sound+pw-devel-0.1.0-1.fc41.x86_64.rpm > /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml > $ rpm -qlp > rust-vhost-device-sound+alsa-backend-devel-0.1.0-1.fc41.x86_64.rpm > /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml > $ rpm -qlp rust-vhost-device-sound-devel-0.1.0-1.fc41.x86_64.rpm > /usr/bin/vhost-device-sound > /usr/lib/.build-id > /usr/lib/.build-id/90/3d5dba274233482f13c18992668a913053a95d > /usr/share/cargo/registry/vhost-device-sound-0.1.0 > /usr/share/cargo/registry/vhost-device-sound-0.1.0/.cargo-checksum.json > /usr/share/cargo/registry/vhost-device-sound-0.1.0/CHANGELOG.md > /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml > /usr/share/cargo/registry/vhost-device-sound-0.1.0/LICENSE-APACHE > /usr/share/cargo/registry/vhost-device-sound-0.1.0/LICENSE-BSD-3-Clause > /usr/share/cargo/registry/vhost-device-sound-0.1.0/LICENSE.dependencies > /usr/share/cargo/registry/vhost-device-sound-0.1.0/README.md > /usr/share/cargo/registry/vhost-device-sound-0.1.0/rustfmt.toml > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends.rs > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa.rs > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa/ > test_utils.rs > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/null.rs > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/ > pipewire > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/ > pipewire.rs > > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/ > pipewire/test_utils.rs > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/device.rs > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/lib.rs > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/main.rs > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/stream.rs > /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/virtio_sound.rs > /usr/share/cargo/registry/vhost-device-sound-0.1.0/vhost-device-sound.1 > /usr/share/man/man1/vhost-device-sound.1.gz > ``` > I would have expected different content there. > > > ===== 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. > Note: Checking patched sources after %prep for licenses. Licenses > found: "Unknown or generated", "*No copyright* Apache License 2.0", > "BSD 3-Clause License", "*No copyright* Apache License (v2.0) or > bsd_-3-Clause_clause". 8 files have unknown license. Detailed output > of licensecheck in licensecheck.txt > [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. > [x]: Package requires other packages for directories it uses. > [x]: %build honors applicable compiler flags or justifies otherwise. > [x]: Package contains no bundled libraries without FPC exception. > [x]: Changelog in prescribed format. > [x]: Sources contain only permissible code or content. > [-]: Package contains desktop file if it is a GUI application. > [x]: 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. > Note: package is requiring an ExcludeArch and provides justification > for it > [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]: The License field must be a valid SPDX expression. > [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]: 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]: Large documentation must go in a -doc subpackage. Large could be size > (~1MB) or number of files. > Note: Documentation size is 1221 bytes in 2 files. > [x]: Packages must not store files under /srv, /opt or /usr/local > > ===== SHOULD items ===== > > Generic: > [x]: 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). > [?]: Fully versioned dependency in subpackages if applicable. > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in vhost- > device-sound , rust-vhost-device-sound-devel , rust-vhost-device- > sound+default-devel , rust-vhost-device-sound+alsa-backend-devel , > rust-vhost-device-sound+pw-devel , rust-vhost-device-sound+pw-backend- > devel , rust-vhost-device-sound+xen-devel > [x]: Package functions as described. > [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. > Note: gpgverify is not used. > [x]: Package should compile and build into binary rpms on all supported > architectures. > [x]: %check is present and all tests pass. > [!]: Packages should try to preserve timestamps of original installed > files. > Note: in %install you're installing a man page, just add -p to the > install command to preserve the timestamp. > [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]: 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]: Spec use %global instead of %define unless justified. > > ===== EXTRA items ===== > > Generic: > [x]: Rpmlint is run on debuginfo package(s). > Note: No rpmlint messages. > [x]: Rpmlint is run on all installed packages. > Note: There are rpmlint messages (see attachment). > [x]: Large data in /usr/share should live in a noarch subpackage if package > is arched. > [x]: Spec file according to URL is the same as in SRPM. > > > Rpmlint > ------- > Checking: vhost-device-sound-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound-devel-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound+default-devel-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound+alsa-backend-devel-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound+pw-devel-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound+pw-backend-devel-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound+xen-devel-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound-debugsource-0.1.0-1.fc41.x86_64.rpm > rust-vhost-device-sound-0.1.0-1.fc41.src.rpm > =========================================================================== > rpmlint session starts > ========================================================================== > rpmlint: 2.5.0 > configuration: > /usr/lib/python3.12/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/tmplj91uhk_')] > checks: 32, packages: 9 > > rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error > ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error > ('virtio', '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error > ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error > ('virtio', '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound.src: E: spelling-error ('Virtio', 'Summary(en_US) > Virtio -> Virtuoso') > rust-vhost-device-sound.src: E: spelling-error ('virtio', '%description -l > en_US virtio -> virtuoso') > rust-vhost-device-sound-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > vhost-device-sound.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) > Virtio -> Virtuoso') > vhost-device-sound.x86_64: E: spelling-error ('virtio', '%description -l > en_US virtio -> virtuoso') > rust-vhost-device-sound.spec:124: W: setup-not-quiet > vhost-device-sound.x86_64: W: no-manual-page-for-binary vhost-device-sound > rust-vhost-device-sound+alsa-backend-devel.x86_64: W: no-documentation > rust-vhost-device-sound+default-devel.x86_64: W: no-documentation > rust-vhost-device-sound+pw-backend-devel.x86_64: W: no-documentation > rust-vhost-device-sound+pw-devel.x86_64: W: no-documentation > rust-vhost-device-sound+xen-devel.x86_64: W: no-documentation > rust-vhost-device-sound.spec: W: %ifarch-applied-patch Patch2: > build-fix-for-i386.patch > ==================================== 9 packages and 0 specfiles checked; 16 > errors, 8 warnings, 44 filtered, 16 badness; has taken 0.8 s > ==================================== > > > > > Rpmlint (debuginfo) > ------------------- > Checking: rust-vhost-device-sound-devel-debuginfo-0.1.0-1.fc41.x86_64.rpm > =========================================================================== > rpmlint session starts > ========================================================================== > rpmlint: 2.5.0 > configuration: > /usr/lib/python3.12/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/tmp2j90pp_6')] > checks: 32, packages: 1 > > ====================================== 1 packages and 0 specfiles checked; 0 > errors, 0 warnings, 5 filtered, 0 badness; has taken 0.2 s > ===================================== > > > > > > Rpmlint (installed packages) > ---------------------------- > ============================ rpmlint session starts > ============================ > rpmlint: 2.5.0 > configuration: > /usr/lib/python3.12/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: 32, packages: 9 > > rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+pw-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+xen-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > vhost-device-sound.x86_64: E: spelling-error ('Virtio', 'Summary(en_US) > Virtio -> Virtuoso') > vhost-device-sound.x86_64: E: spelling-error ('virtio', '%description -l > en_US virtio -> virtuoso') > rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+default-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error > ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+alsa-backend-devel.x86_64: E: spelling-error > ('virtio', '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error > ('Virtio', 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound+pw-backend-devel.x86_64: E: spelling-error > ('virtio', '%description -l en_US virtio -> virtuoso') > rust-vhost-device-sound-devel.x86_64: E: spelling-error ('Virtio', > 'Summary(en_US) Virtio -> Virtuoso') > rust-vhost-device-sound-devel.x86_64: E: spelling-error ('virtio', > '%description -l en_US virtio -> virtuoso') > vhost-device-sound.x86_64: W: no-manual-page-for-binary vhost-device-sound > rust-vhost-device-sound+pw-devel.x86_64: W: no-documentation > rust-vhost-device-sound+xen-devel.x86_64: W: no-documentation > rust-vhost-device-sound+default-devel.x86_64: W: no-documentation > rust-vhost-device-sound+alsa-backend-devel.x86_64: W: no-documentation > rust-vhost-device-sound+pw-backend-devel.x86_64: W: no-documentation > 9 packages and 0 specfiles checked; 14 errors, 6 warnings, 46 filtered, 14 > badness; has taken 0.6 s > > > > Source checksums > ---------------- > https://crates.io/api/v1/crates/vhost-device-sound/0.1.0/download#/vhost- > device-sound-0.1.0.crate : > CHECKSUM(SHA256) this package : > 8d89731cfe36eb3b57b6eec55b7ed0f79e34b34274fd4f672035b548417c7f09 > CHECKSUM(SHA256) upstream package : > 8d89731cfe36eb3b57b6eec55b7ed0f79e34b34274fd4f672035b548417c7f09 > > > Requires > -------- > vhost-device-sound (rpmlib, GLIBC filtered): > ld-linux-x86-64.so.2()(64bit) > libasound.so.2()(64bit) > libasound.so.2(ALSA_0.9)(64bit) > libasound.so.2(ALSA_0.9.0rc4)(64bit) > 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) > libpipewire-0.3.so.0()(64bit) > rtld(GNU_HASH) > > rust-vhost-device-sound-devel (rpmlib, GLIBC filtered): > (crate(clap/default) >= 4.4.0 with crate(clap/default) < 5.0.0~) > (crate(clap/derive) >= 4.4.0 with crate(clap/derive) < 5.0.0~) > (crate(env_logger/default) >= 0.11.0 with crate(env_logger/default) < > 0.12.0~) > (crate(log/default) >= 0.4.0 with crate(log/default) < 0.5.0~) > (crate(thiserror/default) >= 1.0.0 with crate(thiserror/default) < > 2.0.0~) > (crate(vhost-user-backend/default) >= 0.13.0 with > crate(vhost-user-backend/default) < 0.14.0~) > (crate(vhost/default) >= 0.10.0 with crate(vhost/default) < 0.11.0~) > (crate(vhost/vhost-user-backend) >= 0.10.0 with > crate(vhost/vhost-user-backend) < 0.11.0~) > (crate(virtio-bindings/default) >= 0.2.1 with > crate(virtio-bindings/default) < 0.3.0~) > (crate(virtio-queue/default) >= 0.11.0 with crate(virtio-queue/default) > < 0.12.0~) > (crate(vm-memory/default) >= 0.14.0 with crate(vm-memory/default) < > 0.15.0~) > (crate(vmm-sys-util/default) >= 0.12.0 with crate(vmm-sys-util/default) > < 0.13.0~) > cargo > ld-linux-x86-64.so.2()(64bit) > libasound.so.2()(64bit) > libasound.so.2(ALSA_0.9)(64bit) > libasound.so.2(ALSA_0.9.0rc4)(64bit) > 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) > libpipewire-0.3.so.0()(64bit) > rtld(GNU_HASH) > > rust-vhost-device-sound+default-devel (rpmlib, GLIBC filtered): > cargo > crate(vhost-device-sound) > crate(vhost-device-sound/alsa-backend) > crate(vhost-device-sound/pw-backend) > > rust-vhost-device-sound+alsa-backend-devel (rpmlib, GLIBC filtered): > (crate(alsa/default) >= 0.8.0 with crate(alsa/default) < 0.9.0~) > cargo > crate(vhost-device-sound) > > rust-vhost-device-sound+pw-devel (rpmlib, GLIBC filtered): > (crate(pipewire/default) >= 0.8.0 with crate(pipewire/default) < 0.9.0~) > cargo > crate(vhost-device-sound) > > rust-vhost-device-sound+pw-backend-devel (rpmlib, GLIBC filtered): > cargo > crate(vhost-device-sound) > crate(vhost-device-sound/pw) > > rust-vhost-device-sound+xen-devel (rpmlib, GLIBC filtered): > (crate(vhost-user-backend/xen) >= 0.13.0 with > crate(vhost-user-backend/xen) < 0.14.0~) > (crate(vhost/xen) >= 0.10.0 with crate(vhost/xen) < 0.11.0~) > (crate(vm-memory/xen) >= 0.14.0 with crate(vm-memory/xen) < 0.15.0~) > cargo > crate(vhost-device-sound) > > rust-vhost-device-sound-debugsource (rpmlib, GLIBC filtered): > > > > Provides > -------- > vhost-device-sound: > vhost-device-sound > vhost-device-sound(x86-64) > > rust-vhost-device-sound-devel: > crate(vhost-device-sound) > rust-vhost-device-sound-devel > rust-vhost-device-sound-devel(x86-64) > > rust-vhost-device-sound+default-devel: > crate(vhost-device-sound/default) > rust-vhost-device-sound+default-devel > rust-vhost-device-sound+default-devel(x86-64) > > rust-vhost-device-sound+alsa-backend-devel: > crate(vhost-device-sound/alsa-backend) > rust-vhost-device-sound+alsa-backend-devel > rust-vhost-device-sound+alsa-backend-devel(x86-64) > > rust-vhost-device-sound+pw-devel: > crate(vhost-device-sound/pw) > rust-vhost-device-sound+pw-devel > rust-vhost-device-sound+pw-devel(x86-64) > > rust-vhost-device-sound+pw-backend-devel: > crate(vhost-device-sound/pw-backend) > rust-vhost-device-sound+pw-backend-devel > rust-vhost-device-sound+pw-backend-devel(x86-64) > > rust-vhost-device-sound+xen-devel: > crate(vhost-device-sound/xen) > rust-vhost-device-sound+xen-devel > rust-vhost-device-sound+xen-devel(x86-64) > > rust-vhost-device-sound-debugsource: > rust-vhost-device-sound-debugsource > rust-vhost-device-sound-debugsource(x86-64) > > > > Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 > Command line :/usr/bin/fedora-review -b 2274028 > Buildroot used: fedora-rawhide-x86_64 > Active plugins: Shell-api, Generic > Disabled plugins: Java, Python, Perl, fonts, R, Ruby, SugarActivity, PHP, > Haskell, Ocaml, C/C++ > Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH fixed: warnings about Package contains duplicates in %files! fixed: Man page is shipped in devel package instead of in the main package. fixed: Packages should try to preserve timestamps of original I don't understand the issue you highlited here `I think there's some packaging issue with devel packages for the backends`
I might misunderstand the purpose of the devel packages with rust language as I don't know the language at all. But from a package named `rust-vhost-device-sound+pw-devel-0.1.0-1.fc41.x86_64.rpm` I would have expected content like: ``` /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/pipewire /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/pipewire.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/pipewire/test_utils.rs ``` and in `rust-vhost-device-sound+alsa-backend-devel-0.1.0-1.fc41.x86_64.rpm` ``` /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa.rs /usr/share/cargo/registry/vhost-device-sound-0.1.0/src/audio_backends/alsa/test_utils.rs ``` instead of having them containing both the same file: ``` /usr/share/cargo/registry/vhost-device-sound-0.1.0/Cargo.toml ``` But as I said, if it's ok for RUST team packagers, it's ok for me.
(In reply to blinxen from comment #2) > I took a quick peek and have the following comments: > > * Both patches ("ignore-pw-server-test.patch" and > "build-fix-for-alsa-test.patch") simply add the `ignore` attribute to some > tests. > Therefore I see them as redundant and would just skip them in the `%check` > section like you did with `result::tests::async_seq_panic`. > * The patch for `i386` should be called "build-fix-for-i686", since this is > what we actually build in Fedora. > `i386` is only used in copr. > * Is the `i386` patch forwarded to upstream? I think it might be useful to > them too. > * Why do all packages have "BuildArch: noarch"? > * The man page should not be added by using a patch. Use `Source` and copy > the man page in the `%install` section. Also this can / should be forwarded > to upstream. > * The comment "# FIXME: paste output of %%cargo_license_summary here" should > be removed since you wrote down the output of `%cargo_license_summary`. > * Please add the output of "%cargo_license_summary" as a comment in the spec > file above `License`. We do this for all Rust packages and it makes it > easier to double check the license tags. > * Is there a specific reason for including the binary file and the man page > in the devel subpackage? I think this was added by mistake since the binary > package does not contain the man page. > > > I think there's some packaging issue with devel packages for the backends. I see for example: > > I would have expected different content there. > > The output looks correct to me. What content would you have expected? fixed: removed patches to ignore tests by skipping them in the `%check` fixed: renamed The patch for `i386` to "build-fix-for-i686" fixed: removed the The comment "# FIXME" added a new comment indicating the License fixed: put binary file and man page in the proper location (main package) > * Why do all packages have "BuildArch: noarch"? the package dependencies specfiles specifies "BuildArch: noarch", I think it'll be better to clarify with the maintainer of those packages. > * The man page should not be added by using a patch. Use `Source` and copy > the man page in the `%install` section. Also this can / should be forwarded > to upstream. for now I let the man page to be a patch, I would send this upstream ASAP.
> I would like someone from RUST team to review as well, but here's my review: Ok, I see several issues with this package, some of which seem to have been added as workarounds to work around other issues ... > I think there's some packaging issue with devel packages for the backends. I see for example: > I would have expected different content there. Why? The "rust-%{crate}-devel" subpackage contains the source code for the crate. The "rust-%{crate}+$feature-devel subpackages are only present for metadata / dependency resolution and are empty. > rust-vhost-device-sound-devel (rpmlib, GLIBC filtered): > (crate(clap/default) >= 4.4.0 with crate(clap/default) < 5.0.0~) > (crate(clap/derive) >= 4.4.0 with crate(clap/derive) < 5.0.0~) > (crate(env_logger/default) >= 0.11.0 with crate(env_logger/default) < 0.12.0~) > (crate(log/default) >= 0.4.0 with crate(log/default) < 0.5.0~) > (crate(thiserror/default) >= 1.0.0 with crate(thiserror/default) < 2.0.0~) > (crate(vhost-user-backend/default) >= 0.13.0 with crate(vhost-user-backend/default) < 0.14.0~) > (crate(vhost/default) >= 0.10.0 with crate(vhost/default) < 0.11.0~) > (crate(vhost/vhost-user-backend) >= 0.10.0 with crate(vhost/vhost-user-backend) < 0.11.0~) > (crate(virtio-bindings/default) >= 0.2.1 with crate(virtio-bindings/default) < 0.3.0~) > (crate(virtio-queue/default) >= 0.11.0 with crate(virtio-queue/default) < 0.12.0~) > (crate(vm-memory/default) >= 0.14.0 with crate(vm-memory/default) < 0.15.0~) > (crate(vmm-sys-util/default) >= 0.12.0 with crate(vmm-sys-util/default) < 0.13.0~) > cargo > ld-linux-x86-64.so.2()(64bit) > libasound.so.2()(64bit) > libasound.so.2(ALSA_0.9)(64bit) > libasound.so.2(ALSA_0.9.0rc4)(64bit) > 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) > libpipewire-0.3.so.0()(64bit) > rtld(GNU_HASH) This looks very weird. The "-devel" subpackage should NEVER be arched or depend on shared libraries. This might happen if the source code accidentally ships binaries (which it MUST NOT, it's supposed to be noarch). The -devel package is also missing README / LICENSE files. Not sure why they were removed from the generated spec file. > * The patch for `i386` should be called "build-fix-for-i686", since this is what we actually build in Fedora. > `i386` is only used in copr. The supported architectures in COPR and koji are the same, just the name is different (for whatever reason). > * Why do all packages have "BuildArch: noarch"? All -devel subpackages should have it, it's normal for Rust packages. They *are* architecture-independent, since these packages either ship source code (architecture-independent!) or are empty and only carry RPM metadata. > * The comment "# FIXME: paste output of %%cargo_license_summary here" should be removed since you wrote down the output of `%cargo_license_summary`. This is wrong, the output of %cargo_license_summary looks different, it's an itemized list. Right now the spec file just contains the same information twice. > fixed: warnings about Package contains duplicates in %files! This is not an issue, it's a useless warning provided by RPM and actually is a false positive. Now the files *are* missing %doc and %license tags. > I might misunderstand the purpose of the devel packages with rust language as I don't know the language at all. > But from a package named `rust-vhost-device-sound+pw-devel-0.1.0-1.fc41.x86_64.rpm` I would have expected content like: Yes, the "rust-$crate+$feature-devel" subpackages are empty and are only present for RPM metadata / dependency resolution. The "rust-$crate-devel" package contains all the actual source code. Splitting source code across multiple packages would break everything. > fixed: removed patches to ignore tests by skipping them in the `%check` This is not working as intended. Now you're just running tests 10 times and skipping *one* different test each time ... the list of skipped tests needs to be passed *to the same %cargo_test* invocation *once*. > the package dependencies specfiles specifies "BuildArch: noarch", > I think it'll be better to clarify with the maintainer of those packages. What do you mean here? The places where "rust2rpm" puts "BuildArch: noarch" should just not be changed at all. ================================================================================ Other issues: Applying a source code patch only on specific architectures will break things (as you have discovered). The source code MUST be architecture-independent. In this case, the code should be fixed to be architecture-agnostic. The fact that the code currently fails to compile on 32-bit architectures should be considered an upstream bug. The source of the issue seems to be that the functions in alsa take differently sized arguments on different architectures: hwp.set_period_size(i64::from(period_frames), alsa::ValueOr::Less) hwp.set_buffer_size_near(2 * i64::from(period_frames)) The type if the first argument is in each case a "Frames", which is defined as "alsa::snd_pcm_sframes_t", which is in turn defined as a C "long" in the alsa header files - which has a different size on different architectures. I would recommend to apply a patch like this instead (this should also be upstreamable): src/audio_backends/alsa.rs:163: - let period_frames = period_bytes / frame_size; + let period_frames = alsa::pcm::Frames::from(period_bytes / frame_size); src/audio_backends/alsa.rs:165: -hwp.set_period_size(i64::from(period_frames), alsa::ValueOr::Less)?; +hwp.set_period_size(period_frames, alsa::ValueOr::Less)?; src/audio_backends/alsa.rs:177: -if let Err(err) = hwp.set_buffer_size_near(2 * i64::from(period_frames)) { +if let Err(err) = hwp.set_buffer_size_near(2 * period_frames) { This patch can be applied on all architectures since it does the correct conversion regardless of target architecture. It makes the code both simpler *and* architecture-agnostic (*and* is simpler than the currently selectively applied patch) so I would recommend to send something like this upstream. At this point I would recommend starting fresh from a spec file generated by "rust2rpm" to drop all workarounds that were mistakenly added to work around *real issues*, and add the missing things from there.
Thank you for the review @decathorpe I just created a new spec file and sent a PR for the alsa patch upstream.
Thank you! Looks much better now, just three minor things left from my side: 1. I wonder why you added "as i32" in the first line of the patch compared to what I suggested? Looking at the code for the types of the variables involved, it's unnecessary on both 64-bit and 32-bit architectures. 2. The output of "%cargo_license_summary" is still missing from the spec file (i.e. the itemized list of licenses, not their AND-combined result). 3. Tests are skipped with "test does not panic" as only explanation. What does this mean? The only reasons where tests won't panic as expected should be when they either mistakenly use debug_assertions instead of normal assertions, or rely on "panic on integer overflow" behaviour that is specific to "debug" build mode. If either of these two issues is the cause for some tests not failing as expected, please document it. If tests fail for other (valid) reasons, it would be good to file an upstream ticket about it.
(In reply to Fabio Valentini from comment #8) > Thank you! Looks much better now, just three minor things left from my side: > > 1. I wonder why you added "as i32" in the first line of the patch compared > to what I suggested? > Looking at the code for the types of the variables involved, it's > unnecessary on both 64-bit and 32-bit architectures. > I added that line because the code still failed on i386 in copr see build error for reference https://download.copr.fedorainfracloud.org/results/dorinda/vhost-device-sound/fedora-rawhide-i386/07286508-rust-vhost-device-sound/builder-live.log.gz build-id - https://copr.fedorainfracloud.org/coprs/dorinda/vhost-device-sound/build/7286508/ > 2. The output of "%cargo_license_summary" is still missing from the spec > file (i.e. the itemized list of licenses, not their AND-combined result). > > 3. Tests are skipped with "test does not panic" as only explanation. What > does this mean? > The only reasons where tests won't panic as expected should be when they > either mistakenly use debug_assertions instead of normal assertions, or rely > on "panic on integer overflow" behaviour that is specific to "debug" build > mode. > > If either of these two issues is the cause for some tests not failing as > expected, please document it. > If tests fail for other (valid) reasons, it would be good to file an > upstream ticket about it. Noted, I will update the spec file
> I added that line because the code still failed on i386 in copr Makes sense then. I could have sworn that I checked whether an additional cast was needed, but I apparently missed this. > Noted, I will update the spec file Thanks!
I updated the spec file following changes were made 1: skipped build on 32 bit architectures because upstream does not allow 32 bit compilation. 2: Updated reasons why test are skipped, also sent PR upstream to fix failing test - https://github.com/rust-vmm/vhost-device/pull/649 3: added the output of "%cargo_license_summary" in spec file
Fabio can you give this another review round? Looks good to me.
Looks OK to me - just replace "i386 i686" with "%{ix86}". Also assuming that the syntax for skipping tests works as espected - in the past there have been issues with splitting %cargo_test arguments across multiple lines - it caused *all* tests to be skipped instead of just the ones specified.
(In reply to Fabio Valentini from comment #13) > Looks OK to me - just replace "i386 i686" with "%{ix86}". > Ok thanks > Also assuming that the syntax for skipping tests works as espected - in the > past there have been issues with splitting %cargo_test arguments across > multiple lines - it caused *all* tests to be skipped instead of just the > ones specified. do you think it's better to put it all in one line?
> do you think it's better to put it all in one line? Not sure if it's still necessary in some cases, but in the past this used to be necessary to work around issues with RPM's macro handling.
ok noted, I updated the spec file
Hi, Fabio do you mind to take a look? Thank you.
Hello @decathorpe could you please help with the review process for this package? Thank you.
(In reply to Fabio Valentini from comment #15) > > do you think it's better to put it all in one line? > > Not sure if it's still necessary in some cases, but in the past this used to > be necessary to work around issues with RPM's macro handling. As far as I can tell, this is not resolved? Other than that, looks good to me.
Ok Thanks, I updated the spec file now to have the skipped test in one line.
:thumbsup:
Thanks Fabio for your time reviewing this!
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-vhost-device-sound
FEDORA-2024-c7d61a5bd1 (rust-vhost-device-sound-0.1.0-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-c7d61a5bd1
FEDORA-2024-c7d61a5bd1 (rust-vhost-device-sound-0.1.0-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.