Bug 482884

Summary: Review Request: clc-intercal - Compiler for the INTERCAL language
Product: [Fedora] Fedora Reporter: Iain Arnell <iarnell>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, jochen, notting
Target Milestone: ---Flags: j: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0-0.1.1._94._2.fc9 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-03-27 14:49:31 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:

Description Iain Arnell 2009-01-28 17:22:35 UTC
Spec URL: http://iarnell.fedorapeople.org/rpms/CLC-INTERCAL.spec
SRPM URL: http://iarnell.fedorapeople.org/rpms/CLC-INTERCAL-0-0.0.fc11.src.rpm
Description: Compiler Language With No Pronounceable Acronym.

Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1089200

Notes:
CLC-INTERCAL uses a novel perversion (rather than "version", for correctness) numbering system consisting of a floating point number with independent signs for the integer and fractional parts. The current number is 1.-94.-2. Unfortunately, RPM is unable to cope with such numbering, so I took some liberties with sub-packages to make the resulting binary RPMs look almost right. 

The license is almost-but-not-quite 3 clause BSD. The clauses are the same, but the introduction slightly different: "In addition to the above, permission is hereby granted to use, misuse, modify, distribute, break, fix again, etcetera CLC-INTERCAL-1.-94.-2 provided that the following conditions are met".

Comment 1 Jochen Schmitt 2009-01-28 17:47:46 UTC
Question: It this your first Fedora package?

I ask this, because the package look like it's need some love.

I try to summarize my first mind about this package:

1. At first, you should try to get a better description the package, so a user
which want to install the package may got a idea, why he should try out this new programming language

2. Your have specified 0 as the global package version and add version information on the definition of subpackage. This is not ok, you should put the version information on the Version statement and the subpackage shouldn't have any version information in thier name.

3.) It's look very ugly, that you are using rpm macros for the whole content of the Summary and Group statement.

Unfortunately, I either a sponsor or a perl packaging specialist, so I can't review your package, but I would show your some points which are needs some homework.

If this you first package and you need a sponsor please set the FE-NEEDSPONSOR bug as a blocker.

Best Regards:

Jochen Schmitt

Comment 2 Iain Arnell 2009-01-29 00:40:14 UTC
Answer: No, I already maintain several other perl packages. No sponsor required.


1. Okay, description was a little weak. New text ripped from Debian package:

Summary: Compiler for the INTERCAL language

Description:
This package provides a Perl-based compiler for the INTERCAL programming language, usable either from the command line or as a Perl module.

CLC-INTERCAL is designed to be almost compatible with the original (Princeton 1972) compiler. It also implements several extensions to the original language, including support for object orientation, operator overloading and quantum computing. 


2. Indeed. But RPM can't cope with hyphens in version numbers (and even if it could, it probably wouldn't realise that 1.-94.-2 is a higher version number than 1.-94.-3). It seemed appropriate for INTERCAL to seek a particularly obtuse solution, but on second thought...

Since it's not yet a 1.0 release, how about Version: 0 and Release: 0.%{X}.1._94._2 (as a non-numeric pre-release tag).


3. %{summary} comment is moot if I stick the proper version into release tag; the offending sub-package is no longer necessary. 

Only Group: %{group} remains in UI-X sub-package. 


New Spec: http://iarnell.fedorapeople.org/rpms/CLC-INTERCAL.spec
New SRPM: http://iarnell.fedorapeople.org/rpms/CLC-INTERCAL-0-0.0.1._94._2.fc11.src.rpm
New Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1090063

Comment 3 Jason Tibbitts 2009-03-07 19:33:21 UTC
This is so horrible.  And perverse.  Which makes the package absolutely essential.

It builds and installs fine for me.  I haven't the slightest idea how to make it do anything, and frankly I don't want to learn for fear of mental contamination.

Some comments:

There's no need at all for the Group: tag in the subpackage, unless you want it to be different from the main package.  In this case it's just pointless.

I would urge the lower-casing of the package name, as Debian seems to do.

I have some concerns about /usr/bin/sick as being insufficiently unique.  A search turns up no instances anywhere except in the Debian package, so it seems low risk, but I wonder if it's worth it for what's essentially a joke package.  I'm going to leave that up to you.  You can read http://fedoraproject.org/wiki/Common_package_names_packaging_guideline_draft for guidance; it's still being drafted, but should grow to encompass potentially conflicting executables as well.

I tried parallel make but the package then fails to build.  Please add a comment to this effect so that folks looking to save a bit of build time won't be tripped up as I was.

It's not immediately clear how the UI-X subpackage provides a graphical interface.  I guess the other executables look for the presence of the X module and use it if present.  Is it reasonable to provide a desktop file and an icon in that case?

* source files match upstream.  sha256sum:
   6f3db1dc35c9217c6590007238d444e30730ee021b4ea4338b26bdb7b8ff29ac  
   CLC-INTERCAL-1.-94.-2.tar.gz
* package meets naming and versioning guidelines (as much as is possible)
  I suggest downcasing the package name.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
  CLC-INTERCAL-0-0.0.1._94._2.fc11.noarch.rpm
   perl(Language::INTERCAL::ArrayIO)
   perl(Language::INTERCAL::Arrays)
   perl(Language::INTERCAL::Arrays::Hybrid)
   perl(Language::INTERCAL::Arrays::Tail)
   perl(Language::INTERCAL::Backend)
   perl(Language::INTERCAL::Backend::ListObject)
   perl(Language::INTERCAL::Backend::Object)
   perl(Language::INTERCAL::Backend::Perl)
   perl(Language::INTERCAL::Backend::Run)
   perl(Language::INTERCAL::ByteCode)
   perl(Language::INTERCAL::Charset)
   perl(Language::INTERCAL::Charset::Baudot)
   perl(Language::INTERCAL::Charset::EBCDIC)
   perl(Language::INTERCAL::Charset::Hollerith)
   perl(Language::INTERCAL::CrawlingHorror)
   perl(Language::INTERCAL::DataItem)
   perl(Language::INTERCAL::Distribute)
   perl(Language::INTERCAL::DoubleOhSeven)
   perl(Language::INTERCAL::Exporter)
   perl(Language::INTERCAL::Generate)
   perl(Language::INTERCAL::GenericIO)
   perl(Language::INTERCAL::GenericIO::ARRAY)
   perl(Language::INTERCAL::GenericIO::COUNT)
   perl(Language::INTERCAL::GenericIO::FILE)
   perl(Language::INTERCAL::GenericIO::OBJECT)
   perl(Language::INTERCAL::GenericIO::REMOTE)
   perl(Language::INTERCAL::GenericIO::STRING)
   perl(Language::INTERCAL::GenericIO::TCP)
   perl(Language::INTERCAL::GenericIO::TEE)
   perl(Language::INTERCAL::GenericIO::UFILE)
   perl(Language::INTERCAL::GenericIO::UTCP)
   perl(Language::INTERCAL::HostIP)
   perl(Language::INTERCAL::InstallModule)
   perl(Language::INTERCAL::Interface)
   perl(Language::INTERCAL::Interface::Curses)
   perl(Language::INTERCAL::Interface::Line)
   perl(Language::INTERCAL::Interface::Line::IN)
   perl(Language::INTERCAL::Interface::Line::WOBJ)
   perl(Language::INTERCAL::Interface::None)
   perl(Language::INTERCAL::Interface::common)
   perl(Language::INTERCAL::Interpreter)
   perl(Language::INTERCAL::Numbers)
   perl(Language::INTERCAL::Numbers::Spot)
   perl(Language::INTERCAL::Numbers::Twospot)
   perl(Language::INTERCAL::Object)
   perl(Language::INTERCAL::Optimiser)
   perl(Language::INTERCAL::Parser)
   perl(Language::INTERCAL::Rcfile)
   perl(Language::INTERCAL::ReadNumbers)
   perl(Language::INTERCAL::Reggrim)
   perl(Language::INTERCAL::Server)
   perl(Language::INTERCAL::SharkFin)
   perl(Language::INTERCAL::Sick)
   perl(Language::INTERCAL::Splats)
   perl(Language::INTERCAL::SymbolTable)
   perl(Language::INTERCAL::Theft)
   perl(Language::INTERCAL::Whirlpool)
   perl(Language::INTERCAL::WriteNumbers)
   CLC-INTERCAL = 0-0.0.1._94._2.fc11
  =
   /usr/bin/perl
   perl >= 0:5.005
   perl(Carp)
   perl(Config)
   perl(Curses)
   perl(Exporter)
   perl(ExtUtils::MakeMaker)
   perl(File::Basename)
   perl(File::Spec)
   perl(File::Spec::Functions)
   perl(FindBin)
   perl(Getopt::Long)
   perl(IO::File)
   perl(IO::Socket::INET)
   perl(Language::INTERCAL::ArrayIO)
   perl(Language::INTERCAL::Arrays)
   perl(Language::INTERCAL::Backend)
   perl(Language::INTERCAL::Backend::Object)
   perl(Language::INTERCAL::ByteCode)
   perl(Language::INTERCAL::Charset)
   perl(Language::INTERCAL::Charset::Baudot)
   perl(Language::INTERCAL::CrawlingHorror)
   perl(Language::INTERCAL::DataItem)
   perl(Language::INTERCAL::DoubleOhSeven)
   perl(Language::INTERCAL::Exporter)
   perl(Language::INTERCAL::GenericIO)
   perl(Language::INTERCAL::GenericIO::FILE)
   perl(Language::INTERCAL::GenericIO::TCP)
   perl(Language::INTERCAL::HostIP)
   perl(Language::INTERCAL::Interface)
   perl(Language::INTERCAL::Interface::common)
   perl(Language::INTERCAL::Interpreter)
   perl(Language::INTERCAL::Numbers)
   perl(Language::INTERCAL::Object)
   perl(Language::INTERCAL::Optimiser)
   perl(Language::INTERCAL::Parser)
   perl(Language::INTERCAL::Rcfile)
   perl(Language::INTERCAL::ReadNumbers)
   perl(Language::INTERCAL::Reggrim)
   perl(Language::INTERCAL::Server)
   perl(Language::INTERCAL::SharkFin)
   perl(Language::INTERCAL::Sick)
   perl(Language::INTERCAL::Splats)
   perl(Language::INTERCAL::SymbolTable)
   perl(Language::INTERCAL::Whirlpool)
   perl(POSIX)
   perl(Socket)
   perl(Term::ReadLine)
   perl(constant)
   perl(strict)
   perl(vars)

  CLC-INTERCAL-UI-X-0-0.0.1._94._2.fc11.noarch.rpm
   perl(Language::INTERCAL::Interface::X)
   CLC-INTERCAL-UI-X = 0-0.0.1._94._2.fc11
  =
   CLC-INTERCAL = 0-0.0.1._94._2.fc11
   perl(:MODULE_COMPAT_5.10.0)
   perl(Carp)
   perl(Gtk2)
   perl(Language::INTERCAL::Exporter)
   perl(Language::INTERCAL::Interface::common)
   perl(strict)
   perl(vars)

* %check is present and all tests pass:
  All tests successful.
  Files=15, Tests=6131, 17 wallclock secs ( 0.96 usr  0.10 sys + 15.77 cusr  
   0.77 csys = 17.60 CPU)

  All tests successful.
  Files=7, Tests=148,  7 wallclock secs ( 0.05 usr  0.02 sys +  0.36 cusr  0.06 
   csys =  0.49 CPU)

  All tests successful.
  Files=3, Tests=12,  0 wallclock secs ( 0.01 usr  0.02 sys +  0.20 cusr  0.02 
   csys =  0.25 CPU)

  All tests successful.
  Files=1, Tests=3,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.12 cusr  0.01 
   csys =  0.14 CPU)

  All tests successful.
  Files=1, Tests=3,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.13 cusr  0.02 
   csys =  0.16 CPU)

  All tests successful.
  Files=1, Tests=3,  0 wallclock secs ( 0.01 usr  0.01 sys +  0.16 cusr  0.03 
   csys =  0.21 CPU)

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
? possible generically named files (/usr/bin/sick)
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
? Maybe there should be a desktop file and icon in the UI-X subpackage.

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Comment 4 Iain Arnell 2009-03-08 09:48:51 UTC
Thanks for the review. And I will start to reciprocate now that I've got a few under my belt.

I agree with lowercasing - better aesthetically and consistent with debian.

I've kept the group tag in the sub-package since at least on my setup, rpm is making it 'unspecified' if I don't (and I think rpm 4.4 requires it, so may still be necessary for F-9?).

I'm happy with /usr/bin/sick - it certainly seems to be unique at the minute and should a possible conflict occur in future, I don't see too many problems if we do need to rename it later (anyone relying on intercal has more serious problems to worry about).

You're absolutely right about the UI-X sub-package. I've added a sentence to its description to make this clearer. I'm not sure that a desktop file is necessary, though.


New spec: http://iarnell.fedorapeople.org/rpms/clc-intercal.spec
New SRPM: http://iarnell.fedorapeople.org/rpms/clc-intercal-0-0.1.1._94._2.fc11.src.rpm
New koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1229460

Comment 5 Jason Tibbitts 2009-03-08 20:00:20 UTC
You're right about the group tag; it seems that newer rpm versions handle this better, but older ones don't.

I think we've touched on any potential issues, and everything significant has been addressed, so I think we're done.

APPROVED

Comment 6 Iain Arnell 2009-03-09 04:38:27 UTC
New Package CVS Request
=======================
Package Name: clc-intercal
Short Description: Compiler for the INTERCAL language
Owners: iarnell
Branches: F-9 F-10
InitialCC:

Comment 7 Kevin Fenzi 2009-03-09 16:05:43 UTC
cvs done.

Comment 8 Fedora Update System 2009-03-10 16:24:57 UTC
clc-intercal-0-0.1.1._94._2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/clc-intercal-0-0.1.1._94._2.fc10

Comment 9 Fedora Update System 2009-03-10 16:25:02 UTC
clc-intercal-0-0.1.1._94._2.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/clc-intercal-0-0.1.1._94._2.fc9

Comment 10 Fedora Update System 2009-03-11 18:01:58 UTC
clc-intercal-0-0.1.1._94._2.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 clc-intercal'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2613

Comment 11 Fedora Update System 2009-03-11 18:02:45 UTC
clc-intercal-0-0.1.1._94._2.fc9 has been pushed to the Fedora 9 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-newkey update clc-intercal'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-2622

Comment 12 Fedora Update System 2009-03-27 14:49:25 UTC
clc-intercal-0-0.1.1._94._2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2009-03-27 14:52:21 UTC
clc-intercal-0-0.1.1._94._2.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.