Bug 579925

I don't know much about tcl and we don't get many tcl packages submitted, but we do have a guidelines page.  I suppose that anyone who was interested in tcl would have reviewed this already, so I'll go ahead and take care of it.

The package does not meet the naming guidelines, which require that tcl packages have a "tcl-" prefix.  You can have Provides: tclreadline if you want.

2.1.0 still seems to be the latest version.  In fact, upstream seems to be thoroughly dead; the last commit was something like nine years ago.  Perhaps you and the Debian folks could get together and fork the package.  You've already changed the API of the package (with the prompt2 stuff) so it's not much of a stretch.  Otherwise, who is going to ensure compatibility with future tcl development?

Can you comment on the 20+ rpmlint complaints of the form
  tclreadline.x86_64: W: undefined-non-weak-symbol
   /usr/lib64/libtclreadline-2.1.0.so Tcl_DoOneEvent

Regarding the naming guidelines: I was too much focussed on how the packaging
itself is done, sorry. I've no idea regarding the rpmlint complaints so far, I
will have to investigate here, but:

I won't fork tclreadline, because I'm using it to rarely and I'm not really
familiar with tcl programming. The patches applied are mostly from Debian, so
I would like to keep everything how it is, because there are some further 
tclreadline users out there. If that makes the review from your point of view
a no-go, please let me know.

Spec URL: http://labs.linuxnetz.de/bugzilla/tcl-tclreadline.spec
SRPM URL: http://labs.linuxnetz.de/bugzilla/tcl-tclreadline-2.1.0-2.src.rpm

I guess the primary question is why you would want to push unmaintained software into the distribution when by your own admission you aren't sufficiently familiar with the software to maintain it if problems arise.  For example, what happens if Fedora updates to a readline version which is no longer ABI compatible?

As to the package itself, I think it looks fine assuming you intend to push this to EL4 or EL5; otherwise you can drop several bits.

I'm not sure why you apply memuse.patch; did our tcl become threaded at some point?  (I guess it doesn't really hurt anything, but generally you patch to fix something that's actually broken.)

prompt2.patch seems too actually change the API of the library.  Are you really sure you didn't intend to fork this software?  Debian policies may permit this kind of thing but it's really a bad idea.

There are Fedora users out there that expect tclreadline. And no, not every
Fedora users wants to get a contributor. That may has to do with new target
audience of Fedora...

In case Fedora updates to a API incompatible version (I think, you meant API
rather ABI), I'll try to solve things, otherwise the package gets orphaned.

The package is intended to be pushed to EPEL branches.

The tcl in Fedora was from time to time threaded by accident, more or less
each time the maintainer on Red Hat side switched and the new one wanted to
give a try to the threaded tcl.

I'm not going to create a new upstream, even it's "in" at Fedora nowadys to
fork any software. From my point of view, the prompt2.patch changes API, but
it only adds something new & optional. Thus backward compatibility is given.

Shouldn't reviews that are stuck after a certain amount of time be closed?

I'm not aware of formal policies, but if the original submitter (Robert Scheck) agrees this should be put out of the way. IMHO this is better than introducing a package and then little time later deprecate it.

Robert, is this package still relevant after so much time without any upstream activity?

Answering myself and justifying the action here:

https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews?rd=Extras/Policy/StalledReviews

So I'm closing this BZ. Reopen if this is still a itch you want to scratch.

I am still interested in introducing this package into Fedora EPEL (and maybe
also into Fedora), reopening.

MUST rpmlint - fails please address malformed conditional in spec ; devel package has no documentation ; debuginfo has non-binaries in /usr/lib
	rpmlint /var/lib/mock/epel-7-x86_64/result/tcl-tclreadline-2.1.0-2.el7.centos.src.rpm 
	tcl-tclreadline.src: W: spelling-error Summary(en_US) Readline -> Deadline, Headline, Breadline
	tcl-tclreadline.src: W: spelling-error Summary(en_US) Tk -> Tl, T, K
	tcl-tclreadline.src: W: spelling-error %description -l en_US Readline -> Deadline, Headline, Breadline
	tcl-tclreadline.src: W: spelling-error %description -l en_US tk -> kt, t, k
	tcl-tclreadline.src: W: malformed-fedora-conditional tcl-tclreadline.spec:22
	1 packages and 0 specfiles checked; 0 errors, 5 warnings.

	rpmlint /var/lib/mock/epel-7-x86_64/result/tcl-tclreadline-2.1.0-2.el7.centos.x86_64.rpm 
	tcl-tclreadline.x86_64: W: spelling-error Summary(en_US) Readline -> Deadline, Headline, Breadline
	tcl-tclreadline.x86_64: W: spelling-error Summary(en_US) Tk -> Tl, T, K
	tcl-tclreadline.x86_64: W: spelling-error %description -l en_US Readline -> Deadline, Headline, Breadline
	tcl-tclreadline.x86_64: W: spelling-error %description -l en_US tk -> kt, t, k
	tcl-tclreadline.x86_64: W: incoherent-version-in-changelog 2.1.0-2 ['2.1.0-2.el7.centos', '2.1.0-2.centos']
	1 packages and 0 specfiles checked; 0 errors, 5 warnings.

	rpmlint /var/lib/mock/epel-7-x86_64/result/tcl-tclreadline-devel-2.1.0-2.el7.centos.x86_64.rpm 
	tcl-tclreadline-devel.x86_64: W: no-documentation
	1 packages and 0 specfiles checked; 0 errors, 1 warnings.

	rpmlint /var/lib/mock/epel-7-x86_64/result/tcl-tclreadline-debuginfo-2.1.0-2.el7.centos.x86_64.rpm 
	1 packages and 0 specfiles checked; 0 errors, 0 warnings.

MUST Package Naming Guidelines - the tcl- requirement has been addressed and met.  I find no other issue, but this is my first informal review.

MUST Specfile Naming - PASS

MUST Packaging Guidelines - FAIL - rpmlint issues ; debuginfo package issue
	No External Kernel Modules - OK
	No Inclusion of Pre-Built Binaries or Libraries - OK
	Spec Legibility - OK
	Architecture Support
		Builds for Fedora 21 - koji task 7204827 - PASS
		Builds for Fedora 20 - koji task 7204082 - PASS
		Builds for Fedora 19 - koji task 7204835 - PASS
		Builds for EPEL-7 - koji task 7204882 - PASS
		Builds for EPEL-6 - kpjo task 7204885 - PASS
		Builds for EPEL-5 - kpjo task 7204923 - PASS
	Filesystem Heirarchy Standard Compliance - OK
	rpmlint on installed packages
		<mock-chroot>[root@mythbox /]# rpmlint -i tcl-tclreadline
		tcl-tclreadline.x86_64: W: incoherent-version-in-changelog 2.1.0-2 ['2.1.0-2.el7.centos', '2.1.0-2.centos']
		The latest entry in %changelog contains a version identifier that is not
		coherent with the epoch:version-release tuple of the package.

		1 packages and 0 specfiles checked; 0 errors, 1 warnings.

		<mock-chroot>[root@mythbox /]# rpmlint -i tcl-tclreadline-debuginfo
		tcl-tclreadline-debuginfo.x86_64: W: only-non-binary-in-usr-lib
		There are only non binary files in /usr/lib so they should be in /usr/share.

		1 packages and 0 specfiles checked; 0 errors, 1 warnings.

		<mock-chroot>[root@mythbox /]# rpmlint -i tcl-tclreadline-devel
		tcl-tclreadline-devel.x86_64: W: no-documentation
		The package contains no documentation (README, doc, etc). You have to include
		documentation files.

		1 packages and 0 specfiles checked; 0 errors, 1 warnings.

	Changelog readability - OK
	Tag usage check - OK
		Packager Tag - not present GOOD
		Vendor Tag - not present GOOD
		Copyright Tag - not present GOOD
		License Tag used - GOOD
		Summary Tag does not end in period - GOOD
		PreReq Tag not used - GOOD
		Requires Tag used - GOOD
		Source Tag - used and resovles to full URL - GOOD
	BuildRequires - OK
		mock used and no missing build requires found
	Trademarks in summary or description - GOOD
	Documentation - No obvious irregulararities found
	DebugInfo - This Package is made by rpmbuild and yet not addressed in the .spec.
                    rpmlint is unhappy with it composure so th spec needs to address this.
	Devel Package - Other than the missing documentation for this package the spec looks good.
	Library usage and linking - Can find no issues as the spec pays good attention to this
	Macros - are used as intended as far as reviewer can tell
	
MUST Licensing Guidelines - PASS
	license is found in the COPYING file and meets esablished BSD Format.

MUST license and spec file license must agree - PASS

MUST docs must include license file - PASS

MUST spec file uses American English - PASS

MUST spec file must be legible - PASS

MUST sources must match upstream source - PASS
	SOURCE0: sha256sum matches
	SOURCE1: sha256sum matches

MUST compile and build on at least one architecture

MUST work on architecture or exclude in spec - PASS

MUST BuildRequires used -PASS

MUST handle locales - UNKNOWN
	no %find_lang macro is present,but reviewer is unable to test this package for multiple locales so it may be okay.

MUST call ldconfig in %post and %postun - PASS

MUST not package system libraries - PASS

MUST specify if relocatable - PASS

MUST package must own all directories it creates - PASS

MUST no duplicate files in %files - PASS

MUST set file permissions correctly - PASS

MUST consistently use macros - PASS

MUST codes or permissable content - PASS

MUST documentation absence does not affect usage - UNKNOWN - reviewer was not given a test case protocol and is not familiar enough to create one

MUST handle static libraries in a -static package - DOES not appear to apply

MUST handle devleopment linraries in a -devel package - PASS

NOT a GUI Package

MUST filenames in UTF-8 format - PASS

SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. - PASS

SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. - UNKNOWN
	No translations provided but not sure if this applies

SHOULD: The reviewer should test that the package builds in mock - PASS
	See above was tested in mock and koji scratch build

SHOULD: The package should compile and build into binary rpms on all supported architectures - PASS
	See koji results above

SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. - UNKNOWN
	The description of how it should operate is unclear to this reviewer

SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. - PASS
	Scripts appear sanely used

SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. - PASS
	No subpackages other than devel seen

SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. - PASS
	No .pc files used

SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. - PASS
	This condition appears met.

SHOULD: your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense. - PASS
	manpage does define the usage of libraries

I am not yet approved as a packager yet but have provided the preliminary review above to assist any packager who would want to undetake a full review.  The packager may also want to address the rpmlint and locale / language issues discovered.

Just read another review and should also point out that COPYING probably belongs in %license not %doc.

Hi, Robert Lightfoot

I get a different set of objections from my rpmlint evaluation ... obviously some of the 'spelling error' stuff are inapplicable

[herrold@centos-6 tclreadline]$ rpmlint /home/herrold/rpmbuild/SRPMS/tcl-tclreadline-2.1.0-2.orc6.src.rpm /home/herrold/rpmbuild/RPMS/x86_64/tcl-tclreadline-2.1.0-2.orc6.x86_64.rpm /home/herrold/rpmbuild/RPMS/x86_64/tcl-tclreadline-devel-2.1.0-2.orc6.x86_64.rpm
tcl-tclreadline.src: W: spelling-error %description -l en_US tk -> kt, t, k
tcl-tclreadline.src:57: W: macro-in-comment %doc
tcl-tclreadline.src:72: W: macro-in-comment %{_libdir}
 ... addressed in a patch I shall apply in a moment

tcl-tclreadline.x86_64: W: spelling-error %description -l en_US tk -> kt, t, k

tcl-tclreadline-devel.x86_64: W: no-documentation
.... placeholder 'documentation' in a sub-package makes no sense, as it is tied to its parent package when properly installed:
   Requires:       %{name} = %{version}-%{release}
 -- PASS

3 packages and 0 specfiles checked; 0 errors, 5 warnings.
[herrold@centos-6 tclreadline]$ rpmlint  -V
rpmlint version 1.5 Copyright (C) 1999-2007 Frederic Lepied, Mandriva
[herrold@centos-6 tclreadline]$

I am not seeing locate issues in my run ... what version of rpmlint are you running?


Robert Scheck

Could you please apply the patch if you find it acceptable, and note a URL where the revised package set might be pulled from, and we can move this along directly

With the patched spec file locally I now get:

[herrold@centos-6 tclreadline]$ rpmlint /home/herrold/rpmbuild/SRPMS/tcl-tclreadline-2.1.0-3.orc6.src.rpm /home/herrold/rpmbuild/RPMS/x86_64/tcl-tclreadline-2.1.0-3.orc6.x86_64.rpm
tcl-tclreadline.src: W: spelling-error %description -l en_US tk -> kt, t, k
tcl-tclreadline.x86_64: W: spelling-error %description -l en_US tk -> kt, t, k
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

Created attachment 923994 [details]
patch at proposed rel 3 to address remainint rpmlint issues

I was running the current version of rpmlint for centos6.  That system no longer exists having been migrated to Centos7.

Spec URL: http://labs.linuxnetz.de/bugzilla/tcl-tclreadline.spec
SRPM URL: http://labs.linuxnetz.de/bugzilla/tcl-tclreadline-2.1.0-3.src.rpm

looks good

[herrold@centos-6 tclreadline]$ rpmlint /home/herrold/rpmbuild/SRPMS/tcl-tclreadline-2.1.0-3.orc6.src.rpm /home/herrold/rpmbuild/RPMS/x86_64/tcl-tclreadline-2.1.0-3.orc6.x86_64.rpm
tcl-tclreadline.src: W: spelling-error %description -l en_US tk -> kt, t, k
tcl-tclreadline.x86_64: W: spelling-error %description -l en_US tk -> kt, t, k
2 packages and 0 specfiles checked; 0 errors, 2 warnings.
[herrold@centos-6 tclreadline]$


PASS

flip on the satisfactory review flag

Thank you very much for the review!


New Package SCM Request
=======================
Package Name: tcl-tclreadline
Short Description: GNU Readline extension for Tcl/Tk
Upstream URL: http://tclreadline.sourceforge.net/
Owners: robert
Branches: f19 f20 f21 el5 el6 epel7
InitialCC:

RP Herrold - I still get an error on the src.rpm from rpmlint.
[mock@mythbox ~]$ rpmlint /var/lib/mock/epel-7-x86_64/result/tcl-tclreadline-2.1.0-3.el7.centos.src.rpm /var/lib/mock/epel-7-x86_64/result/tcl-tclreadline-2.1.0-3.el7.centos.x86_64.rpm 
tcl-tclreadline.src: E: specfile-error sh: tclsh: command not found
tcl-tclreadline.x86_64: W: incoherent-version-in-changelog 2.1.0-3 ['2.1.0-3.el7.centos', '2.1.0-3.centos']
2 packages and 0 specfiles checked; 1 errors, 1 warnings.
[mock@mythbox ~]$ rpm -qi rpmlint
Name        : rpmlint
Version     : 1.5
Release     : 4.el7
Architecture: noarch
Install Date: Mon 04 Aug 2014 08:22:55 PM EDT
Group       : Development/Tools
Size        : 1225006
License     : GPLv2
Signature   : RSA/SHA256, Fri 04 Jul 2014 12:50:56 AM EDT, Key ID 24c6a8a7f4a80eb5
Source RPM  : rpmlint-1.5-4.el7.src.rpm
Build Date  : Mon 09 Jun 2014 11:28:19 PM EDT
Build Host  : worker1.bsys.centos.org
Relocations : (not relocatable)
Packager    : CentOS BuildSystem <http://bugs.centos.org>
Vendor      : CentOS
URL         : http://sourceforge.net/projects/rpmlint/
Summary     : Tool for checking common errors in RPM packages
Description :
rpmlint is a tool for checking common errors in RPM packages.  Binary
and source packages as well as spec files can be checked.
[mock@mythbox ~]$

(In reply to Robert Lightfoot from comment #19)
> tcl-tclreadline.src: E: specfile-error sh: tclsh: command not found

This is caused by tclsh(1) being not available on your system. Usually this
is also case on builders until the build environment has been set up. Caused
by this line:

%{!?tcl_version: %global tcl_version %((echo '8.5'; echo 'puts $tcl_version' | tclsh) | tail -1)}

If it makes you more happy, we can silent it by using instead:

%{!?tcl_version: %global tcl_version %((echo '8.5'; echo 'puts $tcl_version' | tclsh 2> /dev/null) | tail -1)}

However this is just hiding the error message from STDERR.

I'll defer to others wiser than I as you response just confuses me.  If tclsh is needed on the build system then why is there no BuildRequires in the spec file.  This was done on a fresh Centos7 system with mock.

Command tclsh is provided by package tcl which is a build requires.

The "build" in mock usually consists out of two steps (see "ENTER do") in the
build.log; it is usually there twice. The first one is to build the *.src.rpm
and the second one is for the binary package. The build requires are satisfied
at the second one while at the first there is only a minimal environment. In
that minimal environment there's usually no tclsh, thus the "echo '8.5'" from
above macro wins. If the binary package gets build the build requires of the
source rpm have been definately satisfied and building happens against exactly
that specified version that is returned by tclsh.

Git done (by process-git-requests).

tcl-tclreadline-2.1.0-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/tcl-tclreadline-2.1.0-3.fc20

tcl-tclreadline-2.1.0-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/tcl-tclreadline-2.1.0-3.fc19

tcl-tclreadline-2.1.0-3.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/tcl-tclreadline-2.1.0-3.el6

tcl-tclreadline-2.1.0-3.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/tcl-tclreadline-2.1.0-3.el5

tcl-tclreadline-2.1.0-3.el5 has been pushed to the Fedora EPEL 5 testing repository.

tcl-tclreadline-2.1.0-3.fc19 has been pushed to the Fedora 19 stable repository.

tcl-tclreadline-2.1.0-3.fc20 has been pushed to the Fedora 20 stable repository.

tcl-tclreadline-2.1.0-3.el5 has been pushed to the Fedora EPEL 5 stable repository.

tcl-tclreadline-2.1.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository.