Bug 430978 - Review Request: libsoup22 - Backward compatibility package for libsoup 2.2 API
Review Request: libsoup22 - Backward compatibility package for libsoup 2.2 API
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
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:
Last Closed: 2008-03-05 15:48:34 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
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


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 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"
- 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
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.

Comment 11 Mamoru TASAKA 2008-03-04 05:34:48 EST
For 2.2.105-1:

* URL not correct. The correct one seems

* 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)
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.