Bug 188359
Summary: | Review Request: bugzilla - bug tracking tool | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | John Berninger <john> | ||||||||||
Component: | Package Review | Assignee: | Jason Tibbitts <j> | ||||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||||
Severity: | medium | Docs Contact: | |||||||||||
Priority: | medium | ||||||||||||
Version: | rawhide | CC: | daleemoore, dennis, ed, j, jwboyer, laurent.rineau__fedora, mike, mkanat, nkadel, paul, sstarr | ||||||||||
Target Milestone: | --- | Flags: | wtogami:
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-06-30 01:20:29 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
John Berninger
2006-04-08 13:27:14 UTC
Any chance of a review on this package? I'll try to look into this soon. In the meantime, I think someone might ping the RH bugzilla guy to take a look as well. A couple questions: 1) why use a CVS tarball instead of a release? 2) BuildArch: is sufficient. No need to spell Architecture out. Also, rpmlint doesn't throw any warnings or errors so that is good. 1) Overthinking. CVS tarball isn't really needed, so I've reverted to a released tarball (2.20.1). New packages: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.20.1-2.src.rpm I took a quick look and things do look pretty good. I have a couple of suggestions: Remove the Provides: filter stuff if you didn't want to actually filter anything. Unless somehow you actually need to filter out perl(unwanted_provide). Package the documentation files (QUICKSTART, README, UPGRADING*) as %doc instead of putting them in the web directory. Don't package the requires and provides filters. Provides filter removed, docs split into -doc package. New packages: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.20.1-3.src.rpm This is really shaping up so I'll go ahead and sign on for a review. (Others are certainly welcome to join in.) Let's dispense with rpmlint stuff: W: bugzilla no-documentation You moved all of the documentation off to the -doc subpackage. The usual practise seems to be to keep some basic documentation like changelogs or readmes in the main package and move extensive stuff off to the subpackage; I'd suggest keeping QUICKSTART, README and UPGRADING* in the base package but it's certainly up to you as there's no hard rule here. E: bugzilla script-without-shellbang /var/www/bugzilla/template/en/default/admin/keywords/list.html.tmpl E: bugzilla script-without-shellbang /var/www/bugzilla/template/en/default/admin/keywords/edit.html.tmpl E: bugzilla script-without-shellbang /var/www/bugzilla/contrib/gnatsparse/README E: bugzilla script-without-shellbang /var/www/bugzilla/contrib/gnatsparse/magic.py E: bugzilla script-without-shellbang /var/www/bugzilla/template/en/default/admin/keywords/create.html.tmpl E: bugzilla script-without-shellbang /var/www/bugzilla/template/en/default/admin/keywords/rebuild-cache.html.tmpl E: bugzilla script-without-shellbang /var/www/bugzilla/Bugzilla/Bug.pm E: bugzilla script-without-shellbang /var/www/bugzilla/template/en/default/admin/keywords/created.html.tmpl E: bugzilla script-without-shellbang /var/www/bugzilla/template/en/default/admin/keywords/confirm-delete.html.tmpl E: bugzilla script-without-shellbang /var/www/bugzilla/contrib/gnatsparse/gnatsparse.py These all have executable permission, but they shouldn't. Perhaps the python scripts should, but they would need to start with #!/usr/bin/python. E: bugzilla version-control-internal-file /var/www/bugzilla/template/en/.cvsignore E: bugzilla version-control-internal-file /var/www/bugzilla/template/.cvsignore E: bugzilla version-control-internal-file /var/www/bugzilla/Bugzilla/.cvsignore E: bugzilla-doc version-control-internal-file /var/www/bugzilla/docs/.cvsignore These should all be deleted. E: bugzilla non-executable-script /var/www/bugzilla/contrib/gnats2bz.pl 0644 E: bugzilla non-executable-script /var/www/bugzilla/contrib/cvs-update.pl 0644 E: bugzilla non-executable-script /var/www/bugzilla/contrib/sendbugmail.pl 0644 E: bugzilla non-executable-script /var/www/bugzilla/contrib/jb2bz.py 0644 E: bugzilla non-executable-script /var/www/bugzilla/contrib/sendunsentbugmail.pl 0644 E: bugzilla non-executable-script /var/www/bugzilla/contrib/yp_nomail.sh 0644 E: bugzilla-doc non-executable-script /var/www/bugzilla/docs/makedocs.pl 0644 I think it's safe to ignore these, but we'll have to think about consistency. W: bugzilla non-conffile-in-etc /etc/httpd/conf.d/bugzilla.conf Safe to ignore. E: bugzilla wrong-script-interpreter /var/www/bugzilla/contrib/jb2bz.py "/usr/local/bin/python" Should probably be fixed. About the contrib directory: Is it safe, or even appropriate to stick this stuff in the webroot? I would argue that it isn't, or that access to it from the web should be severely restricted. Generally this kind of thing is packaged (execute bits off) with the documentation as examples or under /usr/lib Is everything in /var/www/bugzilla intended to be visible from the web or accessed by one of the scripts run by the web server? Stuff that's run from cron jobs shouldn't be there. What about the t directory? (In reply to comment #6) > W: bugzilla non-conffile-in-etc /etc/httpd/conf.d/bugzilla.conf > > Safe to ignore. Why ignore it? Why not mark the file as %config(noreplace) instead, and shut rpmlint up? I believe I've fixed all of the rpmlint issues above, but can't be certain as rpmlint continues to refuse to display those issues for me. Anyway, new packages here: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.20.1-4.src.rpm Paul: The issue was simply that I didn't understand what rpmlint was trying to tell me. John: What rpmlint version are you running? I have rpmlint-0.76-1.fc5.noarch. I'll try another build as soon as my local mirror finishes updating. (In reply to comment #9) > The issue was simply that I didn't understand what rpmlint was trying to tell me. rpmlint has a handy "-I" option if you don't know what it's complaining about: $ rpmlint -I non-conffile-in-etc non-conffile-in-etc : A non-executable file in your package is being installed in /etc, but is not a configuration file. All non-executable files in /etc should be configuration files. Mark the file as %config in the spec file. Created attachment 128270 [details] Another specfile for cross checking Just a FYI, I have a local (not really maintained since 2.16, more or less blindly updated) package of this too, perhaps cross check if there's something useful worth adopting in it. Specfile attached, full SRPM at http://cachalot.mine.nu/5/SRPMS/bugzilla-2.20-0.2.src.rpm Paul: Yep, I know about -I; I just confused that message for something else. I guess I need to tighten the screws so that my brain won't fall out as often. Ville: Thanks for sending your spec; I took a look at it and there are some things that I personally like: Your handling of python and ruby shebang lines is nice and more comprehensive than trying to write patch files. You filter out some dependencies that John's packages don't. (*.pl, DBD::*). I hadn't started looking into that yet but I it will need to be done. To the package at hand: It builds; rpmlint still complains about four .cvsignore files. A quick find will get rid of those. rpmlint doesn't like not having any binaries in /usr/lib. Does anyone have any suggestions as to whether it's reasonable to keep the contrib stuff here? If not, where should it go? Perhaps /usr/share since there's no arch-dependent stuff there. I like the current docs split, although after more thought I wonder how much of that documentation is for the installer/configurer and how much is expected to be web-accessible. Will there be broken internal links if the -docs package isn't installed? And if the docs shouldn't be web-accessible, they should go into /usr/share/doc. Ville's package installs a crontab. I don't know enough about bugzilla to know if it really needs one, but if so then one should probably be installed with everything commented out. (An everything install shouldn't have crontabs and such running by default.) Ville's package restarts the web server. Is it a good idea to do this here? Ville's package has Requires for perl-Template-Toolkit, perl(Chart::Lines), perl(GD) and Perl(PatchReader) which aren't in this package. The Template-Toolkit is found by RPM, and GD is a dependency so that's taken care of. Chart::Lines and PatchReader are in extras already so they should probably be added as dependencies if Bugzilla will use them. It looks like the Requires: filter didn't run at all; I still those things in the final list. I think you need to escape the parentheses when you call sed, but you might also want to consider either emitting the script from the specfile as Ville does, or do some substitution on the script in order to get the proper value of %{__perl_requires} in there. It's not guaranteed to live in /usr/lib/rpm/perl_requires. Bugzilla's contrib stuff depends on being where it is, for the most part. The first time you run ./checksetup.pl it creates a various .htaccess files that prevents access to files that people shouldn't be accessing. If you want, you can be clever and generate these files in advance and then include it as a patch with the distro. They don't have anything actually dynamic in them, I think. (And actually, that makes me wonder why we don't just have them in CVS.) You can actually modify Bugzilla to be relocatable, but it's easier to not do it if you don't have to. Looking over the spec file, things look pretty good to me from a Bugzilla standpoint, except that you should probably just have a separate -contrib package but keep the contrib stuff in /var/www/bugzilla/, because a lot of them depend on the Bugzilla/ libraries in that directory and it would be irritating to have to type /usr/lib/bugzilla/script.pl from the /var/www/bugzilla directory. Also, I released 2.22 lately, so the package should probably be updated to that. Maybe if you bug me I'll add a switch to checksetup.pl in future versions that will create all the dynamically-generated files and set all the file permissions properly without trying to connect to the database or compile the templates. :-) (In reply to comment #13) > Bugzilla's contrib stuff depends on being where it is, for the most part. > > The first time you run ./checksetup.pl it creates a various .htaccess files that > prevents access to files that people shouldn't be accessing. If you want, you > can be clever and generate these files in advance and then include it as a patch > with the distro. They don't have anything actually dynamic in them, I think. > (And actually, that makes me wonder why we don't just have them in CVS.) Would it not be possible to create a /etc/httpd/conf.d/bugzilla.conf with the appropriae permissions instead of having a bunch of .htaccess files (which are slower at runtime and rpmlint will whinge about them)? (In reply to comment #15) > Would it not be possible to create a /etc/httpd/conf.d/bugzilla.conf with the > appropriae permissions instead of having a bunch of .htaccess files (which are > slower at runtime and rpmlint will whinge about them)? No, because we need to be in control of them ourselves. That is, the .htaccess files are required for security on Bugzilla, so we can't leave it up to a directory that's out of our control. Evening, folks. I wrote a bugzilla SRPM myself a while back, that I'd be happy to send the .spec for, but yours is not bad. However, rather than doing all explicit file naming and transfers and exclusions, why not use a tar to transfer everything to the target dir for installation, like this, then put the documentation and contrib directory in as a normal '%doc' set of files, like this? %install mkdir -p ${RPM_BUILD_ROOT}/%{bzinstallprefix}/bugzilla # Chicanery because bugzilla lacks an install tool tar cf - . | tar xf - -C ${RPM_BUILD_ROOT}/%{bzinstallprefix}/bugzilla \ --exclude=CVS \ --exclude=.cvsignore \ --exclude=t \ --exclude=README \ --exclude=UPGRADING \ --exclude=UPGRADING-pre-2.8 \ --exclude=docs \ --exclude=contrib Lots of good input here over the past couple of week; we even have upstream willing to help us out. Any input from the submitter? Oops... it seems I've not kept up. Apologies, my bugzilla mail started going to a mailbox I wasn't paying attention to. Give me a few days to catch myself up... OK, packages updated with (I think) all input above. Specifically: - shifted /usr/local path handling - redid provides/requires filtering - updated to upstream 2.22 - split off contrib/ into -contrib package (but still installing it in /var/www/bugzilla/contrib) SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.22-0.src.rpm I'm looking at the bugzilla-httpd-conf: I really don't think it needs the +FollowSymLinks or the +Indexes options. I haven't needed them in any of the Bugzillas I've installed. Hi folks, the -2.22-0.src.rpm in comment #20 builds on my FC-5 system but fails to install. Here are the commands and the resulting errors: yum localinstall bugzilla-2.22-0.noarch.rpm \ bugzilla-contrib-2.22-0.noarch.rpm bugzilla-doc-2.22-0.noarch.rpm Error: Missing Dependency: perl(BugzillaEmail) is needed by package bugzilla-contrib Error: Missing Dependency: perl(globals.pl) is needed by package bugzilla Error: Missing Dependency: perl(globals.pl) is needed by package bugzilla-contrib and it happens because the automatic dependency handling within RPM finds lines such as: require "globals.pl"; One possible way to "fix" this would be to first save a list of all the Requires: items, then add AutoReqProv: no to the main package and the -contrib sub-package, and finally add in all the Requires: bits manually [being carefull to delete the unwanted bits]. This is not a pretty solution and it will be un-fun to maintain (for instance, if upstream ever adds any new requirements). So, perhaps someone else knows of a better way to fix this annoying issue? Honestly it's far better to do the standard provide filtering. We have to do a lot of this for Perl modules, and it's pretty simple. There's info on how to do this at http://fedoraproject.org/wiki/Packaging/Perl. Sorry for being inactive on this review for so long; it seems to have slipped my mind. I'll try to churn out a spec patch and a full review later today. A large number of comments were lost in the crash. I don't want to miss something by attempting to summarize them, so I'll add them back in verbatim. Sorry for the spam. ------- Additional Comments From tibbs.edu 2006-06-10 00:17 EST ------- Well, that took longer than I anticipated. I found that the requires filtering in the spec wasn't working at all. Plus, it looks like it was attempting to filter out more than necessary. I found that if I just filter out the perl(globals.pl) bit, everything looks sane. Also, the .cvsignore files are still being packaged; the statement in %build to delete them is looking for directories named .cvsignore, not files. Finally, this needs to live under /usr/share, not /var/www, which is a simple edit of the first line plus the httpd.conf file. I'll attach the specfile I used to build; with this there are only the two "no-documentation" warnings from rpmlint can be ignored. Care to look it over and push a fresh package for a final review? Note that I no longer have the attachment mentioned in the previous comment, so I cannot upload it again. ------- Additional Comments From mkanat 2006-06-10 00:58 EST ------- (In reply to comment #24) > Finally, this needs to live under /usr/share, not /var/www, which is a simple > edit of the first line plus the httpd.conf file. Will SELinux actually allow Apache to execute CGIs that live in /usr/share? Particularly ones that need to write to directories? ------- Additional Comments From tibbs.edu 2006-06-10 01:22 EST ------- I suggest you peruse the gallery2 review, which deals with this issue: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=181599 My understanding is that selinux will allow apache to run things out of /usr/share as long as the appropriate selinux boolean is set. And if it didn't, we'd need to work with the selinux folks to get the necessary bits in. It's simply not permissible for us to put things in /var/www. Now, what does bugzilla need to write? I thought it was entirely database driven. Obviously it can't write to /usr/share, so we have to use another location. The gallery2 package uses a directory under /srv for this, but it does take selinux bits. The bug for that is https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=183140 ------- Additional Comments From mkanat 2006-06-10 01:37 EST ------- It has to write to the data/ directory and various subdirectories under that. The data directory is created by checksetup.pl. You can change where Bugzilla expects the data directory to be by editing the $datadir variable in Bugzilla/Config.pm. Why can't we put something in /var/www? It's where we normally put Bugzilla. Also note that Bugzilla requires *either* DBD::Pg or DBD::mysql, but it doesn't need both. I'm not sure how to handle that in RPM. The automatic deps will probably pick up both. ------- Additional Comments From tibbs.edu 2006-06-10 11:37 EST ------- (In reply to comment #28) > Why can't we put something in /var/www? It's where we normally put Bugzilla. The naive answer is because the packaging guidelines indicate that it's not the proper place; see the end of http://fedoraproject.org/wiki/Packaging/Guidelines. The point is that once this is in Extras it's essentially a system component, and the system shouldn't install important pieces of itself into /var. > Also note that Bugzilla requires *either* DBD::Pg or DBD::mysql, but it doesn't > need both. I'm not sure how to handle that in RPM. The automatic deps will > probably pick up both. RPM has no way to indicate this kind of either-or requirement; it's probably simpler to just install both unless we can somehow make two subpackages, bugzilla-postgres and bugzilla-mysql that provide bugzilla-db and pull in the necessary Perl modules for each specific database. I doubt it's worth it. ------- Additional Comments From mkanat 2006-06-10 19:00 EST ------- (In reply to comment #29) > RPM has no way to indicate this kind of either-or requirement; it's probably > simpler to just install both unless we can somehow make two subpackages, > bugzilla-postgres and bugzilla-mysql that provide bugzilla-db and pull in the > necessary Perl modules for each specific database. I doubt it's worth it. Okay. The problem is that those perl modules also pull in the databases themselves. So installing Bugzilla will now always install both postgresql and mysql. ------- Additional Comments From tibbs.edu 2006-06-10 19:18 EST ------- Surely it only pulls in the client libraries? I can see no evidence that it will actually pull in the database servers; that would be nuts. Admittedly the mysql client libraries are a bit large (5MB) but that could be seen as a packaging bug since it contains the client command line interface as well. (perl-DBD-MySQL just wants libmysqlclient.) The Postgres client libs are only 500K. In any case, I'm not sure it would be acceptable to filter the perl-DBD dependencies and require that the end user know that they need to install one or the other. I guess it depends on whether or not then can be warned at setup time; if that's possible then it would be reasonable to do so. This isn't exactly and install-and-go package so I think it's acceptable to have them go back and pull in another package. ------- Additional Comments From mkanat 2006-06-10 19:22 EST ------- (In reply to comment #31) > This isn't > exactly and install-and-go package so I think it's acceptable to have them go > back and pull in another package. If they only pull in the client libraries, it's okay to have both of the perl modules be installed. It's true that running ./checksetup.pl will tell the user that they need to install the correct module, but it will give them CPAN instructions, and I think we'd prefer that they use RPMs. ------- Additional Comments From tibbs.edu 2006-06-10 20:44 EST ------- We shouldn't fear patching things like checksetup to give the appropriate instructions, but in this case I think just requiring both is going to be our best bet. So what's left? We need to pick a location for the data directory; /srv/bugzilla makes the most sense to me. I need to get with the selinux gurus to see what's required there. ------- Additional Comments From mkanat 2006-06-10 23:34 EST ------- I would think that /var/bugzilla would make more sense, since that seems to fit the purpose of /var, yes? ------- Additional Comments From tibbs.edu 2006-06-10 23:48 EST ------- Well, that's a reasonable question. To address /var/bugzilla specifically, FHS forbids it: Applications must generally not add directories to the top level of /var. Such directories should only be added if they have some system-wide implication, and in consultation with the FHS mailing list. If it had to be in /var, /var/lib/bugzilla would be better. Here's what FHS has to say about /var and /srv: /var contains variable data files. This includes spool directories and files, administrative and logging data, and transient and temporary files. /srv contains site-specific data which is served by this system. Seems to me like /srv is more appropriate. And in any case I'm just following gallery2's lead, which was packaged by the same person (John Berninger). I assume he'll chime in with why he chose that location and what he'd like to do about this package. ------- Additional Comments From mkanat 2006-06-11 00:01 EST ------- Okay. Very little of the data in data/ is technically "served" to the user--just certain graphics generated by graphviz. So /var/lib/bugzilla makes sense to me, but it's up to you guys in the end. ------- Additional Comments From tibbs.edu 2006-06-11 00:18 EST ------- Ah, I had the impression that it held data such as uploaded attachments. If it's just transient data, why doesn't it just put it in /tmp? ------- Additional Comments From mkanat 2006-06-11 00:29 EST ------- It's definitely not transient data. Bugzilla's per-server operating parameters are stored there, but they're written by a CGI. It also stores pre-compiled templates. It does store a few attachments, but most attachments are stored in the database (depending on user settings). Basically, it stores any file that the web server needs to write to or anything that Bugzilla generates dynamically. ------- Additional Comments From nkadel 2006-06-11 00:34 EST ------- If I may suggest, please don't stuff bugzilla in /srv: very few programs actually use it or /opt these days, and /var is an old and well-recognized place for data that gets a lot of disk access, and where the files experience churn and thus need frequent backup. /var/lib/bugzilla makes a certain amount of sense if /var/bugzilla is not easily negotiated for. ------- Additional Comments From paul 2006-06-12 07:52 EST ------- My vote would be for the app to go in /usr/share and the data to be in /var/lib/bugzilla (via a symlink if necessary). The only SELinux fiddling that should be necessary would I think to be to have /var/lib/bugzilla(/.*)? as httpd_var_lib_t. ------- Additional Comments From tibbs.edu 2006-06-12 12:40 EST ------- Now that I finally understand what bugzilla needs to write, I agree that /var/lib/bugzilla is indeed the proper place. I'm rolling a new package now which installs into /var/lib/bugzilla ; sorry for the delay in response, I've been on vacation for a while and am still getting caught up. OK, new packages are up: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.22-1.src.rpm Looks like there's been some miscommunication. The new packages put everything into /var/lib/bugzilla; the point was that the bulk of the package needs to be installed in /usr/share/bugzilla and the data directory needs to be /var/lib/bugzilla. As far as I can determine, this package can't be split uplike that. In looking at this, it seems that the directory where BZ writes stuff is determined (in part) by the $libdir variable in Bugzilla/Config.pm - which has a value of '.'. Normally, I would simply try changing this value, but there is a note just above that that entry that says changing $libdir will cause breakage. There's also a note that "it is not yet possible to mvoe the .pms elsewhere", which I assume means the Bugzilla/*.pm files must be in the same directory as BZ is writing stuff to. Would anyone from upstream care to comment on this? If I'm incorrect, I'd love to find out so I can finish packaging this up... Right. You can't change where the .pms are very easily. But you don't need to change that. You want to change $datadir to /var/lib/bugzilla/data and that's it. OK, that was easier than I expected. Thanks, Max. New packages: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.22-2.src.rpm you need to fix the http conf file to point to /usr/share/bugzilla and it would be nice to add a README.fedora file in the documentation stating what is needed for the user to do to geta functional bugzilla setup ie cd /usr/share/bugzilla and then run ./checksetup.pl after setting up a db and checking the settings in localconfig Your post scriptlet runs checksetup.pl, which outputs a *lot* of stuff. You might want to make it output to /dev/null, since this is an RPM. Also, why do you create the fake localconfig file? Is that necessary? There are a few optional "requires" that Bugzilla also needs: graphviz (for generating dependency graphs) and patchutils (for interdiff). Bugzilla also requires specific minimum versions of various CPAN modules, and the versions are specified in checksetup.pl. Will the automatic "requires" gathering figure that out? (In reply to comment #33) > Bugzilla also requires specific minimum versions of various CPAN modules, and > the versions are specified in checksetup.pl. Will the automatic "requires" > gathering figure that out? There's an easy way to find out - use: $ rpm -qp --requires bugzilla-xxx.noarch.rpm Max: The fake localconfig is created so that it can be included in the payload as a config file. If it isn't created until %post, then the RPMdb doesn't know about it and it could potentially get whacked during a package upgrade. The automatic requires gathering doesn't seem to include perl module versions, just presence or absence. That may be a blocker, in which case requires filtering will start getting ugly... New packages: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.22-3.src.rpm I thought rpm would pull versioned dependencies out of a "use Module VERSION" statement. It won't find them any other way as far as I can tell. In the case of this package, all of the module dependencies are unversioned. If the necessary versions were provided in base FC4 or the oldest version ever in extras then we're OK. Otherwise then there is the possibility that someone who has never done any updates might have an old module installed, so we'd have to include versioned dependency information. If we can get away with doing so for a couple of modules then it might not be too bad. But if not, it's probably easier to just turn off RPM's dependency generation altogether and just supply a list. It shouldn't be too hard to hack checksetup to dump a dependency list in the proper format and then just call it in place of the regular perl dependency generator. One additional thing bothers me. rpmlint has this complaint: E: bugzilla file-in-usr-marked-as-conffile /usr/share/bugzilla/localconfig We cannot assume that anything under /usr/share is writable except at install time, so it's really not the best place to put a config file. I looked through the source and it does look like it expects the configuration to live in $libdir. However, this seeps to be specified in only one location, Bugzilla::Config.pm, so it should be possible to patch two lines and get it to look in /etc/bugzilla instead. Do you think this is feasible? the README file is for gallery not bugzilla I just noticed that myself. As to the config file, I'll move that to /etc/bugzilla and symlink to it. I'll re-roll new packages as soon as I can this afternoon, I've been handed some other high priority items for today. The reason I suggested patching is because of the multi-project configuration thing which uses localconfig.$project for the name of the config file. Just having one symlinked configuration doesn't seem like it would work in general. Bugzilla tarballs never themselves include localconfig. Also, the RPM should never include it. Thus, it should never be overwritten in an upgrade. The best way to move it is just to edit the $localconfig variable in Bugzilla::Config. Don't change $libdir--that doesn't work. As far as the dependencies, it should be pretty easy, as you said, to get checksetup or some variant of it to print out the requirements in the format you need. If you search for "my $modules" and any calls to "have_vers" in checksetup.pl you can see where it stores the requirements. In the CVS version of Bugzilla this will be even easier since the requirements will mostly be in a separate .pm file, and you can write a script that just uses that module to print out the info. I think all the minimums included with FC4 are probably fine. I think we don't require anything too recent, for 2.22. My idea was just to edit the setting of $localconfig in Bugzilla/Config.pm lines 64 and 67. This file already gets patched in bugzilla-data-dir.patch so I doubt it would be much of a problem. Honestly I think that's the last thing holding this up. Well, that and the readme file talking about Gallery. Removed localconfig file generation / packaging from specfile per Max K-A, patched Config.pm to put localconfig in /etc/bugzilla, and made sure I had the right readme file this time. New packages: SPEC: http://www.berningeronline.net/bugzilla.spec SRPM: http://www.berningeronline.net/bugzilla-2.22-4.src.rpm OK, everything's looking good. But I did turn up a few issues wieh working through my checklist. First off, the spec says GPL but it doesn't look like it to me. The first file I checked clearly says "Mozilla Public License" and the FAQ I found with a quick search seems to back this up. So s/GPL/MPL/. In addition, there doesn't seem to be an actual copy of the license included; upstream should be bugged about this but I believe they're listening. Also, there are files like README and QUICKSTART which are marked as %doc but which get installed under /usr/share/bugzilla instead of /usr/share/doc/bugzilla-2.22 as I'd expect. I think that when you mark packages as %doc that are already installed they don't get moved under docdir. I don't think this is a blocker but you might want to have a look. Finally, I'm pretty sure there will be selinux problems with this package. Unfortunately I have no real way to test it at the moment, and I don't think selinux issues are blockers right now. But I do suggest you work with the selinux folks and possibly Paul Howarth, who has been doing a bunch of work on packaging selinux rules. rpmlint says: W: bugzilla-contrib no-documentation W: bugzilla-doc no-documentation both of which are fine. Since the only required change is G->M in the License: tag, I'll go ahead and approve this so we can put it to bed. Review: * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. X license field matches the actual license. * license is open source-compatible. License text not included upstream. * source files match upstream: bbf2f1ec5607978d39855df104231973 bugzilla-2.22.tar.gz * latest version is being packaged. * BuildRequires are proper. * package builds in mock (development, x86_64). O rpmlint is silent. * final provides and requires are sane (see full list at end) * no shared libraries are present. * package is not relocatable. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * %clean is present. * %check is not present; no test suite upstream. * scriptlets are present and OK (%post script to generate default setup) * code, not content. * large documentation is in a -doc subpackage * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no libtool .la droppings. * not a GUI app. Full dependency list: bugzilla-2.22-4.fc6.noarch.rpm config(bugzilla) = 2.22-4.fc6 perl(Bugzilla) perl(Bugzilla::Attachment) perl(Bugzilla::Auth) perl(Bugzilla::Auth::Login::WWW) perl(Bugzilla::Auth::Login::WWW::CGI) perl(Bugzilla::Auth::Login::WWW::CGI::Cookie) perl(Bugzilla::Auth::Login::WWW::Env) perl(Bugzilla::Auth::Verify::DB) perl(Bugzilla::Auth::Verify::LDAP) perl(Bugzilla::Bug) perl(Bugzilla::BugMail) perl(Bugzilla::CGI) perl(Bugzilla::Chart) perl(Bugzilla::Classification) perl(Bugzilla::Component) perl(Bugzilla::Config) perl(Bugzilla::Config::Admin) perl(Bugzilla::Config::Attachment) perl(Bugzilla::Config::Auth) perl(Bugzilla::Config::BugChange) perl(Bugzilla::Config::BugFields) perl(Bugzilla::Config::BugMove) perl(Bugzilla::Config::Common) perl(Bugzilla::Config::Core) perl(Bugzilla::Config::DependencyGraph) perl(Bugzilla::Config::GroupSecurity) perl(Bugzilla::Config::L10n) perl(Bugzilla::Config::LDAP) perl(Bugzilla::Config::MTA) perl(Bugzilla::Config::PatchViewer) perl(Bugzilla::Config::Query) perl(Bugzilla::Config::ShadowDB) perl(Bugzilla::Config::UserMatch) perl(Bugzilla::Constants) perl(Bugzilla::DB) perl(Bugzilla::DB::Mysql) perl(Bugzilla::DB::Pg) perl(Bugzilla::DB::Schema) perl(Bugzilla::DB::Schema::Mysql) perl(Bugzilla::DB::Schema::Pg) perl(Bugzilla::Error) perl(Bugzilla::Field) perl(Bugzilla::Flag) perl(Bugzilla::FlagType) perl(Bugzilla::Group) perl(Bugzilla::Milestone) perl(Bugzilla::Product) perl(Bugzilla::Search) perl(Bugzilla::Search::Quicksearch) perl(Bugzilla::Series) perl(Bugzilla::Template) perl(Bugzilla::Template::Plugin::Bugzilla) perl(Bugzilla::Template::Plugin::Hook) perl(Bugzilla::Template::Plugin::User) perl(Bugzilla::Token) perl(Bugzilla::User) perl(Bugzilla::User::Setting) perl(Bugzilla::Util) perl(Bugzilla::Version) perl(Support::Files) perl(Support::Systemexec) perl(Support::Templates) bugzilla = 2.22-4.fc6 = /bin/sh /usr/bin/perl config(bugzilla) = 2.22-4.fc6 graphviz patchutils perl(AnyDBM_File) perl(Bugzilla) perl(Bugzilla::Attachment) perl(Bugzilla::Auth) perl(Bugzilla::Auth::Login::WWW) perl(Bugzilla::Bug) perl(Bugzilla::BugMail) perl(Bugzilla::CGI) perl(Bugzilla::Chart) perl(Bugzilla::Classification) perl(Bugzilla::Component) perl(Bugzilla::Config) perl(Bugzilla::Config::Common) perl(Bugzilla::Constants) perl(Bugzilla::DB) perl(Bugzilla::DB::Schema) perl(Bugzilla::Error) perl(Bugzilla::Field) perl(Bugzilla::Flag) perl(Bugzilla::FlagType) perl(Bugzilla::Group) perl(Bugzilla::Milestone) perl(Bugzilla::Product) perl(Bugzilla::Search) perl(Bugzilla::Search::Quicksearch) perl(Bugzilla::Series) perl(Bugzilla::Template) perl(Bugzilla::Token) perl(Bugzilla::User) perl(Bugzilla::User::Setting) perl(Bugzilla::Util) perl(Bugzilla::Version) perl(CGI) perl(CGI::Carp) perl(DBD::Pg) perl(DBI) perl(Data::Dumper) perl(Date::Format) perl(Date::Parse) perl(Errno) perl(Exporter) perl(Fcntl) perl(File::Basename) perl(File::Find) perl(File::Spec) perl(File::Temp) perl(Getopt::Long) perl(IO::Handle) perl(MIME::Base64) perl(MIME::Parser) perl(MIME::QuotedPrint) perl(Mail::Address) perl(Mail::Header) perl(Mail::Mailer) perl(Net::LDAP) perl(Pod::Usage) perl(Safe) perl(Socket) perl(Storable) perl(Support::Files) perl(Template::Stash) perl(Test::Harness) perl(Text::Wrap) perl(XML::Twig) perl(base) perl(constant) perl(diagnostics) perl(lib) perl(strict) perl(vars) smtpdaemon webserver bugzilla-contrib-2.22-4.fc6.noarch.rpm bugzilla-contrib = 2.22-4.fc6 = /bin/sh /usr/bin/env /usr/bin/perl /usr/bin/python /usr/bin/ruby perl(Bugzilla) perl(Bugzilla::BugMail) perl(Bugzilla::Config) perl(Bugzilla::Constants) perl(Bugzilla::DB) perl(Bugzilla::User) perl(Bugzilla::Util) perl(BugzillaEmail) perl(Getopt::Long) perl(MIME::Parser) perl(Net::LDAP) perl(Pod::Usage) perl(constant) perl(lib) perl(strict) bugzilla-doc-2.22-4.fc6.noarch.rpm bugzilla-doc = 2.22-4.fc6 = /usr/bin/perl perl(File::Basename) perl(diagnostics) perl(strict) APPROVED Changes made, imported, sync requested. (In reply to comment #43) > Finally, I'm pretty sure there will be selinux problems with this package. > Unfortunately I have no real way to test it at the moment, and I don't think > selinux issues are blockers right now. But I do suggest you work with the > selinux folks and possibly Paul Howarth, who has been doing a bunch of work on > packaging selinux rules. If anyone would like to test bugzilla with FC5 (or later) SELinux, I've spent half an hour knocking together a first pass at a policy. I'll attach the relevant files (bugzilla.{if,fc,te}) shortly. Instructions: Enable CGI scripts: # setsebool -P httpd_enable_cgi 1 Create an SELinux module-build environment: # yum install checkpolicy # cd /root # mkdir selinux.local # cd selinux.local # chcon -R -t usr_t . # ln -s /usr/share/selinux/devel/Makefile . Download the bugzilla policy source files to the /root/selinux.local directory and run "make" in that directory, which should create a policy module package, bugzilla.pp Load the policy module: # semodule -i bugzilla.pp Fix up directory contexts: # restorecon -rv /var/lib/bugzilla /usr/share/bugzilla Then give it a try and please report back any AVCs you get, and how you triggered them. Created attachment 131555 [details]
SELinux file contexts for bugzilla
Created attachment 131556 [details]
SELinux interface definitions for bugzilla
Created attachment 131557 [details]
SELinux policy rules for bugzilla
The attachments in comments 46 to 48 should be named as follows: Comment #46: bugzilla.fc Comment #47: bugzilla.if Comment #48: bugzilla.te See Comment #45 for how to use them. Pretty neat; thanks. A couple of questions: Have things been worked out so that these declarations can go into the bugzilla package, or do they still have to be pushed into the core policy? Is anything special needed for the config files under /etc/bugzilla? Will Postres support require additional rules, or do the mysql rules cover it? (Note that I write "rules" because I don't know the proper terms for the various selinux bits.) Hmmm. Looks like two of the scripts in the contrib directory have "use BugzillaEmail" statements which make it into the dependency list and which, it seems, I didn't notice. Since nothing in the main package provides this, the package is uninstallable. Anyone know what these scripts actually want, or if there's any chance at all that they'd actually run? Should they just be removed or should this dependency just be filtered out? (In reply to comment #43) > Full dependency list: > ... > bugzilla-contrib-2.22-4.fc6.noarch.rpm ... > perl(BugzillaEmail) Nothing is providing this. Sorry about Comment #52, which duplicates Comment #51; got a mid-air collision and had already re-pasted the comment and hit "save changes" before I notuced. (In reply to comment #50) > Pretty neat; thanks. A couple of questions: > > Have things been worked out so that these declarations can go into the bugzilla > package, or do they still have to be pushed into the core policy? As it stands they could go into the bugzilla package, but I'd rather a few people tested it and then get it into the Core policy. > Is anything special needed for the config files under /etc/bugzilla? I tested it by following the instructions in the README.Fedora and running my mysql on the same host. Nothing special was needed for /etc/bugzilla/localconfig > Will Postres support require additional rules, or do the mysql rules cover it? I expect it will need extra rules. If anyone wants to give it a try, try adding this to bugzilla.te: optional_policy(` postgresql_stream_connect(httpd_bugzilla_script_t) ') > (Note that I write "rules" because I don't know the proper terms for the various > selinux bits.) "Rules" is fine, at least for the .te file :-) (In reply to comment #51) > Hmmm. Looks like two of the scripts in the contrib directory have "use > BugzillaEmail" statements which make it into the dependency list and which, it > seems, I didn't notice. Since nothing in the main package provides this, the > package is uninstallable. This is actually inside the contrib package itself. There should be a BugzillaEmail.pm somewhere inside of the contrib package. You could just filter out the dependency requirement. Requires:perl(BugzillaEmail) has been filtered out in CVS. Should this review request remain open until the SELinux issue is resolved, or should that be moved to a separate bugzilla? (In reply to comment #55) > Requires:perl(BugzillaEmail) has been filtered out in CVS. Should this review > request remain open until the SELinux issue is resolved, or should that be moved > to a separate bugzilla? Is anyone here actually going to test it with SELinux? If so, it's probably easier just to keep going here as it should be possible to resolve it quite quickly. If not, it may as well be closed and you can wait for an SELinux user to raise a bug... Since no one has spoken up about testing with SELinux and the package shipped in Extras 4/5/devel several days ago, I'm going to close this review request and just wait for another bug to show up (if one should show up, that is) on SELinux issues. Anecdotal report of success using the SELinux policy as described in Comment #45: https://www.redhat.com/archives/fedora-selinux-list/2007-January/msg00021.html Package Change Request ====================== Package Name: bugzilla New Branches: EL-4 EL-5 Received private email asking for EPEL bugzila packages. |