Bug 539388

Summary: Review Request: xmlrpc-epi - An implementation of the XML-RPC protocol in C.
Product: [Fedora] Fedora Reporter: Bryan O'Sullivan <bos>
Component: Package ReviewAssignee: Thomas Kowaliczek <linuxdonald>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: eldermarco, fedora-package-review, linuxdonald, mtasaka, notting, seg, tcallawa
Target Milestone: ---Flags: linuxdonald: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-03-25 16:56:52 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Bryan O'Sullivan 2009-11-19 23:56:31 UTC
Spec URL: http://www.serpentine.com/bos/files/xmlrpc-epi.spec
SRPM URL: http://www.serpentine.com/bos/files/xmlrpc-epi-0.54.1-1.fc12.src.rpm
Description: An implementation of the XML-RPC protocol in C.

This is needed by the Second Life client.

Comment 1 Jason Tibbitts 2009-11-20 00:44:17 UTC
*** Bug 231809 has been marked as a duplicate of this bug. ***

Comment 2 Thomas Kowaliczek 2010-01-26 23:05:32 UTC
I will review this on Sunday.

Comment 3 Thomas Kowaliczek 2010-01-31 17:48:57 UTC
MUST: rpmlint must be run on every package. The output should be posted in the review.

rpmlint xmlrpc-epi-0.54.1-1.fc12.x86_64.rpm  xmlrpc-epi-debuginfo-0.54.1-1.fc12.x86_64.rpm  xmlrpc-epi-devel-0.54.1-1.fc12.x86_64.rpm
xmlrpc-epi-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

MUST: The package must be named according to the  Package Naming Guidelines .
okay

MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption.
okay

MUST: The package must meet the  Packaging Guidelines .
okay

MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines .
okay

MUST: The License field in the package spec file must match the actual license.
is it not the BSD license?

MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.
okay

MUST: The spec file must be written in American English.
okay

MUST: The spec file for the package MUST be legible.
okay

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the  Source URL Guidelines for how to deal with this.
okay

MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
okay

MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line.
okay

MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.
okay

MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
okay

MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
okay

MUST: Packages must NOT bundle copies of system libraries.
okay

MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker.
okay

MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
okay

MUST: A Fedora package must not list a file more than once in the spec file's %files listings.
okay

MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line.
okay

MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
okay

MUST: Each package must consistently use macros.
okay

MUST: The package must contain code, or permissable content.
okay

MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity).
okay

MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present.
okay

MUST: Header files must be in a -devel package.
okay

MUST: Static libraries must be in a -static package.
okay

MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability).
okay

MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package.
okay

MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
it is Requires: xmlrpc-epi = %{version}-%{release} but it must be Requires:	%{name} = %{version}-%{release}

MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
okay

MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation.
okay

MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time.
okay

MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
okay

MUST: All filenames in rpm packages must be valid UTF-8.
okay

I only find this problem

MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
it is Requires: xmlrpc-epi = %{version}-%{release} but it must be Requires:	%{name} = %{version}-%{release}

and that (but it don´t must be fixed it´s only an warning.:

rpmlint xmlrpc-epi-0.54.1-1.fc12.x86_64.rpm  xmlrpc-epi-debuginfo-0.54.1-1.fc12.x86_64.rpm  xmlrpc-epi-devel-0.54.1-1.fc12.x86_64.rpm
xmlrpc-epi-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

Comment 4 Thomas Kowaliczek 2010-01-31 17:52:32 UTC
Mamoru Tasaka can you please check my review please?

Comment 5 Mamoru TASAKA 2010-01-31 18:59:19 UTC
(In reply to comment #4)
> Mamoru Tasaka can you please check my review please?    
Almost okay, thank you.

Some comments from me:
* %setup directory / debuginfo rpm creation
-------------------------------------------------------------
    25  %prep
    26  %setup -q -n xmlrpc
-------------------------------------------------------------
  - As shown here, when the source tarball is to be expanded,
    the directory named "xmlrpc" is created and source codes
    are expanded there.
    This means that when -debuginfo rpm is created, the included
    source codes are installed through debuginfo rpm under
    /usr/src/debug/xmlrpc .

    However Fedora already has "xmlrpc" package and
    xmlrpc-debuginfo uses "/usr/src/debug/xmlrpc-2.0.1/", so
    using /usr/src/debug/xmlrpc is confusing.

    Please expand source tarball under %_builddir/%name-%version
    like below:
-------------------------------------------------------------
@@ -1,12 +1,12 @@
 Name:      xmlrpc-epi
 Version:   0.54.1
-Release:   1%{?dist}
+Release:   2%{?dist}
 Summary:   An implementation of the XML-RPC protocol in C
 Group:     System Environment/Libraries
 License:   MIT
 URL:       http://xmlrpc-epi.sourceforge.net/
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
-Source:    http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
+Source0:    http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
 BuildRequires: expat-devel
 BuildRequires: libtool
 
@@ -23,15 +23,19 @@
 developing applications that use xmlrpc-epi.
 
 %prep
-%setup -q -n xmlrpc
+%setup -q -c -T -a 0
 
 %build
+cd xmlrpc
+cp -p [A-Z]* ..
+
 %configure --disable-static --includedir=%{_includedir}/xmlrpc-epi
 make %{?_smp_mflags}
 
 %install
 rm -rf %{buildroot}
-make install DESTDIR=%{buildroot}
+cd xmlrpc
+make install DESTDIR=%{buildroot} INSTALL="%{__install} -p"
-------------------------------------------------------------
    ! Note
      - Source is changed to Source"0" to use "-a 0" usage in
        %setup -q.

* Timestamps
  - As shown above, please consider to use
    'INSTALL="%{__install} -p"' option to "make install" to
    keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated by recent
    Makefiles.

Comment 6 Mamoru TASAKA 2010-01-31 19:02:03 UTC
spot, would you audit the following license?
http://xmlrpc-epi.sourceforge.net/main.php?t=license

This is something like MIT, however it seems that its
appearance is somewhat different from MIT.

Comment 7 Thomas Kowaliczek 2010-01-31 21:44:23 UTC
It´s BSD license i have asked upstream:

Thomas Kowaliczek wrote:
> > Message body follows:
> > 
> > Can you please say which license xmlrpc-epi use? is it the
> > BSD one?
> > 
Yes its a BSD style licence, see the COPYING file in the tar ball for
the exact wording.

The MPL only applies to the expat/* files

Regards

Robin

Comment 8 Bryan O'Sullivan 2010-02-05 23:23:27 UTC
I'll have a detailed look at the comments above tomorrow.

Comment 9 Bryan O'Sullivan 2010-02-07 02:54:36 UTC
Thanks for the comments.  Here's an update that addresses all of them:

Spec URL: http://www.serpentine.com/bos/files/xmlrpc-epi.spec
SRPM URL: http://www.serpentine.com/bos/files/xmlrpc-epi-0.54.1-1.fc12.src.rpm

Comment 10 Bryan O'Sullivan 2010-02-07 02:54:43 UTC
Thanks for the comments.  Here's an update that addresses all of them:

Spec URL: http://www.serpentine.com/bos/files/xmlrpc-epi.spec
SRPM URL: http://www.serpentine.com/bos/files/xmlrpc-epi-0.54.1-2.fc12.src.rpm

Comment 11 Thomas Kowaliczek 2010-02-07 14:41:38 UTC
@ Bryan O'Sullivan what about the license? Is it really MIT?

@ Mamoru Tasaka for me looks the packages now okay. But the license?
What do you think?

Comment 12 Mamoru TASAKA 2010-02-12 15:52:48 UTC
I don't think this is BSD. And this also seems to differ from MIT.

Comment 13 Tom "spot" Callaway 2010-02-18 17:47:08 UTC
(In reply to comment #6)
> spot, would you audit the following license?
> http://xmlrpc-epi.sourceforge.net/main.php?t=license
> 
> This is something like MIT, however it seems that its
> appearance is somewhat different from MIT.    

Yeah, but it is functionally identical to MIT. Added to the Licensing/MIT list of variants, please use License: MIT here.

Lifting FE-Legal.

Comment 14 Mamoru TASAKA 2010-02-20 14:37:13 UTC
Thank you, spot.

Now this package looks good to me (by the way please add %changelog
entry for 0.54.1-2)

Comment 15 Thomas Kowaliczek 2010-02-20 22:00:09 UTC
Bryan O'Sullivan @ Please fix/add the last one and i will approve this :)

Comment 16 Bryan O'Sullivan 2010-02-20 22:31:37 UTC
Thanks, folks. URLs remain the same, but I added the forgotten changelog comment.

Spec URL: http://www.serpentine.com/bos/files/xmlrpc-epi.spec
SRPM URL: http://www.serpentine.com/bos/files/xmlrpc-epi-0.54.1-2.fc12.src.rpm

Comment 17 Thomas Kowaliczek 2010-02-21 02:32:47 UTC
################################
This Packages is Apporved by me.
################################

@Bryan O'Sullivan: Please make an CVS Request http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

@Mamoru Tasaka: Thank you for you help :)

Comment 18 Mamoru TASAKA 2010-03-04 15:23:03 UTC
Bryan, would you write CVS admin request?

Comment 19 Bryan O'Sullivan 2010-03-05 04:30:23 UTC
New Package CVS Request
=======================
Package Name: xmlrpc-epi
Short Description: An implementation of the XML-RPC protocol in C.
Owners: bos
Branches: 
InitialCC:

Comment 20 Kevin Fenzi 2010-03-06 05:10:56 UTC
CVS done (by process-cvs-requests.py).

Comment 21 Mamoru TASAKA 2010-03-17 07:47:38 UTC
Bryan, would you rebuild this package on koji?

Comment 22 Mamoru TASAKA 2010-03-25 16:41:11 UTC
ping again, Bryan?