Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 203864 - Review Request: tripwire - IDS
Summary: Review Request: tripwire - IDS
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-08-24 04:05 UTC by Brandon Holbrook
Modified: 2007-11-30 22:11 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-12-22 15:19:12 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Brandon Holbrook 2006-08-24 04:05:34 UTC
Spec URL: http://theholbrooks.org/RPMS/tripwire.spec
SRPM URL: http://theholbrooks.org/RPMS/tripwire-2.4.0.1-1.fc6.2.src.rpm

Description: tripwire has been orphaned for some time now, and neglected upstream for almost as long.  I talked with Warren a while back about taking over and have finally decided to do so.  However, upstream is still active on their forums and has been promising the next release since March.  Nonetheless, I have contacted upstream in hopes of spurring some activity, and hopefully a new release that is gcc4-compatible out of the box, but no replies as of yet.

This SRPM builds and runs on my FC5 i386 (has been running for the last 3 days without incident) as well as builds in mock FC6 i386, but I don't have access to x86_64 hardware to test builds.  The spec file _does_ specify ExclusiveArch ix86, but that is leftover from 2.3.0 and some posts in the forums vaguely indicate that 2.4.0 builds and runs fine on 64bit hardware.  I'd appreciate anybody willing to remove the ExclusiveArch and test building/executing on an x86_64 machine.

It looks ugly while it's building, throws LOTS of warnings, 95% of which are complaining about non-virtual dtor's.  I brought this up in my letter to upstream, but the binaries seem to run fine despite being narrowly compilable.

Comment 1 Brandon Holbrook 2006-08-24 04:11:38 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.

Comment 2 Joost Soeterbroek 2006-08-24 10:23:06 UTC
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






Comment 3 Jason Tibbitts 2006-08-25 00:13:52 UTC
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.


Comment 4 Joost Soeterbroek 2006-08-25 09:10:29 UTC
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. 

Comment 5 Joost Soeterbroek 2006-08-25 09:14:52 UTC
> 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


Comment 6 Joost Soeterbroek 2006-08-25 09:40:15 UTC
(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.


Comment 7 Kev 'Kyrian' Green 2006-08-25 18:29:19 UTC
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.

Comment 8 Brandon Holbrook 2006-08-26 05:48:00 UTC
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.

Comment 9 Dominik 'Rathann' Mierzejewski 2006-10-17 21:47:31 UTC
(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.


Comment 10 Zing 2006-10-19 15:08:50 UTC
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!

Comment 11 Dominik 'Rathann' Mierzejewski 2006-11-01 20:30:55 UTC
(?)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.

Comment 12 Dominik 'Rathann' Mierzejewski 2006-11-13 20:46:05 UTC
*ping*

Comment 13 Brandon Holbrook 2006-11-14 14:58:15 UTC
I'm here... I've just been putting my time into other FE pursuits.  I'll try to
address these issues tonight.

Comment 14 Brandon Holbrook 2006-11-16 04:55:21 UTC
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!

Comment 15 Dominik 'Rathann' Mierzejewski 2006-11-19 20:55:55 UTC
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.

Comment 16 Dominik 'Rathann' Mierzejewski 2006-12-18 20:37:40 UTC
*ping*

Comment 17 Brandon Holbrook 2006-12-20 05:14:38 UTC
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

Comment 18 Dominik 'Rathann' Mierzejewski 2006-12-21 22:04:02 UTC
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.

Comment 19 Brandon Holbrook 2006-12-22 05:02:31 UTC
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!

Comment 20 Dominik 'Rathann' Mierzejewski 2006-12-22 11:07:43 UTC
Thank you, too. Good work. Remember to close this as RESOLVED NEXTRELEASE when
it builds.

Comment 21 Brandon Holbrook 2006-12-22 15:19:12 UTC
Build successfully in devel and FC6 branch requested

Comment 22 Michael Schwendt 2007-01-11 17:48:34 UTC
Please don't close before  owners/owners.list  in CVS has been
updated. Package is still listed as orphaned.


Note You need to log in before you can comment on or make changes to this bug.