Bug 226800

Created attachment 147231 [details]
Build log from mock

Hi Tom.  I can't build this package from mock (or outside of it even).	I've
attached the build log.

Sorry about that.  It worked fine on my FC5 box, but I see now that
it fails on FC6.  I get a different error fwiw.
I will fix this soon.

Could you try this?

http://people.redhat.com/~tromey/bbdb-srpm/emacs-bbdb-2.35-2.src.rpm

This includes a makefile patch that I needed on FC6.

Thanks Tom.  This one builds for FC6 in mock.

rpmlint has a single complaint about this package...

# rpmlint emacs-bbdb-2.35-2.fc6.noarch.rpm
W: emacs-bbdb file-not-utf8 /usr/share/info/bbdb.info.gz

You can fix this with the following spec file patch...

--- emacs-bbdb.spec~    2007-02-03 12:06:44.000000000 -0800
+++ emacs-bbdb.spec     2007-02-05 16:35:51.000000000 -0800
@@ -34,6 +34,10 @@
 %prep
 %setup -q -n %{pkg}-%{version}
 %patch0 -p1
+pushd texinfo
+iconv --from=ISO-8859-1 --to=UTF-8 bbdb.texinfo > bbdb.texinfo.new
+mv bbdb.texinfo.new bbdb.texinfo
+popd

 %build



Actually, rpmlint has a "no documentation" complaint about the -el
package as well, but we can ignore it.

Thanks once again.  I applied this patch.
I saw this rpmlint error but somehow managed to ignore it :-/
I also saw the no documentation complaint but, I agree,
we can ignore it.  There's simply no docs to put in there.

I've uploaded the latest.

http://people.redhat.com/~tromey/bbdb-srpm/emacs-bbdb-2.35-3.src.rpm

One new little annoyance...

# rpmlint /usr/src/redhat/RPMS/noarch/emacs-bbdb-2.35-3.noarch.rpm 
W: emacs-bbdb no-version-in-last-changelog

I'll go through the full review list on your next update, but I don't believe I
can approve this until you've been sponsored for Fedora Extras.   I've blocked
this bugzilla on FE-NEEDSPONSOR for now.

*** Bug 227230 has been marked as a duplicate of this bug. ***

Hi Tom,

Interestingly I submitted this around the same time as you, so please look at
the spec file in bug 227230, as that contains a number of things I think you
should consider, including:

0) You're building and packaging *only* the bbdb bindings for VM, whereas bbdb
comes with bindings for Gnus and MH-E, both included with Emacs - you need to do
make all to build those as well.

1) BBDB requires TeX for the bbdb-print functionality to work, as bbdb-print
creates a TeX file, and so I included a Requires: tetex. I'm on the fence with
this one, I have to say.

2) Your package description doesn't mention that the BBDB is an add on for Emacs
- it should.

3) You should use $RPM_BUILD_ROOT *or* %{buildroot} in your spec consistently,
not a mixture of both.

4) You need to remove [ "$RPM_BUILD_ROOT" != "/" ] && from the %clean section

5) There are a number of shell scripts and extra bits which are very useful to
bbdb users - I packaged these as documentation, along with the ChangeLog with 
    %doc bits misc utils ChangeLog
in the %files section - I strongly urge you to do the same.

6) Assuming you do (5) above, you'll need to make the *.pl files non executeable
with a 
    find -name '*.pl' -exec chmod -x {} \;
in the %install section, and you'll also need to remove some irrelevant files:
    rm -f bits/make.bat
    rm -f utils/Makefile*

7) What was your reason for putting bbdb-autoloads in site-start.d ? If I recall
correctly, this is unecessary, all of the files can live happily in
site-lisp/bbdb (which is on the load-path by default). Perhaps I missed
something here though.

8) The make install-pkg target in the bbdb makefile is totally useless for our
purposes - for readability I'd really recommend not using that, but rather doing
a few simple copies into the buildroot. Example from my spec:
# There is no usable install make rule - install by hand.
%define lispdir %{_datadir}/emacs/site-lisp/bbdb
mkdir -p $RPM_BUILD_ROOT%{lispdir}
cp lisp/*.{elc,el} $RPM_BUILD_ROOT%{lispdir}

%define texdir %{_datadir}/texmf/tex/generic/bbdb
mkdir -p $RPM_BUILD_ROOT%{texdir}
cp tex/*.tex $RPM_BUILD_ROOT%{texdir}

mkdir -p $RPM_BUILD_ROOT%{_infodir}
cp texinfo/bbdb.info $RPM_BUILD_ROOT%{_infodir}

9) I think your BuildRequires for emacs-vm-el is unecessary, as although
warnings will pop up, I don't think they matter. I *may* be wrong here though. I
sort of hope so, since I recently added a patch to the emacs-vm package (the
vmrf patch) which ultimately will lead to a BuildRequires: emacs-bbdb, at which
point we'll have a circular dependency. 

I need to think more about point 9.

Also, if you want a co-maintainer, I'm happy to do that.

Great to see someone else interested in bbdb!

Hi Tom - you've been quiet a while on this, any progress?

Sorry, I've been on the road and I've just been catching up.
Why wouldn't we start with your spec file instead of mine?
You've clearly thought through the issues more completely.
Is there any particular thing mine did better?

About the buildreq issue -- I did not look to see whether VM
defines any macros that BBDB must use.  I think that would
be the only potential problem.

In re #7, I just copied some other package.  I don't think there
is any deeper thing than that.

Hi Tom, OK, when I get a chance, I'll confirm (or otherwise) on #7 and the
buildreq issue. One thing you spotted that I hadn't was the need to convert the
info file to UTF-8. Doesn't matter to me which spec we use as the starting point
though, but it's great to have a couple of us working on it!

OK, have put a "best of breeds" (mongrel?) spec file and src.rpm here:

SPEC: http://jgu.nonlogic.org/emacs-bbdb.spec
SRPM: http://jgu.nonlogic.org/emacs-bbdb-2.35-3.src.rpm

See what you think.

OK, I have been conversing with the upstream maintainer about bbdb and VM - he
is upstream maintainer of both. He has this to say regarding building of bbdb
and VM:

"At least BBDB requires VM at built time as it uses VM
macros.  If you built without VM sources then the ELCs
will contain functions calls where macros should be used
and thus it will fail, i.e. they are broken!

IMHO for VM it is the same just the other way around.

This is no problem if your distro is using binary packages
with precompiled ELCs.  Then the package maintainer needs to
care for correct compilation, but the user is not forces to
install both packages."

And so that means that there's a chicken and egg issue with building these
packages because bbdb needs a BuildRequires: emacs-vm-el and vm needs a
BuildRequires: emacs-bbdb-el.

I am unsure how to deal with that, I shall ask on the maintainers list.

I talked to Bill Nottingham on IRC about this issue. He thinks it is fine to
have both emacs-vm and emacs-bbdb built in the buildsystem without the circular
BuildRequires, and then once packages exist, add in the BuildRequires and
rebuild both packages. So, issue resolved.

People who bootstrap these packages on new archs or derivative distros would
probably appreciate a comment in the specfile(s) explaining the scenario.

SPEC: http://jgu.nonlogic.org/emacs-bbdb.spec
SRPM: http://jgu.nonlogic.org/emacs-bbdb-2.35-4.src.rpm

* Tue May 29 2007 Jonathan G. Underwood <jonathan.underwood> - 2.35-4
- Add BuildRequires: emacs-vm-el
- Add macro to determine emacsversion at package build time
- Add Requires emacs-common >= emacsversion
- Add notes about bootstrapping with VM to top of spec file

Hey, we have approved emacs guidelines now.  Finally.  It doesn't look like this
package quite follows the guidelines, but Jonathan wrote them so I suppose he'd
know better than I.  Any chance it could be fixed up?

Who's actually submitting this package, anyway?

I am happy to fix the package up in the next day or two (need sleep right now).
(In reply to comment #17)
> Hey, we have approved emacs guidelines now.  Finally.  It doesn't look like this
> package quite follows the guidelines, but Jonathan wrote them so I suppose he'd
> know better than I.  Any chance it could be fixed up?
> 

Happy to - will get to it tomorrow (need bed right now).

> Who's actually submitting this package, anyway?

Well, if Tom is happy to have me as a co-maintainer, I'd suggest we both submit
it - it requires some coordination with VM, as there is an element of
bootstrapping between the two packages required (I'm the emacs-vm packager, but
again, would welcome co-maintainers!).

Updated spec file:

http://jgu.fedorapeople.org/emacs-bbdb.spec

Hven't had chance to test it yet.

spec file: http://jgu.fedorapeople.org/emacs-bbdb.spec
SRPM: http://jgu.fedorapeople.org/emacs-bbdb-2.35-6.fc7.src.rpm

Hi.  Sorry I've been ignoring this once again :(
I think it would be fine for Jonathan to be the primary packager.

I looked at the newest spec file and I read through the Emacs
packaging draft.  Looking good!

Going back to the bbdb-autoloads.el question -- I think the reason that
this is in site-start.d is so that the autoloads will be evaluated at
Emacs startup (see site-lisp/site-start.el).  This means that the user
doesn't have to add an explicit require or anything to their .emacs to
start using BBDB.  I was wrong about this back in comment #10.  (FWIW
this is mentioned in the packaging guidelines.)

In my copy of the latest RPM, bbdb-autoloads.el ends up in site-lisp/bbdb/.
So, I think it should be moved again.

BTW ... BBDB and VM requiring each other seems pretty ugly!  Perhaps one or
the other could be fixed upstream.

(In reply to comment #21)
> Hi.  Sorry I've been ignoring this once again :(
> I think it would be fine for Jonathan to be the primary packager.
> 

Fine with me, but would very much welcome a co-maintaniner, hint hint :).

> I looked at the newest spec file and I read through the Emacs
> packaging draft.  Looking good!
> 

Thanks.

> Going back to the bbdb-autoloads.el question -- I think the reason that
> this is in site-start.d is so that the autoloads will be evaluated at
> Emacs startup (see site-lisp/site-start.el).  This means that the user
> doesn't have to add an explicit require or anything to their .emacs to
> start using BBDB.  I was wrong about this back in comment #10.  (FWIW
> this is mentioned in the packaging guidelines.)

Yes - actually the best way of dealing with this IMO is to add an init file to
site-lisp/site-start.d which does a (requires 'bbdb-autoloads) - that's what
most packages do, and is the spirit of the guidelines - have updated the spec
file to create such a file. Is this ok with you?

> 
> In my copy of the latest RPM, bbdb-autoloads.el ends up in site-lisp/bbdb/.
> So, I think it should be moved again.
> 

As I say above - I would prefer to leave bbdb-autoloads in the site-lisp/bbdb
directory, and add a file to site-lisp/site-start.d which requires bbdb-autolods

> BTW ... BBDB and VM requiring each other seems pretty ugly!  Perhaps one or
> the other could be fixed upstream.

Yeah, it's very horrible. Fortunately recently it has transpired that the same
person has taken over upstream leadership of both packages, so I will begin a
campaign with him to change this situation :) He is away until the end of
October though.

Updated
SPEC: http://jgu.fedorapeople.org/emacs-bbdb.spec
SRPM: http://jgu.fedorapeople.org/emacs-bbdb-2.35-7.fc7.src.rpm

> Fine with me, but would very much welcome a co-maintaniner, hint hint :).

:-).  Sounds good.

[ init file that requires bbdb-autoloads ]
> Is this ok with you?

Yes, that's great.  Thanks.

I have a little time; let's start with the rpmlint output:

Oops.  Let's try that again:

emacs-bbdb.src: W: spelling-error-in-description recieved received
  Funny rpmlint, but correct.

emacs-bbdb.src: W: invalid-license GPL
  The license needs some adjusting to meet the current guidelines.  I believe 
  the proper tag for this package is "GPLv2+" according to the comment block at 
  the head of bbdb.el

emacs-bbdb.noarch: W: file-not-utf8 /usr/share/info/bbdb.info.gz
emacs-bbdb.noarch: W: file-not-utf8 /usr/share/doc/emacs-bbdb-2.35/ChangeLog
  A quick call to iconv should fix these up.

emacs-bbdb.noarch: E: wrong-script-interpreter
/usr/share/doc/emacs-bbdb-2.35/utils/bbdb-cid.pl "/usr/local/bin/perl5"
emacs-bbdb.noarch: E: wrong-script-interpreter
/usr/share/doc/emacs-bbdb-2.35/utils/bbdb-unlazy-lock.pl "/usr/local/bin/perl"
emacs-bbdb.noarch: E: wrong-script-interpreter
/usr/share/doc/emacs-bbdb-2.35/utils/bbdb-srv.pl "/usr/local/bin/perl"
  Even though these are packaged as documentation, it would still be nice if 
  these were fixed up with some quick calls to sed.

emacs-bbdb-el.noarch: W: no-documentation
  This is OK.

Updated to fix the rpmlint warnings:

SPEC: http://jgu.fedorapeople.org/emacs-bbdb.spec
SRPM: http://jgu.fedorapeople.org/emacs-bbdb-2.35-8.fc7.src.rpm

I finally found time to get back to this.  rpmlint is indeed down to:
   emacs-bbdb-el.noarch: W: no-documentation
which is fine.

Looking through the source, a good portion of this is actually GPL+, but I'm no
licensing expert so I can't really say if you need to mention that in your
License: tag.  I'm going to make the assumption that the package as a whole is
GPLv2+.

There are a few bits in here that aren't in the emacs guidelines, such as the
handling of emacs packages without pkgconfig support.  Should those make it into
the guidelines?  It looks like the emacs in F7 doesn't have pkgconfig support,
so the guidelines seem to be missing support for, basically, all of the releases.

Honestly, though, I see nothing wrong with this package.

* source files match upstream:
   3fb1316e2ed74d47ca61187fada550e58797467bd9e8ad67343ed16da769f916  
   bbdb-2.35.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  emacs-bbdb-2.35-8.fc8.noarch.rpm
   emacs-bbdb = 2.35-8.fc8
  =
   /bin/sh
   emacs(bin) >= 22.1

  emacs-bbdb-el-2.35-8.fc8.noarch.rpm
   emacs-bbdb-el = 2.35-8.fc8
  =
   emacs-bbdb = 2.35-8.fc8

* %check is not present; no test suite upstream.  I can't remember enough about 
   emacs to be able to test this package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (info file registration)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

APPROVED

Since Jonathan's submitting this, I've removed FE-NEEDSPONSOR and I suppose it's
ready to go.  Do let me know if there's still a sponsorship issue.

(In reply to comment #27)
> Looking through the source, a good portion of this is actually GPL+, but I'm no
> licensing expert so I can't really say if you need to mention that in your
> License: tag.  I'm going to make the assumption that the package as a whole is
> GPLv2+.
> 

I will check this once more and confirm the licensing.

[snip]

> Since Jonathan's submitting this, I've removed FE-NEEDSPONSOR and I suppose it's
> ready to go.  Do let me know if there's still a sponsorship issue.

This really depends on Tom (Tromey). Tom has previously said he's happy for me
to be the package owner, and that is fine with me. On the other hand if Tom
wanted to use this package in order to become sponsored (though with such an
"old hand" I wonder if sponsorship is necessary), that is fine as well, and I'm
happy to be a co-maintainer. Unless Tom replies to the contrary, I'll import the
package into CVS at the weekend.

Yes, please go for it.

If Tom needs sponsorship, he need only apply and I'll be happy to click the
appropriate button.

New Package CVS Request
=======================
Package Name: emacs-bbdb
Short Description: A contact management utility for use with Emacs
Owners: jgu
Branches: F-7 
InitialCC: tromey)
Cvsextras Commits: Yes

Tom - I've taken the liberty of adding you to the inital-cc list. Once you're
sponsored and have a FAS username, we can add that to package owners.

We can't add arbitrary email addresses to InitialCC. It must be a Fedora Account
system account name. ;( 

cvs done. (except for the cc)

Any reason this ticket can't be closed now?

oops, sorry, this skipped my mind. Closing.