[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: new package for review: logjam



On Thu, 10 Feb 2005 10:45:18 +0100, Matthias Saou wrote:

> Tom 'spot' Callaway wrote :
> 
> > On Wed, 2005-02-09 at 20:08 -1000, Warren Togami wrote:
> > > Tom 'spot' Callaway wrote:
> > > > On Wed, 2005-02-09 at 19:54 -1000, Warren Togami wrote:
> > > > 
> > > > 
> > > >>Consider adding "%_smp_mflags  -j3" to your .rpmmacros, which
> sometimes 
> > > >>exposes Makefile races even on uniprocessor build machines.  Everyone
> 
> > > >>that does packaging should do this, in order to avoid a surprise
> failure 
> > > >>on a SMP build system.
> > 
> > Sneaking back on topic, with -j3, it still builds for me. I suspect the
> > missing gettext in the buildroot was what caused the translation error.
> 
> Yup. And Michael pointed that out in his first review mail : gettext-devel
> is most certainly the guilty one, and you'll need to add it to
> BuildRequires (as he wrote). Note that for RH/RHEL or FC < 3, it's
> "gettext" and not "gettext-devel" since the split happened very recently.

Yes, and it is really just needed for building. The added explicit
dependency on

  gettext

is not correct, and the other dependencies on

  gtk2, xmms, gtkhtml3, librsvg2

are redundant, not needed, and would only require unnecessary rebuilds
if a library were moved into a package with a different name.
'rpm -qR logjam' shows the automatic dependencies on the libraries
already.


Also, Tom mentioned "requested cleanups" in his cvs commit. The
following added line is not cleanup, but bad habit, in particular
since a good buildroot is set at the top of the spec file already.
It should simple become 'rm -rf $RPM_BUILD_ROOT' here:

 %install
+[ -n "$RPM_BUILD_ROOT" -a "$RPM_BUILD_ROOT" != / ] && rm -rf $RPM_BUILD_ROOT


Rest of the packaging appears good and not unusual. As I don't run a blog,
I won't comment on the run-time behaviour.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]