Bug 2266450

Summary: Review Request: rust-fts-sys - File hierarchy traversal functions (FTS)
Product: [Fedora] Fedora Reporter: Michel Lind <michel>
Component: Package ReviewAssignee: Fabio Valentini <decathorpe>
Status: ASSIGNED --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: decathorpe, package-review
Target Milestone: ---Flags: decathorpe: fedora-review?
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
URL: https://crates.io/crates/fts-sys
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
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: 2266451    

Description Michel Lind 2024-02-27 21:50:49 UTC 
Spec URL: https://salimma.fedorapeople.org/specs/rust-fts-sys.spec
SRPM URL: https://salimma.fedorapeople.org/specs/rust-fts-sys-0.2.8-1.fc39.src.rpm

Description:
File hierarchy traversal functions (FTS).

Fedora Account System Username: salimma
Comment 1 Fedora Review Service 2024-02-27 22:06:14 UTC 
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7066109
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2266450-rust-fts-sys/fedora-rawhide-x86_64/07066109-rust-fts-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.
Comment 2 Fabio Valentini 2024-02-29 16:18:29 UTC 
Package looks fine, just one weird thing I noticed in Cargo.toml:

> links = "c"

This looks wrong. I don't think this crate should claim that it has exclusive rights to link against "c". Or is it a typo and it should be "libc"? Even then, it would be wrong, I think.
Comment 3 Fabio Valentini 2024-02-29 16:22:09 UTC 
PS: You might also want to add a dependency on glibc-devel - looks like <fts.h> that is imported by this package is from glibc.
Comment 4 Michel Lind 2024-02-29 17:04:08 UTC 
huh, yes. It sounds like it should be libc which explains the dependency on glibc-devel anyway? I wonder how it got transitively pulled in.
Comment 5 Fabio Valentini 2024-03-05 17:02:07 UTC 
From what I can tell:

- remove `links = "c"` from Cargo.toml
- add glibc-devel as build dependency and dependency for the -devel package

from the cargo docs:

> Primarily, Cargo requires that there is at most one package per links value.
> In other words, it is forbidden to have two packages link to the same native library.

ref. https://doc.rust-lang.org/cargo/reference/build-scripts.html#the-links-manifest-key

I'm pretty sure this means that `links = "c"` should be removed from upstream as well, and that this is only working by accident, because not even the "libc" crate (*the* libc bindings) claims to link with libc: https://github.com/rust-lang/libc/blob/main/Cargo.toml