https://bugzilla.redhat.com/show_bug.cgi?id=1141711 --- Comment #13 from Sylvain Beucler <beuc@xxxxxxxx> --- (In reply to Lubomir Rintel from comment #9) > Sorry it took so long and thanks for the patience. > > Reviewing a package of this complexity requires some extra setup, especially > as you seem to do very dangerous things from the scriptlets. > > So, here we go; it still needs some amount of work: > > 0.) % characters in comments: > > # Marking /etc as conffiles and exclude locales (cf. mandatory %find_lang) > # Not using recursive dirs listing because that is processed when all > # packages are mixed in the common install dir (so using %dir instead) > > Please escape them: s/%/%%/. Otherwise RPM attempts to expand them. Done. > 1.) Your %descriptions for subpackage do not make much sense > > They're all the same. Please describe what the package is for. They begin with a general stanza about FusionForge but continue with a description of the sub-package. > The summaries can be improved a bit too. E.g.: > > Summary: Collaborative development tool - CAS authentication > > Can be better written as: > > Summary: CAS authentication plugin for Fusionforge "CAS authentication" is an existing gettext string that is directly extracted from the plugin. I changed to "FusionForge plugin - CAS authentication" to be able to reuse translated strings. > 2.) Please try to keep the whitespace less crazy > > Please crop the four line breaks above %changelog to two. Done. > 3.) Your scriptlets seem to do insane things Following the discussion above we can conclude that post-install scripts need to be removed. README.Fedora still explains what to do after install. Done. Please let me know directly when a fix is incomplete - this issue was adressed 2 weeks ago (comment #4) by adding documentation, and we only knew that this eventually wasn't enough yesterday. > Some tips: > * Do _NOT_ assume database is running > * Do not fail under any circumstances > * Do not spew any output > To verify, try to use install your packages from a kickstart -- do an > unattended installation or run livecd-creator and ensure your log is empty. > > 3.0.) Do not attempt to create directories that you already ship in the > package. > > Installing : fusionforge-common-5.3.50+201410011756-1.fc20.noarch > 77/136 > useradd: warning: the home directory already exists. > Not copying any file from skel directory into it. > > 3.1.) Do not enable or disable sevices > > Installing : fusionforge-db-local-5.3.50+201410011756-1.fc20.noarch > 99/136 Note: Forwarding request to 'systemctl enable postgresql.service'. > ln -s '/usr/lib/systemd/system/postgresql.service' > '/etc/systemd/system/multi-user.target.wants/postgresql.service' > ... > Note: Forwarding request to 'systemctl enable httpd.service'. > ... > > That is system operator's duty. With systemd, you may want to create a target > that depends on services you need. > > 3.2.) Avoid debugging output > > Installing : fusionforge-db-local-5.3.50+201410011756-1.fc20.noarch > 99/136 > ... > Importing initial database... > Running script: 20100308-drop-forum-attachment-type.sql > /usr/share/fusionforge/db/20100308-drop-forum-attachment-type.sql ran > correctly > > Installing : fusionforge-plugin-scmsvn-5.3.50+201410011756-1.fc20.noarch > 132/136 > Running /usr/share/fusionforge/plugins/scmsvn/bin/install.sh configure > Modifying inetd for Subversion server > TODO: xinetd support > > Direct it to a log if needed. > > 3.3.) Note that there might not be enough entropy available > > Generating RSA private key, 1024 bit long modulus > .++++++ > .........++++++ > e is 65537 (0x10001) > > Ensure this can not lock up system forever. It's probably better done in a > separate service, on startup and asynchronously. See what sshd does. > > 3.4.) Use systemd integration properly > > Installing : fusionforge-plugin-admssw-5.3.50+201410011756-1.fc20.noarch > 102/136 > Redirecting to /bin/systemctl reload httpd.service > > If you need to refresh a service, do not call /sbin/service. See: > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd > > 3.5.) Do not assume a database is running > Installing : fusionforge-shell-5.3.50+201410011756-1.fc20.noarch > 109/136 > ... > Could not connect to database > > Scriptlet might be running from a kickstart or during system installation. > > 3.6.) Do not .... do this: > > Installing : fusionforge-shell-5.3.50+201410011756-1.fc20.noarch > 109/136 > ... > useradd: cannot create directory /nonexistent > > The nonexistent directory is supposedly nonexistent. Also, it makes the > security policy angry: > > type=AVC msg=audit(1412781994.848:906): avc: denied { create } for > pid=28546 comm="useradd" name="nonexistent" > scontext=unconfined_u:system_r:useradd_t:s0-s0:c0.c1023 > tcontext=system_u:object_r:etc_runtime_t:s0 tclass=dir permissive=0 For the record, the upstream installation script didn't use "useradd -M" which forces disabling homedir creation, which itself is normally disabled by default already but enabled in Fedora's /etc/login.defs. > 3.6.) Never let the scriptlets fail: > > Installing : fusionforge-shell-5.3.50+201410011756-1.fc20.noarch > 109/136 > ... > warning: %post(fusionforge-shell-5.3.50+201410011756-1.fc20.noarch) > scriptlet failed, exit status 12 > non-fatal postin scriptlet failure in rpm package fusionforge-shell > > even worse, they fail during uninstall (always a bug): > > useradd: cannot create directory /nonexistent > error: %preun(fusionforge-shell-5.3.50+201410011756-1.fc20.noarch) scriptlet > failed, exit status 12 3.x comments obsoleted by post-install scripts removal. > 4.) Don't leave garbage in after uninstall: > > [root@belphegor SPECS]# find /etc/fusionforge /usr/share/fusionforge > /var/lib/fusionforge |xargs rpm -qf > file /etc/fusionforge is not owned by any package > file /etc/fusionforge/ssl-cert.pem is not owned by any package > file /etc/fusionforge/config.ini.d is not owned by any package > file /etc/fusionforge/config.ini.d/post-install.ini is not owned by any > package > file /etc/fusionforge/config.ini.d/sysauthldap-secrets.ini is not owned by > any package > file /etc/fusionforge/config.ini.d/post-install-secrets-ssh_akc.ini is not > owned by any package > file /etc/fusionforge/config.ini.d/post-install-secrets.ini is not owned by > any package > file /etc/fusionforge/ssl-cert.key is not owned by any package > file /etc/fusionforge/httpd.conf.d is not owned by any package > file /etc/fusionforge/httpd.conf.d/vhost-list.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/auth-main.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/plugin-scmsvn.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/post-install-secrets.inc is not owned by > any package > file /etc/fusionforge/httpd.conf.d/plugin-moinmoin.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/vhost-main.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/ssl-really-on.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/block-trace.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/04-config-vendor.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/plugin-generic.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/plugin-scmbzr.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/plugin-ckeditor.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/40-vhosts-extra.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/auth-projects.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/ssl-off.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/plugin-scmdarcs.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/20-vhosts-lists.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/50-wsgi-scmbzr.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/01-namevhost.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/projects-in-mainvhost.inc is not owned by > any package > file /etc/fusionforge/httpd.conf.d/50-vhosts-scm.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/30-vhosts-projects.conf is not owned by > any package > file /etc/fusionforge/httpd.conf.d/plugin-scmgit.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/10-vhosts-main.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/plugin-sysauthldap-secrets.inc is not > owned by any package > file /etc/fusionforge/httpd.conf.d/50-wsgi-moinmoin.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/vhost-projects.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/02-config-main.conf is not owned by any > package > file /etc/fusionforge/httpd.conf.d/plugin-mediawiki.inc is not owned by any > package > file /etc/fusionforge/httpd.conf.d/httpd.vhosts is not owned by any package > file /etc/fusionforge/httpd.conf.d/log.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/ssl-on.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/vhost-scm.inc is not owned by any package > file /etc/fusionforge/httpd.conf.d/plugin-authhttpd.inc is not owned by any > package > file /etc/fusionforge/httpd.conf is not owned by any package > file /usr/share/fusionforge is not owned by any package > file /usr/share/fusionforge/www is not owned by any package > file /usr/share/fusionforge/www/plugins is not owned by any package > file /usr/share/fusionforge/www/plugins/authhttpd is not owned by any package > file /usr/share/fusionforge/www/plugins/projectlabels is not owned by any > package > file /usr/share/fusionforge/www/plugins/message is not owned by any package > file /usr/share/fusionforge/www/plugins/extsubproj is not owned by any > package > file /usr/share/fusionforge/www/plugins/compactpreview is not owned by any > package > file /usr/share/fusionforge/www/plugins/headermenu is not owned by any > package > file /usr/share/fusionforge/www/plugins/mediawiki is not owned by any package > file /usr/share/fusionforge/www/plugins/admssw is not owned by any package > file /usr/share/fusionforge/www/plugins/globalsearch is not owned by any > package > file /usr/share/fusionforge/www/plugins/webanalytics is not owned by any > package > file /usr/share/fusionforge/www/plugins/blocks is not owned by any package > file /usr/share/fusionforge/www/plugins/contribtracker is not owned by any > package > file /usr/share/fusionforge/www/plugins/scmgit is not owned by any package > file /usr/share/fusionforge/www/plugins/scmhook is not owned by any package > file /usr/share/fusionforge/www/plugins/moinmoin is not owned by any package > file /usr/share/fusionforge/www/plugins/online_help is not owned by any > package > file /usr/share/fusionforge/www/plugins/authldap is not owned by any package > file /usr/share/fusionforge/www/plugins/hudson is not owned by any package > file /usr/share/fusionforge/www/plugins/authcas is not owned by any package > file /var/lib/fusionforge is not owned by any package > file /var/lib/fusionforge/etc is not owned by any package > file /var/lib/fusionforge/etc/postfix-transport.db is not owned by any > package > [root@belphegor SPECS]# Currently install/uninstall is clean, given that no post-install script is run. Most of these files are generated passwords, potentially customized configuration files and user-uploaded content, so if we also need an unconditionnal "rm -rf /etc/fusionforge /usr/share/fusionforge /var/lib/fusionforge" on uninstall, no problem with removing critical user data without notice, but I'll need to see the official guideline about it. > (In reply to Sylvain Beucler from comment #8) > > > Here I rely on the upstream snapshoting system which produces independent > > tarballs with versions such as 5.3.50+201410011756, and I don't intend to > > upload a pre-release to Fedora. If you want to witnessing a modified .spec > > from me with a -1.20141001git in the Release field, let me know. > > We're reviewing exactly what goes into Fedora and that has to be aligned > with guidelines be it a snapshot or not. > > As I've stated on IRC: I now disconnected from IRC to keep the conversation here. > Everyone neds to be able to get the very same tarball > as you've used. So either include a proper _public_ link or a comment on how > is the tarball created from the VCS snapshot. And always either use an > official release version number or a proper pre-release tag. > "5.3.50+201410011756" does not seem to exist publicly. Your link is dead. On > the other hand, "-1.20141001git" looks like a proper pre-release tag. Done. http://fusionforge.fusionforge.org/fedora/fusionforge-5.3.50+201410091603-1.fc20.src.rpm http://fusionforge.fusionforge.org/fedora/fusionforge.spec -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review