[Bug 2307668] New: Review Request: rust-google-cloud-storage - Google Cloud Platform storage client library
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Bug ID: 2307668 Summary: Review Request: rust-google-cloud-storage - Google Cloud Platform storage client library Product: Fedora Version: rawhide URL: https://crates.io/crates/google-cloud-storage Status: NEW Component: Package Review Assignee: nobody@fedoraproject.org Reporter: sanjay.ankur@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-g... SRPM URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-g...
Description: Google Cloud Platform storage client library.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value Depends On| |2307663
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2307663 [Bug 2307663] Review Request: rust-google-cloud-auth - Google Cloud Platform server application authentication library
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7939811 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2309375
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2309375 [Bug 2309375] Review Request: rust-taskchampion - Personal task-tracking
https://bugzilla.redhat.com/show_bug.cgi?id=2307668 Bug 2307668 depends on bug 2307663, which changed state.
Bug 2307663 Summary: Review Request: rust-google-cloud-auth - Google Cloud Platform server application authentication library https://bugzilla.redhat.com/show_bug.cgi?id=2307663
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #2 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- Spec URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-g... SRPM URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-g...
- feat: update to latest version (ready for review now that google-cloud-auth has been imported)
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #3 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8042191 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |decathorpe@gmail.com Flags| |fedora-review? CC| |decathorpe@gmail.com Status|NEW |ASSIGNED
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- Three minor things:
- Why include so many pre-import changelog entries? They might be useful for you (as the submitter of the review), but not really for the package itself, especially after importing.
- Please don't use `sed` for patching Cargo.toml. The only *supported* way of patching Cargo.toml is with `rust2rpm -p` - all other methods are not guaranteed to give you valid spec files. Additionally, for many fellow packagers (me included) sed is just a bit too magical, and a patch is much much easier to read.
- Open-ended dependency version ranges like >=0.5.0 are *really* bad in the Rust ecosystem context, and should be avoided if possible. For example, If you know that something is compatible with versions 0.3, 0.4, 0.5 of one of its dependencies, don't use ">=0.3" (because an update to 0.6 could very well break everything!), use a closed range like ">=0.3,<0.6" instead - it signals to packagers that you need to *check* for compatibility and *explicitly* mark it as compatible.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #5 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- Thanks for the review Fabio,
(In reply to Fabio Valentini from comment #4)
Three minor things:
- Why include so many pre-import changelog entries? They might be useful for
you (as the submitter of the review), but not really for the package itself, especially after importing.
I use a git repo for WIP packages, so these get included by rpmautospec. They are indeed very useful for me, but I drop them before the import.
- Please don't use `sed` for patching Cargo.toml.
The only *supported* way of patching Cargo.toml is with `rust2rpm -p` - all other methods are not guaranteed to give you valid spec files. Additionally, for many fellow packagers (me included) sed is just a bit too magical, and a patch is much much easier to read.
Cool, updated.
- Open-ended dependency version ranges like >=0.5.0 are *really* bad in the
Rust ecosystem context, and should be avoided if possible. For example, If you know that something is compatible with versions 0.3, 0.4, 0.5 of one of its dependencies, don't use ">=0.3" (because an update to 0.6 could very well break everything!), use a closed range like ">=0.3,<0.6" instead - it signals to packagers that you need to *check* for compatibility and *explicitly* mark it as compatible.
So, in this specific case, upstream pins a couple of deps to versions that are older than the versions of the deps we have in Fedora. Is pinning to old versions a common occurrence in the ecosystem too, and how should one handle this? The package builds with the version in Fedora, so I've tweaked the version pin to it for the moment.
https://github.com/yoshidan/google-cloud-rust/blob/main/storage/Cargo.toml#L...
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #6 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- Spec URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-g... SRPM URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-g...
- feat: update to latest release, review related updates: - use rust2rpm -p - update dep version pinning to have upper bound - remove unneeded extra license source
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #7 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2049847 --> https://bugzilla.redhat.com/attachment.cgi?id=2049847&action=edit The .spec file difference from Copr build 8042191 to 8096760
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged
--- Comment #8 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8096760 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #9 from Ankur Sinha (FranciscoD) sanjay.ankur@gmail.com --- Hi Fabio,
Could you please take a look at this when you have a minute? I think I've addressed the issues that were pointed out in the review.
Cheers,
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #10 from Fabio Valentini decathorpe@gmail.com --- Thank you for the update, looks good to me!
# these tests require setting up tokens + net access etc. # %%cargo_test -- --lib %cargo_test -- --doc
You can automate generating this with these settings in rust2rpm.toml:
``` [tests] run = ["doc"] comments = ["skip tests that require internet access and valid authentication tokens"] ```
===
Package was generated with rust2rpm, 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 (some tests disabled with justification) ✅ latest version of the crate is packaged ✅ license matches upstream specification 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:
- 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)
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RELEASE_PENDING
--- Comment #11 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-google-cloud-storage
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RELEASE_PENDING |MODIFIED
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-eacc1a8185 (rust-google-cloud-storage-0.22.1-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2024-eacc1a8185
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA Last Closed| |2024-10-23 09:58:09
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-eacc1a8185 (rust-google-cloud-storage-0.22.1-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-9bf53f78a5 (rust-google-cloud-storage-0.22.1-1.fc41) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-9bf53f78a5
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-9bf53f78a5 has been pushed to the Fedora 41 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-9bf53f78a5 *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-9bf53f78a5
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2307668
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-9bf53f78a5 (rust-google-cloud-storage-0.22.1-1.fc41) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com