Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.6.1-1.fc12.src.rpm Description: Prosody is a flexible communications server for Jabber/XMPP written in Lua. It aims to be easy to use, and light on resources. For developers it aims to be easy to extend and give a flexible system on which to rapidly develop added functionality, or prototype new protocols.
rpmlint is not quiet for this package, but all warning should be safely ignored: $ rpmlint prosody-0.6.1-1.fc12.src.rpm prosody.src:40: W: configure-without-libdir-spec This one should be ok since upstream does not use autotools but a specific configure script ; which does not provide any '--libdir' flag. $ rpmlint prosody-0.6.1-1.fc12.x86_64.rpm prosody.x86_64: W: non-standard-uid /var/run/prosody prosody prosody.x86_64: W: non-standard-gid /var/run/prosody prosody prosody.x86_64: W: non-standard-uid /var/lib/prosody prosody prosody.x86_64: W: non-standard-gid /var/lib/prosody prosody prosody.x86_64: W: incoherent-subsys /etc/rc.d/init.d/prosody $prog non-standard uid/gid are ok because the package creates it own user for the daemon to run. incoherent-subsys is due to the use of '$prog' in the initd file. Package builds fine in mock. Note that to use ssl possibilities, we need to have lua-sec which is not yet in the repositories (I've made a review request for this one also : https://bugzilla.redhat.com/show_bug.cgi?id=551763)
I've upgraded to the latest stable version (0.6.2): Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.6.2-1.fc13.src.rpm
I've upgraded to the latest stable version (0.7.0): Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.7.0-1.fc13.src.rpm
I wonder if you should just go ahead without luasec and SSL support, and not block on luasec which has issues for Fedora.
Indeed that should be a good solution. Anyways, since nobody seems to be interested in reviewing prosody for now, luasec issue is not really a blocker :)
I've started testing this package because I wanted to set up a simple XMPP server. I ran into a fist issue when I tried this as root : prosodyctl adduser username@mydomain Apparently, that command drops privileges to the "prosody" user, which is good. But it tries to create /var/lib/prosody/mydomain/accounts/username.dat and all of its parent directories, but fails. My first guess is that you should add an empty /var/lib/prosody owned by the "prosody" user to the package.
The missing /var/lib/prosody might also be because I have /usr/com/prosody since I've tested on EL5 where _sharedstatedir probably evaluates to that. Maybe use %{_var}/lib/prosody instead? The programs are using the default of /var/lib/prosody anyway in all cases since --datadir= isn't passed to configure. A second issue is that the included crt/key pair comes as-is from the "certs" directory of the source package. It will expire on October 17th 2010, in little over a month. It also eases man-in-the-middle attacks since the default certificate is identical on all servers. The best would be to generate a unique long-lasting key/crt pair upon package install, like the mod_ssl package does.
(In reply to comment #6) > My first guess is that you should add an empty /var/lib/prosody owned by the > "prosody" user to the package. The package already ships that directory: $ rpm -ql prosody | grep /var /var/lib/prosody /var/run/prosody (In reply to comment #7) > The missing /var/lib/prosody might also be because I have /usr/com/prosody > since I've tested on EL5 where _sharedstatedir probably evaluates to that. > Maybe use %{_var}/lib/prosody instead? The programs are using the default of > /var/lib/prosody anyway in all cases since --datadir= isn't passed to > configure. I've not yet tested on EL-5 (Fedora 12 and 13 only for now), I'll try. > > A second issue is that the included crt/key pair comes as-is from the "certs" > directory of the source package. It will expire on October 17th 2010, in little > over a month. It also eases man-in-the-middle attacks since the default > certificate is identical on all servers. The best would be to generate a unique > long-lasting key/crt pair upon package install, like the mod_ssl package does. You are right, I will change the specfile so it will generate a ssl cert at install time. Thank you :)
About the /var/lib/prosody : If you test on EL-5 you'll see what I mean. The macro you use creates and owns /usr/com/prosody there instead, so things fail. You'll also notice that _initddir doesn't exist, you might want to change to _sysconfdir/init.d or similar. Another issue (I find them while moving forward) : The crt and key files are mode 644, which means that any user of the machine can get them. Bad. I suggest you include them as 600 which then requires them to be readable by the "prosody" user. You can either/also change /etc/prosody/certs to be mode 700, and/or /etc/prosody too. I'm unsure as if there is a strict policy about key and crt files, but another option would be to put them in /etc/pki/tls/{certs,private}/ with all the other files and make them mode 600 and owned by "prosody".
Ok, that iss in fact documented on Fedora's wiki: https://fedoraproject.org/wiki/Packaging:RPMMacros « Differences in EPEL 4 & 5 %{_initddir} does not exist in EPEL 4 & 5, use the deprecated %{_initrddir} macro instead %{_sharedstatedir} expands to %{_prefix}/com in EPEL 4 & 5 » No luck, I've throwed the two big differences between Fedora and EPEL in this SPEC :-D I'm working on having it build properly under EL-5, and will take a look at the same time for the SSL certs generation. I'll search if a specific policy exists for such certificates, that is the first RPM I build that needs such certs.
Ok, here is the new version (this one is only for Fedora, using recommended macros): Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.7.0-2.fc13.src.rpm For EL-5: Spec URL: http://odysseus.x-tnd.be/fedora/prosody/EL-5/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/EL-5/prosody-0.7.0-2.src.rpm Note that lua-expat (required by prosody) is not available on EPEL repositories. such as lua-socket (required by lua-sec). Those two missing dependencies (rebuilt from F-13 SRPMs) and a build of lua-sec for EL-5 are available on: http://odysseus.x-tnd.be/fedora/prosody/EL-5/ I did not found any policy for SSL certificates, so I did it exactly the same way mod_ssl does (including permissions, certificate duration, etc.).
A new version (fedora only for now) that fix a build issue on i686 systems: Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.7.0-3.fc14.src.rpm
I no longer maintain any package in Fedora repositories ; what to do with thie review? Should it be closed? Since I continue to use Prosody on my own server, I've made a new version of the specfile which take care of both Fedora and EL-5. If someone else is interested in packaging prosody, my actual specfile can be found here: https://bitbucket.org/trashy/rpm/src/c44f843cde0a/prosody/prosody.spec The main issue for this package is still lua-sec (to add Prosody SSL support) I guess. That point was discussed in the relevant bz entry: https://bugzilla.redhat.com/show_bug.cgi?id=551763
Yeah, if someone doesn't want to take this up right away, it's better to close this -- the next person who wants to work on it can either reopen or file a new review request. hopefully they'll see your spec file and use it as a base to start from.
Please reopen if prosody can be built without lua-sec.
(In reply to comment #15) > Please reopen if prosody can be built without lua-sec. Wouldn't the Fedora way be porting it to the luasocket? (not that I would do it ;))
As far as I know, Prosody can be compiled without lua-sec ; but as I personnaly was not interested in a jabber server without SSL support (I guess that SSL is the only thing lua-sec provides that lua-socket does not ; but I may be wrong, I do not know lua at all). Anyways, I've never tried such a build.
Good news, maybe IPv6 is coming in luasocket? http://code.google.com/p/lxmppd/issues/detail?id=68#c11
Wouldn't it be worthy to keep prosody and lua-spec packages at least in http://repos.fedorapeople.org/ until the issue will be reconciled somehow?
Note that the problem with this review is more about lua-sec (that is about SSL support, which is more or less "must have" for Jabber server). IPV6 is another problem, but it doesn't make Prosody useless in practical usage.
(In reply to comment #19) > Wouldn't it be worthy to keep prosody and lua-spec packages at least in > http://repos.fedorapeople.org/ until the issue will be reconciled somehow? Just to note that the repository on http://repos.fedorapeople.org/repos/mcepl/prosody/ has been updated to prosody 0.8 (I am missing lua-dbi package, so still using old plain text only storage).
Reporter, if you reopen this review, I would gladly make you review.
I've been recently working on an upgrade to use systemd on Fedora; but I'm not done yet (need some tests). I also need to make some changes regarding the comments you've made on lua-sec review (__mkdir macros, lua requirement, and so on); I'll post a new specfile soon.
(In reply to comment #23) > I've been recently working on an upgrade to use systemd on Fedora; but I'm not > done yet (need some tests). Cool, but please keep sysvinit files as an alternative ... this is a server software, so it should be running on RHEL-6 (at least) as well.
I've made a new version of the package: Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.8.2-2.fc16.src.rpm The main change is systemd integration for Fedora. It should also work on EL-6. Mock builds fine (tested on F-16/x86_64 and EL-6/X86_64). rpmlint is still not clean : prosody.src:40: W: configure-without-libdir-spec This one should be ok since upstream does not use autotools but a specific configure script ; which does not provide any '--libdir' flag. prosody.x86_64: W: non-standard-uid /var/run/prosody prosody prosody.x86_64: W: non-standard-gid /var/run/prosody prosody prosody.x86_64: W: non-standard-uid /var/lib/prosody prosody prosody.x86_64: W: non-standard-gid /var/lib/prosody prosody prosody.x86_64: W: no-manual-page-for-binary prosody non-standard uid/gid are ok because the package creates it own user for the daemon to run. no-manual-page: well.. there is no manual page. I guess that is OK as well. On EL only: prosody.x86_64: W: incoherent-subsys /etc/rc.d/init.d/prosody $prog incoherent-subsys is due to the use of '$prog' in the initd file. lua-expat and lua-socket are now available on EL-6 as well, so missing dependencies are now lua-sec (approved today - bz #551763) and lua-dbi (bz #707016).
Legend: + = PASSED, - = FAILED, 0 = Not Applicable - MUST: rpmlint must be run on every package. The output should be posted in the review wycliff:build $ rpmlint RPMS/x86_64/prosody-*.el7.x86_64.rpm SRPMS/prosody-0.8.2-2.el7.src.rpm prosody-debuginfo.x86_64: W: only-non-binary-in-usr-lib prosody.x86_64: W: non-standard-uid /var/run/prosody prosody prosody.x86_64: W: non-standard-gid /var/run/prosody prosody prosody.x86_64: W: non-ghost-in-var-run /var/run/prosody prosody.x86_64: W: non-standard-uid /var/lib/prosody prosody prosody.x86_64: W: non-standard-gid /var/lib/prosody prosody prosody.x86_64: W: no-manual-page-for-binary prosody prosody.x86_64: W: incoherent-subsys /etc/rc.d/init.d/prosody $prog prosody.src:64: W: configure-without-libdir-spec 3 packages and 0 specfiles checked; 0 errors, 9 warnings. wycliff:build $ All these are warnings and are either false positives or unimportant, with the exception of prosody.x86_64: W: non-ghost-in-var-run /var/run/prosody A file or directory in the package is located in /var/run. Files installed in this directory should be marked as %ghost and created at runtime to work properly in tmpfs /var/run setups. This should probably be fixed. + MUST: package named according to the Package Naming Guidelines + MUST: The spec file name must match the base package %{name} - MUST: The package must meet the Packaging Guidelines . Per above mentioned Lua Packaging Guidelines spec file should (for package which should be packaged for RHEL-6 as well) contain %if 0%{?fedora} >= 16 || 0%{?rhel} >= 7 Requires: lua(abi) = %{luaver} %else Requires: lua >= %{luaver} %endif Also, please don't use only Fedora specific %ifs. E.g., instead of %if 0%{?fedora} >= 15 write %if 0%{?fedora} >= 15 || 0%{?rhel} >= 7 Also, your spec file doesn't use macros suggested in the above PackagingGuidelines. Yes, it is just a draft, but I believe that following the examples of other language-specific guidelines it is best to use those macros. + MUST: The package licensed with a Fedora approved license and meets the Licensing Guidelines + MUST: The License field in the package spec file matches the actual license MIT + MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. COPYING is included. + MUST: The spec file must be written in American English. + MUST: The spec file for the package MUST be legible. + MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task MD5: 6e907bf0d0acf24f1011083020ba6ffb + MUST: The package successfully compiles and builds into binary rpms on at least one primary architecture - build in koji, no problems 0 MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch + MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines Builds in koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=4007065) 0 MUST: The spec file handles locales properly. This is done by using the %find_lang macro No locales are present. 0 MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. No libraries provided. + MUST: Packages must NOT bundle copies of system libraries 0 MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker + MUST: Package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory + MUST: Package must not list a file more than once in the spec file's %files listings + MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + MUST: Each package must consistently use macros + MUST: The package must contain code, or permissible content 0 MUST: Large documentation files must go in a -doc subpackage + MUST: If a package includes something as %doc, it must not affect the runtime of the application 0 MUST: Header files must be in a -devel package 0 MUST: Static libraries must be in a -static package 0 MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' 0 MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package 0 MUST: devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} + MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built 0 MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section + MUST: Packages must not own files or directories already owned by other packages - MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) + MUST: All filenames in rpm packages must be valid UTF-8 Please fix suggested mistakes.
(In reply to comment #26) > - MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} > (or $RPM_BUILD_ROOT) Sorry, this is copy&paste mistake ... in this package it is all right.
BTW, I am really suspicious about this line %{_bindir}/openssl genrsa -rand /proc/apm:/proc/cpuinfo:/proc/dma:/proc/filesystems:/proc/interrupts:/proc/ioports:/proc/pci:/proc/rtc:/proc/uptime 1024 > %{sslkey} 2> /dev/null What did poor old /dev/random (which is I guess used per default anyway) do to you, that you don't want to use it? I was asking some security people around here and their unanimously think that this is really not a bright idea (among other things, some of these files are not that random), but mostly it seems like a useless mannerism to me.
Thanks Matěj for your feedback! About the ssl key generation, it was probably taken from the mod_ssl package, as I recall it does the same thing. Are there any guidelines about install-time generated SSL keys and certificates?
By default /dev/urandom is used by openssl - it is used always on start so the -rand specification should not be able to break things f.e. by adding constant data. So it is harmless to use it but unnecessary.
I've made changes about requires and %ifs. Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.8.2-2.fc16.src.rpm I've made two successfull koji scratch builds: - f16 - http://koji.fedoraproject.org/koji/taskinfo?taskID=4057689 - el6 - http://koji.fedoraproject.org/koji/taskinfo?taskID=4057692 I'm a bit lost with that ssl certificate generation. Should I finally change something in prosody's spec? And if yes, what to change? Are there existing specfiles in the repositories that could be used as example? Also, Is that a blocker for this review?
I've missed the ghost... Here is the new build: Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.8.2-4.fc16.src.rpm
There are still %if 0%{?fedora} >= 15 without %{?rhel} part in %preun, %post, and %postun scripts, but that's certainly not a bug preventing this package to be APPROVED.
(In reply to comment #31) > I've made changes about requires and %ifs. > > Spec URL: http://odysseus.x-tnd.be/fedora/prosody/prosody.spec > SRPM URL: http://odysseus.x-tnd.be/fedora/prosody/prosody-0.8.2-2.fc16.src.rpm > > I've made two successfull koji scratch builds: > - f16 - http://koji.fedoraproject.org/koji/taskinfo?taskID=4057689 > - el6 - http://koji.fedoraproject.org/koji/taskinfo?taskID=4057692 > > I'm a bit lost with that ssl certificate generation. Should I finally change > something in prosody's spec? And if yes, what to change? Are there existing > specfiles in the repositories that could be used as example? > Also, Is that a blocker for this review? My opinion would be to just get rid of whole -rand parameter of openssl and let default reign. I.e., I would have just %{_bindir}/openssl genrsa 1024 > %{sslkey} 2> /dev/null
Thanks a lot for that review :) I'll fix remainings %ifs before the first commit. If there are no objections, I'll also use Matěj's proposition for the SSL cert generation.
New Package SCM Request ======================= Package Name: prosody Short Description: Flexible communications server for Jabber/XMPP Owners: trasher Branches: f16 f17 el6 InitialCC:
Git done (by process-git-requests).
I was working on the last few changes when I see the %ghost seems to prevent /var/run/prosody to be created at install time - prosody refuses to start. It was ok on next reboot only. Without the %ghost; the directory is created at install and prosody can run. tmpfiles.d guidelines (https://fedoraproject.org/wiki/Packaging:Tmpfiles.d#Example_spec_file) does not mention use of %ghost for the directory, but only for files created in that directory (that may be ommited) as far as I understand. So, I guess that removing the %ghost should be ok, should'nt it?
Correct. Files in that directory should be %ghost'ed but not directories.
prosody-0.8.2-5.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/prosody-0.8.2-5.fc17
prosody-0.8.2-5.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/prosody-0.8.2-5.fc16
prosody-0.8.2-5.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/prosody-0.8.2-5.el6
prosody-0.8.2-5.el6 has been pushed to the Fedora EPEL 6 testing repository.
prosody-0.8.2-5.fc17 has been pushed to the Fedora 17 stable repository.
prosody-0.8.2-5.fc16 has been pushed to the Fedora 16 stable repository.
prosody-0.8.2-5.el6 has been pushed to the Fedora EPEL 6 stable repository.
Package Change Request ====================== Package Name: prosody New Branches: epel7 Owners: robert