Bug 482884 - Review Request: clc-intercal - Compiler for the INTERCAL language
Review Request: clc-intercal - Compiler for the INTERCAL language
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-28 12:22 EST by Iain Arnell
Modified: 2009-03-27 10:52 EDT (History)
3 users (show)

See Also:
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 10:49:31 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Iain Arnell 2009-01-28 12:22:35 EST
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 12:47:46 EST
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-28 19:40:14 EST
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 14:33:21 EST
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 05:48:51 EDT
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 16:00:20 EDT
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 00:38:27 EDT
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 12:05:43 EDT
cvs done.
Comment 8 Fedora Update System 2009-03-10 12:24:57 EDT
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 12:25:02 EDT
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 14:01:58 EDT
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 14:02:45 EDT
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 10:49:25 EDT
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 10:52:21 EDT
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.

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