Bug 458548 - Review Request: uriparser - URI parsing library - RFC 3986
Review Request: uriparser - URI parsing library - RFC 3986
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-09 14:52 EDT by Rakesh Pandit
Modified: 2008-10-01 02:43 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-09-08 14:47:23 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Rakesh Pandit 2008-08-09 14:52:20 EDT
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 14:53:46 EDT
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 16:01:54 EDT
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 02:55:11 EDT
@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 13:20:16 EDT
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 06:11:16 EDT
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 06:30:43 EDT
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 06:40:27 EDT
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 06:48:13 EDT
(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 06:58:29 EDT
okay - Will update soon.
Thanks
Comment 12 Rakesh Pandit 2008-09-04 14:35:37 EDT
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 12:16:15 EDT
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 15:37:14 EDT
- 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 11:20:23 EDT
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 12:24:25 EDT
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 11:47:20 EDT
Okay.

----------------------------------------------------------------------------
     This package (uriparser) is APPROVED by mtasaka
----------------------------------------------------------------------------
Comment 18 Rakesh Pandit 2008-09-07 12:14:54 EDT
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 11:17:31 EDT
cvs done.
Comment 20 Fedora Update System 2008-09-08 14:44:04 EDT
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 14:45:54 EDT
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 02:32:43 EDT
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 02:43:56 EDT
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.