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)

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.

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

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.

Thanks Fabio! I opened https://github.com/felinira/gvdb-rs/issues/11 for clarifying the svg file licensing, https://github.com/felinira/gvdb-rs/pull/12 to add the missing license file, and https://github.com/felinira/gvdb-rs/issues/13 for the s390x build issue.

OK, so I haven't heard anything back from upstream yet, but I think we can still move forward with this. For s390x, I went ahead and just disabled the tests - the failures still need investigating, but it shouldn't block the packaging work for other arches.

As for the test-data directory, I believe I found the source for the icons after some googling - it appears to be https://www.svgrepo.com/svg/442121/document-send-symbolic and licensed under CC-BY (which is a Fedora allowed license). So I think we don't need to exclude these files from the tarball, but I went ahead and removed from the distributed files that end up in the rpm just in case. This also saves us from having to list them as a downstream-modification in the license tag.

Here's my comment in my local git commit for the package:

---
    Exclude test-data directory that has unclear licensing (rhbz#2230729)
    
    As per discussion in https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2230729#c3
    
    The svg icons in test-data directory appear to be licensed under CC-BY,
    but still waiting for a confirmation from gvdb-rs upstream. Since they
    appear to be free software and under fedora approved license, we don't
    need to respin the tarball, but out of caution, exclude them from
    distributed files for now. This also saves us from having to manually
    update the license tag.
    
    Likely source for the icons:
    https://www.svgrepo.com/svg/442121/document-send-symbolic
    
    Upstream issue:
    https://github.com/felinira/gvdb-rs/issues/11
---

What do you think, does it make sense?

Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=105087553

Spec URL: https://kalev.fedorapeople.org/rust-gvdb.spec
SRPM URL: https://kalev.fedorapeople.org/rust-gvdb-0.5.1-3.fc40.src.rpm

(In reply to Kalev Lember from comment #6)
> OK, so I haven't heard anything back from upstream yet, but I think we can
> still move forward with this. For s390x, I went ahead and just disabled the
> tests - the failures still need investigating, but it shouldn't block the
> packaging work for other arches.
> 
> As for the test-data directory, I believe I found the source for the icons
> after some googling - it appears to be
> https://www.svgrepo.com/svg/442121/document-send-symbolic and licensed under
> CC-BY (which is a Fedora allowed license). So I think we don't need to
> exclude these files from the tarball, but I went ahead and removed from the
> distributed files that end up in the rpm just in case. This also saves us
> from having to list them as a downstream-modification in the license tag.
> 
> What do you think, does it make sense?

Thanks for investigating! Yes, I think this is correct.
If the files are under a license that allows redistribution, then having them in the SRPM is fine.
And since they're not shipped, they don't affect the License tag of the package.

As for the failing tests on big-endian, skipping tests on s390x is a good solution for now.
Not building the package on s390x would have unpleasant knock-on effects in dependent packages, as you noted.

===

Package was generated with rust2rpm, simplifying the review.

- package builds and installs without errors on rawhide
- test suite is run and all unit tests pass
- latest version of the crate is packaged
- license matches upstream specification and is acceptable for Fedora
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter: alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

- add @rust-sig with "commit" access as package co-maintainer
  (should happen automatically)

- set bugzilla assignee overrides to @rust-sig (optional)

- track package in koschei for all built branches
  (should happen automatically once rust-sig is co-maintainer)

===

Three minor things that don't block approval:

- include the link to the upstream issue for the s390x failures above the "%ifnarch s390x"
- include the link to the upstream issue for the test-data license above the patch
- include the link to the upstream issue for the missing license file above Source1

Having the links to the issues in the spec file alongside the code that refers to them is really nice when you're looking at packages that are maintained by others (or that you have last touched yourself months or years ago).

Excellent, thank you for the review!

Good point about the links -- I usually add them but somehow forgot them this time. I think it may be the autogenerated nature of the rust spec files that throws me off a bit. I've amended my local git commits and added the links now.

The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-gvdb

FEDORA-2023-1d9a7f4b72 has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2023-1d9a7f4b72

FEDORA-2023-1d9a7f4b72 has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2023-1b6875778d has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-1b6875778d

FEDORA-2023-1b6875778d has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2023-d02a5bd514 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-d02a5bd514

FEDORA-2023-434850c6f3 has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2023-434850c6f3

FEDORA-2023-d02a5bd514 has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2023-434850c6f3 has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.