Bug 226494 - Merge Review: tk
Merge Review: tk
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 16:10 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-23 08:20:05 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wart: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:10:57 EST
Fedora Merge Review: tk

http://cvs.fedora.redhat.com/viewcvs/devel/tk/
Initial Owner: mmaslano@redhat.com
Comment 1 Robert Scheck 2007-02-16 08:56:50 EST
Marcela, again, again and again: Watch out _before_ you cause unowned/orphaned 
directories, please! The directory /usr/share/tk8.4 is no longer owned by any 
package but should be owned by tk (like before):

--- tk.spec       2007-02-15 16:25:56.000000000 +0100
+++ tk.spec.rsc   2007-02-16 14:56:01.000000000 +0100
@@ -105,11 +105,7 @@
 %files
 %defattr(-,root,root,-)
 %{_bindir}/wish*
-%{_datadir}/%{name}%{majorver}/demos/
-%{_datadir}/%{name}%{majorver}/images/
-%{_datadir}/%{name}%{majorver}/msgs/
-%{_datadir}/%{name}%{majorver}/*.tcl
-%{_datadir}/%{name}%{majorver}/tclIndex
+%{_datadir}/%{name}%{majorver}
 %{_libdir}/lib%{name}%{majorver}.so
 %{_libdir}/%{name}%{majorver}
 %{_mandir}/man1/*
Comment 2 Wart 2007-02-19 23:52:07 EST
rpmlint output:
E: tk invalid-soname /usr/lib/libtk8.4.so libtk8.4.so
   As noted in the Tcl merge review, this is a historical oddity
   that would probably break lots of stuff if changed.  The rpmlint
   warning can be ignored.
W: tk dangerous-command-in-%pre rm
   It appears that the %pre script attempts to remove a link named
   /usr/{lib,lib64}/tk8.4.  Why does it do this?  Is this an attempt
   to clean up a link that was erroneously made in a previous version
   of Tk?


GOOD
====
* Package named appropriately
* Sources matches upstream
* Spec file legible and in Am. English
* BSD license ok, license file included
* Build dependencies ok
* No locales
* No need for a .desktop file
* ldconfig called correctly for libraries in %{_libdir}
* Not relocatable
* No duplicate %files
* No need for -doc subpackage
* headers and unversioned .so files in -devel subpackage

MUSTFIX
=======
* The 'URL:' tag is the -devel package description is redundant; the url
  defaults to the one specified for the main package.

* As mentioned in comment #1, the /usr/share/tk8.4 directory is unowned.
  The patch in comment #1 will fix it, and simplify the %files section.

* Mixed use of $RPM_BUILD_ROOT and %{buildroot}.  Choose either one and use
  it consistently.

* Move the %{_mandir}/mann/* man pages out of the -devel subpackage and
  into the main package; these are man pages for the script-level API, not
  the C-level API.  The %{_mandir}/man3/* man pages can remain in the -devel
  subpackage.

* The TOPDIR=$PWD at the top of %build is unnecessary.  Remove it.

NOTES
=====
* package does not currently build because Tcl is not providing
  the private header files in /usr/include/tcl-private.  This is a
  bug in the tcl package, not tk.

* The summary shouldn't contain the package name.  You should drop the
  'Tk' from the summary and capitalize the 'g':
  Graphical toolkit for the Tcl scripting language
Comment 3 Marcela Mašláňová 2007-02-20 10:23:27 EST
> W: tk dangerous-command-in-%pre rm

It should remove old directory before installation, if exists.
Comment 4 Wart 2007-02-27 00:56:31 EST
All MUSTFIX items fixed.  Directory ownership looks ok nw.  /Resetting the
'assigned to' field based on the recently adopted review guidelines:
https://www.redhat.com/archives/fedora-maintainers/2007-February/msg00682.html

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