[Bug 2292522] New: Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

Bug ID: 2292522
           Summary: Review Request: rust-zune-jpeg - Fast, correct and
                    safe jpeg decoder
           Product: Fedora
           Version: rawhide
            Status: NEW
         Component: Package Review
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL: https://decathorpe.fedorapeople.org/rust-zune-jpeg.spec
SRPM URL:
https://decathorpe.fedorapeople.org/rust-zune-jpeg-0.4.11-1.fc40.src.rpm

Description:
A fast, correct and safe jpeg decoder.

Fedora Account System Username: decathorpe

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

--- Comment #1 from Fabio Valentini  ---
This package built on koji: 
https://koji.fedoraproject.org/koji/taskinfo?taskID=119071015

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

Fedora Review Service  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |AutomationTriaged
                URL|                            |https://crates.io/crates/zu
                   |                            |ne-jpeg

--- Comment #2 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7617617
(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.

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

wojnilowicz  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]    |[e-mail]
                   |                            |m
              Flags|                            |fedora-review?
                 CC|                            |[e-mail]
                   |                            |m
             Status|NEW                         |ASSIGNED

--- Comment #3 from wojnilowicz  ---
1) Shouldn't the following subpackage:

%package     -n %{name}+x86-devel
Summary:        %{summary}
BuildArch:      noarch

be somehow restricted to x86_64, and not be noarch?
At
https://github.com/etemesi254/zune-image/blob/dev/crates/zune-jpeg/README.md...
it says that this "Enables x86 specific instructions"

2) x86 requires x86_64-v2, x86_64-v3. Wouldn't that be an issue for x86_64-v1
when used in an application using this as a dependency?

[fedora-review-service-build]

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

--- Comment #4 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7689515
(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.

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

--- Comment #5 from Fabio Valentini  ---
(In reply to wojnilowicz from comment #3)
> Shouldn't the following subpackage:
>
> %package     -n %{name}+x86-devel
> Summary:        %{summary}
> BuildArch:      noarch
>
> be somehow restricted to x86_64, and not be noarch?

No, this is not possible.
The "-devel" package must be noarch and be available on all architectures,
otherwise you get broken dependencies somewhere.

We even have special machinery in rust2rpm for crates that only compile on
specific architectures (the "supported-arches" setting in rust2rpm.toml)
because of this.
> At
> https://github.com/etemesi254/zune-image/blob/dev/crates/zune-jpeg/README.
> md#crate-features it says that this "Enables x86 specific instructions"

This should not be an issue according to the README:

https://github.com/etemesi254/zune-image/blob/dev/crates/zune-jpeg/README.md...

"""
Note that the x86 features are automatically disabled on platforms that aren't
x86 during compile time hence there is no need to disable them explicitly if
you are targeting such a platform.
"""
> x86 requires x86_64-v2, x86_64-v3. Wouldn't that be an issue for
>
> x86_64-v1 when used in an application using this as a dependency?

That is a good question. It looks like there is code to guard against using
instructions on CPUs where they are not available:

https://github.com/etemesi254/zune-image/blob/dev/crates/zune-core/src/optio...

This should work under most circumstances. The only way this could break (as
far as I can tell) would be if 1) zune-jpeg is built without "std" support 2)
with "x86" feature enabled *and* 3) the feature in question is available in the
environment where the code is compiled but *not* on the machine the code gets
executed on.

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

wojnilowicz  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #6 from wojnilowicz  ---
Package was generated with rust2rpm, simplifying the review.

✅ 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 matches upstream specification and is acceptable for Fedora
✅ license files are 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 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)

===

Thanks for the explanations. They seem reasonable to me. What about this?

(In reply to Fabio Valentini from comment #5)
> (In reply to wojnilowicz from comment #3)
> > Shouldn't the following subpackage:
> >
> > %package     -n %{name}+x86-devel
> > Summary:        %{summary}
> > BuildArch:      noarch
> >
> > be somehow restricted to x86_64, and not be noarch?
>
> No, this is not possible.
> The "-devel" package must be noarch and be available on all architectures,
> otherwise you get broken dependencies somewhere.
>
> We even have special machinery in rust2rpm for crates that only compile on
> specific architectures (the "supported-arches" setting in rust2rpm.toml)
> because of this.

How is it different from https://bugzilla.redhat.com/show_bug.cgi?id=2276290 ?
Is it because "supported-arches" there would be used for the whole crate,
because the crate as a whole wouldn't work,
while here it would have to be used only for a subpackage of the crate, because
only this subpackage makes no sense? I would assume that's impossible to filter
it out from the rpm point of view, however it's still doable to package it
friction less thanks to the following note:
> """
> Note that the x86 features are automatically disabled on platforms that
> aren't x86 during compile time hence there is no need to disable them
> explicitly if you are targeting such a platform.
> """

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

--- Comment #7 from Fabio Valentini  ---
Thank you for the thorough review!
> Is it because "supported-arches" there would be used for the whole crate, because the crate as a whole wouldn't work,
> while here it would have to be used only for a subpackage of the crate, because only this subpackage makes no sense?
> I would assume that's impossible to filter it out from the rpm point of view, however it's still doable to package it friction less thanks to the following note (...)

Yeah. supported-arches only affects which architectures a crate is built on and
where tests are run. It doesn't affect the package contents at all.
Packages that have "BuildArch: noarch" also aren't filtered out by architecture
(they are supposed to be architecture-independent after all), so there's not
much that can be done here.

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

Fedora Admin user for bugzilla script actions  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST

--- Comment #8 from Fedora Admin user for bugzilla script actions  ---
The Pagure repository was created at
https://src.fedoraproject.org/rpms/rust-zune-jpeg

[Bug 2292522] Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder

https://bugzilla.redhat.com/show_bug.cgi?id=2292522

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Fixed In Version|                            |rust-zune-jpeg-0.4.11-1.fc4
                   |                            |1
         Resolution|---                         |RAWHIDE
             Status|POST                        |CLOSED
        Last Closed|                            |2024-07-07 18:01:32

--- Comment #9 from Fabio Valentini  ---
Imported and built:
https://bodhi.fedoraproject.org/updates/FEDORA-2024-70b523927a