#1510 Rust guidelines cleanups and refresh for changes in rust2rpm v27 / v28
Merged by james. Opened by decathorpe.
Unknown source master

Download 1510.patch

I've split this into two commits to make reviewing hopefully a bit easier.

First commit:

I moved two sections into more suitable places:

  • The RPM Macros documentation (which is quite verbose) moved to a top-level section more towards the bottom of the page.
  • The guidelines for dynamically generated BuildRequires moved from a subsection to a top-level section more towards the top of the page.

Second commit:

  • Cleanups for things that are now outdated or no longer necessary.
  • Simplifications for the Naming and Source guidelines that were just way too complicated.
  • Updates to account for changes in rust2rpm v27 and v28.

Notably, rust2rpm no longer supports generating (incomplete) spec files for "non-crate" projects - instead, I've just added two basic spec templates for two common "non-crate" package cases.

In total, cleanups result in about ~100 fewer lines of documentation, and the added templates add ~100 lines, the remaining diffstat is minor cleanups, formatting fixes, and moving things around.

:thumbsup:

Would make sense to include the command for rust2rpm in here? Maybe there are some flags that need to be passed on the second rerun to re-apply patches and etc..

:thumbsup:

Would make sense to include the command for rust2rpm in here? Maybe there are some flags that need to be passed on the second rerun to re-apply patches and etc..

I'm kind of intentially keeping rust2rpm documentation separate from the actual Rust Packaging Guidelines. The manual pages for rust2rpm and rust2rpm.toml should be pretty comprehensive, and I don't want to duplicate this information here too.

SourceLicense here?

The MUST here has double meaning:
- can only be called in xxx scriplet
- it must be present in xxx scriplet

The former could be covered by a general statement or a table, which would clear up the MUST to focus on if it needs to be used or not

Calling this macro *MAY* be skipped
if the crate is not supported on the current CPU architecture.

You lost me here. What is CPU architecture have to do here?

It would be good to point where the compiled artifacts are if %cargo_install does not work. Dunno what causes it to fail, but it happens under workspace I believe

SourceLicense here?

No

The other three comments are for sections that are not touched by this PR so if you have issues with them, please comment elsewhere.

Could you add some "litmus test" for the non-rust audience to figure out if a package is a crate, binary, workspace or combination of those.

Could you add some "litmus test" for the non-rust audience to figure out if a package is a crate, binary, workspace or combination of those.

The Rust guidelines docs are already quite long. I don't want to spend too many characters explaining things that are not specific to packaging. I can't write them assuming that people know nothing about how Rust or cargo work :(

Should specify somewhere that rust2rpm.toml must be included in the dist-git. Might as well codify it for including it even if it is empty.

We should recommend %pyproject_save_files -l rustypackage here. It’s always best to have the -l when you are expecting your license file(s) to be handled in %{pyproject_files}. We might want to recommend double-checking that both the upstream license file and the LICENSE.dependencies file are handled as expected, using rpm -qL -p ….

Consider recommending defining a macro to avoid repeating the description in the spec file, as in https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file.

This seems to be a “GitHub-like” URL scheme despite the generic forge URL. For GitHub, %{url}/archive/v%{version}/rustypackage-%{version}.tar.gz is a better URL because the archive name is more recognizable and matches the extraction directory.

https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file has %autosetup -p1 -n ...-%{version} here for a good reason: this will usually need to be %autosetup -p1 -nrustypackage-%{version}. Only in rare cases, where the canonical name starts withpython-, like https://pypi.org/project/python-irodsclient/, and the source package is namedpython-foorather thanpython-python-foo(https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_library_naming) can-n` be avoided for Python library packages.

The template at https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file still has BuildRequires: python3-devel, but it’s not necessary if using %pyproject_buildrequires (or the pyproject declarative buildsystem). See https://pagure.io/packaging-committee/pull-request/1379 and https://pagure.io/packaging-committee/pull-request/1384. It’s not harmful, but it could be omitted from the template.

We should probably make this match the style of the License in the non-crate Rust project template above,

License:        %{shrink:
    WTFPL AND
    ...
}

1 new commit added

  • Rust: more small improvements and clarifications

Thank you for your feedback - I think I have addressed all comments in the third commit that I just pushed - please take a look.

Can we please add a recommendation to include it as a source in the SRPM? Since these are used as the preferred input control mechanism for creating/updating the spec, it makes sense to do so.

Pull-Request has been merged by james

Metadata