Bug 1018859

Summary:	Review Request: perl-Term-Clui - Term::Clui Perl module
Product:	[Fedora] Fedora	Reporter:	Kostas Georgiou <k.georgiou>
Component:	Package Review	Assignee:	Petr Pisar <ppisar>
Status:	CLOSED ERRATA	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	rawhide	CC:	k.georgiou, package-review, ppisar
Target Milestone:	---	Flags:	ppisar: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	perl-Term-Clui-1.68-3.fc18	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2022-09-16 15:39:18 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 Kostas Georgiou 2013-10-14 14:31:08 UTC

Spec URL: http://georgiou.fedorapeople.org//perl-Term-Clui.spec
SRPM URL: http://georgiou.fedorapeople.org//perl-Term-Clui-1.68-1.fc19.src.rpm

Description:
Term::Clui offers a high-level user interface to give the user of command-
line applications a consistent "look and feel". Its metaphor for the
computer is as a human-like conversation-partner, and as each
question/response is completed it is summarised onto one line, and remains
on screen, so that the history of the session gradually accumulates on the
screen and is available for review, or for cut/paste. This user interface
can therefore be intermixed with standard applications which write to
STDOUT or STDERR, such as make, pgp, rcs etc.

Comment 1 Kostas Georgiou 2013-10-14 14:31:14 UTC

This package built on koji:  http://koji.fedoraproject.org/koji/taskinfo?taskID=6058648

Comment 2 Petr Pisar 2013-10-16 11:09:40 UTC

URL is usable. Ok.
Source0 is usable. Ok.
Source tar ball is original (SHA-256: 007f1cc9c5779fe9b2bacefec8535157ae647499347ffbbea931da63e793f216). Ok.

FIX: The summary is not descriptive. See README or Clui.pm for better text.

Description verified from Clui.pm. Ok.

License verified from Clui.pm, Clui/FileSelect.pm, test.pl, examples/login_shell, examples/test_script, examples/linux_admin, examples/audio_stuff, examples/choose. The py/test_script is not part of binary package. Ok.
No XS code. noarch BuildArch is Ok.

TODO: Remove the BuildRoot definition and cleaning in the %install section and remove the whole %clean section. They are not needed for Fedora.

TODO: You can replace `%{__perl}' macro with plain `perl' command.

TODO: You can replace PERL_INSTALL_ROOT argument with standard DESTDIR argument in the %install section.

TODO: You can remove deleting empty directories in the %install section. ExtUtils::MakeMaker does not create empty directories anymore.

FIX: Remove %defattr macro from %files section. This is not needed anymore (since rpm-4.4).

TODO: You could package the `example' subdirectory as a documentation.

FIX: Build-require `perl(Exporter)' (Clui.pm:13).

TODO: Build-require `perl(strict)' (Clui.pm:20).
TODO: Build-require `perl(warnings)' (Clui.pm:20).

The Term::ReadKey, Term::ReadLine::Gnu, Term::Size modules are recommended. Ok.

All tests pass. Ok.

$ rpmlint perl-Term-Clui.spec ../SRPMS/perl-Term-Clui-1.68-1.fc21.src.rpm ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm 
perl-Term-Clui.src: W: spelling-error %description -l en_US summarised -> summarized, summarize
perl-Term-Clui.src: W: spelling-error %description -l en_US pgp -> pg, pp, pep
perl-Term-Clui.src: W: spelling-error %description -l en_US rcs -> rs, cs, arcs
perl-Term-Clui.noarch: W: spelling-error %description -l en_US summarised -> summarized, summarize
perl-Term-Clui.noarch: W: spelling-error %description -l en_US pgp -> pg, pp, pep
perl-Term-Clui.noarch: W: spelling-error %description -l en_US rcs -> rs, cs, arcs
2 packages and 1 specfiles checked; 0 errors, 6 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm
drwxr-xr-x    2 root    root                        0 Oct 16 12:46 /usr/share/doc/perl-Term-Clui
-rw-r--r--    1 root    root                     3848 Jul  3 02:59 /usr/share/doc/perl-Term-Clui/Changes
-rw-r--r--    1 root    root                     3422 Nov 10  2010 /usr/share/doc/perl-Term-Clui/README
-rw-r--r--    1 root    root                     7715 Oct 16 12:46 /usr/share/man/man3/Term::Clui.3pm.gz
-rw-r--r--    1 root    root                     3528 Oct 16 12:46 /usr/share/man/man3/Term::Clui::FileSelect.3pm.gz
drwxr-xr-x    2 root    root                        0 Oct 16 12:46 /usr/share/perl5/vendor_perl/Term
drwxr-xr-x    2 root    root                        0 Oct 16 12:46 /usr/share/perl5/vendor_perl/Term/Clui
-rw-r--r--    1 root    root                    55522 Jul  3 10:48 /usr/share/perl5/vendor_perl/Term/Clui.pm
-rw-r--r--    1 root    root                     9843 Mar 23  2013 /usr/share/perl5/vendor_perl/Term/Clui/FileSelect.pm
File layout and permissions is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm | sort -i | uniq -c
      1 perl(Exporter)
      1 perl(:MODULE_COMPAT_5.18.1)
      1 perl(Term::ReadKey)
      1 perl(Term::ReadLine::Gnu)
      1 perl(Term::Size)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
TODO: Run-require `perl(strict)' (Clui.pm:20).
TODO: Run-require `perl(warnings)' (Clui.pm:20).

$ rpm -q --provides -p ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm | sort -i | uniq -c
      1 perl(Term::Clui) = 1.68
      1 perl-Term-Clui = 1.68-1.fc21
      1 perl(Term::Clui::FileSelect) = 1.68
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Term-Clui-1.68-1.fc21.noarch.rpm 
Binary dependencies resolvable. Ok.

Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6064821). Ok.

Otherwise the package is in line with Fedora and Perl packaging guidelines.

Please correct all `FIX' items, consider fixing `TODO' items, and provide new SPEC file.
Resolution: Package NOT approved.

Comment 3 Kostas Georgiou 2013-10-16 16:55:37 UTC

Thanks for the review, here are the new files and the changes.

Spec URL: http://georgiou.fedorapeople.org/perl-Term-Clui.spec
SRPM URL: http://georgiou.fedorapeople.org/perl-Term-Clui-1.68-2.fc21.src.rpm

> FIX: The summary is not descriptive. See README or Clui.pm for better text.
DONE

> TODO: Remove the BuildRoot definition and cleaning in the %install section and
> remove the whole %clean section. They are not needed for Fedora.
DONE, I can always add them back if we need the package in epel-5

> TODO: You can replace `%{__perl}' macro with plain `perl' command.
DONE

> TODO: You can replace PERL_INSTALL_ROOT argument with standard DESTDIR argument
> in the %install section.
This doesn't seem to work:
  make pure_install DISTDIR=/builddir/build/BUILDROOT/perl-Term-Clui-1.68-2.fc21.x86_64
  ERROR: Can't create '/usr/share/man/man3'
  Do not have write permissions on '/usr/share/man/man3'

> TODO: You can remove deleting empty directories in the %install section.
> ExtUtils::MakeMaker does not create empty directories anymore.
DONE

> FIX: Remove %defattr macro from %files section. This is not needed anymore
> (since rpm-4.4).
DONE

> TODO: You could package the `example' subdirectory as a documentation.
DONE

> FIX: Build-require `perl(Exporter)' (Clui.pm:13).
> TODO: Build-require `perl(strict)' (Clui.pm:20).
> TODO: Build-require `perl(warnings)' (Clui.pm:20).
> TODO: Run-require `perl(strict)' (Clui.pm:20).
> TODO: Run-require `perl(warnings)' (Clui.pm:20).
DONE

Comment 4 Petr Pisar 2013-10-17 06:35:55 UTC

Spec file changes:
--- perl-Term-Clui.spec.old     2013-10-14 16:30:16.000000000 +0200
+++ perl-Term-Clui.spec 2013-10-16 18:53:57.000000000 +0200
@@ -1,22 +1,26 @@
 Name:           perl-Term-Clui
 Version:        1.68
-Release:        1%{?dist}
-Summary:        Term::Clui Perl module
+Release:        2%{?dist}
+Summary:        Perl module offering a Command-Line User Interface
 License:        GPL+ or Artistic
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Term-Clui/
 Source0:        http://www.cpan.org/authors/id/P/PJ/PJB/Term-Clui-%{version}.tar.gz
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(Term::ReadKey)
 BuildRequires:  perl(Term::ReadLine::Gnu)
 BuildRequires:  perl(Term::Size)
 BuildRequires:  perl(Test::Simple)
+BuildRequires:  perl(Exporter)
+BuildRequires:  perl(strict)
+BuildRequires:  perl(warnings)
 Requires:       perl(Term::ReadKey)
 Requires:       perl(Term::ReadLine::Gnu)
 Requires:       perl(Term::Size)
-Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
+Requires:       perl(strict)
+Requires:       perl(warnings)
+Requires:       perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))

 %description
 Term::Clui offers a high-level user interface to give the user of command-
@@ -30,33 +34,31 @@

 %prep
 %setup -q -n Term-Clui-%{version}
+#Don't pull in the examples dependencies
+chmod -x examples/*

 %build
-%{__perl} Makefile.PL INSTALLDIRS=vendor
+perl Makefile.PL INSTALLDIRS=vendor
 make %{?_smp_mflags}

 %install
-rm -rf %{buildroot}
-
 make pure_install PERL_INSTALL_ROOT=%{buildroot}

 find %{buildroot} -type f -name .packlist -exec rm -f {} \;
-find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null \;

 %{_fixperms} %{buildroot}/*

 %check
 make test

-%clean
-rm -rf %{buildroot}
-
 %files
-%defattr(-,root,root,-)
-%doc Changes README
+%doc Changes README examples
 %{perl_vendorlib}/*
 %{_mandir}/man3/*

 %changelog
+* Wed Oct 16 2013 Kostas Georgiou <georgiou> 1.68-2
+- Review changes/fixes #1018859.
+
 * Wed Oct 02 2013 Kostas Georgiou <georgiou> 1.68-1
 - Specfile autogenerated by cpanspec 1.78.

> FIX: The summary is not descriptive. See README or Clui.pm for better text.
+Summary:        Perl module offering a Command-Line User Interface
Ok.

> TODO: Remove the BuildRoot definition and cleaning in the %install section
> and remove the whole %clean section. They are not needed for Fedora.
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

-rm -rf %{buildroot}

-%clean
-rm -rf %{buildroot}
Ok.

> TODO: You can replace `%{__perl}' macro with plain `perl' command.
Ok.

> > TODO: You can replace PERL_INSTALL_ROOT argument with standard DESTDIR
> > argument in the %install section.
> This doesn't seem to work:
>  make pure_install DISTDIR=/builddir/build/BUILDROOT/perl-Term-Clui-1.68-2.fc21.x86_64
>  ERROR: Can't create '/usr/share/man/man3'
>  Do not have write permissions on '/usr/share/man/man3'
I guess you misspelled DESTDIR as DISTDIR.

> TODO: You can remove deleting empty directories in the %install section.
> ExtUtils::MakeMaker does not create empty directories anymore.
-find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null \;
Ok.

> FIX: Remove %defattr macro from %files section. This is not needed anymore
> (since rpm-4.4).
-%defattr(-,root,root,-)
Ok.

> TODO: You could package the `example' subdirectory as a documentation.
-%doc Changes README
+%doc Changes README examples
Ok.

> FIX: Build-require `perl(Exporter)' (Clui.pm:13).
+BuildRequires:  perl(Exporter)
Ok.

> TODO: Build-require `perl(strict)' (Clui.pm:20).
+BuildRequires:  perl(strict)
Ok.

>TODO: Build-require `perl(warnings)' (Clui.pm:20).
+BuildRequires:  perl(warnings)
Ok.

> TODO: Run-require `perl(strict)' (Clui.pm:20).
+Requires:       perl(strict)
Ok.

> TODO: Run-require `perl(warnings)' (Clui.pm:20).
+Requires:       perl(warnings)
Ok.

$ rpmlint perl-Term-Clui.spec ../SRPMS/perl-Term-Clui-1.68-2.fc21.src.rpm ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm 
perl-Term-Clui.src: W: spelling-error %description -l en_US summarised -> summarized, summarize
perl-Term-Clui.src: W: spelling-error %description -l en_US pgp -> pg, pp, pep
perl-Term-Clui.src: W: spelling-error %description -l en_US rcs -> rs, cs, arcs
perl-Term-Clui.noarch: W: spelling-error %description -l en_US summarised -> summarized, summarize
perl-Term-Clui.noarch: W: spelling-error %description -l en_US pgp -> pg, pp, pep
perl-Term-Clui.noarch: W: spelling-error %description -l en_US rcs -> rs, cs, arcs
2 packages and 1 specfiles checked; 0 errors, 6 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Oct 17 08:28 /usr/share/doc/perl-Term-Clui
-rw-r--r--    1 root    root                     3848 Jul  3 02:59 /usr/share/doc/perl-Term-Clui/Changes
-rw-r--r--    1 root    root                     3422 Nov 10  2010 /usr/share/doc/perl-Term-Clui/README
drwxr-xr-x    2 root    root                        0 Oct 17 08:28 /usr/share/doc/perl-Term-Clui/examples
-rw-r--r--    1 root    root                    32203 Apr 11  2013 /usr/share/doc/perl-Term-Clui/examples/audio_stuff
-rw-r--r--    1 root    root                     3256 Sep 27  2009 /usr/share/doc/perl-Term-Clui/examples/choose
-rw-r--r--    1 root    root                    15558 Mar 18  2010 /usr/share/doc/perl-Term-Clui/examples/linux_admin
-rw-r--r--    1 root    root                     5824 Mar 10  2012 /usr/share/doc/perl-Term-Clui/examples/login_shell
-rw-r--r--    1 root    root                     8696 Mar 26  2012 /usr/share/doc/perl-Term-Clui/examples/test_script
-rw-r--r--    1 root    root                     7715 Oct 17 08:28 /usr/share/man/man3/Term::Clui.3pm.gz
-rw-r--r--    1 root    root                     3528 Oct 17 08:28 /usr/share/man/man3/Term::Clui::FileSelect.3pm.gz
drwxr-xr-x    2 root    root                        0 Oct 17 08:28 /usr/share/perl5/vendor_perl/Term
drwxr-xr-x    2 root    root                        0 Oct 17 08:28 /usr/share/perl5/vendor_perl/Term/Clui
-rw-r--r--    1 root    root                    55522 Jul  3 10:48 /usr/share/perl5/vendor_perl/Term/Clui.pm
-rw-r--r--    1 root    root                     9843 Mar 23  2013 /usr/share/perl5/vendor_perl/Term/Clui/FileSelect.pm
File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm | sort -i | uniq -c
      1 perl(:MODULE_COMPAT_5.18.1)
      1 perl(Exporter)
      1 perl(Term::ReadKey)
      1 perl(Term::ReadLine::Gnu)
      1 perl(Term::Size)
      1 perl(strict)
      1 perl(warnings)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Term-Clui-1.68-2.fc21.noarch.rpm 
Binary dependencies resolvable. Ok.

Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6069374). Ok.

Package is good.

Please consider replacing the PERL_INSTALL_ROOT with DESTDIR before building this package.
Resolution: Package APPROVED.

Comment 5 Kostas Georgiou 2013-10-17 10:52:45 UTC

New Package SCM Request
=======================
Package Name: perl-Term-Clui
Short Description: Perl module offering a Command-Line User Interface
Owners: georgiou
Branches: f18 f19 f20 el6
InitialCC: perl-sig

Comment 6 Gwyn Ciesla 2013-10-17 12:37:10 UTC

Git done (by process-git-requests).

Comment 7 Petr Pisar 2014-07-18 11:59:38 UTC

This package in all Fedora stable repositories.

It only waits in EPEL-6 testsing repository <https://admin.fedoraproject.org/updates/FEDORA-EPEL-2013-12001/perl-Term-Clui-1.68-3.el6>.

Comment 8 Petr Pisar 2018-04-25 08:18:26 UTC

Could you please submit the EPEL-6 build to stable repository?

Comment 9 Petr Pisar 2022-09-16 15:39:18 UTC

A development of EPEL 6 has already been closed.