Bug 2334841

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8453909
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2334841-rust-libphosh-sys/fedora-rawhide-x86_64/08453909-rust-libphosh-sys/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

Please know that there can be false-positives.

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

Spec URL: https://pagure.io/rust-libphosh-sys/raw/1c5d2f12950487fc8628720dd1291b8dbc3c78b7/f/rust-libphosh-sys.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/5771/127375771/rust-libphosh-sys-0.0.4-2.fc42.src.rpm

Created attachment 2064159 [details]
The .spec file difference from Copr build 8453909 to 8453952

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8453952
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2334841-rust-libphosh-sys/fedora-rawhide-x86_64/08453952-rust-libphosh-sys/fedora-review/review.txt

Please take a look if any issues were found.


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

Looks mostly OK, with some minor things:

0. Don't mix tabs and spaces in the same file. :)

1. Why did you manually add "BuildRequires: gcc"? If it's to placate some warning, you can ignore it - the Rust compiler already hard-codes a dependency on gcc.

2. "ExcludeArch: i686" is better written as "ExcludeArch: %{ix86}" to avoid problems with systems that define this differently (it's i686 in koji but i386 in COPR, IIRC).

3. You also need to add "Requires: libphosh-devel" to the "devel" subpackage, otherwise it won't be pulled in when building packages that depend on libphosh-sys.

4. Replace "%license LICENSE" with "%license %{crate_instdir}/LICENSE" to avoid including the LICENSE file in built packages twice.

You might be interested in also adding a rust2rpm.toml config file to automate some parts of this package.

For example, this snippet would cause the dependency on libphosh-devel to be automatically injected where required:

```
[requires]
build = ["libphosh-devel"]
lib = ["libphosh-devel"]
```

0. Oops xD

1. Yeah I was mostly trying to placate fedora-review-service. I suspected something like what you're saying, but couldn't see the compiler dep when I went looking at the rust package. Guess I just didn't look carefully enough ;) I'll remove this unnecessary dep in the next revision.

2. Interesting, guess I didn't run into the inconsistency because I've only been building against aarch64/x64 in COPR

3. Ah, right. Oops! I'll add a rust2rpm.toml per your suggestion and fix this up, thanks.

4. I see, I'll do that in the next revision. Interestingly I've already done this in some local changes that build rust-libphosh-sys 0.0.5, but I can't submit that yet because libphosh 0.44 hasn't made its way into the repos.

I'll push up a new revision soon, thanks for the review!

Spec URL: https://pagure.io/rust-libphosh-sys/raw/94440753295a75b0caecce51151b2cef02a6d032/f/rust-libphosh-sys.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/8055/127498055/rust-libphosh-sys-0.0.4-3.fc42.src.rpm

---

Notes:

 * I've added a basic [rust2rpm.toml](https://pagure.io/rust-libphosh-sys/blob/main/f/rust2rpm.toml) with the suggested content.
 * I didn't bother to incorporate the manually patched LICENSE Source: + script to copy that file to rust2rpm.toml, because those will be removed in libphosh 0.0.5 (which includes the LICENSE in the packaged crate on crates.io)
 * As noted in previous comment, I can't do the 0.0.5 bump yet because it depends on Phosh being updated to 0.44 first.
 * I couldn't see a way to add ExcludeArch in rust2rpm.toml. `[package.supported-arches]` seems to be a different thing, right?
 * While investigating the ExcludeArch thing, I came across one of [your comments last year](https://bugzilla.redhat.com/show_bug.cgi?id=2266544#c5) indicating that a comment as to why i686 isn't necessary - so I took some initiative and removed the ExcludeArch comment.

Created attachment 2064551 [details]
The .spec file difference from Copr build 8453952 to 8465518

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8465518
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2334841-rust-libphosh-sys/fedora-rawhide-x86_64/08465518-rust-libphosh-sys/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

Please know that there can be false-positives.

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

Oh Oh. looks like phosh was updated to v0.44 (by yourself? https://src.fedoraproject.org/rpms/phosh/pull-request/3), which breaks building the bindings:

> The system library `libphosh-0.43` required by crate `libphosh-sys` was not found.

Ah, yes - I just submitted the PR to help out my homie lihis, who is maintaining the Phosh packages in Fedora. I wasn't keeping an eye on when that stuff was merging or grinding through Bodhi :)

Anyways, here's the next revision. I've changed the build dep to `pkgconfig(libphosh-0.44)` to make it clear that these crates are currently pinned to a rather precise version of the libphosh API (which hasn't stabilized yet, but we're hoping to nail down some stuff for 0.45). I was also able to remove the LICENSE hax as promised earier.

---

Spec URL: https://pagure.io/rust-libphosh-sys/raw/b6b6acaeb9d2522960db07586fd4f43a646c8d11/f/rust-libphosh-sys.spec
SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/9769/127749769/rust-libphosh-sys-0.0.5-1.fc42.src.rpm

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

Created attachment 2065433 [details]
The .spec file difference from Copr build 8465518 to 8500328

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8500328
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2334841-rust-libphosh-sys/fedora-rawhide-x86_64/08500328-rust-libphosh-sys/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/

Please know that there can be false-positives.

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

Thanks! This looks good to me now.

Package was generated with rust2rpm, simplifying the review.

✅ package contains only permissible content
✅ 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 (*NOT* pre-release 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)

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

FEDORA-2025-a65f973ead (rust-libphosh-sys-0.0.5-1.fc42) has been submitted as an update to Fedora 42.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-a65f973ead

FEDORA-2025-a65f973ead (rust-libphosh-sys-0.0.5-1.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.

Looks like you figured out all the necessary steps to land this in the repos - congratulations on your first official package :)
Let me (or Neal) know if you need any help or have any questions. We're both in the #rust:fedoraproject.org room on Matrix.

Yes indeed! I followed the excellent documentation and marvelled at how all the automated tooling and systems worked exactly the way the docs said they would. As a jaded/grizzled Software Person I was sure that literally never actually happens these days ;)

Thanks for all the help you've provided thus far - answering my questions, reviewing these packages, and bringing back rust-libhandy <3

(In reply to Sam Day from comment #19)
> Yes indeed! I followed the excellent documentation and marvelled at how all
> the automated tooling and systems worked exactly the way the docs said they
> would. As a jaded/grizzled Software Person I was sure that literally never
> actually happens these days ;)

There *are* some outdated docs out there, for sure, but at least for common tasks there's a few people who work to keep things up-to-date.

> Thanks for all the help you've provided thus far - answering my questions,
> reviewing these packages, and bringing back rust-libhandy <3

No problem! Looking forward to seeing phrog.