Bug 2124365

Summary: Review Request: rust-selectors - CSS Selectors matching for Rust
Product: [Fedora] Fedora Reporter: Kalev Lember <klember>
Component: Package ReviewAssignee: Fabio Valentini <decathorpe>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: decathorpe, package-review
Target Milestone: ---Flags: decathorpe: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: rust-selectors-0.23.0-1.fc37 rust-selectors-0.23.0-1.fc38 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-10-15 16:33:08 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:
Bug Depends On: 2124329, 2124361    
Bug Blocks:    

Description Kalev Lember 2022-09-05 20:14:12 UTC
Spec URL: https://kalev.fedorapeople.org/rust-selectors.spec
SRPM URL: https://kalev.fedorapeople.org/rust-selectors-0.23.0-1.fc38.src.rpm

Description:
CSS Selectors matching for Rust.

Fedora Account System Username: kalev

Note that this depends on two pending reviews, rust-cssparser and rust-servo_arc. With these two installed, the package builds fine in mock locally.

I'm trying to get librsvg2 to build using system rust crates (instead of vendored crates as it is right now) and this package is one of the dependencies for librsvg2.

Comment 1 Fabio Valentini 2022-09-05 20:50:55 UTC
Taking this review. Will continue once dependencies are settled.

Comment 2 Fabio Valentini 2022-10-13 19:17:37 UTC
Package looks good to me (latest version packaged, builds and installs without errors, tests pass, etc.), with one exception:

There's no license file (:sad trombone:), and it looks like the MPL-2.0 license also requires that distributed sources contain a copy of the license terms.

Comment 3 Kalev Lember 2022-10-14 17:50:51 UTC
Thanks!

Why do you say that a license file is required? I skimmed through the license text (https://www.mozilla.org/en-US/MPL/2.0/) and it says 'You must inform recipients that the Source Code Form of the Covered Software is governed by the terms of this License, and how they can obtain a copy of this License.'

... and the license header used in the code says:

/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. */

I don't see anywhere that says that the full license text must be included? The text says to include a way to obtain a copy of this license and that's clearly done with the inclusion of the link above.

Comment 4 Fabio Valentini 2022-10-14 18:17:27 UTC
Huh, you're right - it requires license and copyright *notice*, but not the text of the license itself. It's still unusual that there's no copy of the license included in the published sources ... if you don't want to file an issue with the upstream project about that, then I'll do that myself.

===

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 (MPL-2.0) 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:

- add @rust-sig with "commit" access as package co-maintainer

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

- 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

- track package in koschei for all built branches

Comment 5 Kalev Lember 2022-10-14 19:03:06 UTC
Oh no, I can file the ticket, I just didn't want it to block the new package review while upstream adds the license text :)

Thanks again for the quick review!

Comment 6 Gwyn Ciesla 2022-10-14 19:15:51 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-selectors

Comment 7 Kalev Lember 2022-10-15 16:33:08 UTC
Package imported and built.