Bug 203864
Summary: | Review Request: tripwire - IDS | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Brandon Holbrook <fedora> |
Component: | Package Review | Assignee: | Dominik 'Rathann' Mierzejewski <dominik> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | joost.soeterbroek, kyrian, mgml, zing |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-12-22 15:19:12 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
Brandon Holbrook
2006-08-24 04:05:34 UTC
I also requested access to his SVN repository (he houses the code outside of sourceforge), or if there's even anything in there worth having, so we could possibly pacakge newer SVN code without an official release. Only 1 rpmline error: $ rpmlint tripwire-2.4.0.1-1.fc6.2.i386.rpm E: tripwire executable-marked-as-config-file /etc/cron.daily/tripwire-check BYI, this also builds fine on x86_64. The compiler warnings are almost exclusively "warning: 'class blah' has virtual functions but non-virtual destructor." I don't see anything about incompatible sizes or the like. No %check, though, and I'm not sure how I'd go about doing a quick test. Major issue - Tripwire initialisation fails, missing twinstall.sh file: According to /etc/cron.daily/tripwire-check (and quickstart guide) you need to: **** Run "/etc/tripwire/twinstall.sh" and/or "tripwire --init". **** - /etc/tripwire/twinstall.sh file does not exist - $ tripwire --init ### Error: File could not be opened. ### Filename: /etc/tripwire/tw.cfg ### No such file or directory ### Configuration file could not be read. ### Exiting... Minor issue - version mismatch documentation: Quickstart Guide /usr/share/doc/tripwire-2.4.0.1/quickstart.txt refers multiple times to Tripwire v2.3.
> Minor issue - version mismatch documentation:
>
> Quickstart Guide /usr/share/doc/tripwire-2.4.0.1/quickstart.txt refers multiple
> times to Tripwire v2.3.
Man pages also refer to Tripwire 2.3.1
(In reply to comment #4) > Major issue - Tripwire initialisation fails, missing twinstall.sh file: > > According to /etc/cron.daily/tripwire-check (and quickstart guide) you need to: > > **** Run "/etc/tripwire/twinstall.sh" and/or "tripwire --init". **** Ok, I missed '/usr/share/doc/tripwire-2.4.0.1/README.RPM' which states you should run /usr/sbin/tripwire-setup-keyfiles after RPM install (in stead of /etc/tripwire/twinstall.sh). Maybe this information should be included in file /etc/cron.daily/tripwire-check as well, because it could lead to confusion with users during initialisation. The recipie used here is about the same as I came up with to create an RPM that built with GCC4, using the patch detailed under the SF.net project for Tripwire. It is possible to get prior versions to compile using backward-compatible libstdc++ and gcc-c++ packages, but I don't think that should be a concern any more. The version and documentation changes required should be basically trivial to do. I have no idea what rpmlint does, but I'd be willing to find out, and I think we should be sure to include the 'COMMERCIAL' and 'TRADEMARK' documents from the original source tarball. As per threads on Fedora Extras list, and in the IRC meeting, I would be happy to step forward as formal maintainer of a Tripwire package for FC5 and onwards. In theory at least, I should be able to do this for i386 and x86_64 as I have machines of both architectures, but I am hesitant about the latter since I am actively trying NOT to use x86_64 native stuff on it because it breaks LOADS of Desktop stuff on Fedora at this time. Spec URL: http://theholbrooks.org/RPMS/tripwire.spec SRPM URL: http://theholbrooks.org/RPMS/tripwire-2.4.0.1-1.4.src.rpm Thanks for the feedback all, I have updated all the external documentation to reflect version 2.4, but not the internal text files (quickstart.txt) or man pages. I've submitted a bug to upstream asking them to correct the documentation in the next release. In the mean time, is it Fedora's (and thus ours as the packagers) responsibility to fix silly things like forgetting to bump version numbers in your documentation? Obviously some quick command-line perl will do the job nicely in %pre... what is everyone's feeling on the subject? I have removed the %config flag from the cron script... I'm not entire sure why it was there to begin with. rpmlint output is now nonexistent :) Also, I now echo the contents of README.RPM upon package install to let users know about the next step required to initialize their tripwire database. I also now include the COMMERCIAL file as a %doc (TRADEMARK was already there) Kyrian, I would love to have you join this package as a co-maintainer once this gets approved for FC5/6, especially as an x86_64 sanity check and another pair of eyes at maintaining this rarely-updated code. Are there any other differences between this approach and the RPM that you created? rpmlint is a quick rpm 'sanity check' utility that scans through source as well as binary RPM files for common mistakes and guideline violations. It is usually the first tool run by package reviewers to make sure there isn't anything majorly wrong with a submitted package before diving in and running checks manually. You can 'yum update rpmlint' to get it, and then run it against any RPM. The output is usually self-explanatory ('E'rrors and 'W'arnings), and SOME output can be justified on a per-package basis, but no complaints from rpmlint is usually desired. (In reply to comment #8) [...] > I've submitted a bug to upstream asking them to correct the > documentation in the next release. In the mean time, is it Fedora's (and thus > ours as the packagers) responsibility to fix silly things like forgetting to > bump version numbers in your documentation? Obviously some quick command-line > perl will do the job nicely in %pre... what is everyone's feeling on the > subject? I think a packager is not obliged but he should at least poke the upstream about that. Of course sending upstream a patch is the best solution. > Also, I now echo the contents of README.RPM upon package install to let users > know about the next step required to initialize their tripwire database. I don't think this is the best idea, but, as long as it's non-interactive, I don't mind. I'll take this review, because I'm interested in keeping this package in Fedora. I still use it on some servers. please don't spam the rpm installation with a cat of the README.RPM file. I'd say a note in the %description pointing the user to the /path/to/README.RPM is a fine place for that. At most, _maybe_ a one line echo of the /path/to/README.RPM... and even that doesn't really sit to well with me... IMHO! (?)1. package meets naming and packaging guidelines. Release: 1%{?dist}.4 This should be: Release: 4%{?dist} 2. specfile is properly named, is cleanly written and uses macros consistently. 3. dist tag is present. 4. build root is correct. %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 5. license field matches the actual license. 6. license is open source-compatible (GPL). License text included in package. 7. source files match upstream: b371f79ac23cacc9ad40b1da76b4a0c4 tripwire-2.4.0.1-src.tar.bz2 8. latest version is being packaged. 9. BuildRequires are proper. (?)10. package builds in mock ( ). 11. rpmlint is silent. (?)12. final provides and requires are sane: config(tripwire) = 2.4.0.1-1.4 tripwire = 2.4.0.1-1.4 = config(tripwire) = 2.4.0.1-1.4 gawk grep gzip libcrypto.so.6()(64bit) libgcc_s.so.1()(64bit) libm.so.6()(64bit) libstdc++.so.6()(64bit) sed tar What is Requires: sed grep gzip tar gawk needed for? 13. no shared libraries are present. 14. package is not relocatable. 15. owns the directories it creates. 16. doesn't own any directories it shouldn't. 17. no duplicates in %files. (?)18. file permissions are appropriate. %attr(0755,root,root) %dir %{_sysconfdir}/tripwire possibly, this should be 0700 19. %clean is present. 20. %check is not present (?)21. scriptlets are ok Requires(post): sed is missing. Also, don't print anything in %post. 22. code, not content. 23. documentation is small, so no -docs subpackage is necessary. 24. %docs are not necessary for the proper functioning of the package. 25. no headers. 26. no pkgconfig files. 27. no libtool .la droppings. 28. not a GUI app. 29. not a web app. Please take care of 1, 12, 18 and 21 while I build it in mock. *ping* I'm here... I've just been putting my time into other FE pursuits. I'll try to address these issues tonight. Spec URL: http://theholbrooks.org/RPMS/tripwire.spec SRPM URL: http://theholbrooks.org/RPMS/tripwire-2.4.0.1-2.src.rpm I've addressed the issues you laid out above. Thanks for the thorough review! Builds fine in devel/x86_64 mock. Some more issues: BuildRequires: autoconf seems to be unnecessary. Was there any reason to have it? Could you remove that: # Print getting started help message if [ $1 -eq 1 ]; then %{__cat} %_docdir/%{name}-%{version}/README.RPM fi from %post and (optionally) rename README.RPM to README.Fedora? Additionally, you can change %defattr to read: %defattr(644,root,root,755) and remove %attr(644,root,root) from some files below. I'm also unsure if %dir %{_var}/lib/tripwire/report should be readable by everyone. Or maybe %{_var}/lib/tripwire, too. After all, I think only root will be using tripwire, as it needs access to all files. *ping* Spec URL: http://theholbrooks.org/RPMS/tripwire.spec SRPM URL: http://theholbrooks.org/RPMS/tripwire-2.4.0.1-3.src.rpm Removed BR: autoconf Renamed README.RPM to README.Fedora instead of cat'ing the README file during install, just a one-liner pointing users to read the file themselves. chmod'ed /var/lib/tripwire and /var/lib/tripwire/report to 0700 Please remove that as well: # Print getting started help message if [ $1 -eq 1 ]; then echo To configure tripwire, read: %_docdir/%{name}-%{version}/README.Fedora fi I am of the opinion that any rpm console output during installation should be an indication of either an error or a warning and other packagers agree with me. Otherwise looks fine. It is APPROVED provided the above is done. I've removed all output from %post and imported this package for devel/ and requested a build. You're welcome to check it out and see for yourself, and move this bug to block FE-ACCEPT. Thanks for all your time! Thank you, too. Good work. Remember to close this as RESOLVED NEXTRELEASE when it builds. Build successfully in devel and FC6 branch requested Please don't close before owners/owners.list in CVS has been updated. Package is still listed as orphaned. |