Bug 2230729 - Review Request: rust-gvdb - Read and write GLib GVariant database files
Summary: Review Request: rust-gvdb - Read and write GLib GVariant database files
Keywords:
Status: ASSIGNED
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://crates.io/crates/gvdb
Whiteboard:
Depends On: 2230010 2230724
Blocks: 2230732
TreeView+ depends on / blocked
 
Reported: 2023-08-09 22:11 UTC by Kalev Lember
Modified: 2023-08-11 20:44 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
decathorpe: fedora-review?


Attachments (Terms of Use)

Description Kalev Lember 2023-08-09 22:11:04 UTC
Spec URL: https://kalev.fedorapeople.org/rust-gvdb.spec
SRPM URL: https://kalev.fedorapeople.org/rust-gvdb-0.5.1-1.fc40.src.rpm
Description:
Read and write GLib GVariant database files.

Fedora Account System Username: kalev

Comment 1 Kalev Lember 2023-08-09 22:13:21 UTC
Note that this depends on rust-safe-transmute and rust-litrs reviews, and updating rust-quick-xml (I've put it in a PR for now, https://src.fedoraproject.org/rpms/rust-quick-xml/pull-request/2)

Comment 2 Fedora Review Service 2023-08-09 22:15:36 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6261773
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2230729-rust-gvdb/fedora-rawhide-x86_64/06261773-rust-gvdb/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 3 Fabio Valentini 2023-08-11 20:27:09 UTC
Two issues I can note immediately while I'm running a scratch build for all architectures:

1. Please file an upstream issue for the missing license file. The MIT license is one of the licenses that requires redistributed sources to contain a copy of the license text. This applies to crates distributed via crates.io and to distro packages (it can be solved by including the file manually from upstream git for now, but that should be a temporary solution until upstream publishes tarballs / crates with the license text included).

2. You might want to prevent test data from being installed / packaged. They include some icons where I'm not sure where they're from (looks like either the hicolor icon theme or Adwaita icon theme), so it's not clear which license applies to them (if any). It would be good to clarify the origin / license of these files with upstream. Until then, the least that you'd need to do is prevent them from being included in built RPMs, simplest solution would be a patch for Cargo.toml that does this:

-exclude = ["test/c"]
+exclude = ["test/c", "test-data"]

It doesn't look like anything from the test-data directory is non-free / non-redistributable, but I can't be sure about that since it's not documented where the files come from, so it would be great if upstream could clarify this. In the unlikely event that the files indeed are non-redistributable, you would need to re-package a "clean" tarball without the test data. You can look at the rust-statrs package for an example of how to do that. Another solution would be to ask upstream to exclude the test-data directory from published crate tarballs (i.e. push the 1-line-patch included above to be upstream instead of downstream).

Comment 4 Fabio Valentini 2023-08-11 20:44:47 UTC
Ran a scratch build now that all dependencies are available in rawhide, and it failed:
https://koji.fedoraproject.org/koji/taskinfo?taskID=104697659

As I suspected (I'm getting old), the tests are failing on s390x.
There's two possibilities for the root cause(s):
1. Tests hard-code bytes that are not byte-swapped for big-endian architectures (i.e. s390x) so the code works correctly, but the tests are broken because they compare correct results with a broken expected value.
2. Tests are correct, but the underlying code doesn't handle big-endian architectures correctly.

Case 1 is usually much easier to handle (either by fixing test fixtures or by ignoring failing tests).
Case 2 can be annoying, since it requires working with upstream to find the places in the code base where little-endianness is (wrongly) assumed, and fixing them properly.


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