Bug 1078588 - Review Request: ts - Task Spooler
Review Request: ts - Task Spooler
Status: NEW
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-NEEDSPONSOR
  Show dependency treegraph
 
Reported: 2014-03-19 19:04 EDT by Jean-Marie Renouard
Modified: 2015-12-29 10:15 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Jean-Marie Renouard 2014-03-19 19:04:54 EDT
Spec URL: http://www.jmrenouard.fr/repo/ts.spec
SRPM URL: http://www.jmrenouard.fr/repo/generic/sources/ts-0.7.4-1.el6.src.rpm 
Description: 
Task Spooler is a Unix batch system where the tasks are executed 
run one after the other. The amount of jobs to run at once can 
be set at any time. Each user in each system has his own job queue. 
The tasks are run in the correct context (that of en-queue) 
from any shell/process, and its output/results can be easily 
watched. It is very useful when you know that your commands 
depend on a lot of RAM, a lot of disk use, give a lot of output, 
or for whatever reason it's better not to run them all at the 
same time, while you want to keep your resources busy for maximum benefit. 
Its interface allows using it easily in scripts.

Fedora Account System Username: jmrenouard

rpmlint seems ok on SRPMS, RPMS and SPEC file.
$ rpmlint /var/www/html/repo/ts.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 1 Christopher Meng 2014-03-21 07:48:42 EDT
Hi,

Welcome to Fedora.

Please take a look at:

https://fedoraproject.org/wiki/Join_the_package_collection_maintainers

Thanks.
Comment 2 Michael Schwendt 2014-03-29 11:28:25 EDT
Consider pointing the fedora-review tool at this ticket,

  fedora-review -b 1078588

since it performs many helpful checks (it takes the package from the Spec/SRPM URLs).


> License:	GPLv2+ 

Can't confirm that. See:

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses


> BuildRoot:	$RPM_BUILD_ROOT	

This is a no-op, because if you wanted to use the default $RPM_BUILD_ROOT, you would not need to set the BuildRoot tag. Also notice:

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> BuildRequires:	make gcc 

https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2
( https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2 )


> %install
> rm -rf ${RPM_BUILD_ROOT}

Buildroot is emptied automatically nowadays.
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

> %{make_install} PREFIX=${RPM_BUILD_ROOT}/usr
> install -m 755 -d ${RPM_BUILD_ROOT}

That's a questionable order of those two commands. How could "make install" create anything in $RPM_BUILD_ROOT, if that one didn't exist yet?


> %clean
> rm -rf $RPM_BUILD_ROOT

https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

If you want to target EL5, either be explicit about that (and reuse the same spec for multiple dist targets) or consider forking the spec for EL.


> %files
> %defattr(-,root,root,-)

%defattr is not needed anymore for any of the active Fedora releases and not even current EL5 either.

https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions


> %doc %{_mandir}/man1/*

Files below %_mandir are marked as %doc implicitly. See "rpm -E %__docdir_path".


rpmlint tells:

> Checking: ts-0.7.4-1.fc20.x86_64.rpm
>           ts-0.7.4-1.fc20.src.rpm
> ts.x86_64: W: invalid-url URL: http://viric.name/soft/ts/ <urlopen error timed out>
> ts.src: W: invalid-url URL: http://viric.name/soft/ts/ <urlopen error timed out>
> ts.src: E: specfile-error warning: bogus date in %changelog: Wed Mar 18 2014 Jean-Marie Renouard <jmrenouard@gmail.com> - 0.7.4-1
> 2 packages and 0 specfiles checked; 1 errors, 2 warnings.

Especially the %changelog errors ought to be corrected, since e.g. "date" tells that Mar 18 2014 is a Tuesday.


> $ rpmls -p ts-0.7.4-1.fc20.x86_64.rpm 
> -rwxr-xr-x  /usr/bin/ts
> -rw-r--r--  /usr/share/man/man1/ts.1.gz

License file is not included:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

And no documentation files are included except for the manual page:
https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation


> build.log

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags


This has been a brief first look only.
Comment 3 Jean-Marie Renouard 2014-03-30 17:21:13 EDT
Thanks for your review, I will take all remarks into account during this week.

Jean-Marie
Comment 4 Jean-Marie Renouard 2014-04-07 18:18:09 EDT
Hello, 

I have install fedora-review and mock so I can see clearly what is expected on this package.

This is a new version.
Spec URL: http://www.jmrenouard.fr/repo/ts.spec
SRPM URL: http://www.jmrenouard.fr/repo/generic/sources/ts-0.7.4-2.el6.src.rpm 


Included licence files and doc files.

I have taken into account all remarks done by Michael Schwendt.
Comment 5 Jean-Marie Renouard 2014-04-07 18:20:48 EDT
This is a new version is:

Spec URL: http://www.jmrenouard.fr/repo/ts.spec
SRPM URL: http://www.jmrenouard.fr/repo/generic/sources/ts-0.7.4-2.fc20.src.rpm 

Best regards,
Comment 6 Michael Schwendt 2014-04-08 03:46:07 EDT
> License:	GPLv2+

Where does it say "GPLv2 or later"?

  https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses

The file "COPYING" is "License: GPLv2", and the source files don't tell "or later":

 * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Clarification

 * https://www.gnu.org/licenses/gpl-faq.html#NoticeInSourceFile


> %clean
> rm -rf $RPM_BUILD_ROOT

same as comment 2


> %doc %{_datadir}/ts/*

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
 -> https://fedoraproject.org/wiki/Packaging:UnownedDirectories


> %build
> make %{?_smp_mflags}

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

In particular, it will be a small exercise to figure out whether you can use a trick such as

  %configure || :
  make %{?_smp_mflags}

to reuse the flags exported by the %configure macro (see rpm -E %configure), or whether it will be necessary to activate the flags via a different way (e.g. by patching the Makefile).


* fedora-review also says:

[!]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0:
     http://viric.name/soft/ts/ts-0.7.4.tar.gz
     See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags

ts.x86_64: W: invalid-url URL: http://viric.name/soft/ts/ <urlopen error [Errno -2] Name or service not known>


> ts.x86_64: E: incorrect-fsf-address /usr/share/ts/COPYING

https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address
Comment 7 Jean-Marie Renouard 2014-04-08 17:06:15 EDT
A quick analysis shows that all files refers to COPYING file:
/*
    Task Spooler - a task queue system for the unix user
    Copyright (C) 2007-2009  Lluís Batlle i Rossell

    Please find the license in the provided COPYING file.
*/

But 2 files have no headers: ./ttail.c et main.h


[makerpm@localhost ts-0.7.4]$ find . -type f -iname '*.c' -o -iname '*.h' |sort > /tmp/all.txt
[makerpm@localhost ts-0.7.4]$ find . -type f -iname '*.c' -o -iname '*.h' | xargs -n 50 grep COPYING |cut -d: -f1| sort >/tmp/withCOPYING.txt
[makerpm@localhost ts-0.7.4]$ sdiff /tmp/all.txt /tmp/withCOPYING.txt 
./client.c                                                      ./client.c
./env.c                                                         ./env.c
./error.c                                                       ./error.c
./execute.c                                                     ./execute.c
./info.c                                                        ./info.c
./jobs.c                                                        ./jobs.c
./list.c                                                        ./list.c
./mail.c                                                        ./mail.c
./main.c                                                        ./main.c
./main.h                                                      <
./msg.c                                                         ./msg.c
./msgdump.c                                                     ./msgdump.c
./print.c                                                       ./print.c
./server.c                                                      ./server.c
./server_start.c                                                ./server_start.c
./signals.c                                                     ./signals.c
./tail.c                                                        ./tail.c
./ttail.c                                                     <
Comment 8 Jean-Marie Renouard 2014-04-08 18:39:22 EDT
1° Since this week, web site is no more available: 
 http://viric.name/soft/ts/
Strange situation :)

2° Licence RPM changed to License: GPLv2

3° main.h and ttail.c doesn't contains reference to COPYING file for licence.
Source files make reference to COPYING file,is it right for Fedora team ?

4° incorrect-fsf-address in COPYING
Nothing done for moment due to legal reason. (No patches on Licence)


5° Correct flags generated
I add a patch in source with a empty configure so no errors are generated.

6° %clean section is now empty

7° Unowned directory
/usr/bin/ts
/usr/share/man/man1/ts.1.gz
/usr/share/ts-0.7.4
/usr/share/ts-0.7.4/COPYING
/usr/share/ts-0.7.4/Changelog
/usr/share/ts-0.7.4/OBJECTIVES
/usr/share/ts-0.7.4/PROTOCOL
/usr/share/ts-0.7.4/README
/usr/share/ts-0.7.4/TRICKS
/usr/share/ts-0.7.4/buglist.bug
/usr/share/ts-0.7.4/web
/usr/share/ts-0.7.4/web/article_linux_com.html
/usr/share/ts-0.7.4/web/index.html
/usr/share/ts-0.7.4/web/ts-0.2.1.png
/usr/share/ts-0.7.4/web/ts-0.5.4.ebuild

All directories are included in the package now.

For point 1,3,4, I direcly send a mail to ts developer for having his feddback.

Spec URL: http://www.jmrenouard.fr/repo/ts.spec
SRPM URL: http://www.jmrenouard.fr/repo/generic/sources/ts-0.7.4-3.fc20.src.rpm
Comment 9 Michael Schwendt 2015-12-29 10:15:51 EST
Running into this ticket again while cleaning up my filtered mail folders:


0.7.5 has been released on 2015-03-06.


autosetup: option requires an argument -- 'n'
error: Unknown option n in autosetup(a:b:cDn:TvNS:p:)


+ make -j3
cc -pedantic -ansi -Wall -g -O0 -D_XOPEN_SOURCE=500 -D__STRICT_ANSI__ -c main.c
cc -pedantic -ansi -Wall -g -O0 -D_XOPEN_SOURCE=500 -D__STRICT_ANSI__ -c server.c

Global compilation flags are not applied yet:
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags


> %clean

An empty %clean section would override the default and not remove the buildroot. Check end of rpmbuild output.


> /usr/share/ts-0.7.4/COPYING

A %license macro has been introduced in January 2015.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

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