Bug 2184883 - Review Request: rust-minreq - Simple, minimal-dependency HTTP client.
Summary: Review Request: rust-minreq - Simple, minimal-dependency HTTP client.
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/minreq
Whiteboard:
Depends On:
Blocks: 2116151
TreeView+ depends on / blocked
 
Reported: 2023-04-06 06:10 UTC by Felix Wang
Modified: 2023-08-11 02:47 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-08-11 02:47:41 UTC
Type: Bug
Embargoed:
decathorpe: fedora-review?


Attachments (Terms of Use)
The .spec file difference from Copr build 5749677 to 5749678 (255 bytes, patch)
2023-04-06 06:28 UTC, Jakub Kadlčík
no flags Details | Diff

Comment 1 Jakub Kadlčík 2023-04-06 06:15:16 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5749677
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184883-rust-minreq/fedora-rawhide-x86_64/05749677-rust-minreq/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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 3 Jakub Kadlčík 2023-04-06 06:28:07 UTC
Created attachment 1956050 [details]
The .spec file difference from Copr build 5749677 to 5749678

Comment 4 Jakub Kadlčík 2023-04-06 06:28:09 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5749678
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184883-rust-minreq/fedora-rawhide-x86_64/05749678-rust-minreq/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 5 Fabio Valentini 2023-04-20 20:34:23 UTC
There are some immediate problems I see:

1. The package seems to bundle code from the native-tls crate without documenting why or declaring its license correctly. The crate metadata says `license = "ISC"` but the crate clearly bundles code that is licensed MIT, so that should probably be `license = "ISC AND MIT"`. You might want to ask the upstream project why they're bundling part of the native-tls crate instead of depending on it directly, and notify them that the license in their crate's metadata is incomplete and / or wrong.

2. You should not point Source or Patch URLs at temporary GitHub projects. In fact, you don't need to provide a URL for patches at all (unless they are 1:1 backports of upstream commits, for example). rust2rpm also automates part of this process, you might want to use "rust2rpm -p" instead of manually creating the patch and adding it. (It's also missing an explanatory comment what the patch does, something like "relax serde dev-dependency that is too strict".)

3. Some subpackages that are built from this package are not installable in Fedora:

 Problem 1: conflicting requests
  - nothing provides (crate(openssl/vendored) >= 0.10.29 with crate(openssl/vendored) < 0.11.0~) needed by rust-minreq+https-bundled-devel-2.7.0-1.fc39.noarch
 Problem 2: conflicting requests
  - nothing provides (crate(punycode/default) >= 0.4.1 with crate(punycode/default) < 0.5.0~) needed by rust-minreq+punycode-devel-2.7.0-1.fc39.noarch
 Problem 3: package rust-minreq+https-bundled-probe-devel-2.7.0-1.fc39.noarch requires crate(minreq/https-bundled) = 2.7.0, but none of the providers can be installed
  - conflicting requests
  - nothing provides (crate(openssl/vendored) >= 0.10.29 with crate(openssl/vendored) < 0.11.0~) needed by rust-minreq+https-bundled-devel-2.7.0-1.fc39.noarch

The "vendored" feature of the "openssl" crate is not available in Fedora, since we cannot ship a vendored copy of OpenSSL sources and must link to system OpenSSL instead. You will need to patch out the "https-bundled" feature from this crate.

The other problem points to a missing dependency (punycode), which is not packaged for Fedora yet.

4. Tests are disabled without giving a reason for it. Since all dev-dependencies are available in Fedora, I assume you disabled running tests because they fail? Please add an explanation for why that is the case, or - if possible - instead, enable tests, but skip those that fail for expected reasons (for example, because they need internet access, or because they require test files that are not included with published crate tarballs).

5. This crate depends on a lot of rustls projects for optional features. These crates are only available on limited architectures (due to limited cross-platform functionality of the underlying "ring" crate). You will need to take this into account in some way or another (possibly by only building this crate on x86_64, aarch64, and i686, or removing the features that depend on rustls - unless you are packaging something that uses the rustls features, of course).

===

Upon further inspection, it looks like you're packaging this crate in order to update the encode_unicode crate.

You might want to just remove the "minreq" dependency there, instead: It appears to only be used in benchmark code (which we neither build nor run during RPM builds), and which would also need internet access to work:
https://github.com/tormol/encode_unicode/blob/master/benches/length.rs

===

Side note: Are you trying to update the prettytable-rs crate? You might need to coordinate this with rpick package / upstream, which currently depends on an old version.

Comment 6 Fabio Valentini 2023-07-30 16:44:51 UTC
Are you still interested in this package?

Comment 7 Felix Wang 2023-08-11 02:47:41 UTC
Thank you for your efforts in providing such a detailed and comprehensive review of the package. However, at the moment I am quite busy with job searching and preparing for graduation, which have my full attention. So I am not interested in pursuing this package. I apologized for making you wait so long without a reply and should have responded in time instead of letting it linger. I want to again express my gratitude for your time and feedback. So I close the review request.


Note You need to log in before you can comment on or make changes to this bug.