1. I think, using "%{_builddir}/feather/redhat-linux-build/bin/feather" in %install section is not portable. Since out of source build was introduced, the directory where build happens changed at least once, and I won't be surprised if it will change in the future again. Better use "%{_vpath_builddir}/bin/feather" instead. Also, it will improve readability.
2. Or even better, try to use %cmake_install to install binary, and resort to manual installation only if this method doesn't work, but do it with "install" command, not "cp".
3. Using "%{_builddir}/feather/" prefix in %install section is unnecessary and harms readability: %install script is already run in this directory.

Please note that this is not a proper complete review, I've just listed things that I found by looking at the spec file.

Thanks for the review!

1.) Thanks, I did not know about _vpath_builddir. Very useful!
2.) Unfortunately that installs the binary to /usr/feather, installs header files at random places, installs libraries that are only used internally, etc.
3.) Thanks, removed.

I updated the spec file (no updated SRPM yet).

Would you be willing to formally review?

"Free and open-source" applies to all packages in Fedora. So that part of the Summary
is not useful. And it you don't know what "monero" is, and I don't, the rest won't tell
you much either… Please try to write the Summary+description so that it is meaningful to
someone who is completely new to the subject.

Also "written in C++ with the Qt framework" is not terribly interesting to users. You
can mention Qt, but other details of implementation are unimportant.

> sed -i 's/^\(Icon\|Exec\)=feather$/&wallet/' %{buildroot}%{_datadir}/applications/featherwallet.desktop
> sed -i 's/^Name=Feather$/& Wallet/' %{buildroot}%{_datadir}/applications/featherwallet.desktop

Those commands should be in %prep. This is where we apply patches and do other modifications
to sources.

> mkdir -p %{buildroot}%{_bindir} %{buildroot}%{_datadir}/applications %{buildroot}%{_datadir}/pixmaps
> cp %{_vpath_builddir}/bin/feather %{buildroot}%{_bindir}/featherwallet
> cp src/assets/feather.desktop %{buildroot}%{_datadir}/applications/featherwallet.desktop

It's a bit nicer to say:
install -D %{buildroot}%{_bindir}/featherwallet %{_vpath_builddir}/bin/feather
install -m0644 -D %{buildroot}%{_datadir}/applications/featherwallet.desktop src/assets/feather.desktop
etc.

I'd recommend dropping the manual Release and %changelog handling.
See https://docs.pagure.org/fedora-infra.rpmautospec/index.html.

> %global debug_package %{nil}

Why? This is usually a sign of other problem with the build systems, usually missing flags.
Please drop this and instead figure out what is needed to for the -debuginfo package to
generate properly.

Since this is a graphical application, it should have an appdata file:
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
(with a pretty screenshot ;))

feather-2.3.0-1.fc37.src.rpm

Sorry, last comment was in accident when changing the title.

I uploaded a new SRPM: https://nil.im/feather-2.3.0-1.fc37.src.rpm
Updated spec file: https://nil.im/feather.spec

PTAL!

> License:       BSD

The License tag must be in SPDX format:

$ license-fedora2spdx 'BSD'
Warning: more options on how to interpret BSD. Possible options: ['BSD-1-Clause', 'BSD-2-Clause-FreeBSD', 'BSD-2-Clause-Views', 'BSD-2-Clause', 'BSD-3-Clause-Modification', 'BSD-3-Clause']
{{pick BSD choice}}

> ExcludeArch: armv7hl

Please use this: ExcludeArch: %{arm} %{ix86}

> gpgv2 --quiet --keyring %{SOURCE2} %{SOURCE1} %{SOURCE0}

Please switch to %{gpgverify} macro:
%{gpgverify} --keyring='%{SOURCE2}' --signature='%{SOURCE1}' --data='%{SOURCE0}'

> -DCMAKE_BUILD_TYPE=RelWithDebInfo

You can use Release here, because `-g` flag is already present in Fedora build flags. RelWithDebInfo will just duplicate it.

> # Their install target is not meant to be used. It installs the binary into
> # /usr (not /usr/bin) and installs a bunch of other useless files.

I think you should port their CMake build manifest to GNUInstallDirs and send a pull request instead of manually calling install binary.

> The License tag must be in SPDX format:

Done

> Please use this: ExcludeArch: %{arm} %{ix86}

Done

> Please switch to %{gpgverify} macro:

Done

> You can use Release here, because `-g` flag is already present in Fedora build flags. RelWithDebInfo will just duplicate it.

This doesn't work - with just Release (the default IIRC), it'll result in an empty debug package. RelWithDebInfo makes it work.

> I think you should port their CMake build manifest to GNUInstallDirs and send a pull request instead of manually calling install binary.

That doesn't help with this release, though, so is something for later :).

> This doesn't work - with just Release (the default IIRC), it'll result in an empty debug package. RelWithDebInfo makes it work.

Then the package ignores the Fedora build flags. You need to check this.

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.

New spec file and SRPM for much newer version.

I checked and the Fedora flags *are* passed to GCC (both during compiling and linking), but RelWithDebInfo is still required as otherwise it seems the build process strips the binaries.

Spec URL: https://nil.im/feather.spec
SRPM URL: https://nil.im/feather-2.6.8-1.fc40.src.rpm

+ package name is OK
+ license is acceptable for Fedora (BSD-3-Clause)
+ license is specified correctly as SPDX
+ latest version
+ gpg signature is checked
+ builds and installs OK
+ builds flags are passed to the build commands
+ BR/P/R look OK
+ appdata file is present
- %check is not present. Maybe add a test run to make sure that the executable works, e.g. just print '--help' output?

rpmlint:
feather.src: E: spelling-error ('Monero', 'Summary(en_US) Monero -> Moreno, Monroe, Monera')
feather.x86_64: E: spelling-error ('Monero', 'Summary(en_US) Monero -> Moreno, Monroe, Monera')
Obviously bogus.

feather.x86_64: W: no-manual-page-for-binary feather
feather.x86_64: W: no-documentation
True, but not a big issue.

feather.x86_64: W: crypto-policy-non-compliance-openssl /usr/bin/feather SSL_CTX_set_cipher_list
Hmm, this requires investigation.

feather-2.6.8/monero/contrib/epee/src/net_ssl.cpp
312:  SSL_CTX_set_cipher_list(ssl_context.native_handle(), "ECDHE-ECDSA-CHACHA20-POLY1305-SHA256:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256");

https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/ says:
> OpenSSL applications:
> If the application doesn’t have a configuration file, ensure that there is no default cipher list specified, or that the default list is set as "PROFILE=SYSTEM". That is, check the source code for SSL_CTX_set_cipher_list(). If it is not present then nothing needs to be done (the default is used). Otherwise, if that call is present and provided a fixed string which does not contain PSK or SRP, replace the string with "PROFILE=SYSTEM", or remove the call.

But also:
> Note however, that there are applications which intentionally set weaker, or custom settings on a purpose (e.g., postfix); those need not adhere to the policy. When in doubt, discuss with the Fedora crypto team.

Feather sets a *stronger* policy, clearly on purpose. I think this clearly falls into the exception quoted above and doesn't need to be discussed with the "Fedora crypto team".

Package is APPROVED.

The Pagure repository was created at https://src.fedoraproject.org/rpms/feather

Thanks for the review!

Re OpenSSL policy: There is another reason to never ever touch this in feather: It will make you immediately stand out among everybody else, ruining your privacy.

FEDORA-2024-0a046f3b59 (feather-2.6.8-1.fc42) has been submitted as an update to Fedora 42.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-0a046f3b59

FEDORA-2024-0a046f3b59 (feather-2.6.8-1.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.