Bug 458548 - Review Request: uriparser - URI parsing library - RFC 3986
Summary: Review Request: uriparser - URI parsing library - RFC 3986
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-08-09 18:52 UTC by Rakesh Pandit
Modified: 2008-10-01 06:43 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-09-08 18:47:23 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Rakesh Pandit 2008-08-09 18:52:20 UTC
Description:
Uriparser is a strictly RFC 3986 compliant URI parsing library written
in C. uriparser is cross-platform, fast, supports Unicode and is
licensed under the New BSD license.

SPEC: http://rakesh.fedorapeople.org/spec/liburiparser.spec
SRPM: http://rakesh.fedorapeople.org/srpm/liburiparser-0.7.1-1.fc9.src.rpm

*Note:
I haven't packaged a doc sub package or anything inside doc directory. The build system for that was separate, and I don't have any reason why. I had asked upstream and hope for some action.

*I have filed a bug at sourceforge project home.

Comment 1 Rakesh Pandit 2008-08-09 18:53:46 UTC
In case reviewer doesn't see any issues i think this packages can go through review without doc until other issue is resolved.

The package will block libkml library which I will file in a while.

Comment 3 Jason Tibbitts 2008-08-09 20:01:54 UTC
Actually I don't see why a separate build system is a problem.  In %build, just add
  cd doc
  %configure
  make %{?_smp_mflags}

and you'll need to figure out how to get them installed properly, since make install in the doc directory doesn't seem to do anything for me.  It's probably just a matter of putting the doc/html directory somewhere.

Comment 4 Rakesh Pandit 2008-08-10 06:55:11 UTC
@jason

Yes, you are very right. Thanks.

I was just concerned why was it like that & never though about a way around.

Updated:
SPEC: http://rakesh.fedorapeople.org/spec/liburiparser.spec
SRPM: http://rakesh.fedorapeople.org/srpm/liburiparser-0.7.1-2.fc9.src.rpm

Comment 5 Jason Tibbitts 2008-08-16 17:20:16 UTC
Unfortunately this doesn't build because the documentation has additional build dependencies.  I think you need doxygen and graphviz.

Comment 7 Mamoru TASAKA 2008-09-04 10:11:16 UTC
First of all, why do you name this srpm as liburiparser instead of using uriparser which
you say is the project name?

Comment 8 Rakesh Pandit 2008-09-04 10:30:43 UTC
I just though about keeping consistency with ubuntu package. what is correct i n your view ? to better use tarball/project name. - Naming guidelines.

Comment 9 Mamoru TASAKA 2008-09-04 10:40:27 UTC
No. Please use debian/ubuntu naming way. Debian packages use many lib-foo names
which Fedora does not use. Use upstream tarball names as much as possible.

Comment 10 Mamoru TASAKA 2008-09-04 10:48:13 UTC
(In reply to comment #9)
> No. Please use debian/ubuntu naming way. 

I meant "Please don't use debian/ubuntu naming way", sorry...

Comment 11 Rakesh Pandit 2008-09-04 10:58:29 UTC
okay - Will update soon.
Thanks

Comment 12 Rakesh Pandit 2008-09-04 18:35:37 UTC
SPEC: http://rakesh.fedorapeople.org/srpm/uriparser-0.7.1-4.fc10.src.rpm
SRPM: http://rakesh.fedorapeople.org/spec/uriparser.spec

Build successfully on koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=807202

rpmlint output: clean

rpmlint uriparser-0.7.1-4.fc10.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint uriparser-0.7.1-4.fc10.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings

rpmlint uriparser-devel-0.7.1-4.fc10.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint uriparser-debuginfo-0.7.1-4.fc10.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 13 Mamoru TASAKA 2008-09-05 16:16:15 UTC
For 0.7.1-4:

* Group
  - Usually "Group" tag for the main package of this type
    is "System Environment/Libraries" (and -devel subpackage
    has "Development/Libraries")

! %setup
  - "-n %{name}-%{version}" is redundant as the default of
    %setup directory is this.

* autotools call
--------------------------------------------
   419  + cd doc
......
   443  config.status: creating Makefile
   444  + make -j8
   445  cd . && /bin/sh /builddir/build/BUILD/uriparser-0.7.1/missing --run aclocal-1.10 
   446  /builddir/build/BUILD/uriparser-0.7.1/missing: line 54: aclocal-1.10: command not found
   447  WARNING: `aclocal-1.10' is missing on your system.  You should only need it if
   448           you modified `acinclude.m4' or `configure.in'.  You might want
   449           to install the `Automake' and `Perl' packages.  Grab them from
   450           any GNU archive site.
   451  cd . && /bin/sh /builddir/build/BUILD/uriparser-0.7.1/missing --run autoconf
......
--------------------------------------------
   - Automated autotool calls after configure is not desired. Usually
     timestamps on some files (aclocal.m4 and so on) are wrong.
     You can avoid this by "touch"ing some files.

* Timestamp
  - Please consider to use
--------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
--------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for recent autotool based Makefiles.

* Documents install
--------------------------------------------
# doc folder make install does nothing
install -d $RPM_BUILD_ROOT%{_docdir}/%{name}-%{version}/html/
install -p -m 0644 doc/html/* $RPM_BUILD_ROOT%{_docdir}/%{name}-%{version}/html/
--------------------------------------------
  - Well, this does not do anything (and can be removed completely) ... 
    Actually in -devel package the directory 
    %{_docdir}/%{name}-%{version}/html/ does not exist.

    This is because of %doc invocation. As build.log shows
    what %doc actually does (when the list in %doc does not 
    begin with /) is:
--------------------------------------------
   663  Processing files: uriparser-0.7.1-4.fc10
   664  Executing(%doc): /bin/sh -e /var/tmp/rpm-tmp.hz0MmE
   665  + umask 022
   666  + cd /builddir/build/BUILD
   667  + cd uriparser-0.7.1
   668  + DOCDIR=/builddir/build/BUILDROOT/uriparser-0.7.1-4.fc10.i386/usr/share/doc/uriparser-0.7.1
   669  + export DOCDIR
   670  + rm -rf /builddir/build/BUILDROOT/uriparser-0.7.1-4.fc10.i386/usr/share/doc/uriparser-0.7.1
   671  + /bin/mkdir -p /builddir/build/BUILDROOT/uriparser-0.7.1-4.fc10.i386/usr/share/doc/uriparser-0.7.1
   672  + cp -pr THANKS AUTHORS COPYING /builddir/build/BUILDROOT/uriparser-0.7.1-4.fc10.i386/usr/share/doc/uripars
er-0.7.1
   673  + exit 0
--------------------------------------------
    See the line 670. %doc once removes all files under 
    $RPM_BUILD_ROOT%{_docdir}/%{name}-%{version}/.

* Documents
  - Please consider to add the following file to %doc.
--------------------------------------------
ChangeLog
--------------------------------------------

Comment 14 Rakesh Pandit 2008-09-05 19:37:14 UTC
- fixed group, removed redundant args for %%setup
- included ChangeLog, fixed html folder path in %%files
- fixed automated autotool calls after configure

SPEC: http://rakesh.fedorapeople.org/spec/uriparser.spec
SRPM: http://rakesh.fedorapeople.org/srpm/uriparser-0.7.1-5.fc10.src.rpm

Comment 15 Mamoru TASAKA 2008-09-06 15:20:23 UTC
Well, it is up to you, however personally I prefer to use "%doc THANKS" and remove all
lines like "install -p -m 0644 doc/html/* $RPM_BUILD_ROOT%{_docdir}/%{name}-%{version}/html/"
or so because
- It is simpler
- Usually -devel subpackage uses %{_docdir}/%{name}-devel-%{version}
- And it is very often that using %_docdir/..... files entry will cause directory
  ownership mistakes (please check your spec file), just as Michael Schwendt wrote:
  https://www.redhat.com/archives/fedora-devel-list/2008-September/msg00381.html

Comment 16 Rakesh Pandit 2008-09-06 16:24:25 UTC
I agree with you, it is always better to use %doc and avoid other movement of files as much as possible.

Thanks,
SPEC: http://rakesh.fedorapeople.org/spec/uriparser.spec
SRPM: http://rakesh.fedorapeople.org/srpm/uriparser-0.7.1-6.fc10.src.rpm

Comment 17 Mamoru TASAKA 2008-09-07 15:47:20 UTC
Okay.

----------------------------------------------------------------------------
     This package (uriparser) is APPROVED by mtasaka
----------------------------------------------------------------------------

Comment 18 Rakesh Pandit 2008-09-07 16:14:54 UTC
New Package CVS Request
=======================
Package Name: uriparser
Short Description: URI parsing library - RFC 3986
Owners: rakesh
Branches: F-8 F-9
InitialCC: rakesh
Cvsextras Commits: yes

Comment 19 Kevin Fenzi 2008-09-08 15:17:31 UTC
cvs done.

Comment 20 Fedora Update System 2008-09-08 18:44:04 UTC
uriparser-0.7.1-6.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/uriparser-0.7.1-6.fc9

Comment 21 Fedora Update System 2008-09-08 18:45:54 UTC
uriparser-0.7.1-6.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/uriparser-0.7.1-6.fc8

Comment 22 Fedora Update System 2008-10-01 06:32:43 UTC
uriparser-0.7.1-6.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2008-10-01 06:43:56 UTC
uriparser-0.7.1-6.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.


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