Bug 2368361
| Summary: | Review Request: rust-pam-bindings - rust bindings for libpam | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Noa Resare <noa> | ||||
| Component: | Package Review | Assignee: | Fabio Valentini <decathorpe> | ||||
| Status: | ASSIGNED --- | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | rawhide | CC: | code, decathorpe, package-review, villapla | ||||
| Target Milestone: | --- | Keywords: | AutomationTriaged | ||||
| Target Release: | --- | Flags: | decathorpe:
fedora-review?
|
||||
| Hardware: | All | ||||||
| OS: | Linux | ||||||
| URL: | https://crates.io/crates/pam-bindings | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | Doc Type: | --- | |||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | Type: | --- | |||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 2404683 | ||||||
| Attachments: |
|
||||||
|
Description
Noa Resare
2025-05-24 12:08:37 UTC
Copr build: https://copr.fedorainfracloud.org/coprs/build/9077020 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2368361-rust-pam-bindings/fedora-rawhide-x86_64/09077020-rust-pam-bindings/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. It looks like you still need to address this:
> # FIXME: no license files detected
It is not entirely clear to me how to add the LICENSE file to the .src.rpm while keeping it generated using rust2rpm. I made an attempt in this commit https://github.com/nresare/rpm-packaging/commit/8327fcb2e4c4cfce137ccfee754e6a97048aca47 which in turn resulted in the following output: Spec URL: https://download.copr.fedorainfracloud.org/results/noa/rust/fedora-rawhide-aarch64/09366875-rust-pam-bindings/rust-pam-bindings.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/noa/rust/fedora-rawhide-aarch64/09366875-rust-pam-bindings/rust-pam-bindings-0.1.1-1.fc43.src.rpm The reason for these files missing in the .crate is that the upstream repository is a cargo workspace, where the LICENSE and README.md lives in the top level directory. I have opened an upstream issue[1], in the interim I am happy to go with whatever workaround seems best to you 1) https://github.com/anowell/pam-rs/issues/17 I assume you noticed that there doesn’t seem to have been any activity upstream in the last three years? I also found a report of unsoundness in https://github.com/anowell/pam-rs/issues/16, which seems concerning for authentication-related code. ---- Ignoring the above for now, we can fix https://github.com/anowell/pam-rs/issues/17 upstream with https://github.com/anowell/pam-rs/pull/18. (Copies of the workspace LICENSE file would have worked as well; symbolic links are a common pattern.) It’s OK if there is no README.md in the crates. Until upstream makes a new release, you can add the license downstream via rust2rpm something like this: [package.license-files] include = [ "LICENSE"] [[package.extra-sources]] number = 10 file = "https://github.com/anowell/pam-rs/raw/3d9318b5b812d9142595928deec23e5843a60995/LICENSE" comments = [ "README.md and LICENSE missing from published pam-bindings crate", "https://github.com/anowell/pam-rs/issues/17", "Fix missing license files in published crates", "https://github.com/anowell/pam-rs/pull/18", ] [scripts.prep] pre = [ "# Copy in the workspace license file", "cp -p '%{SOURCE10}' ." ] Thank you for the rust2rpm.toml hints, this looks a lot better. I have landed an updated version here https://github.com/nresare/rpm-packaging/blob/main/pam-bindings/rust2rpm.toml and rebuilt the COPR packages from this. Stable links for the newly built spec and .src.rpm: https://github.com/nresare/rpm-packaging/releases/download/v20251013/rust-pam-bindings.spec https://github.com/nresare/rpm-packaging/releases/download/v20251013/rust-pam-bindings-0.1.1-1.el10.src.rpm -- It is true that upstream seems to have found better things to do with their time. I will reach out and hear if they are interested in some shared ownership Created attachment 2109718 [details]
The .spec file difference from Copr build 9077020 to 9688424
Copr build: https://copr.fedorainfracloud.org/coprs/build/9688424 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2368361-rust-pam-bindings/fedora-rawhide-x86_64/09688424-rust-pam-bindings/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 pretty good now - I see one minor thing: Since you're including the README file as an extra file after the fact, it's not detected by rust2rpm. You might want to mark it as a %doc file explicitly, similarly to what you're already doing for the %license file: ``` [package] license-files.include = ["LICENSE"] doc-files.include = ["README.md"] ``` |