Bug 962834 - Review Request: libmetalink - a Metalink C library
Summary: Review Request: libmetalink - a Metalink C library
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Michael Schwendt
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2013-05-14 14:29 UTC by Alejandro Alvarez
Modified: 2013-06-18 07:38 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2013-06-18 07:22:06 UTC
Type: Bug
bugs.michael: fedora-review+

Attachments (Terms of Use)

Description Alejandro Alvarez 2013-05-14 14:29:43 UTC
This is a deprecated package of which I would like to take ownership. The previous maintainer stopped supporting it, but upstream is active.

Spec URL: https://aalvarez.web.cern.ch/aalvarez/fedora/libmetalink.spec
SRPM URL: https://aalvarez.web.cern.ch/aalvarez/fedora/libmetalink-0.1.2-1.src.rpm

libmetalink is a Metalink C library. It adds Metalink functionality such as
parsing Metalink XML files to programs written in C.

Fedora Account System Username: aalvarez

rpmlint output
libmetalink.src:84: W: macro-in-%changelog %{version}
libmetalink.src:84: W: macro-in-%changelog %{release}
libmetalink.src:87: W: macro-in-%changelog %{_docdir}
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

Build on EPEL6

Build on F20

Comment 1 Michael Schwendt 2013-06-05 06:44:31 UTC
> rpmlint output

Fix the issues found by rpmlint, please. Also run rpmlint on the _built_ rpms, not just the src.rpm.

> Summary:        A Metalink C library

In Anaconda and package tools, which display these summaries, it looks better
(and more concise) when omitting these leading articles:

  Summary: Metalink library written in C


> BuildRoot: ...

> %install
> rm -rf $RPM_BUILD_ROOT

> %clean
> rm -rf $RPM_BUILD_ROOT

> %defattr(-,root,root,-)

Nowdays these are not necessary anymore. Unless you want to build with the same spec file for EL5. Therefore it's better to remove what's not needed anymore and add a comment what shall be kept:


> %package       devel
> Summary:       A Metalink C library devel package

More clear (as in many -devel packages):

  Summary: Files needed for developing with %{name}

> Requires:      %{name} = %{version}-%{release}


> Requires:      pkgconfig

Not needed. Automatically added for the executable _and_ the .pc file's Provides.

> %package       docs


Probably not worthwhile, but you could see whether to replace this -docs package with a -doc package (dead.package file says: retired on 2012-02-06).

> Group:         Development/Libraries

Rather "Group: Documentation".

> Requires:      %{name} = %{version}-%{release}

Independent documentation -doc packages typically don't require the base
package. It should be possible to install documentation without having to
install a program and all its dependencies.

> %files devel
> %{_includedir}/metalink/
> %{_includedir}/metalink/metalink*.h

Duplicate file entries. The first includes the directory *and* everything in it. The second includes the header files once more. Choose either one:



  %dir %{_includedir}/metalink/

As a packaging hint: It can be helpful to spell out the specific file names of header files, so if one file gets renamed during a version upgrade, the build would fail and serve as an early-warning-system.

Comment 2 Alejandro Alvarez 2013-06-10 09:51:43 UTC
Here is the new version.
By the way, I do want to release in EPEL5. I attach this time a build in el5-candidate.

Spec URL: https://aalvarez.web.cern.ch/aalvarez/fedora/libmetalink.spec
SRPM URL: https://aalvarez.web.cern.ch/aalvarez/fedora/libmetalink-0.1.2-2.fc18.src.rpm

rpmlint output
$rpmlint ./libmetalink-0.1.2-2.fc18.src.rpm 
libmetalink.src: W: spelling-error Summary(en_US) Metalink -> Meta link, Meta-link, Metal ink
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint ./libmetalink-0.1.2-2.fc18.x86_64.rpm 
libmetalink.x86_64: W: spelling-error Summary(en_US) Metalink -> Meta link, Meta-link, Metal ink

Build on F20

Build on EPEL6

Build on EPEL5

Comment 3 Michael Schwendt 2013-06-13 08:52:23 UTC
* The changes are fine, except that I just had a final look at the new -doc package and noticed it contains only 15 manual pages with a total package size of 12092. As such I think it's not justified to create a separate -doc package. 

I recommend that you include the manual pages in the -devel package where they belong (manuals section 3). No Obsoletes are needed, since the -doc package hasn't existed before. The old packages included the man pages in the base package (wrongly): http://koji.fedoraproject.org/koji/packageinfo?packageID=8400

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

The %changelog doesn't tell _why_ you've moved the man pages into a subpackage. In case a future version will include "large" documentation, it would still be possible to introduce a -doc package.

* EPEL5: %defattr is not needed there either, because RPM is new enough.

* You're free to handle the doc changes within pkg git.


Comment 4 Michael Schwendt 2013-06-13 08:55:10 UTC
Btw, "yum search cunit" is successful meanwhile, and the configure script searches for it. Consider looking into it, too.

Comment 5 Michael Schwendt 2013-06-13 08:57:37 UTC
BuildRequires: CUnit-devel

make check

Comment 6 Michael Schwendt 2013-06-13 08:58:45 UTC
    CUnit - A unit testing framework for C - Version 2.1-2

Suite: libmetalink_TestSuite
  Test: test of metalink_list ...passed
  Test: test of metalink_pctrl_file_transaction ...passed
  Test: test of metalink_pctrl_resource_transaction ...passed
  Test: test of metalink_pctrl_checksum_transaction ...passed
  Test: test of metalink_pctrl_piece_hash_transaction ...passed
  Test: test of metalink_pctrl_chunk_checksum_transaction ...passed
  Test: test of metalink_parse_file ...passed
  Test: test of metalink_parse_fp ...passed
  Test: test of metalink_parse_fd ...passed
  Test: test of metalink_parse_memory ...passed
  Test: test of metalink_parse_update ...passed
  Test: test of metalink_parser_update_fail ...passed
  Test: test of metalink_check_safe_path ...passed
  Test: test of metalink_get_version ...passed
  Test: test of metalink_parse_file_v4 ...passed

Run Summary:    Type  Total    Ran Passed Failed Inactive
              suites      1      1    n/a      0        0
               tests     15     15     15      0        0
             asserts    489    489    489      0      n/a

Elapsed time =    0.000 seconds
PASS: metalinktest
1 test passed

Comment 7 Alejandro Alvarez 2013-06-13 09:09:37 UTC
I will have a look at those, thanks!

Package Change Request
Package Name: libmetalink
New Branches: f17 f18 f19 el5 el6
Owners: aalvarez

Comment 8 Gwyn Ciesla 2013-06-13 12:09:18 UTC
Git done (by process-git-requests).

Comment 9 Alejandro Alvarez 2013-06-13 12:45:56 UTC
Sorry. I assumed master would be adopted too, but it hasn't been.

Package Change Request
Package Name: libmetalink
New Branches: master
Owners: aalvarez

Comment 10 Michael Schwendt 2013-06-13 13:10:02 UTC
I think you need to file an unblock request at Fedora Release Engineering and point at this re-review: https://fedorahosted.org/rel-eng/

Comment 11 Alejandro Alvarez 2013-06-13 13:24:23 UTC
I have done that too, but I was surprised I can push to all branches except master.

Well, and in the database devel is the only one still marked as orphan and deprecated.

For the record: https://fedorahosted.org/rel-eng/ticket/5635

Comment 12 Gwyn Ciesla 2013-06-13 13:53:06 UTC
You need to take ownership of it in pkgdb.

Comment 13 Michael Schwendt 2013-06-13 14:00:24 UTC
In pkgdb mortals cannot take ownership of "deprecated" packages. An admin needs to assist.

$ koji list-pkgs --package libmetalink --show-blocked
Package                 Tag                     Extra Arches     Owner          
----------------------- ----------------------- ---------------- ---------------
libmetalink             dist-f9                                  ant            
libmetalink             dist-5E-epel                             aalvarez       
libmetalink             f12-alpha                                ant            
libmetalink             f9-cutoff                                ant            
libmetalink             f9-build-cutoff                          ant            
libmetalink             f12-beta                                 ant            
libmetalink             f12-final                                ant            
libmetalink             dist-6E-epel                             aalvarez       
libmetalink             dist-f15                                 orphan         
libmetalink             f16                                      orphan         
libmetalink             f17                                      aalvarez        [BLOCKED]
libmetalink             f17-final                                orphan          [BLOCKED]
libmetalink             f18-Alpha                                orphan          [BLOCKED]
libmetalink             dist-f15-eol                             orphan         
libmetalink             f18-final                                orphan          [BLOCKED]

Comment 14 Michael Schwendt 2013-06-17 16:12:49 UTC
Alejandro, you can "Take ownership" in pkgdb now thanks to Kevin Fenzi.

Comment 15 Gwyn Ciesla 2013-06-17 16:16:03 UTC
Alejandro, you now own the devel branch.

Comment 16 Alejandro Alvarez 2013-06-18 07:21:24 UTC
Great, thanks!

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