Bug 507660
Summary: | Review Request: xylib - Library for reading x-y data from several file formats | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Marcin Wojdyr <wojdyr> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | andrew, fedora-package-review, mtasaka, notting, susi.lehtola, wojdyr |
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 0.4-4.fc11 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-07-23 16:03:36 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 511758 |
Description
Marcin Wojdyr
2009-06-23 16:53:03 UTC
Hi Marcin, I am not able to sponsor your package but I can hopefully offer some useful help. * BuildRequires: gcc-c++ Is does not have to be declared - http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 * Timestamps You should consider using use of INSTALL="install -p" when you make install Hi Andrew, thanks for your comments. I added gcc-c++ to build the same RPM on OpenSuse Build Service. I know that it's not necessary (on Fedora), although it's not clear to me if having it is allowed or not. When I prepared this RPM I looked at a few other RPMS in Fedora. Some of them have "install -p" and some don't. I have no idea when it should be used, but I'll add it to the SPEC. - Don't call libtool manually. Remove the last three lines in %install, add instead BuildRequires: automake and sed -i "s|noinst_|bin_|g" Makefile.am automake to %setup, this will make the build process install xyconv to %{_bindir}. - Use SMP make flags. Change make to make %{?_smp_mflags} - Change %{_bindir}/* to %{_bindir}/xyconv and %{_includedir}/* to %{_includedir}/xylib/ (no sense in using wildcards in these cases) - The attributes %defattr(-,root,root) should be %defattr(-,root,root,-) SHOULD: Use a more recommended version of the BuildRoot tag http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag I made all the changes you suggested above, with one exception: I kept gcc-g++ in BuildRequires, I hope it's not a problem. Thank you both. Spec URL: http://www.unipress.waw.pl/~wojdyr/spec/xylib.spec SRPM URL: http://www.unipress.waw.pl/~wojdyr/spec/xylib-0.4-1.fc11.src.rpm As you're a new packager you should learn to run rpmlint on your packages: https://fedoraproject.org/wiki/Packaging/Guidelines#Use_rpmlint You've written %defattr in the changelog, which may be expanded. rpmlint complains from this among other things. In the changelog you should use %% to prevent the macro from being expanded. rpmlint does sometimes generate noise, for instance it will comlain about the -devel package having no documentation. Yes, I know about rpmlint, but the last time I checked only RPMs, not the spec. Updated spec and SRPM are in the same place. Your srpm does not build on dist-f12: http://koji.fedoraproject.org/koji/taskinfo?taskID=1453152 By the way please make it sure that you change the release number of your spec file every time you modify your spec file (and post the URLs of new spec/srpm files) to avoid confusion. Thanks for looking at this rpm. The problem is with version mismatch of automake. Should I call libtool manually, like in the first version, i.e. ./libtool --mode=install install -m 755 xyconv $RPM_BUILD_ROOT%{_bindir} or rather run more autotools after changing Makefile.am? (In reply to comment #8) > Thanks for looking at this rpm. > > The problem is with version mismatch of automake. Should I call libtool > manually, like in the first version, i.e. > ./libtool --mode=install install -m 755 xyconv $RPM_BUILD_ROOT%{_bindir} > > or rather run more autotools after changing Makefile.am? The first option is of course to get upstream to change the makefile to install the binary by default. The second one is to run more autotools. (The third one is requiring the necessary version of automake to build the package. Automake 1.10 doesn't seem to be available on F12, though...) Funny, I tried the build with F11 which has the same version of automake as F12, so there shouldn't be any problem with that... Jussi, I'm just wondering why changing Makefile.am is preferred to calling ./libtool --mode=install install ... The latter seems more portable to me. I couldn't find anything about it in Fedora guides. Marcin It's because it isn't supposed to be used that way. It defeats the whole purpose of autotools: a simple(ish) way of configuring and building software on many platforms. (In reply to comment #11) > It's because it isn't supposed to be used that way. It defeats the whole > purpose of autotools: a simple(ish) way of configuring and building software on > many platforms. Sorry, but do you really mean that using well-documented, portable libtool script "defeats the whole purpose of autotools"? It's much more elegant way than hacking Makefile.am and running autotools. If for some reasons using libtool is not recommended in fedora project (but I haven't seen such a statement in docs and there is a number of specs in fedora CVS that _are_ using libtool in this way), the second most portable way is to just call install. Marcin I changed the spec to avoid using automake. Spec URL: http://www.unipress.waw.pl/~wojdyr/spec/xylib.spec SRPM URL: http://www.unipress.waw.pl/~wojdyr/spec/xylib-0.4-3.fc11.src.rpm btw on fedora-devel there has been a discussion raging on whether autotools files are OK to patch and rerun autotools vs hacking the preprocessed files manually, which is analogous to this matter. For -3: * Requires - -devel subpackage should have "Requires: boost-devel" because xylib/cache.h contains: ---------------------------------------------------- 23 #if 0 24 #include <tr1/memory> 25 using std::tr1::shared_ptr; 26 #else 27 #include <boost/shared_ptr.hpp> 28 using boost::shared_ptr; 29 #endif ---------------------------------------------------- Then: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------ I added the missing Require: Spec URL: http://www.unipress.waw.pl/~wojdyr/spec/xylib.spec SRPM URL: http://www.unipress.waw.pl/~wojdyr/spec/xylib-0.4-4.fc11.src.rpm and will try to make another work soon. Okay. Then I will wait for your another review request or your pre-review of other person's review request. I didn't do exactly what you asked yet, but today FTBFS bug was submitted for fityk package, which I'm an upstream author. I made several changes to the fityk spec and submitted a patch as an attachment 353890 [details] to bug 511758. (In reply to comment #18) > I didn't do exactly what you asked yet, but today FTBFS bug was submitted for > fityk package, which I'm an upstream author. I made several changes to the > fityk spec and submitted a patch as an attachment 353890 [details] to bug 511758. Ah, okay. ------------------------------------------------------------------ This package (xylib) is APPROVED by mtasaka ------------------------------------------------------------------ Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Get a Fedora Account". After you request for sponsorship a mail will be sent to sponsor members automatically (which is invisible for you) which notifies that you need a sponsor. After that, please also write on this bug for confirmation that you requested for sponsorship and your FAS (Fedora Account System) name. Then I will sponsor you. If you want to import this package into Fedora 10/11, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. Thanks. I requested sponsorship. FAS name: wojdyr Okay, now I am sponsoring you. Please follow "Join" wiki again. New Package CVS Request ======================= Package Name: xylib Short Description: Library for reading x-y data from several file formats Owners: wojdyr Branches: F-10 F-11 InitialCC: cvs done. xylib-0.4-4.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/xylib-0.4-4.fc11 xylib-0.4-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/xylib-0.4-4.fc10 xylib-0.4-4.fc11 has been pushed to the Fedora 11 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update xylib'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-7894 xylib-0.4-4.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update xylib'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7903 Now closing. xylib-0.4-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. xylib-0.4-4.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. |