Bug 188105 - Review Request: torque
Review Request: torque
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ed Hill
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-05 22:20 EDT by Garrick Staples
Modified: 2015-04-27 15:37 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-04-19 15:17:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
limburgher: fedora‑cvs+


Attachments (Terms of Use)
remove hard-coded rpaths (485 bytes, patch)
2006-04-08 16:10 EDT, Ed Hill
no flags Details | Diff

  None (edit)
Description Garrick Staples 2006-04-05 22:20:46 EDT
Spec Name or Url: http://www-rcf.usc.edu/~garrick/torque.spec
SRPM Name or Url: http://www-rcf.usc.edu/~garrick/torque-2.1.0p0-0.1.200604051756.src.rpm
Description: **This is my first package for FC, I'm looking for a sponsor**
TORQUE (Tera-scale Open-source Resource and QUEue manager) is a resource
manager providing control over batch jobs and distributed compute nodes.
TORQUE is based on OpenPBS version 2.3.12 and incorporates scalability,
fault tolerance, and feature extension patches provided by USC, NCSA, OSC,
the U.S. Dept of Energy, Sandia, PNNL, U of Buffalo, TeraGrid, and many
other leading edge HPC organizations.
Comment 1 Ed Hill 2006-04-07 00:34:36 EDT
Hi Garrick, I started to do a torque review and came across the following
in the PBS_License.txt file:

  2. Redistribution in any form is only permitted for non-commercial,
     non-profit purposes.  There can be no charge for the Software or any
     software incorporating the Software.  Further, there can be no
     expectation of revenue generated as a consequence of redistributing
     the Software.

So I'm afraid the above isn't acceptable within FE since the Fedora 
Project aims to provide FOSS packages that are "free for anyone to use, 
modify and distribute" -- and the "anyone" may include both non- and 
for-profit entities.  So the above license disqualifies this submission:

  http://fedoraproject.org/wiki/Packaging/Guidelines#Legal

Sorry...
Comment 2 Garrick Staples 2006-04-07 01:02:54 EDT
Fortunately we don't have to worry about that.  A few lines up is, "After
December 31, 2001, only conditions 3-6 must be met."

Btw, I just posted torque-2.1.0p0-0.2.200604060235.src.rpm which has better
.desktop files, some menu icons, and matching upstream source (the first srpm
was just pulled out of CVS).
Comment 3 Ed Hill 2006-04-07 07:43:25 EDT
I'm sorry!  I've re-opened the bug and will continue with the review.
Comment 4 Ed Hill 2006-04-07 23:46:41 EDT
An attempt to build this RPM:

  http://www-rcf.usc.edu/~garrick/torque-2.1.0p0-0.3.200604071240.src.rpm

in mock (FC4 i386) resulted in this error:

  RPM build errors:
    File must begin with "/": %{_desktopdir}/*.desktop
Comment 5 Garrick Staples 2006-04-08 00:01:19 EDT
Bah!  _desktopdir is defined by jpackage-utils.

Updated http://www-rcf.usc.edu/~garrick/torque.spec changes that to
%{_datadir}/applications

Comment 6 Ed Hill 2006-04-08 16:10:55 EDT
Created attachment 127513 [details]
remove hard-coded rpaths

This tiny patch removes the hard-coded rpaths from the configure script and
here are the lines to add to the torque.spec:

  Patch0: torque_configure_remove_rpaths.patch

  %patch0 -p1
Comment 7 Ed Hill 2006-04-08 16:54:14 EDT
Hi Garrick, using your latest spec and the patch above I put together:

http://mitgcm.org/eh3/fedora_misc/torque-2.1.0p0-0.4.200604071240.src.rpm
md5sum:
2470a40280b9b65d6b49c9fffe5f46bb  torque-2.1.0p0-0.4.200604071240.src.rpm

which builds locally (and probably in mock since it worked with a 
very similar previous version).  In any case, heres the rpmlint output:

  W: torque invalid-license Freely redistributable (See PBS_License.txt)

[repeated many times] -- probably OK to ignore since the license does appear to 
satisfy the Fedora requirements

  W: torque no-documentation

[repeated many times] -- OK to ignore

  W: torque incoherent-version-in-changelog 2.1.0p0-0.3.200604071240
     2.1.0p0-0.4.200604071240

the naming does not exactly follow the pattern in 
  http://fedoraproject.org/wiki/Packaging/NamingGuidelines 
but it is pretty close -- could you please use the "YYYYMMDDcvs" 
pattern per the guidelines?

  E: torque no-binary
  E: torque non-standard-dir-perm /var/torque/spool 01777
  W: torque non-standard-dir-in-var torque

probably OK to ignore

  W: torque-client unstripped-binary-or-object /usr/sbin/pbs_iff
  E: torque-client setuid-binary /usr/sbin/pbs_iff root 04755
  E: torque-client non-standard-executable-perm /usr/sbin/pbs_iff 04755

These are worrisome.  The unstripped binary can probably be fixed
in the rpm build somehow.  And does the /usr/sbin/pbs_iff really 
need to be suid?

  E: torque-mom non-standard-dir-perm /var/torque/checkpoint 0700
  E: torque-mom non-standard-dir-perm /var/torque/undelivered 01777
  E: torque-mom non-standard-dir-perm /var/torque/mom_priv/jobs 0751
  W: torque-mom non-standard-dir-in-var torque
  W: torque-mom incoherent-init-script-name pbs_mom
  E: torque-scheduler non-standard-dir-perm /var/torque/sched_priv 0750
  W: torque-scheduler non-standard-dir-in-var torque
  W: torque-scheduler incoherent-init-script-name pbs_sched
  E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_groups 0750
  E: torque-server non-standard-dir-perm /var/torque/server_priv/queues 0750
  E: torque-server non-standard-dir-perm /var/torque/server_priv 0750
  E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_hosts 0750
  E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_users 0750
  E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_svr 0750
  E: torque-server non-standard-dir-perm /var/torque/server_priv/jobs 0750
  W: torque-server non-standard-dir-in-var torque
  W: torque-server incoherent-init-script-name pbs_server

The above can probably (?) be ignored.
Comment 8 Garrick Staples 2006-04-10 19:15:14 EDT
(In reply to comment #6)
> Created an attachment (id=127513) [edit]
> remove hard-coded rpaths
> 
> This tiny patch removes the hard-coded rpaths from the configure script and
> here are the lines to add to the torque.spec:
> 
>   Patch0: torque_configure_remove_rpaths.patch
> 
>   %patch0 -p1


Is this necessary?  TORQUE is just doing standard autoconf+automake+libtool stuff.
Comment 9 Garrick Staples 2006-04-10 19:23:14 EDT
(In reply to comment #7)
>   W: torque incoherent-version-in-changelog 2.1.0p0-0.3.200604071240
>      2.1.0p0-0.4.200604071240
> 
> the naming does not exactly follow the pattern in 
>   http://fedoraproject.org/wiki/Packaging/NamingGuidelines 
> but it is pretty close -- could you please use the "YYYYMMDDcvs" 
> pattern per the guidelines?

I'll add a "cvs" on the end.


>   W: torque-client unstripped-binary-or-object /usr/sbin/pbs_iff
>   E: torque-client setuid-binary /usr/sbin/pbs_iff root 04755
>   E: torque-client non-standard-executable-perm /usr/sbin/pbs_iff 04755
> 
> These are worrisome.  The unstripped binary can probably be fixed
> in the rpm build somehow.  And does the /usr/sbin/pbs_iff really 
> need to be suid?

Yes, pbs_iff needs to get a priv port as part of TORQUE's internal authn system.
 
>   E: torque-mom non-standard-dir-perm /var/torque/checkpoint 0700
>   E: torque-mom non-standard-dir-perm /var/torque/undelivered 01777
>   E: torque-mom non-standard-dir-perm /var/torque/mom_priv/jobs 0751
>   W: torque-mom non-standard-dir-in-var torque
>   W: torque-mom incoherent-init-script-name pbs_mom
>   E: torque-scheduler non-standard-dir-perm /var/torque/sched_priv 0750
>   W: torque-scheduler non-standard-dir-in-var torque
>   W: torque-scheduler incoherent-init-script-name pbs_sched
>   E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_groups 0750
>   E: torque-server non-standard-dir-perm /var/torque/server_priv/queues 0750
>   E: torque-server non-standard-dir-perm /var/torque/server_priv 0750
>   E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_hosts 0750
>   E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_users 0750
>   E: torque-server non-standard-dir-perm /var/torque/server_priv/acl_svr 0750
>   E: torque-server non-standard-dir-perm /var/torque/server_priv/jobs 0750
>   W: torque-server non-standard-dir-in-var torque
>   W: torque-server incoherent-init-script-name pbs_server
> 
> The above can probably (?) be ignored.
> 

Yup, that's all correct.
Comment 10 Ed Hill 2006-04-10 20:39:33 EDT
(in reply to comment #8)

Its my understanding that rpaths can be problematic (or at best just 
redundant) as described at:

  http://fedoraproject.org/wiki/Extras/Schedule/RpathCheckBuildsys

But if you can convince me that they're harmless (even on multilib) then
we can ignore the rpath rpmlint warnings that happen without the patch.  
I think its safer/easier to remove them but I could be wrong.
Comment 11 Garrick Staples 2006-04-13 01:06:14 EDT
http://www-rcf.usc.edu/~garrick/torque-2.1.0p0-0.5.200604071240cvs.src.rpm
http://www-rcf.usc.edu/~garrick/torque.spec

These fix the package release and include your rpath patch.

I've done mock builds on FC4 and FC5 on i386.
Comment 12 Ed Hill 2006-04-17 10:52:53 EDT
Hi Garrick, this isn't a full review but I'm hoping to find more time to 
look at it later this week.

good:
 rpmlint output basically unchanged from previous comments
 OK - follows naming guidelines
 OK - license seems acceptable and included
 OK - spec file is not as simple as it could be and it contains a 
    number of conditional options that are a little time-consuming to 
    read and (try to) understand -- but having looked at them I don't
    see any actual blockers
 OK - builds in mock on FC5 i386
 OK - dir ownership and permissions look fine
 OK - libs seem fine and no *.la files
 OK - code not content

nits:
 - the [ "$RPM_BUILD_ROOT" != "/" ] is not necessary for FE
 - If you'd like to have the same version of torque in, say, 
     FE4, FE5, and devel then you'll probably want to add
     %{?dist} per http://fedoraproject.org/wiki/DistTag
 - perhaps the headers currently located at /usr/include/* 
     could go in a subdir such as /usr/include/torque/* since 
     some of the header files have rather unfortunately generic 
     names (eg. "tm.h")

blockers:
 - How can I verify that the source matches upstream?  I found 
   the download pages at:

     http://www.clusterresources.com/downloads/torque/snapshots/

   but I can't seem to find the same .tar.gz file or a way to 
   create an identical one from CVS -- could you please document 
   that step within the spec file as a comment so that I can repeat 
   it?  Or perhaps use one of the "official" tar files?
Comment 13 Garrick Staples 2006-04-17 15:30:27 EDT
(In reply to comment #12)
> nits:
>  - the [ "$RPM_BUILD_ROOT" != "/" ] is not necessary for FE

Ok, will remove.

>  - If you'd like to have the same version of torque in, say, 
>      FE4, FE5, and devel then you'll probably want to add
>      %{?dist} per http://fedoraproject.org/wiki/DistTag

I was wondering about this, but I left it out because it isn't actually in the
naming guidelines.

>  - perhaps the headers currently located at /usr/include/* 
>      could go in a subdir such as /usr/include/torque/* since 
>      some of the header files have rather unfortunately generic 
>      names (eg. "tm.h")

Sure.
 
> blockers:
>  - How can I verify that the source matches upstream?  I found 
>    the download pages at:
> 
>      http://www.clusterresources.com/downloads/torque/snapshots/
> 
>    but I can't seem to find the same .tar.gz file or a way to 
>    create an identical one from CVS -- could you please document 
>    that step within the spec file as a comment so that I can repeat 
>    it?  Or perhaps use one of the "official" tar files?

I'll roll an "official" snapshot upstream and give you a new srpm in a few minutes.

Comment 14 Garrick Staples 2006-04-17 17:25:58 EDT
http://www-rcf.usc.edu/~garrick/torque-2.1.0p0-0.5.200604171430cvs.src.rpm
http://www-rcf.usc.edu/~garrick/torque.spec

* Mon Apr 17 2006 Garrick Staples <garrick@usc.edu> 2.1.0p0-0.6.200604171430cvs
- add %%{dist} tag
- cleanup the cleanups in spec
- bump to matching upstream
- move headers to /usr/include/torque/
Comment 15 Ed Hill 2006-04-17 21:28:38 EDT
The SRPM [please note the "6" instead of "5" in release tag since the 
SRPM URL in comment #14 has a typo] at:

  http://www-rcf.usc.edu/~garrick/torque-2.1.0p0-0.6.200604171430cvs.src.rpm
  sha1sum: ba0a569763d6b91697c9a40723b392be464ab64e

does cleanup everything mentioned in comments #12--13 and heres the
remainder of the review:

very minor nit:
 + please consider adding the "-q" option to %setup so that the 
   build logs are a little shorter and more readable (just a 
   request--by no means a blocker!)

good:
 OK - source matches upstream
 OK - macro usage looks consistent although there are some 
      harmless quirks like having both %__rm and %{__rm}
 OK - proper use of -devel
 OK - desktop files appear to have correct install syntax
 OK - scriptlets look sane to me
 OK - installed and runs with out seg-faulting on a single FC4 i386
      machine (I do need to go dig up the syntax for creating default 
      queues, etc. because a quick "qsub -I" seems to wait forever and 
      I imagine its an incomplete setup and thus my fault.  Other 
      commands such as "pbsnodes -a" and "qmgr" work just fine--no 
      segfaults.)

I don't see any blockers so its APPROVED.

Congrats on the first package and please feel free to contact me if you 
want any help with FE CVS, the build system, etc.
Comment 16 Garrick Staples 2006-04-19 15:16:10 EDT
The builds just went through so I guess we're done here.

Thanks Ed for the help!
Comment 17 Steve Traylen 2009-12-01 17:54:12 EST
Package Change Request
======================
Package Name: torque
New Branches: EL-4 EL-5
Owners: stevetraylen
InitialCC: garrick


I sent mails to the owner on September 23rd and more explicitly
on November 25th requesting the EPEL branches.
Also bug 479672 has been present for a while.

Steve Traylen.
Comment 18 Kevin Fenzi 2009-12-03 01:45:03 EST
cvs done.
Comment 19 Fedora Update System 2009-12-16 05:05:13 EST
torque-2.3.8-1.el4 has been submitted as an update for Fedora EPEL 4.
http://admin.fedoraproject.org/updates/torque-2.3.8-1.el4
Comment 20 Fedora Update System 2009-12-16 05:06:19 EST
torque-2.3.8-1.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/torque-2.3.8-1.el5
Comment 21 David Brown 2015-04-25 00:18:55 EDT
New Package SCM Request
=======================
Package Name: torque
Short Description: cluster job scheduler
Upstream URL: http://www.adaptivecomputing.com/products/open-source/torque/
Owners: dmlb2000 hguemar
Branches: epel7
Comment 22 Jon Ciesla 2015-04-25 12:00:54 EDT
Fedora review flag not set.
Comment 23 David Brown 2015-04-27 12:18:39 EDT
Following the https://fedoraproject.org/wiki/Package_SCM_admin_requests document for existing packages mentions nothing about setting the fedora-review flag.

This is a new branch for an existing package...
Comment 24 Jon Ciesla 2015-04-27 14:25:50 EDT
In that case please use the Package Change Request, rather than New Package.
Comment 25 David Brown 2015-04-27 14:27:09 EDT
Oh, I got the wrong title in my comment...
Comment 26 David Brown 2015-04-27 14:27:48 EDT
Package Change Request
======================
Package Name: torque
Short Description: cluster job scheduler
Upstream URL: http://www.adaptivecomputing.com/products/open-source/torque/
Owners: dmlb2000 hguemar
Branches: epel7
Comment 27 David Brown 2015-04-27 14:28:58 EDT
Package Change Request
======================
Package Name: torque
New Branches: epel7
Owners: dmlb2000 hguemar
Comment 28 Jon Ciesla 2015-04-27 15:37:21 EDT
Git done (by process-git-requests).

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