Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 188105
Summary: | Review Request: torque | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Garrick Staples <garrick> | ||||
Component: | Package Review | Assignee: | Ed Hill <ed> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | david.brown, ewan, steve.traylen | ||||
Target Milestone: | --- | Flags: | gwync:
fedora-cvs+
|
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2006-04-19 19:17:03 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 163779 | ||||||
Attachments: |
|
Description
Garrick Staples
2006-04-06 02:20:46 UTC
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... 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). I'm sorry! I've re-opened the bug and will continue with the review. 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 Bah! _desktopdir is defined by jpackage-utils. Updated http://www-rcf.usc.edu/~garrick/torque.spec changes that to %{_datadir}/applications 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
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. (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. (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. (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. 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. 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? (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. 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> 2.1.0p0-0.6.200604171430cvs - add %%{dist} tag - cleanup the cleanups in spec - bump to matching upstream - move headers to /usr/include/torque/ 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. The builds just went through so I guess we're done here. Thanks Ed for the help! 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. cvs done. 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 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 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 Fedora review flag not set. 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... In that case please use the Package Change Request, rather than New Package. Oh, I got the wrong title in my comment... 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 Package Change Request ====================== Package Name: torque New Branches: epel7 Owners: dmlb2000 hguemar Git done (by process-git-requests). |