Bug 430978 - Review Request: libsoup22 - Backward compatibility package for libsoup 2.2 API
Review Request: libsoup22 - Backward compatibility package for libsoup 2.2 API
Status: CLOSED RAWHIDE
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: 430756
  Show dependency treegraph
 
Reported: 2008-01-30 16:43 EST by Matthew Barnes
Modified: 2008-03-05 15:48 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-03-05 15:48:34 EST
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)
patch to adjust 2.2.x doc install dir (362 bytes, patch)
2008-02-01 10:55 EST, Dan Winship
no flags Details | Diff

  None (edit)
Description Matthew Barnes 2008-01-30 16:43:47 EST
Spec URL: http://mbarnes.fedorapeople.org/libsoup22.spec
SRPM URL: http://mbarnes.fedorapeople.org/libsoup22-2.2.104-2.fc9.src.rpm

Description:

Fedora 9 will include the new libsoup-2.4 API, which breaks the 2.2 API in non-trivial ways.  This is a backward compatibility package for applications that still need the libsoup-2.2 API.
Comment 1 Bill Nottingham 2008-01-30 21:38:49 EST
MUST Items:
 - Package meets naming and packaging guidelines - OK
 - Spec file matches base package name. - OK
 - Spec has consistant macro usage. - OK
 - Meets Packaging Guidelines. - OK
 - License  - ***

No version is ever listed in the source files. Ergo, the license is
any version of LGPL -> LGPLv2+.

 - License field in spec matches - See above
 - License file included in package  - OK
 - Spec in American English  - OK
 - Spec is legible.  - OK
 - Sources match upstream md5sum:
ab3b10b1c97de5abe38a748a3656da4c - OK
 - Package needs ExcludeArch - N/A
 - BuildRequires correct - OK
 - Spec handles locales/find_lang - N/A
 - Package is relocatable and has a reason to be. - N/A
 - Package has %defattr and permissions on files is good. - OK
 - Package has a correct %clean section. - OK
 - Package has correct buildroot - ***

Using one of the preferred buildroots is... preferred, such as:
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

 - Package is code or permissible content. - OK
 - Doc subpackage needed/used. - N/A
 - Packages %doc files don't affect runtime. - OK
 - Headers/static libs in -devel subpackage. - OK
 - Spec has needed ldconfig in post and postun - OK
 - .pc files in -devel subpackage/requires pkgconfig - OK
 - .so files in -devel subpackage. - OK
 - -devel package Requires: %{name} = %{version}-%{release} - OK
 - .la files are removed. - OK
 - Package compiles and builds on at least one arch. - OK (tested x86_64)
 - Package has no duplicate files in %files. - OK
 - Package doesn't own any directories other packages own. - OK
 - Package owns all the directories it creates. - OK
 - No rpmlint output. ***
libsoup22.x86_64: E: explicit-lib-dependency libxml2 - See below.
libsoup22-devel.x86_64: W: no-documentation - ignorable
 - final provides and requires are sane: - ***

Requires: glib2 >= 2.6
Requires: gnutls
Requires: libxml2

All seem superfluous. gnutls and libxml2 are already picked up by normal library
requires, and the glib2 configure test is already looking for 2.12.x, if I'm
reading it right.

SHOULD Items:

 - Should build in mock. - tested x86_64
 - Should function as described. - did not try
 - Should have sane scriptlets.  - OK
 - Should have dist tag - OK
 - Should package latest version - OK

Please fix a) license tag b) buildroot c) requires. Thanks!
Comment 2 Mamoru TASAKA 2008-01-30 21:44:21 EST
- Source must be given by full URL
- support parallel make if possible
- Usually the main library package has "System Environment/Libraries"
  group. 
- Don't use %makeinstall unless unavoidable
Comment 3 Mamoru TASAKA 2008-01-30 21:49:52 EST
- add "make check" in %check section if possible.
Comment 4 Bill Nottingham 2008-01-30 21:57:36 EST
The tests require a running apache server... that's not really practical.
Comment 5 Mamoru TASAKA 2008-01-30 22:07:15 EST
(In reply to comment #4)
> The tests require a running apache server... that's not really practical.

I actually thought so first, however tests/Makefile.am says:
----------------------------------------------------------------------
    45  if HAVE_APACHE
    46  APACHE_TESTS = auth-test proxy-test pull-api
    47  endif
-----------------------------------------------------------------------
So if httpd is not installed, make check simply skips these tests.
Actually I have not httpd installed and "make check" of libsoup22
does 6 checks.

[tasaka1@localhost libsoup-2.2.104]$ LANG=C make check
Making check in libsoup
make[1]: Entering directory `/home/tasaka1/rpmbuild/BUILD/libsoup-2.2.104/libsoup'
make  check-am
make[2]: Entering directory `/home/tasaka1/rpmbuild/BUILD/libsoup-2.2.104/libsoup'
make[2]: Nothing to be done for `check-am'.
make[2]: Leaving directory `/home/tasaka1/rpmbuild/BUILD/libsoup-2.2.104/libsoup'
make[1]: Leaving directory `/home/tasaka1/rpmbuild/BUILD/libsoup-2.2.104/libsoup'
Making check in tests
make[1]: Entering directory `/home/tasaka1/rpmbuild/BUILD/libsoup-2.2.104/tests'
make  check-TESTS
make[2]: Entering directory `/home/tasaka1/rpmbuild/BUILD/libsoup-2.2.104/tests'
context-test: OK
PASS: context-test
date: OK
PASS: date
header-parsing: OK
PASS: header-parsing
uri-parsing: OK
PASS: uri-parsing
ntlm-test: OK
PASS: ntlm-test
SYNCHRONOUS SSL TEST PASSED
ASYNCHRONOUS SSL TEST PASSED
PASS: ssl-test
==================
All 6 tests passed
==================

Comment 6 Dan Winship 2008-02-01 10:55:40 EST
Created attachment 293736 [details]
patch to adjust 2.2.x doc install dir

the next libsoup 2.4 release will install its docs into
/usr/share/gtk-doc/html/libsoup-2.4/. This patch makes 2.2.104 install its docs
into .../libsoup-2.2/, if you'd rather do that.
Comment 7 Dan Winship 2008-02-01 10:59:24 EST
(In reply to comment #5)
> (In reply to comment #4)
> > The tests require a running apache server... that's not really practical.

(They don't require a *running* apache server; they just require that apache be
installed, and then they start it themselves, with a custom httpd.conf.)

> So if httpd is not installed, make check simply skips these tests.
> Actually I have not httpd installed and "make check" of libsoup22
> does 6 checks.

Yeah, and since you only need to test "did the package build correctly", not
"does all of the code still work correctly", then running just the
non-apache-using tests is fine.
Comment 8 Alex Lancaster 2008-02-07 00:03:17 EST
Ping?  Is this review APPROVED?   It seems to have stalled, although most of the
serious issues seem to have been fixed.  Still quite a number of broken packages
in rawhide that need libsoup22 ASAP.
Comment 9 Bill Nottingham 2008-02-07 11:21:49 EST
No, it's still waiting for a new spec/srpm with some of the above issues resolved.
Comment 10 Paul W. Frields 2008-03-01 09:45:54 EST
Here's one that should work.  It passes the mock test for me and I think I
caught all of Bill's requested changes.

http://pfrields.fedorapeople.org/packages/SRPMS/libsoup22-2.2.105-1.fc9.src.rpm
http://pfrields.fedorapeople.org/packages/SPECS/libsoup22.spec
Comment 11 Mamoru TASAKA 2008-03-04 05:34:48 EST
For 2.2.105-1:

* URL not correct. The correct one seems
  ftp://ftp.gnome.org/pub/gnome/sources/libsoup/2.2/%{name}-%{version}.tar.bz2

* Now %defattr(-,root,root,-) is recommended.

* I recommended to use
----------------------------------------------------------------
  make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
----------------------------------------------------------------
  This method usually works for recent autotool-based Makefiles
  and saves most of the installed files.

Other thing seems good to me.
Comment 13 Matthew Barnes 2008-03-04 10:24:50 EST
Weird.  I don't recall getting Bugzilla spam for any of these updates.  Didn't
realize the review had progressed so far.  Apologies for my lack of repsonse.
Comment 14 Mamoru TASAKA 2008-03-04 10:27:06 EST
Okay. For now reassigning this bug to me and I approve this.

--------------------------------------------------------------
   This package (libsoup22) is APPROVED by me
--------------------------------------------------------------
Comment 15 Matthew Barnes 2008-03-05 10:36:56 EST
New Package CVS Request
=======================
Package Name: libsoup22
Short Description: Compatibility package for libsoup 2.2 API
Owners: mbarnes
Branches: (just devel)
InitialCC: 
Cvsextras Commits: yes

(Anyone else want to co-own this thing?)
Comment 16 Kevin Fenzi 2008-03-05 14:43:41 EST
cvs done.
Comment 17 Matthew Barnes 2008-03-05 15:48:34 EST
Built it Rawhide now.

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