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
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.
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"?
(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.
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 :)
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.
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.
> %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.
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
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
(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.
Thanks Dodji. Sinny, I see there is no need then to add "BuildRequires: libzip"
(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}
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
(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.
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
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
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
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.
(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
(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
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 .
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.
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 (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.
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.
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
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
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.
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.
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
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
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.
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.
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
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 :)
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.
I have added python-grabserial (BZ#1202265) package for review. Please provide feedback on that package as well. Thanks
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
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.
(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
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
Did another review of package laszip - https://bugzilla.redhat.com/show_bug.cgi?id=1199296
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
(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.
Hi, Package builds successfully for rawhide in koji - http://koji.fedoraproject.org/koji/taskinfo?taskID=9494573 According to me package looks good.
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.
> 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.
(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?
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.
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.
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.
Thank you!
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
Git done (by process-git-requests).
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
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
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
libabigail-1.0-0.1.20150422gita9582d8.el7 has been pushed to the Fedora EPEL 7 stable repository.
libabigail-1.0-0.1.20150422gita9582d8.fc21 has been pushed to the Fedora 21 stable repository.
libabigail-1.0-0.1.20150422gita9582d8.fc22 has been pushed to the Fedora 22 stable repository.