Bug 2086484 - Review Request: neomutt - Email Client
Summary: Review Request: neomutt - Email Client
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Carl George 🤠
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-05-16 09:41 UTC by Richard Russon
Modified: 2022-10-05 02:39 UTC (History)
7 users (show)

Fixed In Version: neomutt-20220629-1.fc37
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-10-05 02:39:13 UTC
Type: ---
Embargoed:
carl: fedora-review+


Attachments (Terms of Use)
RPM Spec file for NeoMutt (3.56 KB, text/plain)
2022-05-20 01:43 UTC, Richard Russon
no flags Details

Description Richard Russon 2022-05-16 09:41:58 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt/fedora-rawhide-x86_64/04346132-neomutt/neomutt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt/fedora-rawhide-x86_64/04346132-neomutt/neomutt-20220429-1.fc37.src.rpm
Description: NeoMutt - Email Client
  NeoMutt is a fork of the venerable Mutt Email Client.
  It has extra features, such as Notmuch integration
  and a much-improved codebase
Fedora Account System Username: flatcap

Comment 1 Carl George 🤠 2022-05-18 04:58:48 UTC
Here are the things I've noticed that need to be fixed.

===============================================================================

The licensecheck utility found multiple files under different licenses.
These all must be reflected in the License field.  There also must be a
comment explaining the multiple license breakdown.

Public Domain:
contrib/colorschemes/zenburn.neomuttrc
pgpewrap.c
docs/mbox.5

MIT:
contrib/colorschemes/neonwolf-256.neomuttrc
test/acutest.h

BSD:
autosetup/mutt-gettext.tcl
autosetup/mutt-iconv.tcl
contrib/hcache-bench/neomutt-hcache-bench.sh
mutt/queue.h

Unlicense:
contrib/keybase/

https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_field

===============================================================================

Remove the BugURL line.  The Fedora build system (koji) will set this to
https://bugz.fedoraproject.org/<packagename>, which redirects to a list of
open bugzilla bugs.  I can't find any documentation about this, but you can
verify it on a Fedora system by running `rpm -q --qf '%{bugurl}\n'
<anypackagename>`.

===============================================================================

It is recommended to follow the following format for the Source URL.

-Source: https://github.com/neomutt/neomutt/archive/%{version}.tar.gz
+Source: https://github.com/neomutt/neomutt/archive/%{version}/%{name}-%{version}.tar.gz

https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags

===============================================================================

Each patch should have a comment about their upstream status, or
justification why they are downstream-only.

https://docs.fedoraproject.org/en-US/packaging-guidelines/PatchUpstreamStatus/

===============================================================================

No C compiler is listed as a build requirement.  The build currently works
because the perl build requirement recursively pulls in gcc, but this is not
guaranteed to always be the case.  Add `BuildRequires: gcc`.

https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/#_buildrequires_and_requires
https://docs.fedoraproject.org/en-US/packaging-guidelines/#compiler

===============================================================================

The conditional _pkgdocdir macro definition can be removed.  That's already
defined on all maintained versioned of Fedora and RHEL.

===============================================================================

The invocations of ./configure, make, and make install should be replaced by
the standardized macro equivalents of %configure, %make_build, and
%make_install.  This will allow the removal of --sysconfdir, _smp_mflags,
and DESTDIR, as those are already included in the macros.

===============================================================================

The license file should be marked as %license, not %doc.  This ensures the
license is included even when the transaction flag nodocs is set.

-%doc *.md
+%license LICENSE.md
+%doc AUTHORS.md ChangeLog.md INSTALL.md README.md SECURITY.md

https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text

===============================================================================

The package places files in /usr/libexec/neomutt/, but it doesn't own that
directory.  The easiest fix it to just have %files own that directory
recursively.  It should also be referenced as %{_libexecdir}.

-/usr/libexec/neomutt/pgpewrap
-/usr/libexec/neomutt/smime_keys
+%{_libexecdir}/neomutt/

Comment 2 Carl George 🤠 2022-05-18 04:59:33 UTC
I had a few other questions/suggestions that may or may not lead to changes.

===============================================================================

Source1 is just installled as a %doc file.  Could this be added to the
upstream GitHub repo and maintained there?  That would also avoid the need
to manually copy it to the build directory in %prep, as it would already be
there from the tarball extraction.

===============================================================================

Several conditionals are disabled for EL releases.  The comments indicate
this is due to packages missing from RHEL, or not working from RHEL.  Some
of these are incorrect.  Additionally, Fedora packages have the benefit of
being able to be built in EPEL, where lots of packages that are missing from
RHEL are maintained by the community for use on RHEL.  This will probably
allow many of these conditionals to be re-enabled.  If the provided version
is too old to work or is otherwise broken, then the affected conditional can
be left disabled for that release.

idn: libidn-devel in rhel7, rhel8, and epel9
lmdb: lmdb-devel in epel7, rhel8, and rhel9
autocrypt: sqlite-devel in rhel7, rhel8, and rhel9
idn2: libidn2-devel in epel7, rhel8, and rhel9
lua: lua-devel in rhel7, rhel8, and rhel9
lz4: lz4-devel in rhel7, rhel8, and rhel9
notmuch: notmuch-devel in epel8 and epel9
pcre2: pcre2-devel in rhel7, rhel8, and rhel9
zstd: libzstd-devel epel7, rhel8, and rhel9

===============================================================================

Consider replacing instances of $RPM_BUILD_ROOT with %{buildroot}.  This is
not required.  The guidelines only state to pick one and use it
consistently, but I prefer the latter.

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_using_buildroot_and_optflags_vs_rpm_build_root_and_rpm_opt_flags

===============================================================================

Why is the %{_docdir}/neomutt directory being deleted on RHEL during
%install?

===============================================================================

There are a large number of %doc files.  Consider splitting them out to a
neomutt-doc subpackage.

https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Comment 3 Richard Russon 2022-05-18 16:54:52 UTC
Thanks for the thorough review!
I've spent all day trying to understand my .spec file.
For six years, I've been hacking in ignorance :-)

I'll have some updates and questions, soon.

Comment 4 Carl George 🤠 2022-05-19 02:57:43 UTC
Sure thing, take your time.  If you would like interactive help feel free to ping me on either IRC (carlwgeorge on libera.chat) or Matrix (@carlwgeorge:matrix.org).

Comment 5 Richard Russon 2022-05-20 01:43:58 UTC
Created attachment 1881493 [details]
RPM Spec file for NeoMutt

Updated .spec file following review

Comment 6 Richard Russon 2022-05-20 01:47:45 UTC
I've covered most of your requests, and learned a lot along the way.
I think I now understand how the Centos Stream, RHEL, EPEL fit together.

It's all gone OK, except for RHEL7 -- it's just too old.
There's GPGME, but it's not new enough for Autocrypt;
There's Lua, but it's too old to compile;
and our biggest feature, Notmuch, is a non-starter.

If we don't even try to support RHEL7, then the spec file is trivial.

===============================================================================

Simple stuff, first:

> Remove the BugURL line
> format for the Source URL
> Each patch should have a comment
> BuildRequires: gcc
> _pkgdocdir macro definition can be removed
> The package places files in /usr/libexec/neomutt/
> replacing instances of $RPM_BUILD_ROOT with %{buildroot}

Done

===============================================================================

> licensecheck utility

I've done my best to comment the exceptions from GPLv2+.
Suggestions welcome.

===============================================================================

> %configure, %make_build, and make_install

Done.  This _mostly_ works.
RHEL7 seems to require me to add `CC=gcc` to %configure
(other systems seem to work ok without it)

===============================================================================

> license file should be marked as %license, not %doc

> -%doc *.md
> +%license LICENSE.md
> +%doc AUTHORS.md ChangeLog.md INSTALL.md README.md SECURITY.md

I did this, but then it complained that LICENSE.md was installed but not
packaged.  Currently the spec has both %license and %doc for LICENSE.md

===============================================================================

> Source1 is just installed as a %doc file

Good point.
I've upstreamed that and a couple of the patches.
However, the spec file won't work until I make a release with the changes.
I'll do that soon.

===============================================================================

> Several conditionals are disabled for EL releases.
> this is due to packages missing from RHEL, or not working from RHEL

Yeah, the spec file is very old :-)

I've stripped out the unnecessary conditionals, leaving only RHEL7.

EPEL now contains everything NeoMutt needs.

===============================================================================

> Why is the %{_docdir}/neomutt directory being deleted on RHEL during install?

Ah!  I discovered the reason (dating back six years).
NeoMutt installs the docs into /usr/share/doc/neomutt
RHEL7 (and only this) expects its docs in neomutt-VERSION

At the end of every build, Mock would complain about all the unpackaged files.
I've replaced the `rm -fr` with a `mv` and now it's happier.

===============================================================================

> There are a large number of %doc files.  Consider splitting them out

I gave this a try, but I couldn't figure it out.
I was hoping to be able to just move the %doc, etc, into

%package doc
%files doc
...

Unfortunately, this expected files in /usr/share/doc/neomutt-doc/
Is there some simple way to set the install dir to just 'neomutt'?

When you next have some time, I'm ready for your next round of reviewing.
Many thanks!

Comment 7 Carl George 🤠 2022-05-24 04:44:54 UTC
> It's all gone OK, except for RHEL7 -- it's just too old.
> If we don't even try to support RHEL7, then the spec file is trivial.

It's up to you.  You can build for as many or as few EPEL branches as you want.  If you prefer to keep the spec file cleaner, it's ok to avoid EPEL7.  You can always add it later if you change your mind.

> I've done my best to comment the exceptions from GPLv2+.
> Suggestions welcome.

Looks great!

> Done.  This _mostly_ works.
> RHEL7 seems to require me to add `CC=gcc` to %configure
> (other systems seem to work ok without it)

I imagine it's a problem with the much older version of gcc, or perhaps just the older rpm macros.  Probably not worth the hassle of pinning it down when we can just skip building for EPEL7.

> I did this, but then it complained that LICENSE.md was installed but not
> packaged.  Currently the spec has both %license and %doc for LICENSE.md

That is strange.  The one line should be sufficient to copy the file to the correct location and own the file.  Do you have a local ~/.rpmmacros file that is overriding %license?  Does it happen when building for Fedora and EPEL, or just EPEL?

> I gave this a try, but I couldn't figure it out.

That's ok, it's not strictly required.  We don't need to block the review on it.  In the future if anyone strongly wants it they can send a pull request to implement it.

> Is there some simple way to set the install dir to just 'neomutt'?

Yes, you can do `%global _docdir_fmt %{name}`.  Be aware this may cause issues if multiple subpackages share ownership of any of the doc files (see bug 1625015).  I would put AUTHORS.md, ChangeLog.md, README.md, and SECURITY.md only in the main package, and everything else only in neomutt-doc.  Also, the INSTALL.md file should be omitted entirely, as it won't be relevant for people that install the rpm.

Comment 8 Carl George 🤠 2022-05-24 05:00:11 UTC
I took a look at the spec file attachment, and things are looking real good, but two small new issues were introduced.

I imagine you added `rpm --showrc` for troubleshooting, but it should not be included in the final spec file.  Pro-tip, this command also works in a container.  Make sure to install redhat-rpm-config (and epel-rpm-macros for EL releases) for all macros to be available.

Your multi-line configure is missing an escape after --full-doc, so the conditional flags after that won't be included.

One more thing, please post revisions to the package as new `Spec URL:` and `SRPM URL:` lines (with working URLs), similar to the original comment template.  You don't have to repeat the `Description:` or `Fedora Account System Username:` lines.  The fedora-review tool doesn't work with attachments, it expects URLs in that format and will pick the last occurrences of those lines in the comments of the given bug.

Comment 9 Carl George 🤠 2022-05-24 05:27:44 UTC
I see what's happening with LICENSE.md.  %license handles copying the file to /usr/share/licenses/neomutt/, but the upstream Makefile is putting another copy in /usr/share/doc/neomutt/.  I'm not sure if you'd prefer to adjust the upstream Makefile or just create a patch for this spec file to not install that file (or INSTALL.md) only during the rpm build.

https://github.com/neomutt/neomutt/blob/d0f9f8f3da210c085d55e8e33dd64b0996d53957/docs/Makefile.autosetup#L27-L30

Comment 10 Michel Lind 2022-05-26 17:32:46 UTC
Subscribing to it as some of my coworkers want to run neomutt at work

As per the dual license files - yeah I normally just delete them at the end of %install rather than messing with Makefile, unless the changes are clean enough that they can be upstreamed.

Once this is in could it be branched for EPEL 9 and 8 as well? Thanks!

Comment 11 Richard Russon 2022-06-07 10:35:35 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt-test/fedora-36-x86_64/04502025-neomutt/neomutt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt-test/fedora-36-x86_64/04502025-neomutt/neomutt-20220606-1.fc36.src.rpm

Sorry for the slow reply.
Things have been upstreamed and we're down to two Fedora-specific patches.

> > It's all gone OK, except for RHEL7

> it's ok to avoid EPEL7

Thanks.
Then I choose not to support RHEL7/EPEL7.
NeoMutt offers almost nothing over Mutt on that platform.

> I imagine you added `rpm --showrc` for troubleshooting

D'oh.  Removed

> Your multi-line configure is missing an escape

Fixed

> One more thing, please post revisions to the package as new
> `Spec URL:` and `SRPM URL:` lines (with working URLs)

Done

Thanks

Comment 12 Carl George 🤠 2022-06-11 06:25:55 UTC
This is looking real good.  We do still need to put the finishing touches on the documentation/license handling.  I just realized this spec is doing something that is forbidden in the guidelines [0].

> Marking a relative path with %doc in the %files section will cause RPM to copy the referenced file or directory from %_builddir to the proper location for documentation. Files can also be placed in %_pkgdocdir, and the build scripts of the software being packaged may do this automatically when called in %install. However, mixing these methods is problematic and may result in duplicated or conflicting files, so use of %doc with relative paths and installation of files directly into %_pkgdocdir in the same source package is forbidden.

The upstream Makefile is installing the files into %{buildroot}%{_pkgdocdir}, then %doc is doing it again.  This is also why we run into issues not listing all the files as %doc.  What we should do is rm INSTALL.md and LICENSE.md from %{_pkgdocdir} during %install (right after %make_install), then only list %{_pkgdocdir} in %files, removing all %doc lines with relative paths.  I tried this and it worked, except it needed one more tweak to resolve an "ambiguous python shebang" error (this can probably be fixed upstream).


 %{make_install}
+rm %{buildroot}%{_pkgdocdir}/INSTALL.md %{buildroot}%{_pkgdocdir}/LICENSE.md
+sed -e '1 s/python/python3/' -i %{buildroot}%{_pkgdocdir}/keybase/keybase.py


 %license LICENSE.md
-%doc LICENSE.md
-%doc AUTHORS.md ChangeLog.md INSTALL.md README.md SECURITY.md
-%doc docs/CODE_OF_CONDUCT.md docs/CONTRIBUTING.md
-%doc docs/*.txt
-%doc docs/*.html
-%doc docs/mime.types
-%doc contrib/account-command
-%doc contrib/colorschemes
-%doc contrib/hcache-bench
-%doc contrib/keybase
-%doc contrib/logo
-%doc contrib/lua
-%doc contrib/samples
-%doc contrib/vim-keys
-%doc contrib/oauth2
+%{_pkgdocdir}


[0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation

Comment 13 Carl George 🤠 2022-06-28 02:46:27 UTC
While at a conference last week I was chatting with another Fedora packager who uses neomutt (FAS steve).  He offered to co-maintain the package once it's approved.  I can help you set up that permission when then time comes.

Have you had a chance to fix up the documentation as described in the previous comment?

Comment 14 Richard Russon 2022-06-28 09:46:43 UTC
Damn!  Sorry, I missed the notification.  I'm on it.

Since then, we've been shuffling files around.
We've dropped some old contrib cruft and moved all the non-doc files out of /usr/share/doc/neomutt.
Contrib will now be installed to the data dir: /usr/share/neomutt

Comment 15 Richard Russon 2022-06-28 09:47:02 UTC
Hi Steve!
Thanks for your offer to help maintain the package.

Comment 16 Richard Russon 2022-06-28 12:20:40 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt-test/fedora-36-x86_64/04570893-neomutt/neomutt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt-test/fedora-36-x86_64/04570893-neomutt/neomutt-20220628-1.fc36.src.rpm

Note: This has been built against a development version of NeoMutt, which has some bugs.

> We do still need to put the finishing touches on the documentation/license handling.

>  %{make_install}
> +rm %{buildroot}%{_pkgdocdir}/INSTALL.md %{buildroot}%{_pkgdocdir}/LICENSE.md

done

> +%{_pkgdocdir}

done.  much tidier :-)

> one more tweak to resolve an "ambiguous python shebang" error
> +sed -e '1 s/python/python3/' -i %{buildroot}%{_pkgdocdir}/keybase/keybase.py

We decided instead to drop the keybase script -- unmaintained, unused

----------------------

Other changes:

The contrib files (all the non-docs) have been moved to /usr/share/neomutt
I've added: `%{_datadir}` to the `%files` section to cover them.

I've added a patch: neomutt-fedora_colors.patch
(Each distro seems to have its own custom colours)
The spec *used* to grep the config out of another file (now deceased).

License: as keybase.py has gone, so has one of the license types (Unlicense).

Thanks!

Comment 17 Carl George 🤠 2022-06-28 23:49:25 UTC
> I've added: `%{_datadir}` to the `%files` section to cover them.

This should be %{_datadir}/neomutt.  Packages shouldn't duplicate directory ownership with the filesystem package [0].

-%{_datadir}
+%{_datadir}/neomutt

> I've added a patch: neomutt-fedora_colors.patch

This patch creates a single file, which is later read with cat and written to a file in the buildroot.  You can just have that file as an additional source, then put it into place during %install.

+# Fedora color scheme
+Source1: fedora_colors.rc

-# Fedora color scheme
-Patch3: neomutt-fedora_colors.patch

-%patch3 -p1 -b .fedora_colors

-cat contrib/fedora_colors.rc >> %{buildroot}%{_sysconfdir}/neomuttrc
+install -D -p -m 0644 %{SOURCE1} %{buildroot}%{_sysconfdir}/neomuttrc

One more thing to be aware of, the built package is getting an automatic dependency on /usr/bin/python3 due to the shebang lines of executable files in %{_datadir}/neomutt (account-command/macos-keychain/keychain.py and oauth2/mutt_oauth2.py).  If you want to avoid that dependency, chmod those files during %install to make them not executable.

[0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership

Comment 18 Richard Russon 2022-06-29 10:31:36 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt-test/fedora-36-x86_64/04583618-neomutt/neomutt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/flatcap/neomutt-test/fedora-36-x86_64/04583618-neomutt/neomutt-20220629-1.fc36.src.rpm

> This should be %{_datadir}/neomutt

of course.  fixed

> > I've added a patch: neomutt-fedora_colors.patch

> You can just have that file as an additional source

done

> then put it into place during %install.
> +install -D -p -m 0644 %{SOURCE1} %{buildroot}%{_sysconfdir}/neomuttrc

I changed this to `cat` because we want to append the /etc/neomuttrc that we installed earlier.

> the built package is getting an automatic dependency on /usr/bin/python3

ok, understood.  I don't have a problem with that.
I did a quick check of a minimal Fedora install and that included python3 (it was a requirement for dnf).

Thanks!

Comment 19 Carl George 🤠 2022-07-06 03:56:28 UTC
Thanks for making all those adjustments.  The spec file looks good now.  I do need to point out that the changelog entries going forward will need to follow one of the formats specified in the guidelines [0].  Basically just use 20220629-1 instead of NeoMutt-20220629.  The historical entries can stay in a non-compliant format.

I've sponsored you into the packagers group [1], and this package is APPROVED.


[0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs
[1] https://accounts.fedoraproject.org/user/flatcap/


Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


Issues:
=======
- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 3614720 bytes in 20 files.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_documentation


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "GNU General Public License, Version
     2", "*No copyright* GNU General Public License, Version 2", "GNU
     General Public License v2.0 or later", "*No copyright* Public domain",
     "BSD 2-Clause with views sentence", "GNU General Public License v3.0
     or later", "BSD 2-Clause License", "Public domain", "GNU General
     Public License", "BSD 3-Clause License", "MIT License". 119 files have
     unknown license. Detailed output of licensecheck in
     /home/carl/packaging/reviews/neomutt/2086484-neomutt/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0:
     https://github.com/neomutt/neomutt/archive/20220629/neomutt-20220629.tar.gz
     See: https://docs.fedoraproject.org/en-US/packaging-
     guidelines/SourceURL/
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[-]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 7516160 bytes in /usr/share
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.

Comment 20 Gwyn Ciesla 2022-07-08 14:37:38 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/neomutt

Comment 21 Richard Russon 2022-07-09 12:14:01 UTC
> The spec file looks good now.

Great!

> the changelog entries going forward will need to ...
> ... use 20220629-1 instead of NeoMutt-20220629

OK, I've updated my template

> The historical entries can stay in a non-compliant format.

I fixed them too (trivial)

> I've sponsored you into the packagers group [1], and this package is APPROVED.

Thanks for your help


Note You need to log in before you can comment on or make changes to this bug.