Bug 2435758

[fedora-review-service-build]

[fedora-review-service-build]

Spec URL: https://topazus.fedorapeople.org/rust-starship.spec
SRPM URL: https://topazus.fedorapeople.org/rust-starship-1.24.2-1.fc44.src.rpm

Spec URL: https://topazus.fedorapeople.org/rust-starship.spec
SRPM URL: https://topazus.fedorapeople.org/rust-starship-1.24.2-1.fc44.src.rpm

[fedora-review-service-build]

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2435758-rust-starship/fedora-rawhide-x86_64/10093794-rust-starship/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/rust-starship
  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.

Spec URL: https://topazus.fedorapeople.org/rust-starship.spec
SRPM URL: https://topazus.fedorapeople.org/rust-starship-1.24.2-1.fc44.src.rpm

Created attachment 2128187 [details]
The .spec file difference from Copr build 10093794 to 10094000

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2435758-rust-starship/fedora-rawhide-x86_64/10094000-rust-starship/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/rust-starship
  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.

Taking this review.

Thanks for taking the review. The vendor tarball was created with the shell script:


#!/usr/bin/env bash

package=starship
version=$1

spectool -g -R rust-$package.spec

pushd $HOME/rpmbuild/SOURCES

if [ -d $package-$version ]; then
    echo "Removing existing directory: $package-$version"
    rm -rf $package-$version
fi

tar xf $package-$version.crate
cd $package-$version

cargo vendor-filterer --platform x86_64-unknown-linux-gnu \
    --platform powerpc64le-unknown-linux-gnu \
    --platform aarch64-unknown-linux-gnu \
    --platform s390x-unknown-linux-gnu

tar -pc --zstd -f "$package-$version-vendor.tar.zst" vendor
mv $package-$version-vendor.tar.zst ..

popd

Sorry for the delay - initial review comments:

1. If "git" is only needed for tests (could it be just "git-core"?), then please conditionalize it under %if %{with check}".

2. The "Recommends: cascadia-code-nf-fonts" is attached to no package.
You will need to move it under "%package %{crate}", otherwise it has zero effect.

3. The format / syntax in the License tag is incorrect.
Clauses that are joined by "OR" need to be parenthesized when in a larger "AND"ed list.

4. Inconsistent use of macros / mixing of templates.
It looks like you're mixing concepts from rust2rpm-generated spec file with the spec file template for "non-crate" Rust packages. Do one, or the other, but don't mix. For example, you replaced `%cargo_install" with a manuall "install" call, but left "%global cargo_install_lib 0" present, which has no effect when not using "%cargo_install".

5. The folder where you install the elvish shell completions is unowned.
Not even the "elvish" package owns /usr/share/elvish or /usr/share/elvish/lib, so the package will need to (co)own both directories (listing them as %dir). There don't appear to be *any* packages in Fedora that ship completions for elvish, so you could just drop them entirely.

6. There's a bunch of vendored dependencies that are "CC0-1.0" licensed - these are not appropriate for Fedora.
For crates where CC0-1.0 is an OR *option*, you'd need to drop "CC0-1.0" from the options. For crates where "CC0-1.0" is the *only* offered license, you'd need to either drop them, or ask for exceptions on the "legal" mailing list (these might be granted if the crates are already packaged for Fedora).

7. There's a some test data in vendored crates that I'm not sure whether they're redistributable or not.
For exmaple, jsonc-parser/benches/data/citm_catalog.json looks like a "real" dataset, but there's no documentation for what it's from or under the terms of which license the data is made available.

8. Some vendored dependencies have incomplete license information.
For example, the regex* crates include Unicode data / tables when using the "default" or any "unicode-*" features, which are covered by the "Unicode-DFS-2016" license (see rust-regex-syntax package in Fedora). Looks like this is the case here, so your package also contains data / code derived from Unicode-DFS-2016 licensed content.
Conversely, the tzdb_data crate claims to be licensed "MIT-0 OR MIT OR Apache-2.0", but only includes an "MIT" license text - but the Apache-2.0 license requires that the license text is included in redistributed sources. It would be good to contact the tzdb* crate author about clarifying the license terms and / or including the missing license texts.

================================================================================

It looks like the number of "missing" dependencies in Fedora is actually relatively small (after relaxing some version ranges to adapt to what's already available in Fedora). It would be preferable to not vendor dependencies in this case (it would also make it unnecessary to adapt / patch / deal with issues in vendored dependencies):

- guess_host_triple
- jsonc-parser
- process_control (retired)
- starship-battery (retired)
- versions (retired)

All other "missing" dependencies are just (small) version mismatches, as far as I can tell. The Rust SIG can help getting the missing pieces into Fedora for building without vendored dependencies.