Bug 2307668

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7939811
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2307668-rust-google-cloud-storage/fedora-rawhide-x86_64/07939811-rust-google-cloud-storage/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.

Spec URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-google-cloud-storage.spec
SRPM URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-google-cloud-storage-0.22.0-1.fc42.src.rpm

- feat: update to latest version (ready for review now that google-cloud-auth has been imported)

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8042191
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2307668-rust-google-cloud-storage/fedora-rawhide-x86_64/08042191-rust-google-cloud-storage/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.

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.

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#L54

Spec URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-google-cloud-storage.spec
SRPM URL: https://ankursinha.fedorapeople.org/reviews/rust-google-cloud-storage/rust-google-cloud-storage-0.22.1-1.fc42.src.rpm

- feat: update to latest release, review related updates:
- use rust2rpm -p
- update dep version pinning to have upper bound
- remove unneeded extra license source

Created attachment 2049847 [details]
The .spec file difference from Copr build 8042191 to 8096760

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2307668-rust-google-cloud-storage/fedora-rawhide-x86_64/08096760-rust-google-cloud-storage/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.

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,

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)

The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-google-cloud-storage

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

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.

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

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.

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.