Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=767985 --- Comment #12 from T.C. Hollingsworth <tchollingsworth@xxxxxxxxx> 2012-05-10 21:34:06 EDT --- (In reply to comment #11) > So, here are a couple of findings not limited to the items on the > ReviewGuidelines page. This thing is non-trivial to review, but I had expected > that. > > [...] > > * The debian/NEWS file mentions a "man2html-base" package instead of > "man2html-core". Indeed, the Debian packages search lists it as > "man2html-base". That suggests following > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming Generally, Fedora uses "-core" subpackages for the same reasons/purposes Debian uses "-base" subpackages. I used "-core" to be consistent with other Fedora packages, but I can change it if consistency with the Debian package is preferred. > * Licensing, minor issues: > Files man2html/man2html.c and debian/sources/man2html.cgi.c do not > explicitly refer to GPL, just: > > > Permission is granted to distribute, modify and use this program > > as long as this comment is not removed or changed. This looks like "Copyright Only" [1] to me, but I'll mail the legal list for guidance. > The utils.c file only mentions "GPL". Following > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#GPL_and_LGPL > and > https://fedoraproject.org/wiki/Licensing#Good_Licenses > that would result in a license tag: GPL+ > > File manwhatis.c contains a GPLv2 (or later) header. > > So, no big issue. License clarification would be NTH: > > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Clarification > > > * Format string warnings in build output! More often than not these are > worth taking a look at, also to avoid surprises on big endian platforms. Okay, I'll see what's up with these. > * Run-time testing: > > $ hman 7 locale > /usr/bin/hman: line 90: lynx: command not found > > So, if "lynx" isn't a requirement, the browser ought to be configurable. Let's > see: > > $ export MANHTMLPAGER=firefox ; hman 7 locale > --> http://localhost/cgi-bin/man/man2html?7+locale > > "The requested URL /cgi-bin/man/man2html was not found on this server." I'll patch hman to fix this one. > Now returning to lynx after a "yum -y install lynx": > > $ hman 7 locale > > "Alert!: Executable link rejected due to location or path. > > lynx: Can't access startfile > lynxcgi:/usr/lib/man2html/cgi-bin/man/man2html?7+locale" LYNXCGI is disabled by default for security reasons. (See /etc/lynx.cfg at line 964 for the details.) I could ask the lynx maintainer to enable local LYNXCGI and whitelist man2html, but I'm not sure of the security implications of that. IMHO it would be better to default hman to calling `xdg-open` and leave LYNXCGI configuration to interested users who are comfortable with the security issues. > * Fedora related patches and explanations don't seem to be accurate. For > example: > > man2html-dirs.patch > +sharedir = $(DESTDIR)$(PREFIX)/usr/share/man2html > > A path that is not used anywhere in the package. The spec file comment mentions > /var/www/cgi-bin for cgi. Sorry, both are leftover from previous renditions of the packaging. /usr/share/man2html would have been used by the original man2html CGI scripts to store template files. Debian's version of the CGI scripts (which I switched to because they are more robust and less buggy) don't require it. The latter is because I ended up patching away Debian's use of cgi-bin and instead just use an Apache conf file to make http://localhost/man/ work, because mucking about in /var/www isn't allowed in Fedora. I'll clean up this patch and the comment. > * As expected, due to SELinux, httpd is confined as much as not to allow the > CGI scripts to access the MAN search paths. That's a blocker, but still only a > SHOULD in the Review Guidelines: > > | SHOULD: The reviewer should test that the package functions as > | described. A package should not segfault instead of running, for example. > > Currently, the only way to get the scripts to work at all is to change > their file context to httpd_unconfined_script_exec_t. > > Neither the Packaging Guidelines nor the Review Guidelines contain > any section on SELinux. I've found just: > https://fedoraproject.org/wiki/PackagingDrafts/SELinux Thanks, I'll call `semanage fcontext` in the scriptlets for now so it works and file a bug against selinux-policy so it can be fixed properly. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review