Bug 1814887

Summary:	Review Request: rust-libslirp-sys - Rust FFI bindings for libslirp
Product:	[Fedora] Fedora	Reporter:	Marc-Andre Lureau <marcandre.lureau>
Component:	Package Review	Assignee:	Igor Raits <igor.raits>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	igor.raits, package-review
Target Milestone:	---	Flags:	igor.raits: fedora-review+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2020-03-23 12:48:17 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Marc-Andre Lureau 2020-03-18 22:56:39 UTC

Spec URL: https://elmarco.fedorapeople.org/rust-libslirp-sys.spec
SRPM URL: https://elmarco.fedorapeople.org/rust-libslirp-sys-4.0.1-1.fc33.src.rpm
Description: Rust FFI bindings for libslirp
Fedora Account System Username: elmarco

Comment 1 Igor Raits 2020-03-23 08:30:23 UTC

%license files are missing from %files.

Comment 2 Marc-Andre Lureau 2020-03-23 10:08:05 UTC

(In reply to Igor Gnatenko from comment #1)
> %license files are missing from %files.

thanks, upstream doesn't have one, is that required?

Comment 3 Igor Raits 2020-03-23 10:09:25 UTC

Yes. There are ton of MIT variants and we need to know which one it is..

I did not look in upstream, but I guess this is just about copying LICENSE file into subfolder for -sys crate and publishing new version. At least contact upstream please and put link in spec file.

Comment 4 Marc-Andre Lureau 2020-03-23 10:33:25 UTC

Apparently, "MIT" is unambiguously https://spdx.org/licenses/MIT.

Do you think it's still required? I am upstream, fwiw.

Comment 5 Igor Raits 2020-03-23 10:35:56 UTC

(In reply to Marc-Andre Lureau from comment #4)
> Apparently, "MIT" is unambiguously https://spdx.org/licenses/MIT.

Yes, but in the world exists at least 30+ variations of MIT licenses. Better ask @spot about this :)

> Do you think it's still required? I am upstream, fwiw.

Yes. It should not be hard to put the full license text into LICENSE file and release new version, isn't it?

Comment 6 Marc-Andre Lureau 2020-03-23 10:55:12 UTC

Updated:

Spec URL: https://elmarco.fedorapeople.org/rust-libslirp-sys.spec
SRPM URL: https://elmarco.fedorapeople.org/rust-libslirp-sys-4.0.2-1.fc32.src.rpm

Comment 7 Igor Raits 2020-03-23 10:57:52 UTC

LGTM.

Just a few notes:

* You probably forgot to git push in upstream repo
*         pkg_config::find_library("slirp").unwrap(); suggests that any version will work, but in spec file you have >= 4.0.0, you probably want to either drop that part in spec or add some restriction in upstream.

Comment 8 Marc-Andre Lureau 2020-03-23 11:23:45 UTC

(In reply to Igor Gnatenko from comment #7)
> LGTM.
> 
> Just a few notes:
> 
> * You probably forgot to git push in upstream repo

done

> *         pkg_config::find_library("slirp").unwrap(); suggests that any
> version will work, but in spec file you have >= 4.0.0, you probably want to
> either drop that part in spec or add some restriction in upstream.

I added >= 4.0.0 there too.

thanks

Comment 9 Igor Raits 2020-03-23 11:35:31 UTC

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-libslirp-sys