Please, notice that this package is required to upgrade nispor, which is an existing fedora package. https://koji.fedoraproject.org/koji/buildinfo?buildID=1836976

This package is now available for review. Thank you!

* Package name is correct
* Source matches upstream
* License is good for Fedora
* SPEC is reasonably clean, legible and uses macros consistently
* Builds fine in mock
* Provides/Requires look okay

Here's a few things that need fixing or explanation:

0.) The latest version seems to be 0.2.1.

Why are you packaging an old one?

1.) It is not clear what does the license apply to.

None of the *.rs source files indicate how are they licensed.
There's a MIT license file thrown in -- but it's not clear which files
does it apply to.

Please ask upstream to clarify to situation -- ideally by adding
a license statement to each source file. A SPDX tag would do too.

2.) The summary doesn't look good.

It is supposed to explain *what* is in the package and starting it
with a verb is a sure wait to fail at doing that.

Moreover the summary of each subpackage seems to be the same.
Instead, it should help the user understand how do the subpackages
differ.

3.) Expand on the description.

Instead of just repeating the summary line, you should actually explain
what is the package good for. E.g. ("This package contains library used
for communicating via generic netlink protocol from programs written
in Rust language.")

4.) No need to repeat BuildArch everywhere.

All subpackages are noarch. Just include BuildArch before the %package
declarations.

5.) The filelists look suspicious:

=== rust-genetlink-devel-0.1.0-1.fc35.noarch.rpm ===
...
-rw-r--r--    1 root     root                     1778 Jan  1  1970 /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml

=== rust-genetlink+default-devel-0.1.0-1.fc35.noarch.rpm ===
-rw-r--r--    1 root     root                     1778 Jan  1  1970 /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml

=== rust-genetlink+smol_socket-devel-0.1.0-1.fc35.noarch.rpm ===
-rw-r--r--    1 root     root                     1778 Jan  1  1970 /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml

=== rust-genetlink+async-std-devel-0.1.0-1.fc35.noarch.rpm ===
-rw-r--r--    1 root     root                     1778 Jan  1  1970 /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml

=== rust-genetlink+tokio-devel-0.1.0-1.fc35.noarch.rpm ===
-rw-r--r--    1 root     root                     1778 Jan  1  1970 /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml

=== rust-genetlink+tokio_socket-devel-0.1.0-1.fc35.noarch.rpm ===
-rw-r--r--    1 root     root                     1778 Jan  1  1970 /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml

Why do the subpackages even exist, when they all package a file that
rust-genetlink-devel also packages?

6.) LICENSE-MIT is packaged twice in rust-genetlink-devel

-rw-r--r--    1 root     root                     1532 Nov 29  1973 /usr/share/cargo/registry/genetlink-0.1.0/LICENSE-MIT
-rw-r--r--    1 root     root                     1532 Nov 29  1973 /usr/share/licenses/rust-genetlink-devel/LICENSE-MIT

7.) %global debug_package %{nil}

This is unnecessary since you're building all noarch anyway.

(In reply to Lubomir Rintel from comment #3)
> * Package name is correct
> * Source matches upstream
> * License is good for Fedora
> * SPEC is reasonably clean, legible and uses macros consistently
> * Builds fine in mock
> * Provides/Requires look okay

Something that is also important to check is whether Rust packages (and all generated subpackages) *install* correctly, not only that the package builds.

> Here's a few things that need fixing or explanation:
> 
> 0.) The latest version seems to be 0.2.1.
> 
> Why are you packaging an old one?
> 
> 1.) It is not clear what does the license apply to.
> 
> None of the *.rs source files indicate how are they licensed.
> There's a MIT license file thrown in -- but it's not clear which files
> does it apply to.
> 
> Please ask upstream to clarify to situation -- ideally by adding
> a license statement to each source file. A SPDX tag would do too.

That is not necessary. The license specified in the Cargo.toml file is supposed to describe the license of the entire crate / project.
Literally no Rust project I have seen includes license headers in every source file (and I have probably looked at 1000+ Rust projects by now).

> 2.) The summary doesn't look good.
> 
> It is supposed to explain *what* is in the package and starting it
> with a verb is a sure wait to fail at doing that.
> 
> Moreover the summary of each subpackage seems to be the same.
> Instead, it should help the user understand how do the subpackages
> differ.

The Summaries are generated by rust2rpm, fso it would need to be fixed by generating a patch with "rust2rpm -p", so that the change applies consistently everywhere.
Additionally, the -devel and +feature-devel subpackages are also only ever supposed to be consumed by RPM builds as BuildRequires, so making Summary or %description interesting for humans is very low priority (except for the main package's Summary).

> 3.) Expand on the description.
> 
> Instead of just repeating the summary line, you should actually explain
> what is the package good for. E.g. ("This package contains library used
> for communicating via generic netlink protocol from programs written
> in Rust language.")

Please, don't manually change Summary or %desciption tags. Those are automatically generated, and only meant to be minimally useful while not being an empty string.

> 4.) No need to repeat BuildArch everywhere.
> 
> All subpackages are noarch. Just include BuildArch before the %package
> declarations.

That's not how Rust packaging works.

> 5.) The filelists look suspicious:
> 
> === rust-genetlink-devel-0.1.0-1.fc35.noarch.rpm ===
> ...
> -rw-r--r--    1 root     root                     1778 Jan  1  1970
> /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml
> 
> === rust-genetlink+default-devel-0.1.0-1.fc35.noarch.rpm ===
> -rw-r--r--    1 root     root                     1778 Jan  1  1970
> /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml
> 
> === rust-genetlink+smol_socket-devel-0.1.0-1.fc35.noarch.rpm ===
> -rw-r--r--    1 root     root                     1778 Jan  1  1970
> /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml
> 
> === rust-genetlink+async-std-devel-0.1.0-1.fc35.noarch.rpm ===
> -rw-r--r--    1 root     root                     1778 Jan  1  1970
> /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml
> 
> === rust-genetlink+tokio-devel-0.1.0-1.fc35.noarch.rpm ===
> -rw-r--r--    1 root     root                     1778 Jan  1  1970
> /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml
> 
> === rust-genetlink+tokio_socket-devel-0.1.0-1.fc35.noarch.rpm ===
> -rw-r--r--    1 root     root                     1778 Jan  1  1970
> /usr/share/cargo/registry/genetlink-0.1.0/Cargo.toml
> 
> Why do the subpackages even exist, when they all package a file that
> rust-genetlink-devel also packages?

This is intentional. The file is only included as %ghost.
Additionally, the subpackages encode RPM dependency information, they don't contain files.

> 6.) LICENSE-MIT is packaged twice in rust-genetlink-devel
> 
> -rw-r--r--    1 root     root                     1532 Nov 29  1973
> /usr/share/cargo/registry/genetlink-0.1.0/LICENSE-MIT
> -rw-r--r--    1 root     root                     1532 Nov 29  1973
> /usr/share/licenses/rust-genetlink-devel/LICENSE-MIT

This is expected for Rust packages, though it could be improved.
Please report this issue with https://pagure.io/fedora-rust/rust2rpm instead of manual changes that only creating work when rebasing a package for new versions.

(In reply to Lubomir Rintel from comment #4)
> 7.) %global debug_package %{nil}
> 
> This is unnecessary since you're building all noarch anyway.

This is not true. The package is *not* noarch, as a test build and running unit tests happen on all supported architectures. Only the built subpackages are noarch, the source package is not.

(In reply to Lubomir Rintel from comment #3)
> * Package name is correct
> * Source matches upstream
> * License is good for Fedora
> * SPEC is reasonably clean, legible and uses macros consistently
> * Builds fine in mock
> * Provides/Requires look okay
> 
> Here's a few things that need fixing or explanation:
> 
> 0.) The latest version seems to be 0.2.1.
> 
> Why are you packaging an old one?

When I created the BZ this was the latest version. Anyway, I can rebase it later. I don't think this will be a problem at all.

As I noticed that for licenses there is already an issue filed https://pagure.io/fedora-rust/rust2rpm/issue/176, could this request be marked as approved? I will discuss the other comments with the upstream community.

Thanks!

> As I noticed that for licenses there is already an issue filed https://pagure.io/fedora-rust/rust2rpm/issue/176, could this request be marked as approved? I will discuss the other comments with the upstream community.

What do you mean "approved"? The ticket is not about a rules change, but rather about implementing a change in rust2rpm. We don't make changes like this one in individual packages, but push it upstream to the .spec generator so everyone can benefit and nobody has to make manual adjustments.

As Rust SIG member and primary Rust packager in Fedora would approve the package as it is right now, with one exception:

Error: 
 Problem: conflicting requests
  - nothing provides (crate(netlink-proto/smol_socket) >= 0.7.0 with crate(netlink-proto/smol_socket) < 0.8.0~) needed by rust-genetlink+smol_socket-devel-0.1.0-1.fc36.noarch

The subpackage for the smol_socket feature is not installable, because the smol_socket feature in rust-netlink-proto package is missing for some reason.

If this is not a feature you need, drop it from this package (either by patching it out when running "rust2rpm -p", or by supplying a ".rust2rpm.conf" file with the following contents, that you can also import to dist-git to make this preference "permanent" for future updates:

```
[DEFAULT]
unwanted-features =
  smol_socket
```

(In reply to Fabio Valentini from comment #8)
> > As I noticed that for licenses there is already an issue filed https://pagure.io/fedora-rust/rust2rpm/issue/176, could this request be marked as approved? I will discuss the other comments with the upstream community.
> 
> What do you mean "approved"? The ticket is not about a rules change, but
> rather about implementing a change in rust2rpm. We don't make changes like
> this one in individual packages, but push it upstream to the .spec generator
> so everyone can benefit and nobody has to make manual adjustments.
> 
> As Rust SIG member and primary Rust packager in Fedora would approve the
> package as it is right now, with one exception:
> 
> Error: 
>  Problem: conflicting requests
>   - nothing provides (crate(netlink-proto/smol_socket) >= 0.7.0 with
> crate(netlink-proto/smol_socket) < 0.8.0~) needed by
> rust-genetlink+smol_socket-devel-0.1.0-1.fc36.noarch
> 

Agh. Let me fix these packages on Fedora rawhide and 35. Thank you!

> The subpackage for the smol_socket feature is not installable, because the
> smol_socket feature in rust-netlink-proto package is missing for some reason.
> 
> If this is not a feature you need, drop it from this package (either by
> patching it out when running "rust2rpm -p", or by supplying a
> ".rust2rpm.conf" file with the following contents, that you can also import
> to dist-git to make this preference "permanent" for future updates:
> 
> ```
> [DEFAULT]
> unwanted-features =
>   smol_socket
> ```

Thank you, package is APPROVED

Please make sure you address the dependency issue pointed out by Fabio before importing and update to a later version if possible.

It would be excellent if you found a way to make the summary/description more reasonable -- either figure a way of improving it downstream or convince the upstream to use something that would look okay in Fedora.

(In reply to Lubomir Rintel from comment #10)
> Thank you, package is APPROVED
> 
> Please make sure you address the dependency issue pointed out by Fabio
> before importing and update to a later version if possible.
> 
> It would be excellent if you found a way to make the summary/description
> more reasonable -- either figure a way of improving it downstream or
> convince the upstream to use something that would look okay in Fedora.

Thank you Lubomir, I have addressed the dependency issue already. I have updated all the rust-netlink packages in Fedora rawhide and 35. Thanks.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-genetlink

FEDORA-2022-dae6cf6de5 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-dae6cf6de5

FEDORA-2022-dae6cf6de5 has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.