Bug 2332598

This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=126929929

Note: This is a re-review for a package that has been previously retired.

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2332598-rust-elasticlunr-rs/fedora-rawhide-x86_64/08398300-rust-elasticlunr-rs/fedora-review/review.txt

Found issues:

- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/rust-elasticlunr-rs
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names

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.

Looks good to me except 
-license = "MIT/Apache-2.0"
+license = "(MIT OR Apache-2.0) AND MIT"
Did you add "AND MIT" due to the following files:
1) https://github.com/mattico/elasticlunr-rs/blob/master/LICENSE-JS
2) https://github.com/mattico/elasticlunr-rs/blob/master/LICENSE-WORDS
?

BTW, could you review https://bugzilla.redhat.com/show_bug.cgi?id=2344534 in return?

(In reply to wojnilowicz from comment #4)
> Looks good to me except 
> -license = "MIT/Apache-2.0"
> +license = "(MIT OR Apache-2.0) AND MIT"
> Did you add "AND MIT" due to the following files:
> 1) https://github.com/mattico/elasticlunr-rs/blob/master/LICENSE-JS
> 2) https://github.com/mattico/elasticlunr-rs/blob/master/LICENSE-WORDS
> ?

Yes, the README explicitly states that parts of the project are covered by different (i.e. only MIT) license:

> Includes code ported from [elasticlunr.js][eljs] Copyright (C) 2017 by Wei Song, 
> used under license. See LICENSE-JS for details.
>
> Includes stop word lists ported from [stopwords-filter][swft] Copyright (C) 2012 
> David J. Brenes, used under license. See LICENSE-WORDS for details.

(In reply to Fabio Valentini from comment #5)
> (In reply to wojnilowicz from comment #4)
> > Looks good to me except 
> > -license = "MIT/Apache-2.0"
> > +license = "(MIT OR Apache-2.0) AND MIT"
> > Did you add "AND MIT" due to the following files:
> > 1) https://github.com/mattico/elasticlunr-rs/blob/master/LICENSE-JS
> > 2) https://github.com/mattico/elasticlunr-rs/blob/master/LICENSE-WORDS
> > ?
> 
> Yes, the README explicitly states that parts of the project are covered by
> different (i.e. only MIT) license:
> 
> > Includes code ported from [elasticlunr.js][eljs] Copyright (C) 2017 by Wei Song, 
> > used under license. See LICENSE-JS for details.
> >
> > Includes stop word lists ported from [stopwords-filter][swft] Copyright (C) 2012 
> > David J. Brenes, used under license. See LICENSE-WORDS for details.

Shouldn't that be reflected at this line https://github.com/mattico/elasticlunr-rs/blob/master/Cargo.toml#L3 ?

I mean shouldn't the following line
license = "MIT/Apache-2.0"
change to
license = "(MIT OR Apache-2.0) AND MIT"
to firstly remove ambiguity of the deprecated "/" char, and secondly truly reflect what the licensing terms are?
Details at https://doc.rust-lang.org/cargo/reference/manifest.html#the-license-and-license-file-fields

Do you plan any PR for that?

Sorry for the delay in getting back to this.

> Shouldn't that be reflected at this line

For purposes of Fedora packaging, this is true.
But not everyone subscribes to the same school of thought on that topic as Red Hat Legal does.

I have filed an upstream issue to clarify this.
But for downstream packaging, the patch should already correctly reflect the project license(s).

https://github.com/mattico/elasticlunr-rs/issues/55

(In reply to Fabio Valentini from comment #7)
> Sorry for the delay in getting back to this.
> 
> > Shouldn't that be reflected at this line
> 
> For purposes of Fedora packaging, this is true.
> But not everyone subscribes to the same school of thought on that topic as
> Red Hat Legal does.
> 
> I have filed an upstream issue to clarify this.
> But for downstream packaging, the patch should already correctly reflect the
> project license(s).
> 
> https://github.com/mattico/elasticlunr-rs/issues/55

No problem. At least you tried right now. Could you put that link near the license field in your SPEC file?
Other than that it LGTM.

BTW, I already fixed https://bugzilla.redhat.com/show_bug.cgi?id=2344534 Could you look at it as well?

===

Package was generated with rust2rpm (with some features removed due to missing dependencies), 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 almost matches upstream specification and is acceptable for Fedora. Upstream misses MIT license and uses deprecated "/" specification but got notified about it.
✅ licenses of statically linked dependencies are correctly taken into account
✅ 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 (*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)

Thank you - upstream has already published a new version with the corrected license metadata, so I'll use that version and the patch and the spec file link become unnecessary :)

I'll take another look at awatcher ASAP.

Imported and built:
https://bodhi.fedoraproject.org/updates/FEDORA-2025-807ea167e1