Bug 494647

I have a couple of questions about the spec file:

1) Is there some reason to avoid the %configure macro in the spec file?  (And "s/Not/Note" in the comment above the configure invocation if so.)

2) Shouldn't this package "Requires: emacs(bin)"?

You noted a problem with "make mmm.pdf".  The problem is that the texinfo.tex in the mmm-mode distribution is being used, and it is too old (it's from 1999!).  Just rm it, and then the system texinfo.tex gets used, and you get a good build.

Some of the Emacs Lisp warnings have me concerned, especially the notes that regexp-opt is being called with 3 arguments but only accepts 1 or 2.  This is almost certain to lead to broken compiled code.  I'll make a patch for you.  Hang on....

Created attachment 338763 [details]
Patch to quiet the Emacs byte compiler

Oops, my mistake, I managed to compile by hand with XEmacs instead of Emacs while trying out the mmm.pdf fix. :-(  Nevertheless, the attached patch, while not necessary, does quiet the byte compiler down a bit.  Take it or leave it as you see fit.

(In reply to comment #1)
> I have a couple of questions about the spec file:
> 
> 1) Is there some reason to avoid the %configure macro in the spec file?  (And
> "s/Not/Note" in the comment above the configure invocation if so.)

I actually would've, but I was told by another person ("pre-review"?) that it was wise to avoid %configure. I was trying to remember the reason for this, but I noticed that I have done this in many of my spec files, so I did it here too, adding exactly the options I thought was necessary. If I can't remember the reason, I'll change it back. (The "Not" was supposed to be a "Not" - it doesn't really matter here, but rpmlint complains otherwise. I suppose this would only occur if you didn't use %configure)

> 2) Shouldn't this package "Requires: emacs(bin)"?

Yes, it should, sorry.

> You noted a problem with "make mmm.pdf".  The problem is that the texinfo.tex
> in the mmm-mode distribution is being used, and it is too old (it's from
> 1999!).  Just rm it, and then the system texinfo.tex gets used, and you get a
> good build.

Ah - great, I'll do that.

> Some of the Emacs Lisp warnings have me concerned, especially the notes that
> regexp-opt is being called with 3 arguments but only accepts 1 or 2.  This is
> almost certain to lead to broken compiled code.  I'll make a patch for you. 
> Hang on....

Ok, I the changes (switched back -> %configure, required emacs(bin), added patch) you suggested, and the new files are in the same locations as the old were.

(In reply to comment #3)
> (In reply to comment #1)
> > I have a couple of questions about the spec file:
> > 
> > 1) Is there some reason to avoid the %configure macro in the spec file?  (And
> > "s/Not/Note" in the comment above the configure invocation if so.)
> 
> I actually would've, but I was told by another person ("pre-review"?) that it
> was wise to avoid %configure. I was trying to remember the reason for this, but
> I noticed that I have done this in many of my spec files, so I did it here too,
> adding exactly the options I thought was necessary. If I can't remember the
> reason, I'll change it back. (The "Not" was supposed to be a "Not" - it doesn't
> really matter here, but rpmlint complains otherwise. I suppose this would only
> occur if you didn't use %configure)
> 
> > 2) Shouldn't this package "Requires: emacs(bin)"?
> 
> Yes, it should, sorry.
> 
> > You noted a problem with "make mmm.pdf".  The problem is that the texinfo.tex
> > in the mmm-mode distribution is being used, and it is too old (it's from
> > 1999!).  Just rm it, and then the system texinfo.tex gets used, and you get a
> > good build.
> 
> Ah - great, I'll do that.
> 
> > Some of the Emacs Lisp warnings have me concerned, especially the notes that
> > regexp-opt is being called with 3 arguments but only accepts 1 or 2.  This is
> > almost certain to lead to broken compiled code.  I'll make a patch for you. 
> > Hang on....

Okay, now it looks like this bit of %install ...

# Fix an issue that occurs in the info files (not quite sure where in
# the process, so doing it here):

mv %{buildroot}%{_infodir}/mmm.info-2.gz \
   %{buildroot}%{_infodir}/mmm.info-2 && \
   gzip %{buildroot}%{_infodir}/mmm.info-2

... is no longer necessary.  At least, it wasn't necessary for me.

MUST items:
- rpmlint output:
emacs-mmm-el.noarch: W: no-documentation
2 packages and 1 specfiles checked; 0 errors, 1 warnings.
- package name
- spec file name
- packaging guidelines
- Fedora approved license
- license field matches actual license
- license file in %doc
- spec file in American English
- spec file is legible
- sources match upstream
- builds on at least one primary arch
- use of ExcludeArch (N/A)
- build dependencies in BuildRequires
- locale handling (N/A)
- call ldconfig (N/A)
- relocatable package (N/A)
- own all created directories
- no duplicates in %files
- proper permissions
- %clean section
- consistent use of macros
- code or permissible content
- large documentation in -doc (N/A)
- nothing in %doc needed at runtime
- header files in -devel (N/A)
- static libraries in -static (N/A)
- Requires pkgconfig (N/A)
- .so files in -devel (N/A)
- -devel requires main package (N/A)
- no libtool archives (N/A)
- desktop file (N/A)
- do not own previously owned dirs
- clean at start of %install
- filenames are valid UTF-8

SHOULD items:
- ask upstream to include license file (N/A)
- supply available translations (N/A)
? package builds in mock (didn't check)
? builds on all supported arches (didn't check)
- package functions as described
- sane scriptlets
- subpackages require main package
- pkgconfig file in -devel (N/A)
- file vs. package dependencies

Looks good.  APPROVED.

(In reply to comment #5)
> Okay, now it looks like this bit of %install ...
> 
> # Fix an issue that occurs in the info files (not quite sure where in
> # the process, so doing it here):
> 
> mv %{buildroot}%{_infodir}/mmm.info-2.gz \
>    %{buildroot}%{_infodir}/mmm.info-2 && \
>    gzip %{buildroot}%{_infodir}/mmm.info-2
> 
> ... is no longer necessary.  At least, it wasn't necessary for me.

Really? Any idea that is... the problem I found for my system was that mmm.info-2.gz was not actually a zipped archive. Thus, if you run info and navigate to a page that requires mmm.info-2, info actually segfaults. It still seems necessary for me: the result in the RPM is a (once zipped) info file. If this problem didn't occur on your system, then this "fix" would actually break things, so I want to know. (I suppose it might be safer to do file type detection and zip it if it isn't a zip file already.)

Hmm, yes, it now seems unnecessary on my system too - the command isn't actually doing anything (I thought any error would've caused rpmbuild to stop, but on closer inspection the command was having an error - couldn't find the zipped file - and not doing anything...)

(In reply to comment #6)
> (In reply to comment #5)
> > Okay, now it looks like this bit of %install ...
> > 
> > # Fix an issue that occurs in the info files (not quite sure where in
> > # the process, so doing it here):
> > 
> > mv %{buildroot}%{_infodir}/mmm.info-2.gz \
> >    %{buildroot}%{_infodir}/mmm.info-2 && \
> >    gzip %{buildroot}%{_infodir}/mmm.info-2
> > 
> > ... is no longer necessary.  At least, it wasn't necessary for me.
> 
> Really? Any idea that is... the problem I found for my system was that
> mmm.info-2.gz was not actually a zipped archive. Thus, if you run info and
> navigate to a page that requires mmm.info-2, info actually segfaults. It still
> seems necessary for me: the result in the RPM is a (once zipped) info file. If
> this problem didn't occur on your system, then this "fix" would actually break
> things, so I want to know. (I suppose it might be safer to do file type
> detection and zip it if it isn't a zip file already.)

New Package CVS Request
=======================
Package Name: emacs-mmm
Short Description: Emacs minor mode allowing different major modes in the same file
Owners: amdunn
Branches: F-9 F-10 F-11

cvs done.

emacs-mmm-0.4.8-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/emacs-mmm-0.4.8-1.fc9

emacs-mmm-0.4.8-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/emacs-mmm-0.4.8-1.fc10

emacs-mmm-0.4.8-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

emacs-mmm-0.4.8-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.