[Bug 1663090] New: Review Request: direvent - GNU directory event monitoring daemon

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

Bug ID: 1663090
           Summary: Review Request: direvent - GNU directory event
                    monitoring daemon
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://gitlab.com/yagehu/gnu-direvent-rpm-spec/blob/master/direvent.spec
SRPM URL: https://copr.fedorainfracloud.org/coprs/trusty/direvent/build/841271/
Description: This is a GNU project that I've written a spec file for. This is
my first package and I need a sponsor. The official page for the project is
https://www.gnu.org.ua/software/direvent/.
Fedora Account System Username: trusty

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841 (FE-NEEDSPONSOR)

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

Artur Iwicki  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Artur Iwicki  ---
> https://gitlab.com/yagehu/gnu-direvent-rpm-spec/blob/master/direvent.spec

Please provide a link to the raw file next time, not an HTML preview.
> %description
> GNU Direvent monitors...

You should manually insert linebreaks into the description text. There should
be no lines over 80 characters.
https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description
> ./configure --prefix=/usr

Use %{_prefix} here. 
Also, why not just use %configure? Does that not work for some reason?
> %files
> %doc /usr/share/...

Use %{_datadir} here.
https://fedoraproject.org/wiki/Packaging:RPMMacros
> /usr/share/man/man5/direvent.conf.5.gz

Do not assume that man pages will be gzip-compressed.
https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages
> %install
> rm -rf ${buildroot}

Don't do this.
https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

Have you tried running a scratch build in koji? 'cause I'm pretty sure you need
"BuildRequires: gcc" on this one.

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

--- Comment #2 from [e-mail] ---
(In reply to Artur Iwicki from comment #1)
> > https://gitlab.com/yagehu/gnu-direvent-rpm-spec/blob/master/direvent.spec
>
> Please provide a link to the raw file next time, not an HTML preview.
> > %description
> > GNU Direvent monitors...
>
> You should manually insert linebreaks into the description text. There
> should be no lines over 80 characters.
> https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description
> > ./configure --prefix=/usr
>
> Use %{_prefix} here. 
> Also, why not just use %configure? Does that not work for some reason?
> > %files
> > %doc /usr/share/...
>
> Use %{_datadir} here.
> https://fedoraproject.org/wiki/Packaging:RPMMacros
> > /usr/share/man/man5/direvent.conf.5.gz
>
> Do not assume that man pages will be gzip-compressed.
> https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages
> > %install
> > rm -rf ${buildroot}
>
> Don't do this.
> https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections
>
> Have you tried running a scratch build in koji? 'cause I'm pretty sure you
> need "BuildRequires: gcc" on this one.

Got it. Thanks for the advises. Will read docs, fix, and try on koji.

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

--- Comment #3 from [e-mail] ---
Spec URL:
https://gitlab.com/yagehu/gnu-direvent-rpm-spec/raw/master/direvent.spec
SRPM URL: https://copr.fedorainfracloud.org/coprs/trusty/direvent/build/841501/
Description: This is a GNU project that I've written a spec file for. This is
my first package and I need a sponsor. The official page for the project is
https://www.gnu.org.ua/software/direvent/.
Fedora Account System Username: trusty

I fixed mistakes pointed out by Artur and ran koji scratch builds with
el6-candidate and epel7; both completed without errors.

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

--- Comment #4 from Artur Iwicki  ---
> %doc %{_datadir}/info/dir
> [...]
> %doc %{_datadir}/locale/da/LC_MESSAGES/direvent.mo
> [...]
> %doc %{_datadir}/man/man5/direvent.conf.5*

These files should not be marked as %doc. That is for documentation only. Well,
technically man pages and info files are documentation... but still, %doc is
used only extra documents. Think stuff like README.md or
SomeDetailedDescription.txt.
> %doc %{_datadir}/locale/da/LC_MESSAGES/direvent.mo
> [...]
> %doc %{_datadir}/locale/vi/LC_MESSAGES/direvent.mo

You can use a directory wildcard here (**) to avoid having to list every single
path, like this:
%{_datadir}/locale/**/LC_MESSAGES/direvent.mo

Also, you should include the licence file, like this:
%license COPYING
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

Regarding koji builds: do you intend this to be an EPEL-only package, not
available in Fedora?
If this is to go into Fedora, you should make (and post a link to) a koji
scratch build for Rawhide.

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

Elliott Sales de Andrade  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Elliott Sales de Andrade  ---
You should not use a wildcard, but %find_lang:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_handling_locale_...

(Also, the guidelines have moved from the wiki to the above URL.)

[Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

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

--- Comment #6 from Michael Schwendt  ---
Additional hints:
> SRPM URL: https://copr.fedorainfracloud.org/coprs/trusty/direvent/build/841501/

Please link valid src.rpm packages, not service pages which lead to 404 not
found or which are a maze.
> https://copr-be.cloud.fedoraproject.org/results/trusty/direvent/epel-7-x86_6...

configure check reports various "no" items, not limited to flex, bison, gettext
et al. These should be revisited, since BuildRequires may be needed.
> BuildRequires: glibc

glibc-devel actually. Also needed is "BuildRequires: gcc".
https://fedoraproject.org/wiki/Changes/Remove_GCC_from_BuildRoot
> make[4]: Entering directory `/builddir/build/BUILD/direvent-5.1/grecs/src'
>   CC       asprintf.o

No verbose compiler output means one cannot see any compiler/linker options.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags

See what's necessary to make the output verbose, such as removing silent flags
or calling "V=1 make".

Also notice:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make
> %check
> make check
>
> %install
> %make_install

Technically, %check is execute after %install, so the order of the spec file
sections should match that. Btw, often enough, the %check section will be used
to perform tests within the %buildroot, and for that %install would have to be
done.
> %doc %{_datadir}/info/dir
> %doc %{_datadir}/man/man5/direvent.conf.5*
> %doc %{_datadir}/man/man8/direvent.8*

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

If you absolutely want to rely on macros, prefer %{_infodir} and %{_mandir}
here. Further, many such paths are implicitly marked as %doc, so it's not
necessary to add the %doc prefix.

$ rpm -E %__docdir_path
/usr/share/doc:/usr/share/man:/usr/share/info:/usr/share/gtk-doc/html::/usr/share/man:/usr/share/info:/usr/share/javadoc:/usr/doc:/usr/man:/usr/info:/usr/X11R6/man

needinfo canceled: [Bug 1663090] Review Request: direvent - GNU directory event monitoring daemon

Product: Fedora
Version: rawhide
Component: Package Review

Package Review  has canceled Package
Review 's request for
[e-mail]'s needinfo:
Bug 1663090: Review Request: direvent - GNU directory event monitoring daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1663090

--- Comment #8 from Package Review  ---
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.