[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added Version|devel |rawhide -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added Product|Fedora Extras |Fedora [EMAIL PROTECTED] changed: What|Removed |Added OtherBugsDependingO|228177 | nThis|| -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added Status|ASSIGNED|MODIFIED --- Additional Comments From [EMAIL PROTECTED] 2007-02-27 05:30 EST --- Ok, I changed it in version tcl-8.4.13-12. We can discuss it by email. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added OtherBugsDependingO||228177 nThis|| -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
--- Additional Comments From [EMAIL PROTECTED] 2007-02-27 12:58 EST ---
I recently discovered that bz #227200 is really a multilib problem with Tcl.
That is, it's not possible to install both x86_64 and i386 versions of Tcl
extensions simultaneously due to the symlink and missing %{_libdir}/tcl8.4 from
the package path. I'll try to get some guidance from the mailing lists if this
multilib problem needs to be fixed as part of the merge review or not.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
[EMAIL PROTECTED] changed:
What|Removed |Added
Flag|fedora-review- |fedora-review?
--- Additional Comments From [EMAIL PROTECTED] 2007-02-26 23:55 EST ---
Regression: %{buildroot} and $RPM_BUILD_ROOT are both used.
tclConfig.sh looks good now; Tk and others build fine with it.
Fix the buildroot macro and we should be done here.
Once this review is finished, I hope we can start discussing a plan on
addressing some of the other non-blocking issues that will further clean up the
way Tcl and the various extensions are handled in Fedora.
(Setting the fedora-review flag back to '?' per the revised review guidelines)
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
--- Additional Comments From [EMAIL PROTECTED] 2007-02-21 13:40 EST ---
Finally I built tcl with
ln -s %{_datadir}/%{name}%{majorver}
%{buildroot}%{_prefix}/lib/%{name}%{majorver}
When I change it, I couldn't built it on x86_64 so I ended with this solution
for backward compatibility.
Tcl-html wasn't be separated, because:
- many packages have documentation in one package.
- the documentation isn't huge.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added Status|NEEDINFO|ASSIGNED Flag|needinfo? | -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
--- Additional Comments From [EMAIL PROTECTED] 2007-02-16 06:07 EST ---
comment #11
I think correct will be ln -s {_datadir}/%{name}%{majorver}
$RPM_BUILD_ROOT/%{_prefix}/lib/%{name}%{majorver}
The prefix was used for packages for lib or lib64. I don't know, which packages
need this backward compatibility (not sure if they still needed, or if it's
something what was forgoten in spec).
comment #12
yes, you're right
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
[EMAIL PROTECTED] changed:
What|Removed |Added
Status|ASSIGNED|NEEDINFO
--- Additional Comments From [EMAIL PROTECTED] 2007-02-15 10:09 EST ---
E: tcl invalid-soname /usr/lib/libtcl8.4.so libtcl8.4.so
It must stay for backward compatibility.
Use %{_libdir} instead of %{_prefix}/%{_lib}
Please see if it's everything allright.
html
Patch decline to generate useless html source. I think it's better than spend
time with generating. I don't want split tcl into two packages, maybe in new
version. It's small source for html.
Source does not match upstream.
Could you give me http of your source? Because I check it and they are the same.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 --- Additional Comments From [EMAIL PROTECTED] 2007-02-15 14:44 EST --- (In reply to comment #9) E: tcl invalid-soname /usr/lib/libtcl8.4.so libtcl8.4.so It must stay for backward compatibility. Right. That's why I said the error could be ignored. html Patch decline to generate useless html source. I think it's better than spend time with generating. I don't want split tcl into two packages, maybe in new version. It's small source for html. The html: target in the Makefile isn't run by either 'make' or 'make install', so won't get executed by the spec file anymore. You have to explitly run 'make html' to trigger it. Since 'make html' was removed from the spec file, the -html patch isn't needed. Splitting the -html package into a separate spec file isn't a requirement, but it is strongly encouraged because it will: * simplify the tcl spec file * make the tcl src rpm smaller * prevent needless updates to the -html package whenever tcl is rebuilt * shorten the build time for the tcl package since the -html subpackage won't be rebuilt * allow the -html files to be properly marked as 'noarch' Source does not match upstream. Could you give me http of your source? Because I check it and they are the same. # curl -O http://puzzle.dl.sourceforge.net/sourceforge/tcl/tcl8.4.13-src.tar.gz # md5sum tcl8.4.13-src.tar.gz c6b655ad5db095ee73227113220c0523 tcl8.4.13-src.tar.gz ...but this doesn't match the md5sum in the 'sources' file, nor does it match the md5sum from the tcl source tarball that I get when running 'make srpm'. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
--- Additional Comments From [EMAIL PROTECTED] 2007-02-15 15:04 EST ---
(In reply to comment #9)
E: tcl invalid-soname /usr/lib/libtcl8.4.so libtcl8.4.so
It must stay for backward compatibility.
Use %{_libdir} instead of %{_prefix}/%{_lib}
Please see if it's everything allright.
The use of %{_libdir} looks ok, but the backwards-compatible link is still
wrong:
cd %{_datadir}
ln -s %{name}%{majorver} $RPM_BUILD_ROOT/%{_libdir}/%{name}%{majorver}
This creates the same recursive link:
lrwxrwxrwx 1 root root 6 2007-02-15 11:47 /usr/lib/tcl8.4 - tcl8.4
I think you want either:
ln -s %{_datadir}/%{name}%{majorver}
$RPM_BUILD_ROOT/%{_libdir}/%{name}%{majorver}
or
ln -s %{_datadir}/%{name}%{majorver}
$RPM_BUILD_ROOT/%{_prefix}/lib/%{name}%{majorver}
It's unclear to me from the comment in the spec file if this should be
%{_prefix}/lib or %{_libdir} for backwards compatibility. Do you know which
other packages need this backwards compatible link?
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added Status|ASSIGNED|NEEDINFO Flag||needinfo?([EMAIL PROTECTED]) --- Additional Comments From [EMAIL PROTECTED] 2007-02-14 03:37 EST --- I rebuilt it, some minor problems stay. Please see and let me know your opinion. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
[EMAIL PROTECTED] changed:
What|Removed |Added
Status|NEEDINFO|ASSIGNED
Flag|needinfo?([EMAIL PROTECTED]) |
--- Additional Comments From [EMAIL PROTECTED] 2007-02-14 17:14 EST ---
Now for the full review:
rpmlint output:
E: tcl invalid-soname /usr/lib/libtcl8.4.so libtcl8.4.so
- I'm inclined to ignore this as this is how Tcl has always named/versioned
its shared libraries. Yes, it's awkward, but there are 10 years of
Tcl history pressuring it to remain the same.
W: tcl-debuginfo spurious-executable-perm
/usr/src/debug/tcl-8.4.13/tcl8.4.13/generic/tclThreadAlloc.c
- See MUSTFIX below for a simple fix.
GOOD
* Package and spec file named appropriately
* BSD license ok, license file included
* Spec file legible and in Am. English
* No locales
* ldconfig called appropriately
* Not relocatable
* build root cleaned in %clean
* Headers and unversioned .so shared libs in -devel subpackage
* %doc does not affect runtime
* No .desktop file needed
MUSTFIX
===
* Mixed use of $RPM_BUILD_ROOT and %{buildroot}. Please decide on one
or the other and use it consistently in the spec file.
* Use %{_libdir} instead of %{_prefix}/%{_lib}. If you really mean
%{_prefix}/lib, then be aware that %{_lib} evaluates to lib64 on
x86_64 arches.
* The package installs a recursive symlink:
/usr/lib/tcl8.4 - ./tcl8.4
This used to be the symlink from /usr/share/tcl8.4 - /usr/lib/tcl8.4
* Package does not own all directories that it creates. In order to fix,
and simplify the %files section, you can remove all of the %{_datadir}/...
lines and replace them with a single:
%{_datadir}/%{name}%{majorver}
* rpmlint debuginfo warning is harmless, but easily fixed by adding
to %prep:
chmod -x generic/tclThreadAlloc.c
* The %define epoch 1 is unnecessary. If you set the Epoch: tag to '1',
then rpm will implicitly define the %{epoch} variable.
* Source does not match upstream. It appears that the fedora source
tarball has 3 extra files:
$ diff -r tcl8.4.13.upstream tcl8.4.13.fedora
Only in tcl8.4.13.fedora/library/tcltest: constraints.tcl
Only in tcl8.4.13.fedora/library/tcltest: files.tcl
Only in tcl8.4.13.fedora/library/tcltest: testresults.tcl
If these files were added to upstream's tarball after downloading,
then I would recommend adding them as extra SourceX: files in the
spec and committing them to CVS, instead of modifying the upstream
tarball.
If, however, upstream removed these files and replaced the upstream
tarball without telling anyone, then you should just replace the
fedora tarball with the current upstream version.
* %{_mandir}/mann/* should really be part of the main package, since it
contains man pages for all of the script-level commands. %{_mandir}/man3/*
is correctly located in the -devel subpackage.
* Is the -html patch really necessary? It doesn't seem to have any effect
now that the html docs are installed from the upstream tarball and not
generated at build time.
* Don't bother installing/packaging the ldAix shell script. It's a wrapper
for ld on AIX systems, making it pointless on Fedora.
SHOULD
==
* Consider using the recommended BuildRoot:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* Create a new package + spec file for tcl-html. There are no more
source dependencies between tcl and tcl-html, and splitting them
into separate spec files will allow you to tag tcl-html as
BuildArch: noarch
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 --- Additional Comments From [EMAIL PROTECTED] 2007-02-14 17:18 EST --- The old suggested buildroot is now mandatory so that is a must fix item -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added OtherBugsDependingO||228782 nThis|| -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 --- Additional Comments From [EMAIL PROTECTED] 2007-02-12 12:40 EST --- Review will be needed for tcl8.4. Source tk is needed for generating html. BuildRequires are needed (sed is in configure). What do you mean by redundant version? Links for backward compatibily stay in tcl 8.4. I'll fix it for new version. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 --- Additional Comments From [EMAIL PROTECTED] 2007-02-12 13:26 EST --- Is there a chance you will remove the epoch? Rawhide upgrade path breakage seems to be viewed as the lesser evil compared to introducing an epoch. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
--- Additional Comments From [EMAIL PROTECTED] 2007-02-12 14:51 EST ---
(In reply to comment #2)
Review will be needed for tcl8.4.
Ok, then I'll start a full review based on the current state of CVS.
Source tk is needed for generating html.
I checked upstream, and they publish the html pages:
http://superb-east.dl.sourceforge.net/sourceforge/tcl/tcl8.4.13-html.tar.gz
This will let you separate out the -html subpackage into a standalone package
and drop the included tk source tarball. The -html subpackage can also be made
BuildArch: noarch since it contains only html and a few images.
BuildRequires are needed (sed is in configure).
Separating the html documentation into a separate package removes the need for
'BuildRequires: man'. sed is in the minimal build environment, and is
explicitly disallowed by the packaging guidelines:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-4cadce5e79d38a63cad3941de1dadc9d25d67d30
What do you mean by redundant version?
In the main package you specify:
URL: http://tcl.sourceforge.net/
And again in the -devel (and other) subpackage, you specify the url tag again.
If you leave off the URL: tag in the -devel subpackage, then it will inherit the
value from the main package. The same is true for the Version: tag; it's not
necessary to specify it again in the subpackage.
Links for backward compatibily stay in tcl 8.4. I'll fix it for new version.
Ok, but it would still be nice to address the second component of bug #227200,
adding %{_libdir}/tcl%{majorver} to auto_path. This would let us proceed with
updating the extensions in preparation for bug #226893. But this is really an
orthogonal issue to the merge review, so I won't block the review just for this.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
fedora-review requested: [Bug 226479] Merge Review: tcl
Bug 226479: Merge Review: tcl Product: Fedora Extras Version: devel Component: Package Review Wart [EMAIL PROTECTED] has asked for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: tcl https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479 [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED] Flag||fedora-review? -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug, or are watching the QA contact. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
fedora-review denied: [Bug 226479] Merge Review: tcl
Bug 226479: Merge Review: tcl
Product: Fedora Extras
Version: devel
Component: Package Review
Wart [EMAIL PROTECTED] has denied Wart [EMAIL PROTECTED]'s request for
fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
--- Additional Comments from Wart [EMAIL PROTECTED]
Since Tcl is currently flip-flopping between 8.5a5 and 8.4, I'll start
with some of the low-hanging fruit in the spec file, and return to more
version-specific issues once it settles down.
* Source1 should no longer be necessary now that tk has been split out
into a separate package. This will greatly reduce the size of the
src rpm.
* Removing Source1 will let you collapse the build directory by one
level, since the tk sources no longer get unpacked alongside the tcl
sources. Make sure to adjust the setup and patch commands to reflect
this new structure.
* Source2 contains the html sources. Aren't these already built from the
Tcl sources, or do they have to come from a separate tarball? If
they must come from a separate tarball, it would be nice to include
a pointer to the original url, or commands used to generate the html
files.
* BuildRequires: sed not necessary. Likewise, I suspect that
BuildRequires: man isn't needed either.
* The Version: and URL: tags for the subpackages are redundant. They
are copied from the main package if not found.
* In %build, ls %{_tmppath} is pointless. Delete it.
* Move the 'make test' conditionals to a separate '%check' section instead of
putting it in %build.
* %install contains a 'make html' command that appears to be building the
html documentation. This should be in %build, unless it requires
Tcl to be installed before running. Even if that's the case, it
might be possible to run Tcl from the build directory in order to generate
the html docs. Either move 'make html' to %build, or document in the
spec file why it needs to be in %install.
* The backwards compatible symlink might be better addressed by creating
explicit directories. As it is now, it creates a link from /usr/lib to
/usr/share/tcl8.5 on 64-bit systems, and doesn't create and own
/usr/lib/tcl8.5 at all.
See bug #227200 for one possible workaround.
* The %post and %postun sections should be replaced by one-liners:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 226479] Merge Review: tcl
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Merge Review: tcl
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226479
[EMAIL PROTECTED] changed:
What|Removed |Added
AssignedTo|[EMAIL PROTECTED] |[EMAIL PROTECTED]
Flag|fedora-review? |fedora-review-
--- Additional Comments From [EMAIL PROTECTED] 2007-02-08 23:06 EST ---
Since Tcl is currently flip-flopping between 8.5a5 and 8.4, I'll start
with some of the low-hanging fruit in the spec file, and return to more
version-specific issues once it settles down.
* Source1 should no longer be necessary now that tk has been split out
into a separate package. This will greatly reduce the size of the
src rpm.
* Removing Source1 will let you collapse the build directory by one
level, since the tk sources no longer get unpacked alongside the tcl
sources. Make sure to adjust the setup and patch commands to reflect
this new structure.
* Source2 contains the html sources. Aren't these already built from the
Tcl sources, or do they have to come from a separate tarball? If
they must come from a separate tarball, it would be nice to include
a pointer to the original url, or commands used to generate the html
files.
* BuildRequires: sed not necessary. Likewise, I suspect that
BuildRequires: man isn't needed either.
* The Version: and URL: tags for the subpackages are redundant. They
are copied from the main package if not found.
* In %build, ls %{_tmppath} is pointless. Delete it.
* Move the 'make test' conditionals to a separate '%check' section instead of
putting it in %build.
* %install contains a 'make html' command that appears to be building the
html documentation. This should be in %build, unless it requires
Tcl to be installed before running. Even if that's the case, it
might be possible to run Tcl from the build directory in order to generate
the html docs. Either move 'make html' to %build, or document in the
spec file why it needs to be in %install.
* The backwards compatible symlink might be better addressed by creating
explicit directories. As it is now, it creates a link from /usr/lib to
/usr/share/tcl8.5 on 64-bit systems, and doesn't create and own
/usr/lib/tcl8.5 at all.
See bug #227200 for one possible workaround.
* The %post and %postun sections should be replaced by one-liners:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug, or are watching the QA contact.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
