Bug 1663090 - Review Request: direvent - GNU directory event monitoring daemon
Summary: Review Request: direvent - GNU directory event monitoring daemon
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2019-01-03 06:18 UTC by yagejhu
Modified: 2020-08-10 00:58 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-08-10 00:58:36 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description yagejhu 2019-01-03 06:18:15 UTC
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

Comment 1 Artur Frenszek-Iwicki 2019-01-03 15:32:42 UTC
>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.

Comment 2 yagejhu 2019-01-03 17:35:46 UTC
(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.

Comment 3 yagejhu 2019-01-03 23:56:16 UTC
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.

Comment 4 Artur Frenszek-Iwicki 2019-01-05 17:12:59 UTC
>%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.

Comment 5 Elliott Sales de Andrade 2019-01-16 00:38:05 UTC
You should not use a wildcard, but %find_lang: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_handling_locale_files

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

Comment 6 Michael Schwendt 2019-04-20 21:36:50 UTC
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_64/00841271-direvent/build.log.gz

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

Comment 7 Package Review 2020-07-10 00:57:01 UTC
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.

Comment 8 Package Review 2020-08-10 00:58:36 UTC
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.


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