Bug 524545 - Review Request: snacc - Sample Neufeld ASN.1 to C Compiler
Summary: Review Request: snacc - Sample Neufeld ASN.1 to C Compiler
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chitlesh GOORAH
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-09-21 06:49 UTC by Shakthi Kannan
Modified: 2009-10-21 16:22 UTC (History)
4 users (show)

Fixed In Version: 1.3-4.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-10-03 19:03:45 UTC
chitlesh: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
failed built on F-11 (353.00 KB, application/octet-stream)
2009-09-24 18:32 UTC, Chitlesh GOORAH
no flags Details

Description Shakthi Kannan 2009-09-21 06:49:39 UTC
Spec URL: http://shakthimaan.fedorapeople.org/SPECS/snacc.spec
SRPM URL: http://shakthimaan.fedorapeople.org/SRPMS/snacc-1.3-1.fc11.src.rpm
Description: Snacc is a dample Neufeld ASN.1 to C compiler that can produce C, C++ routines or type tables for BER encoding, decoding, printing and freeing.

Successful Koji builds for F-10, F-11 and EL-5 respectively:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1693956
http://koji.fedoraproject.org/koji/taskinfo?taskID=1693961
http://koji.fedoraproject.org/koji/taskinfo?taskID=1693969

Comment 1 Alex Musolino 2009-09-23 09:33:34 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.

Comment 2 Shakthi Kannan 2009-09-23 18:15:43 UTC
==> 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

Comment 3 Alex Musolino 2009-09-24 10:35:28 UTC
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.

Comment 4 Chitlesh GOORAH 2009-09-24 15:42:46 UTC
#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.

Comment 5 Shakthi Kannan 2009-09-24 17:27:27 UTC
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

Comment 6 Chitlesh GOORAH 2009-09-24 18:31:47 UTC
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.

Comment 7 Chitlesh GOORAH 2009-09-24 18:32:54 UTC
Created attachment 362549 [details]
failed built on F-11

Comment 8 Shakthi Kannan 2009-09-25 09:20:52 UTC
(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

Comment 9 Shakthi Kannan 2009-09-26 09:14:04 UTC
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

Comment 10 Chitlesh GOORAH 2009-09-28 20:06:42 UTC
The build fails with tcl-devel installed.

I had to remove tcl-devel so build snacc successfully.

Comment 11 Chitlesh GOORAH 2009-09-28 20:07:53 UTC
- 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

Comment 12 Shakthi Kannan 2009-09-29 14:07:04 UTC
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

Comment 13 Kevin Fenzi 2009-09-29 19:57:15 UTC
cvs done.

Comment 14 Fedora Update System 2009-10-01 05:38:53 UTC
snacc-1.3-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/snacc-1.3-4.fc10

Comment 15 Fedora Update System 2009-10-01 05:38:58 UTC
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

Comment 16 Fedora Update System 2009-10-01 05:39:03 UTC
snacc-1.3-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/snacc-1.3-4.fc11

Comment 17 Fedora Update System 2009-10-02 05:06:35 UTC
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

Comment 18 Fedora Update System 2009-10-03 19:03:40 UTC
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.

Comment 19 Fedora Update System 2009-10-03 19:07:24 UTC
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.

Comment 20 Fedora Update System 2009-10-21 16:22:24 UTC
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.


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