Bug 1078588 - Review Request: ts - Task Spooler
Summary: Review Request: ts - Task Spooler
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: 2014-03-19 23:04 UTC by Jean-Marie Renouard
Modified: 2020-08-10 00:48 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-08-10 00:48:49 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Jean-Marie Renouard 2014-03-19 23:04:54 UTC
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 11:48:42 UTC
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 15:28:25 UTC
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> - 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 21:21:13 UTC
Thanks for your review, I will take all remarks into account during this week.

Jean-Marie

Comment 4 Jean-Marie Renouard 2014-04-07 22:18:09 UTC
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 22:20:48 UTC
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 07:46:07 UTC
> 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 21:06:15 UTC
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 22:39:22 UTC
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 15:15:51 UTC
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

Comment 10 Package Review 2020-07-10 00:49:22 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 11 Package Review 2020-08-10 00:48:49 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.