Bug 927310 - Review Request: libgit2 - The Git linkable library
Summary: Review Request: libgit2 - The Git linkable library
Keywords:
Status: CLOSED DUPLICATE of bug 867959
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-03-25 15:53 UTC by Troy Dawson
Modified: 2013-04-05 19:15 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-05 19:15:04 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Troy Dawson 2013-03-25 15:53:11 UTC
Spec URL: http://tdawson.fedorapeople.org/review/libgit2.spec
SRPM URL: http://tdawson.fedorapeople.org/review/libgit2-0.17.0-1.src.rpm
Description: 
libgit2 is a portable, pure C implementation of the 
Git core methods provided as a re-entrant linkable 
library with a solid API, allowing you to write native 
speed custom Git applications in any language with bindings.

Fedora Account System Username: tdawson

Comment 1 Michael Schwendt 2013-03-26 20:58:14 UTC
It's surprising how you've mispackaged this library. :-(

> License:       GPL2 with linking exemption.

As a valid tag, at most there is "License: GPLv2 with exceptions" in the Fedora Licensing guidelines: https://fedoraproject.org/wiki/Licensing:Main

> pure C implementation

If it's written in C, a working -debuginfo package ought to be built.

> # Cleanup
> rm -rf %{buildroot}/usr/lib/debug %{buildroot}/usr/src/debug

https://fedoraproject.org/wiki/Packaging:Guidelines#Debuginfo_packages

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

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

> %package 	devel
> Summary:	The Git linkable library

This is half-hearted summary for the -devel package. That package does not contain the library, but just the files needed for building software that uses libgit2.

> %description 	devel
> libgit2 is a portable, pure C implementation of the 
> Git core methods provided as a re-entrant linkable 
> library with a solid API, allowing you to write native 
> speed custom Git applications in any language with bindings.

Same here. Just copying the %description of the base package is misleading.

> /usr/lib/%{name}*

And what about target platforms where %{_libdir} is /usr/lib64?

> %files devel
> /usr/include/git2*
> /usr/lib/pkgconfig

Same here. And since the package cannot be built yet, I assume one of the library files is misplaced, too:
https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

A simple test-build for Fedora 18 fails very early, unfortunately.

Comment 2 Troy Dawson 2013-03-27 00:05:13 UTC
Spec URL: http://tdawson.fedorapeople.org/review/libgit2.spec
SRPM URL: http://tdawson.fedorapeople.org/review/libgit2-0.17.0-2.fc20.src.rpm

I'd gotten so used to rubygems I think I'm slipping on normal packages.  Thank you for the comments.

- License: fixed - now GPL2 with exceptions
- Descriptions for devel - fixed
- Debuginfo - it always did create a debuginfo.  But it was leaving all the source code in those directories called "debug"
- /usr/lib/ - I didn't even notice this until you said it.  I was building this on a 64 bit machine, and all my builds have done 64, yet everything is put into /usr/lib/ ... I don't want to change it to %{_libdir} or it might break.
- building - That's sorta funny that it didn't build on F18.  That's usually the platform that I build on, but this time I built on rawhide and F19, and it worked.  I figured if it would build on those, it would build on earlier version.  I was wrong, but I've fixed the buildrequires so that it now builds on F18.

Comment 3 Michael Schwendt 2013-03-27 00:57:23 UTC
> but this time I built on rawhide and F19, and it worked.

Not in a clean buildroot, such as with koji/Mock. You've added "BuildRequires: zlib-devel", which would be missing in Rawhide and F19, too. ;)


> I don't want to change it to %{_libdir} or it might break.

It will be impossible to approve this package then.


Please run "rpmlint" or "rpmlint -i" on all rpms, the src.rpm _and_ the built rpms. Feel free to ignore the obvious false positives (e.g. spelling mistakes).

Comment 4 Michael Schwendt 2013-03-27 10:07:53 UTC
https://fedoraproject.org/wiki/Packaging:Guidelines#Macros
| 
| %{_libdir} must always be used for binary libraries due to multi-lib,
| you may not substitute a hard-coded path. 

The pkgconfig file libgit2.pc also needs a bit of work. libdir=/usr/lib is hardcoded there. And the file is customised for static linking. It sets a pkgconfig inter-dependency on openssl-devel and relinks with zlib. Typically, for shared library builds, one would move such inter-dependencies into the Libs.private and Requires.private fields.

Comment 5 john5342 2013-04-05 18:37:57 UTC
Just a note that this package has already been submitted for review at [1]. You really should check before submitting a review request (it is in the review request guidelines). In addition there are apparently bundled libraries that would require unbundling or an fpc exception. Since the other review was submitted first and it has a reviewer i would suggest you close this review and optionally work with the other submitter to see if you can help there.

[1] - https://bugzilla.redhat.com/show_bug.cgi?id=867959

Comment 6 Troy Dawson 2013-04-05 18:49:40 UTC
You are correct.
I usually do, and thought that I did.  But there was 4 or 5 packages I was checking in Koji and then in the review requests, and I must have missed this in the review requests.  I'll mark this as a duplicate of that one.
Though it's been sitting there for quite a while without any progress.

Comment 7 Michael Schwendt 2013-04-05 19:02:58 UTC
That's a surprise. It's not displayed on the pages at http://fedoraproject.org/PackageReviewStatus/ but only when using the "Quick Review Search" box.

Comment 8 Troy Dawson 2013-04-05 19:15:04 UTC

*** This bug has been marked as a duplicate of bug 867959 ***


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