Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 524545
Summary: | Review Request: snacc - Sample Neufeld ASN.1 to C Compiler | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Shakthi Kannan <shakthimaan> | ||||
Component: | Package Review | Assignee: | Chitlesh GOORAH <chitlesh> | ||||
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | fedora-package-review, musolinoa, notting, shakthimaan | ||||
Target Milestone: | --- | Flags: | chitlesh:
fedora-review+
kevin: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | 1.3-4.el5 | Doc Type: | Bug Fix | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2009-10-03 19:03:45 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: | |||||||
Attachments: |
|
Description
Shakthi Kannan
2009-09-21 06:49:39 UTC
Hi Shakthi, I have a few comments: - The versioned shared libraries should not be in the devel package as they are required by the snacc binary itself. (The *.so symlinks, however, should remain in the devel package). - The *.ans1 files are also required by snacc and so should be moved out of the devel package. - Some of the documentation (*.tex and *.bib) refers to the c-lib and c++-lib APIs (these should go in snacc-devel) and some refer to the use of various binaries in the snacc package (these should go in snacc). - The C/C++ examples should go in devel package (they aren't being packaged at the moment). - The C/C++ header files for the c-lib and c++-lib should go into the devel package. - There's a spelling mistake in the description (s/dample/sample/). Another idea might be to create libsnacc and libsnacc-devel packages for the c and c++ libraries and put just the binaries in snacc. In fact, I think this would be the optimal solution. ==> Thanks for your feedback. - The versioned shared libraries should not be in the devel package as they are required by the snacc binary itself. (The *.so symlinks, however, should remain in the devel package). ==> DONE. - The *.ans1 files are also required by snacc and so should be moved out of the devel package. ==> DONE. Moved to -base package. - Some of the documentation (*.tex and *.bib) refers to the c-lib and c++-lib APIs (these should go in snacc-devel) and some refer to the use of various binaries in the snacc package (these should go in snacc). ==> Since all *.tex files are incleded with snacc.tex file, I have retained it in -devel package. The -base package has the .ps file for snacc user manual. - The C/C++ examples should go in devel package (they aren't being packaged at the moment). ==> DONE. - The C/C++ header files for the c-lib and c++-lib should go into the devel package. ==> They were already in -devel package. They were in /usr/include/snacc/. Now, I have explicitly mentioned them. - There's a spelling mistake in the description (s/dample/sample/). ==> FIXED. Another idea might be to create libsnacc and libsnacc-devel packages for the c and c++ libraries and put just the binaries in snacc. In fact, I think this would be the optimal solution. ==> DONE. SPEC: http://shakthimaan.fedorapeople.org/SPECS/snacc.spec SRPM: http://shakthimaan.fedorapeople.org/SRPMS/snacc-1.3-2.fc11.src.rpm Successful Koji builds for F-10, F-11 and EL-5 respectively: http://koji.fedoraproject.org/koji/taskinfo?taskID=1701352 http://koji.fedoraproject.org/koji/taskinfo?taskID=1701418 http://koji.fedoraproject.org/koji/taskinfo?taskID=1701457 Shakthi, Two more suggestions from me... - The snacc-libs package should not require the snacc package. - I've realised that it should be 'Sample' - Sample and Neufeld are the names of the creators. #001: move the contents of the -lib subpackage to the main package. The -lib subpackage is meant for multilibs hence support multiarchitectures. IF you want to keep the -lib subpackage you will need to 1. add a require N-libs-V-R to the base package 2. move the binaries to the -libs package 3. make the base package a noarch I prefer to eliminate the -lib subpackage entirely. #002: Documentation: *.tex and *.bib are pretty useless and inefficient for the user. I propose to build a PDF out of it in the %prep section and ship only the PDF and not the sources. For best practices, all documentations of the same package should be provided by the same package. This helps the user find the documentation quickly and efficiently. #003: Directory ownership in base package %dir %{_includedir}/%{name}/ #004: .m4 should this go to -devel ? %{_datadir}/aclocal/snacc.m4 #005: Patches naming should start with a %{name} prefix, if not they will overwrite other patches with the same name. FYI: automake17 is provided by Fedora to maintain compatibility with own software. The last real update of automake17 on Fedora is back in 2007. It is recommended for upstream to tune their sources with respect to the newer versions. From Comment #3: - The snacc-libs package should not require the snacc package. ==> Now, snacc-libs has been removed. - I've realised that it should be 'Sample' - Sample and Neufeld are the names of the creators. ==> FIXED. From Comment #4: #001: I prefer to eliminate the -lib subpackage entirely. ==> DONE. #002: Documentation: *.tex and *.bib are pretty useless and inefficient for the user. I propose to build a PDF out of it in the %prep section and ship only the PDF and not the sources. ==> Already .ps file is being shipped in the -base package. I have removed *.tex and *.bib files now. #003: Directory ownership in base package %dir %{_includedir}/%{name}/ ==> DONE. #004: .m4 should this go to -devel ? %{_datadir}/aclocal/snacc.m4 ==> DONE. #005: Patches naming should start with a %{name} prefix, if not they will overwrite other patches with the same name. ==> DONE. These were upstream package names. Now, I have prepended %{name} to them. FYI: automake17 is provided by Fedora to maintain compatibility with own software. The last real update of automake17 on Fedora is back in 2007. It is recommended for upstream to tune their sources with respect to the newer versions. ==> This is a very, very, very old package. Old, as in *1997*. Yet, it is a very useful package. Upstream (Debian alone) has provided it so far. The original code authors' FTP URL doesn't exist at all. Latest at: SPEC: http://shakthimaan.fedorapeople.org/SPECS/snacc.spec SRPM: http://shakthimaan.fedorapeople.org/SRPMS/snacc-1.3-3.fc11.src.rpm Successful Koji builds at F-10, F-11 and EL-5 respectively: http://koji.fedoraproject.org/koji/taskinfo?taskID=1704234 http://koji.fedoraproject.org/koji/taskinfo?taskID=1704239 http://koji.fedoraproject.org/koji/taskinfo?taskID=1704255 It's weird that the scratch build succeeded in koji. On my local machine it failed on F-11. Please see attachment. By the way, you don't have to call a scratch built for each release dump :) One is enough. Created attachment 362549 [details]
failed built on F-11
(In reply to comment #7) > Created an attachment (id=362549) [details] > failed built on F-11 In your log, even though the Makefile is being created for c++-lib/tcl/, it is invoking the build in it with -DTCL with g++. It doesn't invoke the Makefile in the directory in my system or in Koji. I still did pass the --without-tcl option, but, configure still says: configure: WARNING: unrecognized options: --without-tcl 1. Since --without-tcl doesn't affect the build much, I have now removed it. 2. I have checked with Martin (upstream Debian), and he said he never needed Tcl support, so we can disable it. C, C++ is mostly used. So, I have disabled producing c++-lib/tcl/Makefile in AC_OUTPUT in the snacc-configure.patch. It should build fine, now. SPEC: http://shakthimaan.fedorapeople.org/SPECS/snacc.spec SRPM: http://shakthimaan.fedorapeople.org/SRPMS/snacc-1.3-4.fc11.src.rpm Successful Koji builds for F-10, F-11, EL-5 respectively: http://koji.fedoraproject.org/koji/taskinfo?taskID=1708902 http://koji.fedoraproject.org/koji/taskinfo?taskID=1708907 http://koji.fedoraproject.org/koji/taskinfo?taskID=1708924 The build fails with tcl-devel installed. I had to remove tcl-devel so build snacc successfully. - MUST: The package is named according to the Package Naming Guidelines. - MUST: The spec file name matches the base package %{name} - MUST: The package meets the Packaging Guidelines. - MUST: The package is licensed (GPLv2+) with an open-source compatible license and meet other legal requirements as defined in the legal section of Packaging Guidelines. - MUST: The License field in the package spec file matches the actual license. - MUST: the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. - MUST: the package does not contain any duplicate files in the %files - MUST: the package owns all directories that it creates. - MUST: The spec file must be written in American English. - MUST: The spec file for the package is be legible. - MUST: The sources used to build the package must matches the upstream source, as provided in the spec URL. - MUST: The package successfully compiles and builds into binary rpms on at least i586. - MUST: All build dependencies is listed in BuildRequires. - MUST: The spec file handles locales properly.: No locales in this package - MUST: the package is not designed to be relocatable - MUST: Permissions on files are set properly. - MUST: The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). - MUST: The package consistently uses macros, as described in the macros section of Packaging Guidelines. - MUST: The package contains code, or permissible content. This is described in detail in the code vs. content section of Packaging Guidelines. - MUST: There are no Large documentation files - MUST: %doc does not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. - MUST: There are no Header files or static libraries - MUST: The package does not contain library files with a suffix - MUST: Package does NOT contain any .la libtool archives - MUST: Package does not own files or directories already owned by other packages. SHOULD Items: - SHOULD: The source package doesn't include license text(s) as COPYING - SHOULD: mock builds succcessfully in i586. - SHOULD: The reviewer tested that the package functions as described. A package should not segfault instead of running, for example. - SHOULD: Those scriptlets used are sane. Approved New Package CVS Request ======================= Package Name: snacc Short Description: Sample Neufeld ASN.1 to C Compiler Owners: shakthimaan chitlesh Branches: F-10 F-11 EL-5 cvs done. snacc-1.3-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/snacc-1.3-4.fc10 snacc-1.3-4.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/snacc-1.3-4.el5 snacc-1.3-4.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/snacc-1.3-4.fc11 snacc-1.3-4.el5 has been pushed to the Fedora EPEL 5 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 snacc'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/EL-5/FEDORA-EPEL-2009-0563 snacc-1.3-4.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. snacc-1.3-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. snacc-1.3-4.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report. |