Bug 1182261 - Review Request: libabigail - Tool for constructing, manipulating, serializing and de-serializing ABI-relevant artifacts
Summary: Review Request: libabigail - Tool for constructing, manipulating, serializin...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-01-14 18:02 UTC by Sinny Kumari
Modified: 2015-05-26 03:28 UTC (History)
10 users (show)

Fixed In Version: libabigail-1.0-0.1.20150422gita9582d8.fc22
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-04-25 06:17:50 UTC
Type: ---
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
First iteration on proposed changes to the libabigail package (9.77 KB, patch)
2015-01-15 10:05 UTC, Dodji Seketeli
no flags Details | Diff

Description Sinny Kumari 2015-01-14 18:02:52 UTC
Spec URL: https://raw.githubusercontent.com/sinnykumari/libabigail-package/master/spec/libabigail.spec

SRPM URL: https://github.com/sinnykumari/libabigail-package/raw/master/srpm/libabigail-1.0.0-1.fc21.src.rpm

Description: 
Libabigail aims at providing a C++ library for constructing, manipulating, serializing and de-serializing ABI-relevant artifacts. The set of artifacts is made of constructions like types, variables, functions 
and declarations of a given library or program. For a given program or library,
this set of constructions is called an ABI corpus. Provides library to
manipulate ABI corpora, compare them, provides detailed information about their 
differences and help build tools to infer interesting conclusions about these
difference.

This is my first package, and I would like to have this package included into  Fedora rawhide. I have also done scratch build of libabigail pacakge in koji which can be found at http://koji.fedoraproject.org/koji/taskinfo?taskID=8618114 .
It would be awesome if someone can sponsor me as well. I would be happy to have feedback/suggestion.

Fedora Account System Username: sinnykumari

Comment 1 Richard Shaw 2015-01-14 19:18:53 UTC
Ok, pardon my ignorance but as you work at RedHat, do you actually require sponsoring to be a Fedora packager or are you already sponsored?

Quick spec file review:
No major problems jump out at me so very good!

Minor nits:
1. It's preference, not required, but it's a good idea to list your BuildRequires on separate lines. About the only time I list multiple is if they are closely related (autoconf, automake) or let's say you have something that manipulates images then I could see having all the image devel libraries on one line.
2. Requires from the devel package should be arch specific, i.e.:
Requires: %{name} = %{version}-%{release}
to:
Requires: %{name}%{?_isa} = %{version}-%{release}
3. In install since your not doing anything out of the ordinary you could use the "%make_install" macro.
4. In %files unless your using non-default permissions you don't need %defattr.
5. Typically %doc is right under %files. It's not wrong to do so but it's unusual.

Comment 2 Rahul Sundaram 2015-01-14 19:31:18 UTC
Hi

In addition to the above comments, I have one more suggestion.  I see

%{_libdir}/libabigail.la

Refer to http://fedoraproject.org/wiki/Packaging:Guidelines

"Libtool archives, foo.la files, should not be included. Packages using libtool will install these by default even if you configure with --disable-static, so they may need to be removed before packaging. "

Do you need "autoreconf -i"?

Comment 3 Parag AN(पराग) 2015-01-15 08:56:36 UTC
(In reply to Richard Shaw from comment #1)
> Ok, pardon my ignorance but as you work at RedHat, do you actually require
> sponsoring to be a Fedora packager or are you already sponsored?

  Every new package contributor need to get sponsored in packager group irrespective of for which employer you are working. We need to make sure these new contributors knows rpm packaging that follows Fedora packaging guidelines.

Comment 4 Parag AN(पराग) 2015-01-15 09:37:58 UTC
Hi Sinny,
   We have this process http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group to get sponsored into the packager group. Can you either submit few more packages and/or some (4-5) full detailed package reviews? This is needed to make sure package submitter understands the rpm packaging well and follows the fedora packaging guidelines.

Please go through the following links
1) http://fedoraproject.org/wiki/Package_Review_Process

2) https://fedoraproject.org/wiki/PackagingGuidelines

3) To find the packages already submitted for review, check http://fedoraproject.org/PackageReviewStatus/

4) http://fedoraproject.org/wiki/Packaging:ReviewGuidelines is useful while doing package reviews.

5) https://fedorahosted.org/FedoraReview/ this is fedora-review tool to help review packages in fedora. You need to use this and do un-official package reviews of packages submitted by other contributors. While doing so mention "This is un-official review of the package." at top of your review comment.

Good to review packages listed in http://fedoraproject.org/PackageReviewStatus/NEW.html


If you got any questions please ask :)

Comment 5 Dodji Seketeli 2015-01-15 10:05:24 UTC
Created attachment 980401 [details]
First iteration on proposed changes to the libabigail package

Hey Sinny,

As Richard Shaw said in his comment, this looks like great work.
Thank you for that!

I also agree with his comments and I am attaching this patch that
addresses his comments. Namely:

> 1. It's preference, not required, but it's a good idea to list your
>    BuildRequires on separate lines. About the only time I list
>    multiple is if they are closely related (autoconf, automake) or
>    let's say you have something that manipulates images then I could
>    see having all the image devel libraries on one line.
>

I have updated the .spec file in the patch to address this.

> 2. Requires from the devel package should be arch specific, i.e.:
>    Requires: %{name} = %{version}-%{release}
>    to:
>    Requires: %{name}%{?_isa} = %{version}-%{release}

I have made this change too.

> 3. In install since your not doing anything out of the ordinary you
>    could use the "%make_install" macro.

I have made this change.  And I have also added a call to the
'install-man-and-info-doc' target of the Makefile to install the man
pages and info doc.

> 4. In %files unless your using non-default permissions you don't need
>    %defattr.

Fixed.

>
> 5. Typically %doc is right under %files. It's not wrong to
>    do so but it's unusual.

Fixed.

I am also addressing the comments from Rahul Sundaram:


> Libtool archives, foo.la files, should not be included

Removed.

> Do you need "autoreconf -i"?

If the source code tarball was made using make dist, then you are
right, "autoreconf -i" is not needed.  But in the changes I am
proposing here, I am falling back to just using "tar" to build the
source tarball from the git snapshot, so that the person who is
actually constructing the tarball doesn't need to have the autotools
installed just to re-construct the tarball.  And in that case, yes the
"autoreconf -i" is needed.  So I am keeping it in my proposed changes.

In addition to this, I am proposing the changes below to address some
more nits:

* As this packages a pre-release snapshot from git, specify the git
  commit in the Release TAG, as per
  http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages.

* In the comments of the Source0 tag, give more details about how to
  get that specific git commit from the upstream git repository.
  Also, just use a plain 'tar' command to construct the tarball,
  rather than requiring that the user runs autoreconf -i and then
  configure and then make dist.  I think it's nice to ease the work of
  whoever is going to be getting that source tarball.

* Be more factual in the %description of libabigail: introduce the
  actual command line tools that the package contains, a bit like what
  the diffutils package does.  The libabigail tools are not unliked
  the diffutils tools, but for the ABI of shared libraries; that is
  why I think it's interesting to take their wording as an example.
  Also, do not introduce the library itself in the %description of the
  libabigail package, because the library is not what the end-user
  installing the libabigail package would be actually *using*.  I'd
  rather introduce the library in the %description of the
  *libabigail-devel* package.

* Rename the libabigail-man sub-package into libabigail-doc and move
  all the documentation there, as there is quite a lot of
  documentation now (man, texinfo, apidoc, html manual).  This is as
  per http://fedoraproject.org/wiki/Packaging:Guidelines#Documentation
  which says:

      "if there's a lot of documentation, consider putting it into a
      subpackage. In this case, it is recommended to use *-doc as the
      subpackage name".

* Make sure the zip-archive feature is disabled as it can be
  automatically turned on if the libzip library is present at build
  time; and this feature is going to be deprecated in upstream.

* Add texinfo documentation generation at build time.

* Specifically install man page and texinfo documentation using the
  new upstream "install-man-and-info-doc" make target.  For the
  texinfo pages, add post (un)install scriptlets to update the global
  texinfo directory accordingly.

* If make check fails, cat tests/test-suite.log so that we can see
  what the details of the failings are in the output log of the build.
  This is extremely handy to spot and fix causes of test suite errors
  that happen when the package is built remotely on koji.

* Explicitly list the binary tools and libraries that are included in
  the package so that the person who updates the package can see when
  new upstream binaries are added and then choose to add them to the
  package or not.

Comment 6 Sinny Kumari 2015-01-15 12:31:55 UTC
Thanks everyone for feedback. 

Since most of changes are covered by Dodji in his patch (attachment under comments 5), I have updated spec file accordingly.

Updated spec and srpm are available at:
Spec URL: https://github.com/sinnykumari/libabigail-package/raw/master/spec/libabigail.spec
SRPM URL: https://github.com/sinnykumari/libabigail-package/raw/master/srpm/libabigail-1.0-0.1.git.63c81f0.fc21.src.rpm
Koji build link: http://koji.fedoraproject.org/koji/taskinfo?taskID=8623543

Meanwhile, I will also try reviewing other packages.

Comment 7 Michael Schwendt 2015-01-17 13:55:39 UTC
> %global checkout git.%{git_revision}
> Release: 0.1.%{checkout}%{?dist}

Fedora's packaging guidelines want you to include the checkout date her as a prefix:

  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages


> BuildRequires: gzip

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


> Requires: elfutils

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

TLDR: Add a comment that gives the rationale why this explicit Requires are necessary.


> %package -n libabigail-devel
> Provides: libabigail-devel = %{version}-%{release}

That's a very unusual explicit Provides you should delete. It's the same that's added by rpmbuild automatically! ;)


> %package -n libabigail-doc
> Provides: libabigail-doc = %{version}-%{release}

Same here.

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

Please keep Documentation packages completely separate from any such dependencies, so they can be installed without pulling in stuff that's not needed. Unless the documentation can only be displayed with a program included in a separate package. That's not true for HTML files, manual pages and Info pages.

Not shipping the section 7 manual pages in the same package as the tools themselves is a packaging bug.

Blocker: The license files are not included! They must be included in the base package (and preferably also in the separate -doc package to be complete):
https://fedoraproject.org/wiki/Packaging:Guidelines#Licensing


> %check

The section is executed _after_ %install, so it should be placed below %install in the spec file. (Btw, this is especially true, if the test-suite were to be run on %buildroot contents.)


> %{_infodir}/abigail.info.gz

Not a blocker, but just like manual files are included with a '*' wildcard suffix instead of ".gz", doing that also for Info files would be more flexible (with regard to disabling/customising the compression technique used by the build system).


> %post -n libabigail-doc
> /sbin/ldconfig

> %postun -n libabigail-doc
> /sbin/ldconfig

Why is ldconfig run here?


> https://kojipkgs.fedoraproject.org//work/tasks/3547/8623547/build.log

Build output is non-verbose. You cannot see whether Fedora's global compiler/linker flags are used, and you cannot easily verify what options are used during compilation:

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

Try passing V=1 to make, or configure with --disable-silent-rules, or look for extra build options, or patch the Makefile(s) if necessary.


=> Some more work on this package is needed.

Comment 8 Sinny Kumari 2015-01-19 09:49:12 UTC
Hi,

I Updated spec file according to feedback provided by Michael. Updated links are:
Spec Url - https://github.com/sinnykumari/libabigail-package/raw/master/spec/libabigail.spec
SRPM Url - https://github.com/sinnykumari/libabigail-package/raw/master/srpm/libabigail-1.0-0.1.git.20150114git63c81f0.fc21.src.rpm
Koji Build - http://koji.fedoraproject.org/koji/taskinfo?taskID=8660213

> Fedora's packaging guidelines want you to include the checkout date her as a
> prefix:

Done
> > BuildRequires: gzip
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

Removed, my mistake that I provided gzip as explicit BuidRequires

> > Requires: elfutils
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
> 
> TLDR: Add a comment that gives the rationale why this explicit Requires are
> necessary.

Yes, there is no need of elfutils in Require section. So, removed.

> > %package -n libabigail-devel
> > Provides: libabigail-devel = %{version}-%{release}
> 
> That's a very unusual explicit Provides you should delete. It's the same
> that's added by rpmbuild automatically! ;)
> 
> 
> > %package -n libabigail-doc
> > Provides: libabigail-doc = %{version}-%{release}

Thank you for pointing it out. Removed :)
 
> > Requires: %{name} = %{version}-%{release}
> 
> Please keep Documentation packages completely separate from any such
> dependencies, so they can be installed without pulling in stuff that's not
> needed. Unless the documentation can only be displayed with a program
> included in a separate package. That's not true for HTML files, manual pages
> and Info pages.

Yes, there is no need to keep main package as dependency for libabigail-doc package. Removed.

> Not shipping the section 7 manual pages in the same package as the tools
> themselves is a packaging bug.

Keeping man7 files in doc package in order to keep main package size minimal. It will be useful in case of running libabigail on smaller boxes.

> Blocker: The license files are not included! They must be included in the
> base package (and preferably also in the separate -doc package to be
> complete):
> https://fedoraproject.org/wiki/Packaging:Guidelines#Licensing

Added available License file in libabigail main and doc package.

> > %check
> 
> The section is executed _after_ %install, so it should be placed below
> %install in the spec file. (Btw, this is especially true, if the test-suite
> were to be run on %buildroot contents.)

Moved %check after %install
> 
> > %{_infodir}/abigail.info.gz
> 
> Not a blocker, but just like manual files are included with a '*' wildcard
> suffix instead of ".gz", doing that also for Info files would be more
> flexible (with regard to disabling/customising the compression technique
> used by the build system).

Done

> > %post -n libabigail-doc
> > /sbin/ldconfig
> 
> > %postun -n libabigail-doc
> > /sbin/ldconfig
> 
> Why is ldconfig run here?

Sorry, it was my mistake. Not needed as doc package doesn't install any shared library.

> > https://kojipkgs.fedoraproject.org//work/tasks/3547/8623547/build.log
> 
> Build output is non-verbose. You cannot see whether Fedora's global
> compiler/linker flags are used, and you cannot easily verify what options
> are used during compilation:
> 
>   https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
> 
> Try passing V=1 to make, or configure with --disable-silent-rules, or look
> for extra build options, or patch the Makefile(s) if necessary.

Added --disable-silent-rules option in %configure.

Thanks

Comment 9 Parag AN(पराग) 2015-01-23 07:30:01 UTC
Few more fixes required for above updated package.

1) When you prepare updated package its a good practice to do koji scratch build and if its a successful build then check rpmlint output for all generated rpm files. Or you can use fedora-review tool on your own updated package.

rpmlint output on all generated rpm files
libabigail.src: W: spelling-error %description -l en_US abidiff -> abiding
libabigail.src: W: spelling-error %description -l en_US abicompat -> compatible
libabigail.src: W: spelling-error %description -l en_US abidw -> abide
libabigail.src: W: spelling-error %description -l en_US abilint -> ability
libabigail.src: W: spelling-error %description -l en_US applicatipon -> application, supplication, applicator
libabigail.src: W: invalid-url Source0: libabigail-1.0.tar.gz
libabigail.x86_64: W: spelling-error %description -l en_US applicatipon -> application, supplication, applicator
libabigail.x86_64: W: incoherent-version-in-changelog 1.0-0.1.git.63c81f0 ['1.0-0.1.git.20150114git63c81f0.fc22', '1.0-0.1.git.20150114git63c81f0']
libabigail.x86_64: W: no-manual-page-for-binary abidiff
libabigail.x86_64: W: no-manual-page-for-binary abicompat
libabigail.x86_64: W: no-manual-page-for-binary abilint
libabigail.x86_64: W: no-manual-page-for-binary abidw
libabigail-devel.x86_64: W: only-non-binary-in-usr-lib
libabigail-devel.x86_64: W: no-documentation
libabigail-doc.x86_64: W: spelling-error Summary(en_US) texinfo -> tinfoil
libabigail-doc.x86_64: W: spelling-error Summary(en_US) html -> HTML, ht ml, ht-ml
libabigail-doc.x86_64: W: spelling-error %description -l en_US texinfo -> tinfoil
libabigail-doc.x86_64: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
libabigail-doc.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/libabigail-doc/_static/jquery.js
5 packages and 0 specfiles checked; 0 errors, 19 warnings.

=> You need to fix the changelog version-release to 
1.0-0.1.git.20150114git63c81f0

=> Then the wrong-file-end-of-line-encoding warning can be fixed by adding
BuildRequires: dos2unix
and at the end of %install
dos2unix doc/manuals/html/_static/jquery.js

Rest can be ignored.

2) I see you actually need libzip not gzip. You can add following
BuildRequires: libzip
to enable optional feature.

3) As per new guidelines https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text , you need to use %license macro for license file instead of %doc
%license COPYING-LGPLV3

4) Follow the texinfo guidelines http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Texinfo

You have missing Requires lines and need to use preun (not postun)

5) I had a look at source archive and it contains approximately 2.9MB of .git directory which we don't need. So change the source generation step like

tar -cvzf %%{name}-%%{version}.tar.gz %%{name}-%%{version} --exclude .git

and generate new tarball, use it and submit new package for further review.

6) Everytime you update the spec file, increase the release number tag (so next will be "0.2.%{checkout}%{?dist}" and add corresponding changes information in %changelog

Comment 10 Dodji Seketeli 2015-01-23 11:44:25 UTC
(In reply to Parag AN(पराग) from comment #9)

> 2) I see you actually need libzip not gzip. You can add following
> BuildRequires: libzip
> to enable optional feature.

Thank you for your attention to details, that is appreciated.

However, the optional feature you are talking about is meant *not* to be activated in the current context.  As the upstream maintainer of this package, I think it's a good thing to disable it.  I even plan to disable the feature by default upstream (even when libzip is installed) in the future.

So I think the BuildRequires: libzip should not be added.

Comment 11 Parag AN(पराग) 2015-01-23 12:52:38 UTC
Thanks Dodji. 

Sinny, I see there is no need then to add "BuildRequires: libzip"

Comment 12 Richard Shaw 2015-01-23 14:21:27 UTC
(In reply to Parag AN(पराग) from comment #9)
> => You need to fix the changelog version-release to 
> 1.0-0.1.git.20150114git63c81f0

The first git is redundant, just use:
%global checkout %{date}git%{git_revision}

 
> 5) I had a look at source archive and it contains approximately 2.9MB of
> .git directory which we don't need. So change the source generation step like
> 
> tar -cvzf %%{name}-%%{version}.tar.gz %%{name}-%%{version} --exclude .git
> 
> and generate new tarball, use it and submit new package for further review.

Probably easier to use "git archvie..." at this point. Something like:

# This tarball was constructed from pulling the source code of
# libabigail from its Git repository by doing:
#    git clone git://sourceware.org/git/libabigail.git
#    pushd libabigail
#    git archive --prefix %%{name}-%%{version}/ -o %%{name}-%%{version}.tar.gz %%{git_revision}

Comment 13 Sinny Kumari 2015-01-23 14:33:36 UTC
Hi,

Thank you for your feedback. I am making changes into spec file according to your feedback. Have one question though (mentioned in inline comment)
(In reply to Parag AN(पराग) from comment #9)
> Few more fixes required for above updated package.

> 6) Everytime you update the spec file, increase the release number tag (so
> next will be "0.2.%{checkout}%{?dist}" and add corresponding changes
> information in %changelog

Yes, I am aware of updating release number when any modification is done in spec file. But, currently this package is under review and no non-scratch build has been done so far. So, is it really needed now? Asking it because I couldn't find answer to it in Fedora wiki.

Thanks

Comment 14 Parag AN(पराग) 2015-01-23 15:14:11 UTC
(In reply to Richard Shaw from comment #12)
> (In reply to Parag AN(पराग) from comment #9)
> > => You need to fix the changelog version-release to 
> > 1.0-0.1.git.20150114git63c81f0
> 
> The first git is redundant, just use:
> %global checkout %{date}git%{git_revision}
> 

You are right. My eyes missed that.

>  
> > 5) I had a look at source archive and it contains approximately 2.9MB of
> > .git directory which we don't need. So change the source generation step like
> > 
> > tar -cvzf %%{name}-%%{version}.tar.gz %%{name}-%%{version} --exclude .git
> > 
> > and generate new tarball, use it and submit new package for further review.
> 
> Probably easier to use "git archvie..." at this point. Something like:
> 
> # This tarball was constructed from pulling the source code of
> # libabigail from its Git repository by doing:
> #    git clone git://sourceware.org/git/libabigail.git
> #    pushd libabigail
> #    git archive --prefix %%{name}-%%{version}/ -o
> %%{name}-%%{version}.tar.gz %%{git_revision}

Not tested but looks this can also work fine.



(In reply to Sinny Kumari from comment #13)
> Hi,
> 
> Thank you for your feedback. I am making changes into spec file according to
> your feedback. Have one question though (mentioned in inline comment)
> (In reply to Parag AN(पराग) from comment #9)
> > Few more fixes required for above updated package.
> 
> > 6) Everytime you update the spec file, increase the release number tag (so
> > next will be "0.2.%{checkout}%{?dist}" and add corresponding changes
> > information in %changelog
> 
> Yes, I am aware of updating release number when any modification is done in
> spec file. But, currently this package is under review and no non-scratch
> build has been done so far. So, is it really needed now? Asking it because I
> couldn't find answer to it in Fedora wiki.
> 

Yes you do need to update release number. Many new people asks same question but we need to track how the package got updated since its initial submission. This also helps other people to track how this package is reviewed and approved.

I generally don't ask people if its a minor update but in your case there are many changes. Just go through already done package reviews in bugzilla and you will find people did update release number everytime they submitted new changes. You may want to check reviews that got progressed since its submission -> http://fedoraproject.org/PackageReviewStatus/NEEDSPONSOR.html or http://fedoraproject.org/PackageReviewStatus/REVIEW.html

Also, this is needed in case when new update fails to build/work we can go to previous changes and find the issues in new update. 

I too can't find any reference for this in fedora wiki.

Comment 15 Sinny Kumari 2015-01-23 19:05:44 UTC
Updated spec file and also ran basic checks on generated packages. Updated spec and rpms are:
SPEC url - https://github.com/sinnykumari/libabigail-package/raw/master/spec/libabigail.spec
SRPM url - https://github.com/sinnykumari/libabigail-package/raw/master/srpm/libabigail-1.0-0.2.20150114git63c81f0.fc21.src.rpm
Koji Build - http://koji.fedoraproject.org/koji/taskinfo?taskID=8708846


(In reply to Parag AN(पराग) from comment #9)
> Few more fixes required for above updated package.
> 
> 1) When you prepare updated package its a good practice to do koji scratch
> build and if its a successful build then check rpmlint output for all
> generated rpm files. Or you can use fedora-review tool on your own updated
> package.
Yes, rpmlint ran on generated package

> => You need to fix the changelog version-release to 
> 1.0-0.1.git.20150114git63c81f0

Fixed

> => Then the wrong-file-end-of-line-encoding warning can be fixed by adding
> BuildRequires: dos2unix
> and at the end of %install
> dos2unix doc/manuals/html/_static/jquery.js

Added dos2unix

> 3) As per new guidelines
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text ,
> you need to use %license macro for license file instead of %doc
> %license COPYING-LGPLV3

Fixed

> 4) Follow the texinfo guidelines
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Texinfo
>
> You have missing Requires lines and need to use preun (not postun)

Using preun instead of postun.
Not adding 
Requires(post): info
Requires(preun): info
because this dependency should be handled by yum. 

> 5) I had a look at source archive and it contains approximately 2.9MB of
> .git directory which we don't need. So change the source generation step like
> 
> tar -cvzf %%{name}-%%{version}.tar.gz %%{name}-%%{version} --exclude .git
> 
> and generate new tarball, use it and submit new package for further review.

Thank you for telling about it. I really didn't notice it. Fixed.
 
> 6) Everytime you update the spec file, increase the release number tag (so
> next will be "0.2.%{checkout}%{?dist}" and add corresponding changes
> information in %changelog

Updated Release number tag and %changelog information

(In reply to Richard Shaw from comment #12)
> (In reply to Parag AN(पराग) from comment #9)
> > => You need to fix the changelog version-release to 
> > 1.0-0.1.git.20150114git63c81f0
> 
> The first git is redundant, just use:
> %global checkout %{date}git%{git_revision}

Fixed

> Probably easier to use "git archvie..." at this point. Something like:
> 
> # This tarball was constructed from pulling the source code of
> # libabigail from its Git repository by doing:
> #    git clone git://sourceware.org/git/libabigail.git
> #    pushd libabigail
> #    git archive --prefix %%{name}-%%{version}/ -o
> %%{name}-%%{version}.tar.gz %%{git_revision}

Yes, using it to generate archive and it works absolutely fine :)

Thanks

Comment 16 Parag AN(पराग) 2015-01-24 13:28:49 UTC
1) Sinny, you are still not following correct %changelog guidelines. Please read http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs page. I don't see since last release what you have changed.

2) Currently mock is failing for the package you submitted as the newer python-sphinx change has not yet published. The last rawhide published was on 23rd Jan. So, we need to wait till next rawhide is available that will add new package python-sphinx-latex.

3) I don't think info gets pulled automatically by yum. I removed info package from the system and tried to install locally built libabigail package and can see info package is not installed automatically.
So, add those requires in -doc subpackage only. 

4) One more think I found. Generally I have not seen people specifying full subpackage names where the name starts with the main package name. I don't think you need to follow the subpackage names like "-n <pkgname>" where all subpackages start with the main package name. For example for devel subpackage, you can just write as

%package devel
%description devel
%files devel

See http://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch10s04.html

Comment 17 Sinny Kumari 2015-01-24 15:34:50 UTC
Updated spec, srpm and koji build
SPEC Url - https://github.com/sinnykumari/libabigail-package/raw/master/spec/libabigail.spec
SRPM Url - https://github.com/sinnykumari/libabigail-package/raw/master/srpm/libabigail-1.0-0.3.20150114git63c81f0.fc21.src.rpm
Koji build - http://koji.fedoraproject.org/koji/taskinfo?taskID=8718756

(In reply to Parag AN(पराग) from comment #16)
> 1) Sinny, you are still not following correct %changelog guidelines. Please
> read http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs page. I
> don't see since last release what you have changed.

Sorry about not using new changelog entry after each update in spec file. I was updating same changelog on every update because I don't wanted to have lot of changelog entry being added before package being actually build. Now, I am adding new entry for every spec file update. Also, will follow same for future.

> 2) Currently mock is failing for the package you submitted as the newer
> python-sphinx change has not yet published. The last rawhide published was
> on 23rd Jan. So, we need to wait till next rawhide is available that will
> add new package python-sphinx-latex.

When can I expect next rawhide being available?

> 3) I don't think info gets pulled automatically by yum. I removed info
> package from the system and tried to install locally built libabigail
> package and can see info package is not installed automatically.
> So, add those requires in -doc subpackage only. 

I am using Fedora KDE and in my system when I tried to remove already installed info package, dependency was in such a way that it was trying to remove even systemd and emoval fails. Due to that I didn't add explicit requires as info for doc package. It may not be case for other desktop environment on which you tested. So, keeping in mind I have added info as explicit requires. 
 
> 4) One more think I found. Generally I have not seen people specifying full
> subpackage names where the name starts with the main package name. I don't
> think you need to follow the subpackage names like "-n <pkgname>" where all
> subpackages start with the main package name. For example for devel
> subpackage, you can just write as

Fixed

Comment 18 Parag AN(पराग) 2015-01-24 16:02:42 UTC
I think there are some issues going on with daily rawhide compose. Maybe we can expect them to be fixed by Monday. I will further review this package then.

Meanwhile I am not sure if you have started doing package reviews. You need to do informal package reviews. Just based on one package submission and its package review you will not get sponsorship. 

When you do full package review of some packages, provide that review comment here so that I can look how you have reviewed those packages.

Comment 19 Sinny Kumari 2015-01-24 17:48:40 UTC
(In reply to Parag AN(पराग) from comment #18)
> I think there are some issues going on with daily rawhide compose. Maybe we
> can expect them to be fixed by Monday. I will further review this package
> then.

Ok, sure

> Meanwhile I am not sure if you have started doing package reviews. You need
> to do informal package reviews. Just based on one package submission and its
> package review you will not get sponsorship. 
> 
> When you do full package review of some packages, provide that review
> comment here so that I can look how you have reviewed those packages.

I did informal package review for hypre package https://bugzilla.redhat.com/show_bug.cgi?id=1176595 (Comment 4 and 6). I will try reviewing few more packages as well and will update links here.

Thanks

Comment 20 Sinny Kumari 2015-01-28 09:17:59 UTC
(In reply to Parag AN(पराग) from comment #18)
> I think there are some issues going on with daily rawhide compose. Maybe we
> can expect them to be fixed by Monday. I will further review this package
> then.

I think latest python-sphinx is available in rawhide now. It would be great if further feedback is given to this package.

Thanks

Comment 21 Sinny Kumari 2015-01-28 11:14:26 UTC
Hi,

Another package review which I did is https://bugzilla.redhat.com/show_bug.cgi?id=1162234#c5 for nss-securepass  package.

Also I helped in re-building plasma-mediacenter package for version 1.3.0 as seen from Changelog http://koji.fedoraproject.org/koji/buildinfo?buildID=541535 .

Comment 22 Parag AN(पराग) 2015-01-29 06:17:02 UTC
Thanks for doing the review of nss-securepass but it looks to me not complete. You need to do full package reviews using fedora-review tool and mark all the items. Please read the packaging guidelines pages, understand them and provide your suggestions in the package reviews.

About this package review, the last update in comment#17 looks good to get approved. I will now wait for your full package reviews where I can see you find packaging issues and provided fixes in your review comment.

Comment 23 Parag AN(पराग) 2015-01-30 03:38:07 UTC
Sinny, Also, Can you submit few more packages also? That way we can know more about your packaging work and how well you are understanding packaging guidelines.

Comment 24 Dodji Seketeli 2015-01-30 10:01:01 UTC
Quite frankly, with all (In reply to Parag AN(पराग) from comment #23)
> Sinny, Also, Can you submit few more packages also? That way we can know
> more about your packaging work and how well you are understanding packaging
> guidelines.

Quite frankly, with all due respect, I find it almost ridiculous to ask for submitting several packages before getting the inclusion into Rawhide, while what the packager really is interested in is one particular package.

I understand that you are after quality here, but I think you can see the motivation and the ability to grasp packaging concepts from potential Fedora contributors without alienating them with superfluous requests like this.  I believe that is a skill mentors and sponsors should thrive to acquire too.

I know I am not a sponsor myself but really, in the interest of the Fedora Project, I think we shouldn't keep asking folks to submit package B, C, D when they are only interested in package A.  Just asking them to review other packages should be enough.  And even in that case, it shouldn't be a hard pre-requisite, IMHO.

Cheers.

Comment 25 Michael Schwendt 2015-01-30 11:37:07 UTC
If you're unhappy with the sponsoring process, please start a thread on devel@ list and not here in bugzilla. The process is not perfect, but there are several pitfalls awaiting both the sponsors and the new contributors.

Comment 26 Sinny Kumari 2015-01-30 11:43:22 UTC
Hi,

(In reply to Parag AN(पराग) from comment #23)
> Sinny, Also, Can you submit few more packages also? That way we can know
> more about your packaging work and how well you are understanding packaging
> guidelines.

Yes I am interested in submitting other packages too which I have in mind but I would like to do that once my first package get added to Fedora rawhide and I have free time. From my first packaging experience I understand that it takes effort and time to work on one package and due to which I would like to dedicate my time in one good quality packaging rather than  opening multiple packages for review request.

I will be glad to do review of other packages and which I did too for two packages (nss-securepass and hypre). As per you suggestion I will do complete review of nss-securepass soon.

Thanks

Comment 27 Parag AN(पराग) 2015-01-30 14:27:26 UTC
Since this package has been submitted I am seeing some kind of rush to get this package ASAP in fedora. If that is the case why not let upstream developer Dodji submit this package himself? I see Dodji is already a packager. Also, I am not in support of providing the patches that fixes the packaging in the package review. And this is the first package from new contributor/packager. If you give readymade patch and contributor just uses it and provide it as an update how can we find if packager really understood what was packaging mistakes in previous submission?

Sorry, Just based on one package submission and two incomplete peer-reviews, I am unable to sponsor this package. If you can contribute more packages or more full peer-reviews I may still sponsor this package.

Sinny, I think you are still not familiar with fedora-review tool. If you are having any problem using it then ask here.

Please go through also http://fedoraproject.org/wiki/Package_Review_Process#Reviewer

Comment 28 Sinny Kumari 2015-01-30 15:27:26 UTC
There is no rush to get this package into Fedora. I am sure that its very easy for Dodji to submit this package himself. Apparently, it was a bad idea on my part to think that it will be a nice to learn Fedora packaging, given that its preferred for only existing packagers to submit packages.

Anyway, there is another thing I want to understand-
1. For the first package to be submitted, a sponsor is mandatory
2. For having a sponsor, more than one package to be submitted

I'm a bit confused by this because 1 requires 2 and 2 requires 1 back. I do not understand how do I submit my second package while my first one is still incomplete. While I request some clarity on that, I will definitely try to do more peer-reviews.

I have read the docs about the fedora-review tool, and that is what I am currently trying to understand and use. It is feeling quite overwhelming at first so I need more time on that.

Thanks.

Comment 29 Parag AN(पराग) 2015-01-30 15:55:07 UTC
You can submit at any time more than one package but the first package to get reviewed/approved must be by a Sponsor packager and rest all can be by any other packager.

See http://fedoraproject.org/PackageReviewStatus/NEEDSPONSOR.html page which can also show you that people do submit more than one package.

So take your time, get familiar with this fedora review process, provide full quality reviews and get sponsorship.

Comment 30 Michael Schwendt 2015-01-31 10:48:07 UTC
Some hints:

* When offering more than one package and/or a few reviews, you give your potential sponsor(s) more material to examine. That's a good thing. Some sponsors take over the reviewing of _all_ your initial packages. Some will watch/observe your activity in bugzilla anyway and may join with additional comments/guidance.

* Many "new" packagers focus on making a package build on their local system and then stop there. So far, so good. Fedora's packaging guidelines are complex and really can be overwhelming. No doubt about that. They also address lots of issues, which can be considered "minor" only. If someone misses such details, it doesn't necessarily make a package BAD. However, it bears a risk that a packager (re)introduces packaging mistakes because of not looking up the guidelines. On the contrary, the guidelines are not complete either. There are lots of packaging pitfalls not covered by them.

* Where to start? There's this page, which is like a checklist:

  https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

Read it and try to review *your own* package(s) based on the list of MUST/SHOULD items.

* The  fedora-review  tool automates several checks, especially some that can be important, such as checking licensing of source files. It is there to help you. Concentrate on any real errors it reports, also in the rpmlint section. Examine any warnings and ask about them, if there are questions:

  https://fedoraproject.org/wiki/Common_Rpmlint_issues

* After approval of a package, it can get much more difficult depending on how familiar you are with the source code and how much support you get from the upstream authors. There are packagers, who manage to push a package through the review process, but who abandon the package as soon as the first problem reports come in via bugzilla and need activity, such as debugging, patching, forwarding things upstream.

* And probably most important, you need to know where to search for information or where to ask for help. A good entry-point in the Wiki for packagers is this:

  https://fedoraproject.org/wiki/Category:Package_Maintainers

Comment 31 Sinny Kumari 2015-01-31 18:06:51 UTC
Thanks Parag and Michael for helping me with understanding minute details of Fedora packaging.

In order to do complete Fedora-review, I am starting it with my own package and would like to get feedback if I have done complete review of package correctly. Once I get go ahead I will do complete review of other packages as well and will post review link here for easy reference.

Fedora-review of libabigail package 
-----------------------------------

* Fedora review shows one Failed item as [!]: Uses parallel make %{?_smp_mflags} macro. But, libabigail spec file already uses this macro.
* I have done manual review as well. Rest looks good to me.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed

===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

Generic:
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Uses parallel make %{?_smp_mflags} macro.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Package should not use obsolete m4 macros
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: libabigail-1.0-0.3.20150114git63c81f0.fc21.x86_64.rpm
          libabigail-devel-1.0-0.3.20150114git63c81f0.fc21.x86_64.rpm
          libabigail-doc-1.0-0.3.20150114git63c81f0.fc21.x86_64.rpm
          libabigail-1.0-0.3.20150114git63c81f0.fc21.src.rpm
libabigail.x86_64: W: spelling-error %description -l en_US applicatipon -> application, supplication, applicator
libabigail.x86_64: W: no-manual-page-for-binary abidiff
libabigail.x86_64: W: no-manual-page-for-binary abicompat
libabigail.x86_64: W: no-manual-page-for-binary abilint
libabigail.x86_64: W: no-manual-page-for-binary abidw
libabigail-devel.x86_64: W: only-non-binary-in-usr-lib
libabigail-devel.x86_64: W: no-documentation
libabigail-doc.x86_64: W: spelling-error Summary(en_US) texinfo -> tinfoil
libabigail-doc.x86_64: W: spelling-error Summary(en_US) html -> HTML, ht ml, ht-ml
libabigail-doc.x86_64: W: spelling-error %description -l en_US texinfo -> tinfoil
libabigail-doc.x86_64: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
libabigail.src: W: spelling-error %description -l en_US abidiff -> abiding
libabigail.src: W: spelling-error %description -l en_US abicompat -> compatible
libabigail.src: W: spelling-error %description -l en_US abidw -> abide
libabigail.src: W: spelling-error %description -l en_US abilint -> ability
libabigail.src: W: spelling-error %description -l en_US applicatipon -> application, supplication, applicator
libabigail.src: W: invalid-url Source0: libabigail-1.0.tar.gz
4 packages and 0 specfiles checked; 0 errors, 17 warnings.




Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Requires
--------
libabigail-doc (rpmlib, GLIBC filtered):
    /bin/sh
    info

libabigail (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libabigail.so.0()(64bit)
    libc.so.6()(64bit)
    libdw.so.1()(64bit)
    libdw.so.1(ELFUTILS_0.122)(64bit)
    libdw.so.1(ELFUTILS_0.126)(64bit)
    libdw.so.1(ELFUTILS_0.143)(64bit)
    libdw.so.1(ELFUTILS_0.148)(64bit)
    libelf.so.1()(64bit)
    libelf.so.1(ELFUTILS_1.0)(64bit)
    libelf.so.1(ELFUTILS_1.1.1)(64bit)
    libelf.so.1(ELFUTILS_1.2)(64bit)
    libelf.so.1(ELFUTILS_1.5)(64bit)
    libelf.so.1(ELFUTILS_1.6)(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libm.so.6()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libxml2.so.2()(64bit)
    libxml2.so.2(LIBXML2_2.4.30)(64bit)
    libxml2.so.2(LIBXML2_2.5.0)(64bit)
    libxml2.so.2(LIBXML2_2.5.7)(64bit)
    libxml2.so.2(LIBXML2_2.6.0)(64bit)
    rtld(GNU_HASH)

libabigail-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libabigail(x86-64)
    libabigail.so.0()(64bit)
    pkgconfig(libxml-2.0)



Provides
--------
libabigail-doc:
    libabigail-doc
    libabigail-doc(x86-64)

libabigail:
    libabigail
    libabigail(x86-64)
    libabigail.so.0()(64bit)

libabigail-devel:
    libabigail-devel
    libabigail-devel(x86-64)
    pkgconfig(libabigail)



Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14
Command line :/bin/fedora-review -n libabigail
Buildroot used: fedora-21-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

Comment 32 Michael Schwendt 2015-01-31 22:32:27 UTC
fedora-review likely refers to the other "make" invocations:

  make html-doc
  pushd manuals
  make html-doc
  make man
  make info

Whether any such extra make targets would benefit from parallel make jobs is up to the packager:
  https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make

The biggest gain usually is with the compilation step when it's dozens or hundreds of source files to be compiled.

Comment 33 Sinny Kumari 2015-02-01 06:03:30 UTC
I generated html docs with and without parallel make. There is no difference in time consumption. So, I will skip adding %{?_smp_mflags} flag in doc generation. I hope rest reviews are fine. Now, working on complete review of other packages.

Comment 34 Sinny Kumari 2015-02-04 19:24:20 UTC
Hi,

I did full package review of nss-securepass package as per my understanding https://bugzilla.redhat.com/show_bug.cgi?id=1162234#c15 . It would be great if someone can verify it and also add issues/suggestion if something I missed out. I am happy to improve my packaging skill in this way.

Thanks

Comment 35 Parag AN(पराग) 2015-03-04 04:53:17 UTC
Almost one month since above comment and I see only one package review done (rh#1162234). If I see more peer package reviews done, I will sponsor this package :)

Comment 36 Sinny Kumari 2015-03-05 04:32:39 UTC
Hi Parag,

I was looking for another package which can be packaged for Fedora. I looked into http://fedoraproject.org/wiki/Package_maintainers_wishlist for help and also other resources to find out if something is unpackaged and can be relevant to add into Fedora. Package maintainers wishlist wiki page is quite outdated and it took some time to figure out relevant unpackaged project from it. Right now I am working on packaging Grabserial (http://elinux.org/Grabserial) project for adding it into Fedora repository. I will soon send package review for same.

Comment 37 Sinny Kumari 2015-03-16 09:34:31 UTC
I have added python-grabserial (BZ#1202265) package for review. Please provide feedback on that package as well.

Thanks

Comment 38 Sinny Kumari 2015-03-30 11:45:49 UTC
Hi,

Can I get sponsorship now in order to build this package for Fedora rawhide or still more works need to be done from my side?

Thanks

Comment 39 Parag AN(पराग) 2015-03-30 13:22:01 UTC
Correct me if I am wrong but I see your total package submission is 2. Your informal package reviews are 2 which includes a single full package review yet.

I was waiting for more package reviews from you. I have already given what I needed and that is not too much I guess ;-)

I will do your another packager review.

Comment 40 Sinny Kumari 2015-04-12 10:41:57 UTC
(In reply to Parag AN(पराग) from comment #39)
> Correct me if I am wrong but I see your total package submission is 2. Your
> informal package reviews are 2 which includes a single full package review
> yet.

Yes, you are right.

Did another review of package drumgizmo https://bugzilla.redhat.com/show_bug.cgi?id=1210356 .

Thanks

Comment 41 Sinny Kumari 2015-04-12 18:42:25 UTC
Done couple of more package reviews-
1. qmasterpassword - https://bugzilla.redhat.com/show_bug.cgi?id=1193878
2. GBall - https://bugzilla.redhat.com/show_bug.cgi?id=1173846

Comment 42 Sinny Kumari 2015-04-13 20:10:51 UTC
Did another review of package laszip - https://bugzilla.redhat.com/show_bug.cgi?id=1199296

Comment 43 Parag AN(पराग) 2015-04-16 08:43:06 UTC
Let's review your last submission of this package again. I assume your current package links are
SPEC URL: https://raw.githubusercontent.com/sinnykumari/libabigail-package/master/spec/libabigail.spec
SRPM URL: https://raw.githubusercontent.com/sinnykumari/libabigail-package/master/srpm/libabigail-1.0-0.3.20150114git63c81f0.fc21.src.rpm

Comment 44 Sinny Kumari 2015-04-16 19:46:13 UTC
(In reply to Parag AN(पराग) from comment #43)
> Let's review your last submission of this package again. I assume your
> current package links are
> SPEC URL:
> https://raw.githubusercontent.com/sinnykumari/libabigail-package/master/spec/
> libabigail.spec
> SRPM URL:
> https://raw.githubusercontent.com/sinnykumari/libabigail-package/master/srpm/
> libabigail-1.0-0.3.20150114git63c81f0.fc21.src.rpm

Yes, these are the latest spec and srpm. I haven't done any changes after last review done to it. If any thing still missing with this package, will be happy to fix it. I will check once again if it builds correctly with rawhide and will paste koji build link here.

Comment 45 Sinny Kumari 2015-04-17 02:54:03 UTC
Hi,

Package builds successfully for rawhide in koji - http://koji.fedoraproject.org/koji/taskinfo?taskID=9494573

According to me package looks good.

Comment 46 Parag AN(पराग) 2015-04-17 12:00:24 UTC
Few changes I will suggest which is common practice

1) static library files generally removed in %install section so no need to remove them using exclude in %files
We pass the --disable-static to %configure, this will not generate .a file
Then we need to add below %make_install following
find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';'

2) you should write all %post %postun scriptlets before %files section

3) BuildRequires: python-sphinx-latex is not required as that package got removed from repository now.


Rest looks fine. Submit new srpm and spec by increasing release to -4 and adding changelog. I will approve this package.

Comment 47 Michael Schwendt 2015-04-17 13:00:03 UTC
> 1)

Note that .la files are libtool archives and are not related to static libs. It's just Fedora's guidelines where both .a and .la are covered in the same section.


> 2)

Nothing in the guidelines suggests that. Packagers are free to put the scriptlet sections where they want.   *If* there's something to consider related to scriptlet sections, avoid comments in the spec file below such sections. The comments would be included in the scriptlet body, which becomes a problem when the script is not executed via /bin/sh (e.g. the ldconfig scriptlets).


> %license COPYING-LGPLV3

https://www.gnu.org/licenses/gpl-faq.html#v3HowToUpgrade

Please approach the upstream author about the second sentence in the section of the FAQ. The current comment in the file "COPYING" is just lame.

Comment 48 Parag AN(पराग) 2015-04-17 13:29:00 UTC
(In reply to Michael Schwendt (Fedora Packager Sponsors Group) from comment #47)
> > 1)
> 
> Note that .la files are libtool archives and are not related to static libs.
> It's just Fedora's guidelines where both .a and .la are covered in the same
> section.
> 

Yes you are right. I mixed the sentence when I wrote it.

> 
> > 2)
> 
> Nothing in the guidelines suggests that. Packagers are free to put the
> scriptlet sections where they want.   *If* there's something to consider
> related to scriptlet sections, avoid comments in the spec file below such
> sections. The comments would be included in the scriptlet body, which
> becomes a problem when the script is not executed via /bin/sh (e.g. the
> ldconfig scriptlets).
> 

I will still prefer this cosmetic change for easier readability.

> 
> > %license COPYING-LGPLV3
> 
> https://www.gnu.org/licenses/gpl-faq.html#v3HowToUpgrade
> 
> Please approach the upstream author about the second sentence in the section
> of the FAQ. The current comment in the file "COPYING" is just lame.

I suppose you mean here the effective license should be mentioned in COPYING right?

Comment 49 Michael Schwendt 2015-04-17 17:02:54 UTC
Let me quote:

| If you're using LGPLv3 in your project, be sure to include copies
| of both GPLv3 and LGPLv3, since LGPLv3 is now written as a set of
| additional permissions on top of GPLv3.

Comment 50 Sinny Kumari 2015-04-22 09:44:42 UTC
Thanks for the feedback and providing me sponsorship as a packager.

I have made all changes mentioned in comment#46 .

Now, latest package contains both license file COPYING-GPLV3 and COPYING-LGPLV3. Also COPYING file contains appropriate license text. Additionally, now source tar has been updated to latest git commit a9582d8 because license file being updated in latest commit.

Latest spec and srpm are - 
SPEC - https://github.com/sinnykumari/libabigail-package/raw/master/spec/libabigail.spec
SRPM - https://github.com/sinnykumari/libabigail-package/raw/master/srpm/libabigail-1.0-0.1.20150422gita9582d8.fc21.src.rpm
Koji build - http://koji.fedoraproject.org/koji/taskinfo?taskID=9535453

Checked fedora-review tool and rpmlint for error, it looks fine.

Rpmlint
-------
Checking: libabigail-1.0-0.1.20150422gita9582d8.fc21.x86_64.rpm
          libabigail-devel-1.0-0.1.20150422gita9582d8.fc21.x86_64.rpm
          libabigail-doc-1.0-0.1.20150422gita9582d8.fc21.x86_64.rpm
          libabigail-1.0-0.1.20150422gita9582d8.fc21.src.rpm
libabigail.x86_64: W: no-manual-page-for-binary abidiff
libabigail.x86_64: W: no-manual-page-for-binary abicompat
libabigail.x86_64: W: no-manual-page-for-binary abilint
libabigail.x86_64: W: no-manual-page-for-binary abidw
libabigail-devel.x86_64: W: only-non-binary-in-usr-lib
libabigail-devel.x86_64: W: no-documentation
libabigail-doc.x86_64: W: spelling-error Summary(en_US) texinfo -> tinfoil
libabigail-doc.x86_64: W: spelling-error Summary(en_US) html -> HTML, ht ml, ht-ml
libabigail-doc.x86_64: W: spelling-error %description -l en_US texinfo -> tinfoil
libabigail-doc.x86_64: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
libabigail.src: W: spelling-error %description -l en_US abidiff -> abiding
libabigail.src: W: spelling-error %description -l en_US abicompat -> compatible
libabigail.src: W: spelling-error %description -l en_US abidw -> abide
libabigail.src: W: spelling-error %description -l en_US abilint -> ability
libabigail.src: W: invalid-url Source0: libabigail-1.0.tar.gz
4 packages and 0 specfiles checked; 0 errors, 15 warnings.

Comment 51 Parag AN(पराग) 2015-04-24 05:16:54 UTC
As the required license text files are added, spec looks good now.

APPROVED this package.

Please proceed with step 8 from https://fedoraproject.org/wiki/Package_Review_Process#Contributor

Be noted that when you add SCM request you don't need to specify devel/master/rawhide/f23 branch, it will be created by default for your package.

Comment 52 Sinny Kumari 2015-04-24 06:29:11 UTC
Thank you!

Comment 53 Sinny Kumari 2015-04-24 07:17:05 UTC
New Package SCM Request
=======================
Package Name: libabigail
Short Description: Set of ABI analysis tools and library
Upstream URL: https://sourceware.org/libabigail/
Owners: sinnykumari dodji
Branches: f21 f22 epel7 el6
InitialCC: sinnykumari dodji

Comment 54 Gwyn Ciesla 2015-04-24 12:37:22 UTC
Git done (by process-git-requests).

Comment 55 Fedora Update System 2015-04-29 11:27:22 UTC
libabigail-1.0-0.1.20150422gita9582d8.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/libabigail-1.0-0.1.20150422gita9582d8.fc22

Comment 56 Fedora Update System 2015-04-29 11:31:33 UTC
libabigail-1.0-0.1.20150422gita9582d8.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/libabigail-1.0-0.1.20150422gita9582d8.fc21

Comment 57 Fedora Update System 2015-04-29 11:33:15 UTC
libabigail-1.0-0.1.20150422gita9582d8.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/libabigail-1.0-0.1.20150422gita9582d8.el7

Comment 58 Fedora Update System 2015-05-14 20:04:55 UTC
libabigail-1.0-0.1.20150422gita9582d8.el7 has been pushed to the Fedora EPEL 7 stable repository.

Comment 59 Fedora Update System 2015-05-15 13:35:15 UTC
libabigail-1.0-0.1.20150422gita9582d8.fc21 has been pushed to the Fedora 21 stable repository.

Comment 60 Fedora Update System 2015-05-26 03:28:11 UTC
libabigail-1.0-0.1.20150422gita9582d8.fc22 has been pushed to the Fedora 22 stable repository.


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