Bug 374791 - Review Request: libnxml - simple C library for parsing, writing and creating XML
Review Request: libnxml - simple C library for parsing, writing and creating XML
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2007-11-10 11:08 EST by hs1
Modified: 2008-01-20 10:12 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-20 10:12:14 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
libnxml rev 2 spec file (1.79 KB, text/plain)
2007-12-10 03:41 EST, hs1
no flags Details
libnxml rev 3 spec file (1.96 KB, text/x-rpm-spec)
2007-12-10 05:56 EST, hs1
no flags Details
libnxml spec file rev4 (2.09 KB, text/plain)
2007-12-11 03:18 EST, hs1
no flags Details

  None (edit)
Description hs1 2007-11-10 11:08:43 EST
Spec URL: http://hs1.eu/code/rpmbuild/SPECS/libnxml.spec
SRPM URL: http://hs1.eu/code/rpmbuild/SRPMS/libnxml-0.18.1-1.src.rpm
Description: 
This is my first package. rpmlint retuns nothing on regular package but with the -devel one it returns:

libnxml-devel.i386: E: zero-length /usr/share/doc/libnxml-devel-0.18.1/html/nxml_8h__incl.map

libnxml-devel.i386: E: invalid-directory-reference /usr/lib/pkgconfig/nxml.pc (line 1)

---(from author's site)
nXML is a C library for parsing, writing and creating XML 1.0 and 1.1 files or streams. It supports utf-8, utf-16be and utf-16le, ucs-4 (1234, 4321, 2143, 2312).

This library is tested on Linux, Windows, *BSD, Solaris, Minix 3.

Why another XML library? Because it is fast, easy-to-use and -important- it is my personal work about the second layer of the Semantic Web. Other my softwares are based on this library. Annotea (http://www.autistici.org/bakunin/annotea/), Morla (http://www.autistici.org/bakunin/morla/) and libmrss (http://www.autistici.org/bakunin/libmrss/).

Why Nxml ? N is from Naples a beautiful italian city. I was there when I started to write this code. Who doesn't know Naples, maybe he should spend some day in that city because it is full of history, culture and wonderful people.
Comment 1 hs1 2007-11-16 04:39:13 EST
Updated spec file:
- patchs in .pc file fixed
- use %{dist} now
- better BuildRoot
Comment 2 Jason Tibbitts 2007-11-17 22:02:00 EST
A few comments:

Why ExcludeArch?  Do you know for a fact that this doesn't run on Alpha or PPC,
but still runs on, say, PPC64?  And even then, make sure you spell it "alpha",
not "aplha".

Please just use one changelog at the end.

I'm not sure that the second and third paragraphs of the description are really
appropriate.  It is of little use to Fedora to state that the library works on
Windows, and the the rest is more appropriate to the project home page.

Your configure file is generated by autoconf, so you should just use the
%configure macro to pass all of the appropriate options.

Do you really need build dependencies on automake and autoconf?  You don't call
them.

I find it simpler to just delete the .la files instead of using %exclude.

You shouldn't generally package static libraries.  If you absolutely need to,
you must put them in a separate -static package.  See
http://fedoraproject.org/wiki/Packaging/Guidelines (the Exclusion of Static
Libraries section).

The package fails to build:
error: Installed (but unpackaged) file(s) found:
   /usr/lib/libnxml.a
   /usr/lib/libnxml.la
   /usr/lib/libnxml.so
   /usr/lib/libnxml.so.0
   /usr/lib/libnxml.so.0.18.1
   /usr/lib/pkgconfig/nxml.pc

These files are installed to /usr/lib instead of %_libdir, which happens to be
/usr/lib64 on the package I'm building on.  I think calling the configure script
properly would fix this.

You should call parallel make if possible with %{__make} %{?_spm_mflags}.

rpmlint (on the source package, at least, since I can't build the binary one) says:

  libnxml.src:10: W: hardcoded-packager-tag hs1
Don't use Packager.

  libnxml.src:55: E: configure-without-libdir-spec
You should call your configure script with %configure.

  libnxml.src: W: more-than-one-%changelog-section
Mentioned above.

  libnxml.src: W: strange-permission libnxml.spec 0600
  libnxml.src: W: strange-permission libnxml-0.18.1.tar.gz 0600
Generally the files in an srpm are mode 644.
Comment 3 hs1 2007-11-21 17:04:38 EST
I upload a new new version at same URL. I tried follow you suggestions.

I tried to delete *.la and *.la at %build end but I got always this error:
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
Checking for unpackaged file(s): /usr/lib/rpm/check-files
/var/tmp/libnxml-0.18.1-root-hardskinone
error: Installed (but unpackaged) file(s) found:
   /usr/lib/libnxml.a


RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/lib/libnxml.a

========
For permissions: I think why my system umask is 077. Is it a problem? Follows
the rpmlint output for the source package:

]$ rpmlint SRPMS/libnxml-0.18.1-1.fc7.src.rpm 
libnxml.src: W: strange-permission libnxml.spec 0600
libnxml.src: W: strange-permission libnxml-0.18.1.tar.gz 0600

Yes, delete files is simpler but is it a good way? Please explain if you can.

Thanks
Comment 4 Mamoru TASAKA 2007-12-03 08:23:59 EST
Your newest spec/srpm is what is written on your comment 0,
is it okay?
(from next time please change the release number every time you
 modify your spec file to avoid confusion).

By the way, you can check general packaing guidelines from
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

As far as I just glanced at your (perhaps newest) spec file:
* Buildroot tag you are using now is not valid for Fedora
* Remove static archive.
  I prefer to delete these files at %install section like
------------------------------------------------------
%install
(some install process)
%{__rm} -f $RPM_BUILD_ROOT%{_libdir}/lib*.{a,la}
------------------------------------------------------
  ? Maybe %configure accepts --disable-static option to
    avoid static archive creation
* We now recommend %defattr(-,root,root,-)
* By the way, the newest libnxml is already 0.9?
* I am just watching your spec file, however your spec file
  contains
------------------------------------------------------
%doc test/*.c
------------------------------------------------------
  Does this mean that you can execute some check program after
  building libnxml? If so, please create %check section
  and do some test program in %check.

Comment 5 Mamoru TASAKA 2007-12-10 01:46:59 EST
ping?
Comment 6 hs1 2007-12-10 03:41:14 EST
Created attachment 282511 [details]
libnxml rev 2 spec file
Comment 7 hs1 2007-12-10 03:42:14 EST
Pong. I made change as suggested in 374791#c4. Updated spec file is where I
written in #c0. Should I append a rev number to spec file name?

I still don't get how to exclude a file from docdir:
$ rpmlint libnxml-devel-0.18.1-2.fc7.i386.rpm 
libnxml-devel.i386: E: zero-length
/usr/share/doc/libnxml-devel-0.18.1/html/nxml_8h__incl.map

No. test directory contains simple example code.
>------------------------------------------------------
>%doc test/*.c
>------------------------------------------------------
>  Does this mean that you can execute some check program after
>  building libnxml? If so, please create %check section
>  and do some test program in %check.
Comment 8 Mamoru TASAKA 2007-12-10 04:50:08 EST
(In reply to comment #7)
> Pong. I made change as suggested in 374791#c4. Updated spec file is where I
> written in #c0. Should I append a rev number to spec file name?

You don't have to change the name of spec file, however please
also provide the new srpm not only the new spec file 

> I still don't get how to exclude a file from docdir:
> $ rpmlint libnxml-devel-0.18.1-2.fc7.i386.rpm 
> libnxml-devel.i386: E: zero-length
> /usr/share/doc/libnxml-devel-0.18.1/html/nxml_8h__incl.map

If this (zero size) file needed? If so, leave this file as
it is. Otherwise just "rm" this file like
---------------------------------------------------
%prep
%setup -q
rm -f doc/html/nxml_8h__incl.map
----------------------------------------------------
Comment 9 hs1 2007-12-10 05:56:38 EST
Created attachment 282641 [details]
libnxml rev 3 spec file

SRPM file: http://hs1.eu/code/rpmbuild/SRPMS/libnxml-0.18.1-3.fc7.src.rpm
nxml_8h__incl.map file is a zero-length output file create by oxygen, I simple
removed it.
Comment 10 Mamoru TASAKA 2007-12-10 07:33:38 EST
I just tried to rebuild -3.fc7 but it failed.
http://koji.fedoraproject.org/koji/taskinfo?taskID=285232
http://koji.fedoraproject.org/koji/getfile?taskID=285235&name=build.log

I guess at least graphviz is missing from BuildRequires.

By the way, what is sleep 10? Also, the macro %{__rm -f}
does not exist.
Comment 11 hs1 2007-12-11 03:18:25 EST
Created attachment 283721 [details]
libnxml spec file rev4
Comment 12 Mamoru TASAKA 2007-12-11 04:38:29 EST
Please also provide the new srpm.
Comment 14 Mamoru TASAKA 2007-12-11 09:58:32 EST
For 0.18.1-4:

* License
  - From whole tarball license check, this is licensed
    under GPLv2+.

* Parallel make
  - Please support parallel make (i.e. add %{?_smp_mflags} to "make")
    on %build section
  - And please remove %?_smp_mflags from "make install" on %install
    section. Using parallel make on "make install" frequent fails
    due to install directory creating timing problem
    (and not %?_spm_mflags)

* Dependency for -devel subpackage
  - Please check the dependency for -devel subpackage.
    %_includedir/nxml.h contains:
-------------------------------------------------------------
    22  #include <curl/curl.h>
    23  #include <sys/types.h>
    24  #include <sys/stat.h>
    25  #include <fcntl.h>
    26  #include <unistd.h>
    27  #include <stdarg.h>
    28  #include <stdlib.h>
    29  #include <string.h>
    30  #include <errno.h>
-------------------------------------------------------------
    This means libnxml-devel should have "Requires: curl-devel".

* pkgconfig file libdir
  - %_libdir/pkgconfig/nxml.pc contains:
--------------------------------------------------------------
     3  libdir=${exec_prefix}/lib
--------------------------------------------------------------
     This is wrong on 64 bits architecture.

* rpmlint
  - rpmlint complaint shows
-------------------------------------------------------------
libnxml-devel.i386: E: zero-length
/usr/share/doc/libnxml-devel-0.18.1/html/structnxml__namespace__t__coll__graph.map
libnxml-devel.i386: E: zero-length
/usr/share/doc/libnxml-devel-0.18.1/html/struct____nxml__entity__t__coll__graph.map
--------------------------------------------------------------
    If these files are not needed, please remove these.

* Timestamps
  - Please try to use
---------------------------------------------------------------
make install DESTDIR=%{buildroot} INSTALL="install -p"
---------------------------------------------------------------
    to keep timestamps on installed files (here installed header
    files). While sometimes this does not work, this way usually
    works for recent Makefiles.

* %doc
   - IMO
----------------------------------------------------------------
%files devel
%doc test/
----------------------------------------------------------------
     is better than "%doc test/*" to hide all test code under test/
     directory from the top documents directory 
     (%_defaultdocdir/%name-%version)
Comment 15 Mamoru TASAKA 2007-12-18 08:58:09 EST
ping?
Comment 16 Mamoru TASAKA 2007-12-27 11:04:09 EST
ping again?
Comment 17 Mamoru TASAKA 2008-01-06 10:57:53 EST
I will close this bug if no response from the reporter is received
by 2008-01-18.
Comment 18 Mamoru TASAKA 2008-01-20 10:12:14 EST
Closing.

If someone wants to import this package into Fedora, please file a new
review request and mark this bug as a duplicate of the new bug.

Thank you!

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