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 176253
Summary: | Review Request: clement - An application to filter and manage E-mail traffic | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | jmp |
Component: | Package Review | Assignee: | Hans de Goede <hdegoede> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | gauret, hdegoede, jeff |
Target Milestone: | --- | Flags: | kevin:
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-08-22 11:33:08 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 |
Description
jmp
2005-12-20 16:49:00 UTC
This isn't a full review, I'm just noting some things to clean up in the spec. First of all, the %description is huge. I personally feel it should be one or two paragraphs. I don't think that's a blocker. The Summary tag shouldn't contain the package name. Just make it "An application to..." You should get rid of the %define dist at the top. If you want a %dist tag in your personal repository, add it to your .rpmmacros file. Version: %{version} and %Release: %{release}.%{dist} are not defined, and probably not handled very well (as in the build system will probably choke). Just use real relase numbers and such, like 2.1 and 31%{?dist}. The Source should be the full URL where the source can be downloaded. The Vendor tag is deprecated. Please remove it. The BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -- see http://fedoraproject.org/wiki/PackagingGuidelines. The Requires: clamav is probably not necessary. The RPM packaging process will discover this if the package is really linked against clamav. Is the Requires: httpd absolutely necessary? I understand that users won't have access to the quarantine function without it, but will the program run without a web server? If so, you shouldn't require httpd, but leave a note in a README file somewhere. In the %files section, %defattr should be "(-,root,root,-)" -- note final dash. This preserves the mode of directories as well. What is your macro ${prefix} set to? Is it /usr? (As an aside, don't use this macro like you have done, for one it's not defined in your spec file, and second it is better to use the macro %{_prefix} which the RPM subsystem already has defined. Also, a local administrator can change it to suit non-conformist freakish desires.) If it is /usr, your package winds up owning it, which conflicts with the filesystem package. A better example would be something like the following: %{_bindir}/* %{_sbindir}/* %{_datadir}/%{name}/ %{_libdir}/%{name}/ and so on, which will include all files under /usr/bin and /usr/sbin, and the entire directories of /usr/share/clement and /usr/lib/clement including all files therein. It is my adamant opinion that no files in /usr should ever need to be written directly by the administrator or any other user. You have a config file listed in %{prefix}/conf (again, %{prefix} is not defined). All config files should be under /etc. Use the %{_sysconfdir} macro for /etc. Personally, I prefer packages to place their config files in %{_sysconfdir}/%{name}. The attr() on /etc/sysconfig/clement is probably not necessary, if it was installed with the appropriate permissions (defattr should be right). Why should it be writable by the mail user? It is readable by all users, so root owning it should be sufficient and more secure. Also, as above, attr() on /etc/init.d/clement (should be %{_sysconfdir}/rc.d/init.d/clement) is superfluous if it is installed correctly. I really think %files as a whole should come after %prep, %build, %install, and %pre/post/preun/postun. I'm not sure if that's a hard and fast rule, though. If you are going to run chkconfig in %post and %preun, then you need to Requires(post) and Requires(preun): chkconfig. You should not let chkconfig make the program start automatically in %post. Just chkconfig --add it. It will be set to K**clement in every runlevel. Let the administrator turn it on. Also, get rid of the chkconfig --list line. I'm not sure what those %{prefix}/support/add*.sh scripts to, but I assume they help build the default config file. That's nice, but I don't think you should do that in %post. Someone can correct me if I'm wrong, but I think it would be better to include a script and maybe put it in /usr/share/clement (%{_datadir}/%{name}) with a note in the default config file saying "please run /usr/share/clement/*.sh to get a nice baseline config." In %preun, I would change the path /etc/init.d to %{_sysconfdir}/rc.d/init.d. Also, Requires(preun): initscripts, because you require the service command. And if you followed what I was saying above about paths and such, that whole ( cd %{prefix} ... line at the end needs to go. You can use make and rm instead of %{__make} and %{__rm}. That's up to you. I think it's cleaner with the commands instead of the macros. I'm not sure what that mkcron.sh script does exactly, but you might want anacron scripts to just go into /etc/cron.daily. Finally, I don't recall seeing your name or email address on the fedora-extras-list before. If you are a new contributor, you need to identify yourself as such so someone who can sponsor you will help you out. I'm not a sponsor. In spite of my ripping your spec file to shreds (it happens to us all our first time), I want to say "thank you" for writing this program. As an administrator of several email servers and domains, I always appreciate people who help fight spam. I personally had plans to spend my time on something else last week, but I wound up working almost all week on one particular spam problem instead, it was litterally turning into a DoS attack. So thanks for fighting the good fight. Spec File updated according comments. Firt extras package, I would like a sponsor Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-57.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-57.src.rpm Changelog - Further Spec file improvement. - dispatching Clement components according 'FHS' guidelines. Clement-2.1-65 was installed in production for our domain this week-end, so fare so good :-} So, if somebody is willing to be a SPONSOR for Clement, (an application trapping SPAM and virus at SPMT protocol level + mail tracking with a WEB interface), comments and suggestions are more than welcome. Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-65.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-65.src.rpm Changelog - Making sure install and uninstall process are working fine Not building in mock. lvluid.c:17:31: error: security/pam_appl.h: No such file or directory Will need to BuildRequires pam-devel Pam-Devel is now part of the BuildRequires list within the SPEC file. Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-99.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-99.src.rpm Numerous bug-fix/improvement have been done since last bugzilla Clement update (2.1-65), see the Changes file and SPEC Changelog for details. JMP, Your work and responsives above look good (at first glance), so I might be willing to sponsor you. You must understand however that things are currently organised in FE in such a way that once you are sponsored you get full CVS access to all packages. Thus having one good package ready for review usually isn't enough to get you sponsored. There are 2 ways to proceed from here for us (me since I'm concidering sponsering you) to get to learn you better: 1) You review a couple of packages from others see bug 163776 for a list of Review Requests that need a Reviewer, don't worry about not being competent enough todo a review, just add my to the CC-list and I'll watch over your back. 2) Create some more packages and put me in the CC-field of the review request. Or (probably the best) a combiantion of these 2. What also helps is activity in other Fedora projects such as translations etc. Sorry for the long silence I've been busy with other Fedora stuff. As discussed by email I'll review this package and if that goes well I'll sponsor you. This isn't a Full Review yet, but this should give you enough for now to work on: * rpmlint gives: E: clement non-standard-gid /etc/clement-2.1/clement.conf apache This is intentional and can be ignored. Are you sure though you want apache to be able to write to this file, I guess this is for the clement webinterface, but is this safe? * During build on FC5-i386 rpm says: warning: File listed twice: /etc/clement-2.1/clement.conf warning: File listed twice: /etc/clement-2.1/clement.conf warning: File listed twice: /var/www/clement-2.1/clembase.php These must be fixed * All the files under /var/www are owned by mail, this doesn't seem right * The files under /var/lib/clement-2.1/ do not seem very variable (except for maybe dummy-cert?), please find a better place for these. * The remove.sh script called form %preun is evil EVIL *EVIL*, you cannot assume that rpm scripts will be run from an interactive terminal. I think it is best to just leave these files in place and instead in %preun generate a README-fedora in the clement spool dir which explains that since clement is removed these files _may_ be removed to, but that they could contain valuable data and people should be carefull when removing them. * Remove all comments like this one: #----------------------------------------------------------- #Clement SPEC definition #----------------------------------------------------------- People reading a spec file are supposed to know what a specfile is and what the different sections do, these are not helpfull comments, they are noise. * Put the sections in the specfile in the proper order, see for example the /usr/share/fedora/spectemplate-minimal.spec from the fedora-rpmdevtools package. The proper order is Name Version Release Summary Group .... %description %setup %build %install %clean %post(un) (and other scripts) %files %changelog Version 2.1-175 SPECS file is now clean according rpmlint-0.77-1.fc5 rpmbuild give no warning anymore about "File listed twice" Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-175.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-175.src.rpm Numerous bug-fix/improvement have been done since last bugzilla Clement update (2.1-99), see the Changes file and SPEC Changelog for details. According to me SPEC file is really better, there 2 issue not yet addressed: 1) /var/lib/clement-2.1 contents. those file are mainly day to day clement own management, I am really reluctant to put them in /usr/bin as they are for the sole clement purpose. what is the best option in such case? 2) ownership for /var/www/clement-2.1: putting everything own by 'mail' is may be overkilling, this web part is not meaningful if clement daemon is not up and running, Clement itself run with uid 'mail', that why I chose to set the ownership as 'mail'. (In reply to comment #8) > Version 2.1-175 SPECS file is now clean according rpmlint-0.77-1.fc5 > rpmbuild give no warning anymore about "File listed twice" > Good, whats with the release tag inflation (very high number)? > Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-175.spec > SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-175.src.rpm > > Numerous bug-fix/improvement have been done since last bugzilla Clement > update (2.1-99), see the Changes file and SPEC Changelog for details. > > According to me SPEC file is really better, Yes it is, its becoming readable, with the proper order and stuff, it almost looks like a real specfile now :) there 2 issue not yet addressed: > 1) /var/lib/clement-2.1 contents. > those file are mainly day to day clement own management, I am really > reluctant to put them in /usr/bin as they are for the sole clement purpose. > what is the best option in such case? You could & should put them under /usr/lib/clement (%{_libdir}/%{name}) really) > 2) ownership for /var/www/clement-2.1: > putting everything own by 'mail' is may be overkilling, this web part is not > meaningful if clement daemon is not up and running, Clement itself run with > uid 'mail', that why I chose to set the ownership as 'mail'. > I think a default of root root as normal would be better, with an exception for files and or dir which clement needs to write. Also I recently learned that web application packages should not touch /var/www as they might accidently overwrite user content put there, instead the should install their files under /usr/share/%{name}, see: http://fedoraproject.org/wiki/Packaging/Guidelines#head-5d1681fa7cf3714ad490fbf7c095a0cfe16da27f Besides that things are looking good, accept for the: #----------------------------------------------------------- #Clement SPEC definition #----------------------------------------------------------- comment, please remove that. Let me know when you've a new version with these issues fixed. p.s. I'm going on a short vacation from Monday 10 juli till Friday 14 juli, so if I'm quiet for a while thats why. Version 2.1-176 SPECS file fine tuning to use /usr/share and /usr/lib insteed of /var/www and /var/lib. Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-176.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-176.src.rpm :-}} About Release tag inflation, application is kept internally within RCS, developement has been done this last month and we prefere to sauve as many time as necessary to be safe. :-} Tag release number are just that... number, they just need to refer to a consistente and unique binary code. Thanks for support and advices. As promised here is a full review: MUST: ===== * rpmlint output is clean, good! * Package and spec file named appropriately * Packaged according to packaging guidelines * License (GPL) ok, license file included * spec file is legible and in Am. English. 0 Cannot verify if source matches upstream because of broken Source URL, this must be fixed! * Compiles and builds on devel-x86_64 * BR: ok * No locales * No shared libraries * Not relocatable 0 Package does NOT owns / or requires all dirs * No duplicate files & Permissions ok * %clean & macro usage OK * Contains code and permissible content * %doc does not affect runtime, and isn't large enough to warrent a sub package * no -devel package needed, no libs / .la files. * no gui -> no .desktop file required MUST fix: ========= * Proper downloadable Source URL * This is dead wrong: %attr(-,mail,mail) %{_usr}/share/%{name}-%{version}/logs/ Running software cannot shall not and must not write to /usr it could be on a readonly partition or or or .... Please make that: %attr(-,mail,mail) %{_var}/log%{name}-%{version}/ And adjust the software to write it logs there, or am I misinterpreting the dir name here? * Unowned dir %{_usr}/share/%{name}-%{version} Add: "%dir %{_usr}/share/%{name}-%{version}" to %files or better replace: %{_usr}/share/%{name}-%{version}/*.php %{_usr}/share/%{name}-%{version}/reg-icons %{_usr}/share/%{name}-%{version}/local %{_usr}/share/%{name}-%{version}/cgi-bin With just: %{_usr}/share/%{name}-%{version} Should fix: =========== * Spec contains: "#%postun" this will end up as the last line of the %preun line, harmless but it it would be cleaner to just remove it completly. * Replace all occurences of "%{_usr}/share" with "%{_datadir} So all in all its looking good! Once all the must fix items are taken care of I'll sponsor you and you can import this. Version 2.1-186 SPECS file fine tuning, logs are not in /usr/share/clement anymore Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-186.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-186.src.rpm Beside SPEC file fine tuning, numeros improvement and bug-fix since 2.1-176, see Changelog within SPEC file. Almost there, why are these: %attr(-,mail,mail) %{_usr}/bin/%{name} %attr(-,mail,mail) %{_datadir}/%{name}-%{version}/ %attr(-,mail,mail) ? Perhaps the binary is suid? In that case please reflect that in the %attr, even if it already is made suid in %install. And I see no reason for the %{_datadir}/%{name}-%{version} being mail.mail, prhaps this is a leftover from when it contained the log files? I also found some more should fixes, I see you use: %{_usr}/bin in various places, you should replace that with %{_bindir} also you use %{_usr}/lib, which will result in things getting installed under /usr/lib instead of /usr/lib64 on 64 bit archs, is that intentional? If not please replace it with %{_libdir}. We need to be consistent here: rpmlint is complaining about putting an application in setuid how could you suggest to do this? Clement is started un root priviledges and lets them go as soon proper port (SMTP) are open, to do this it seteuid with the application program ownership. So there is NO purpose to put clement setuid, not from the security stand point, not from the rpmlint stand point, not from application stand point. file in %{_usr}/lib are shell for clement application (utilities, support), shell are not archs dependent. (In reply to comment #14) > We need to be consistent here: > rpmlint is complaining about putting an application in setuid how could > you suggest to do this? > Clement is started un root priviledges and lets them go as soon > proper port (SMTP) are open, to do this it seteuid with the application program > ownership. So there is NO purpose to put clement setuid, not from the > security stand point, not from the rpmlint stand point, not from application > stand point. > I think you understood me wrong here, I didn't want to suggest to make %attr(-,mail,mail) %{_usr}/bin/%{name} setuid, I thought it was setuid and that was why it has owner and group mail, if its not setuid, then why not just owner and group root? > file in %{_usr}/lib are shell for clement application (utilities, support), > shell are not archs dependent. > OK. You still haven't explained why you do: %attr(-,mail,mail) %{_datadir}/%{name}-%{version}/ Instead of just: %{_datadir}/%{name}-%{version}/ Or is that just a copy and paste error and will you fix that with the next version? I think datadir is not a problem, I need to double check with the PHP person, should be fixed in the next version. clement is not 'setuid' but must be root open < 1024 port. such the Clement daemon is started as root and clement take the application ownership to become a standard user mail to avoid the have a daemon with root priviledge open on the (wild) outside. I would rather have a "clement" username but rpmlint seems to be rather reluctant to 'give/declare' new username. (In reply to comment #16) > I think datadir is not a problem, I need to double check with the PHP person, > should be fixed in the next version. > > clement is not 'setuid' but must be root open < 1024 port. > such the Clement daemon is started as root and clement take > the application ownership to become a standard user mail > to avoid the have a daemon with root priviledge open on the > (wild) outside. I understand, but then the %{_usr}/bin/%{name} file doesn't have the be owned by mail.mail and could be just root.root, right? My real question al allong has been why is %{_usr}/bin/%{name} owned by mail.mail? > I would rather have a "clement" username but > rpmlint seems to be rather reluctant to 'give/declare' new > username. > Thats possible, add the following lines (at the appropiate places): Requires(pre): /usr/sbin/useradd, /usr/sbin/groupadd %pre /usr/sbin/groupadd -r clement 2> /dev/null || : /usr/sbin/useradd -s /sbin/nologin -M -d / -c "Clement daemon" -r -g clement \ clement 2> /dev/null || : And then you can use %attr (-,clement,clement) in %files. You will ofcourse also need to patch the daemon to drop its root rights to the user clement instead of mail. This might generatre some rpmlint warnings but these may be ignored. Seems I do not explain myself right... %{_usr}/bin/%{name} MUST be own by 'somebody' else than root to have clement to know, once started, under which ID it must run (the application look about the file ownership and say 'ok lets seteuid to this'), if the application is not setuid the only other way is to hard code the effective uid, this is not good from my stand point. I choosed 'mail' because this ID is used by related application. I want to give possibility to change this on the fly by local sysadmin. useradd and groupadd clement where part of the original implementation but removed to comply to rpmlint. If rpmlint is a reference tools to 'the right way to do something' warning can't ignore. IMHO rpmlint warning are 'you are doing something which can work but are against established standard'. > I choosed 'mail' because this ID is used by related application.
This asks for a very close look. Either it is a necessity, by design,
that the program must run as "mail". Or it is a fault, and it runs with
a shared uid/gid it should not have access to.
(In reply to comment #19) > > I choosed 'mail' because this ID is used by related application. > > This asks for a very close look. Either it is a necessity, by design, > that the program must run as "mail". Or it is a fault, and it runs with > a shared uid/gid it should not have access to. > OK.... lets resolv the equation 1) the application must not run as root 2) rpmlint complain if you add/create a user 3) We should not use an existing user (In reply to comment #18) > Seems I do not explain myself right... > %{_usr}/bin/%{name} MUST be own by 'somebody' else than root to have clement to > know, once started, under which ID it must run (the application look about > the file ownership and say 'ok lets seteuid to this'), if the application > is not setuid the only other way is to hard code the effective uid, this is > not good from my stand point. I choosed 'mail' because this ID is used by > related application. I want to give possibility to change this on the fly > by local sysadmin. > I understand. > useradd and groupadd clement where part of the original implementation but > removed to comply to rpmlint. > If rpmlint is a reference tools to 'the right way to do something' warning > can't ignore. IMHO rpmlint warning are 'you are doing something which can work > but are against established standard'. No, a rpmlint warning means you shouldnot be doing this unless you've got a good reason, and it this case we have a good reason so using user and groupadd is ok. (In reply to comment #19) > > I choosed 'mail' because this ID is used by related application. > > This asks for a very close look. Either it is a necessity, by design, > that the program must run as "mail". Or it is a fault, and it runs with > a shared uid/gid it should not have access to. > If I understand jmp correctly its the latter (a fault) jmp if you think it is better to have it run as clement, feel free to add the user, in exceptional cases (which daemons always are) you can ignore the relevant rpmlint warnings, thats why they are warnings, rpmlint deliberatly has 2 levels of compaining, warn and error, and these are only warnings. > 2) rpmlint complain if you add/create a user
Hmm? The "dangerous-command-in" _warning_ about userdel/groupdel?
This is misinformation. Surely there are valid cases when creating
a new user or group is required/justified.
Similarly, there are cases when deleting a user or group during
package removal can be done, i.e. if no files owned by that uid/gid
are left anywhere.
In particular, since you want a "centralized quarantine area" (quote),
you need a special uid/gid and not use an existing uid/gid which is
shared with other programs/services.
So I added clement as group & user as suggested. got this from rpmlint (many lines of this kind) E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement A file in this package is owned by a non standard group. Standard groups are: root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail, news, uucp, man, games, gopher, dip, ftp, lock, nobody, users Does not look like as a 'Warning' to me, how to make it as a Warning? Also got this warning W: clement dangerous-command-in-%preun userdel because I would like to delete created user when removing all the application, there is a kind of ambiguity here as some file could be left if the application was in production previously. My understanding of the last Michael comment is to let the sys-admin delete the remaining user/group himself, right? > E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement Who cares? This is not the first package that creates a new user/group. > My understanding of the last Michael comment is to let the sys-admin > delete the remaining user/group himself, right? Yes. If package removal does not get rid of all files owned by that user/group, the next package might allocate the same uid/gid and give some other software access to the old files, which are still left on the file-system as orphans. (In reply to comment #24) > > E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement > > Who cares? This is not the first package that creates a new user/group. Me!. Agree this is not the first package creating a user/group.... problem, if rpmlint is THE reference and rpmlint is reporting error how can I make the difference between real error which are errors from simple advice... now in my case there is 48 "errors" which are 'who cares',.... hmmmm...... > > > My understanding of the last Michael comment is to let the sys-admin > > delete the remaining user/group himself, right? > > Yes. If package removal does not get rid of all files owned by that > user/group, the next package might allocate the same uid/gid and give > some other software access to the old files, which are still left > on the file-system as orphans. OK, lets keep the user/group then > A bit late due to a bugzilla collision, but I fully agree with Michael: (In reply to comment #23) > So I added clement as group & user as suggested. > > got this from rpmlint (many lines of this kind) > > E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement > A file in this package is owned by a non standard group. > Standard groups are: > root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail, > news, uucp, man, games, gopher, dip, ftp, lock, nobody, users > > > Does not look like as a 'Warning' to me, how to make it as a Warning? > Erm, so its an error (my mistake), still it can be ignored, rpmlint us an automated validation tool and sometimes it can be just plain wrong. Trust me (and Michael) on this between the 2 of us there is a ton of packaging experience. > > Also got this warning > W: clement dangerous-command-in-%preun userdel > because I would like to delete created user when removing all the application, > there is a kind of ambiguity here as some file could be left if the application > was in production previously. > My understanding of the last Michael comment is to let the sys-admin delete > the remaining user/group himself, right? Yes, you should not remove the user, the user may only be removed if your 100.1% sure that no files owned by that user will be left around, which in this case we aren't actually we are pretty sure that files will be left around, so the userdel must be removed from the spec file. There is no policy which says that all rpmlint errors/warnings must be fixed. Look at it from a different perspective to understand. There is nothing about this in the Package Review Guidelines. Nothing at all about creating user/group accounts. Not even the guideline about "setuid root" from fedora.us QACheckList is included. When we agreed on the initial rather long list of MUST/SHOULD items in a FESCO meeting, we didn't cover useradd/groupadd/userdel/groupdel or fedora-useradd and friends. | http://fedoraproject.org/wiki/Packaging/ReviewGuidelines | | - SHOULD: If scriptlets are used, those scriptlets must be sane. | This is vague, and left up to the reviewers judgement to determine | sanity. If anything had changed over time, it should have been announced clearly. Version 2.1-192 SPECS file fine tuning, user/group clement used to run the main daemon Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-192.spec SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-192.src.rpm (In reply to comment #28) > Version 2.1-192 SPECS file fine tuning, user/group clement used to run the main > daemon > > > Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-192.spec > SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-192.src.rpm Looks good, I've sponsored you and its ok to import this now. There is one small thing you should fix before importing, please add " || :" at the end of all the commands (not the if / endif lines) in the pre / post / postun scripts, that way if one of these commands fail it doesn't cause the entire rpm transaction to fail. This is especially important for the pre script, because of a user installs, then removes and then tries to reinstall clement the groupadd and useradd will fail because they already exist (which is ok, the exisiting user may be recycled in this corner case). Part now of the DEVEL EXTRAS, build successful. Changing Summary for tracking purposes. Package Change Request ====================== Package Name: clement New Branches: el5 Owners: jmrcpn Most of the clement installation are el5 distribution type. Having clement in EPEL 5 repo will make update process easier. Git done (by process-git-requests). |