[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 Robert Scheck changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution||RAWHIDE --- Comment #17 from Robert Scheck 2009-02-23 17:39:37 EDT --- Well, gpm-1.20.6-1.fc11 has been built in Rawhide which is newer rather the reviewed package, so closing now. Thank you, Zdenek. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 Robert Scheck changed: What|Removed |Added Flag|fedora-review- |fedora-review+ --- Comment #16 from Robert Scheck 2009-02-05 08:45:39 EDT --- http://cvs.fedoraproject.org/viewvc/devel/gpm/gpm.spec?revision=1.71 seems now fine to me, thus APPROVED. Please close this bug report, once you've built the changes in Rawhide. Thanks. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Comment #15 from Zdenek Prikryl 2009-02-05 08:35:51 EDT --- Ok, it seems reasonable. Committed. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225856
--- Comment #14 from Robert Scheck 2009-02-05
05:55:30 EDT ---
If "/sbin/install-info %{_infodir}/gpm.info.gz --delete %{_infodir}/dir" fails,
this will make rpm and yum breaking the transaction and thus not updating or
deleting. the package and leave stuff in a not-very-well-state. Printing the
error will be independent of that, but updating/uninstalling will work then; a
thing which we should avoid (otherwise user needs rpm --noscripts etc.).
Rest of your explanations is accepted and/or verified in CVS.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225856
--- Comment #13 from Zdenek Prikryl 2009-02-05 04:14:30
EDT ---
(In reply to comment #12)
> Thank you for going on, rpmlint against latest CVS build (from http://
> cvs.fedoraproject.org/viewvc/devel/gpm/gpm.spec?revision=1.69)
>
> > gpm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgpm.so.2.1.0
> > e...@glibc_2.2.5
>
> Any ideas for this? That really looks strange to me - and I do not really
> have a clue what causes this. See also below at the bottom of this comment.
This is caused by exit() function. If something goes wrong, libgpm calls
exit(). This isn't standard library behaviour. Even in the source code there is
a comment that mentions this fact. But anyway, right now there isn't a easy way
how to get rid of exit() calls. I mean, if we want to remove calls, we'll have
to change an architecture of library's reporting stuff. So, IMO we can ignore
this warning.
>
> > %__cc %{?_smp_mflags} -o inputattach %{SOURCE2}
>
> Well, we've lost $RPM_OPT_FLAGS. See build logs: %{?_smp_mflags} only causes
> -jX, not the rest of the flags $RPM_OPT_FLAGS would bring. So please re-add.
Right, re-added.
> Could you perform real integer comparisons rather half string comparisions?
>
> -if [ "$1" = "0" ]; then
> +if [ $1 -eq 0 ]; then
>
> -if [ "$1" -ge "1" ]; then
> +if [ $1 -ne 0 ]; then
I had a conversation about this and string comparison is safer than integer
comparison. So that's why I prefer this.
>
> Following is suggested to not break rpm transaction if something goes wrong:
>
> -/sbin/install-info %{_infodir}/gpm.info.gz --delete %{_infodir}/dir
> +/sbin/install-info %{_infodir}/gpm.info.gz --delete %{_infodir}/dir || :
IMO, if something goes wrong, user has to be notified.
I committed spec with corrections.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225856
--- Comment #12 from Robert Scheck 2009-02-04
16:42:16 EDT ---
Thank you for going on, rpmlint against latest CVS build (from http://
cvs.fedoraproject.org/viewvc/devel/gpm/gpm.spec?revision=1.69)
> gpm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgpm.so.2.1.0
> e...@glibc_2.2.5
Any ideas for this? That really looks strange to me - and I do not really
have a clue what causes this. See also below at the bottom of this comment.
> gpm.src: W: strange-permission gpm.init 0755
> gpm.x86_64: W: service-default-enabled /etc/rc.d/init.d/gpm
> gpm.x86_64: W: service-default-enabled /etc/rc.d/init.d/gpm
> gpm-devel.x86_64: W: no-documentation
> gpm-static.x86_64: W: no-documentation
Ignore so far. GPM should be enabled per default, otherwise the service does
not make so much sense to me. Docs are not available and permission can't be
solved after wrong import (as eplained above).
> %__cc %{?_smp_mflags} -o inputattach %{SOURCE2}
Well, we've lost $RPM_OPT_FLAGS. See build logs: %{?_smp_mflags} only causes
-jX, not the rest of the flags $RPM_OPT_FLAGS would bring. So please re-add.
Could you perform real integer comparisons rather half string comparisions?
-if [ "$1" = "0" ]; then
+if [ $1 -eq 0 ]; then
-if [ "$1" -ge "1" ]; then
+if [ $1 -ne 0 ]; then
Following is suggested to not break rpm transaction if something goes wrong:
-/sbin/install-info %{_infodir}/gpm.info.gz --delete %{_infodir}/dir
+/sbin/install-info %{_infodir}/gpm.info.gz --delete %{_infodir}/dir || :
Except of things raised above, I would say we're fine. Most hard seems to me
shared-lib-calls-exit - can we avoid it or can we just ignore the warning; I
had a look to bug #450011 and if I see correct, it depends on how it is done;
sometimes it can't be avoided. You know code better than me...suggestions?
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Comment #11 from Zdenek Prikryl 2009-02-04 02:37:09 EDT --- I did a few little changes. So now, (hopefully) the correct version of the spec file is in the cvs :-) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Comment #10 from Zdenek Prikryl 2009-02-03 05:58:50 EDT --- Ok, I committed the new spec file into the cvs. The library is now stripped. Please, review it, so I can bump a new release. Also, I did minor clean up in %prep and %bild section. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Comment #9 from Zdenek Prikryl 2009-01-20 03:58:54 EDT --- (In reply to comment #8) > You're currently adding "Requires: bash >= 2.0" to gpm package. Is this really > needed? Bash < 2.0 existed before 1998 in Red Hat Linux - that was just before > Red Hat Linux 5.2. If we still need it, please explain the need for it. It seems that this requires isn't needed any more. > Do we really need the static library? If yes, we need a -static subpackage. > But > personally, I don't see a need for a *.a file - can we remove it? We need the static library. So, I added -static subpackage. > I think, we can ignore macro-in-%changelog warnings, there's nothing which > gets > expanded here. I changed % to %% so this warnings disappear. > Do we really need to package the TODO file as %doc? That seems to be needed > for > upstream, not for downstream, yes? If we need it, we have to convert it to > UTF8 > using e.g. the following: > > iconv -f iso-8859-1 -t utf-8 -o TODO.utf8 TODO > touch -c -r TODO TODO.utf8 > mv -f TODO.utf8 TODO > I my opinion this file is needed. If anyone wants to start writing patches, then he'll look into this file and start writing. So, I added the conversion to the spec file. > We can't fix W: strange-permission gpm.init 0755 as CVS won't let us do this, > AFAIK. Please have a look in the future, that you're importing/adding files > with the correct permissions, please (0644) - thanks. Ok. > BTW, somebody an idea, what causes W: unstripped-binary-or-object /usr/lib64/ > libgpm.so.2.1.0 and how to solve it? I'll take a look on this. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Comment #8 from Robert Scheck 2009-01-17 10:17:28 EDT --- You're currently adding "Requires: bash >= 2.0" to gpm package. Is this really needed? Bash < 2.0 existed before 1998 in Red Hat Linux - that was just before Red Hat Linux 5.2. If we still need it, please explain the need for it. Do we really need the static library? If yes, we need a -static subpackage. But personally, I don't see a need for a *.a file - can we remove it? I think, we can ignore macro-in-%changelog warnings, there's nothing which gets expanded here. Do we really need to package the TODO file as %doc? That seems to be needed for upstream, not for downstream, yes? If we need it, we have to convert it to UTF8 using e.g. the following: iconv -f iso-8859-1 -t utf-8 -o TODO.utf8 TODO touch -c -r TODO TODO.utf8 mv -f TODO.utf8 TODO We can't fix W: strange-permission gpm.init 0755 as CVS won't let us do this, AFAIK. Please have a look in the future, that you're importing/adding files with the correct permissions, please (0644) - thanks. BTW, somebody an idea, what causes W: unstripped-binary-or-object /usr/lib64/ libgpm.so.2.1.0 and how to solve it? Zdenek - please take action, thank you. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Comment #7 from Robert Scheck 2009-01-17 10:08:07 EDT --- Created an attachment (id=329283) --> (https://bugzilla.redhat.com/attachment.cgi?id=329283) Patch to solve some issues in gpm spec file -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 225856] Merge Review: gpm
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=225856
Robert Scheck changed:
What|Removed |Added
Status|NEW |ASSIGNED
CC||zprik...@redhat.com
AssignedTo|nob...@fedoraproject.org|redhat-bugzi...@linuxnetz.d
||e
Flag||fedora-review-
--- Comment #6 from Robert Scheck 2009-01-17 09:54:15
EDT ---
Adding Zdenek Prikryl (zprikryl), seems currently to be the maintainer. So
lets go for beginning the review:
[ DONE ] MUST: rpmlint must be run on every package. The output should be
posted in the review.
$ rpmlint /var/lib/mock/fedora-10-x86_64/result/gpm-*
gpm.src:17: E: prereq-use /sbin/chkconfig /sbin/ldconfig /sbin/install-info
gpm.src:459: W: macro-in-%changelog config
gpm.src:518: W: macro-in-%changelog preun
gpm.src: W: summary-ended-with-dot A mouse server for the Linux console.
gpm.src: W: strange-permission gpm.init 0755
gpm.x86_64: W: file-not-utf8 /usr/share/doc/gpm-1.20.5/TODO
gpm.x86_64: W: summary-ended-with-dot A mouse server for the Linux console.
gpm.x86_64: W: unstripped-binary-or-object /usr/lib64/libgpm.so.2.1.0
gpm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgpm.so.2.1.0
e...@glibc_2.2.5
gpm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgpm.so.2.1.0
exit@@GLIBC_2.2.5
gpm.x86_64: W: service-default-enabled /etc/rc.d/init.d/gpm
gpm.x86_64: W: service-default-enabled /etc/rc.d/init.d/gpm
gpm-devel.x86_64: W: no-documentation
gpm-devel.x86_64: W: summary-ended-with-dot A mouse server for the Linux
console.
4 packages and 0 specfiles checked; 1 errors, 13 warnings.
$
[ OK ] MUST: The package must be named according to the Package Naming
Guidelines
[ OK ] MUST: The spec file name must match the base package %{name}, in the
format %{name}.spec unless your package has an exemption
[FAILED] MUST: The package must meet the Packaging Guidelines
-> See below for details
[ OK ] MUST: The package must be licensed with a Fedora approved license and
meet the Licensing Guidelines
[ OK ] MUST: The License field in the package spec file must match the
actual license.
[ OK ] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of
the license(s) for the package must be included in %doc
[ OK ] MUST: The spec file must be written in American English.
[ OK ] MUST: The spec file for the package MUST be legible.
[ OK ] MUST: The sources used to build the package must match the upstream
source, as provided in the spec URL. Reviewers should use md5sum for
this task. If no upstream URL can be specified for this package,
please see the Source URL Guidelines for how to deal with this.
-> 3915bdd6bf947ef867752a30b4be2387 gpm-1.20.5.tar.gz
-> 3915bdd6bf947ef867752a30b4be2387 gpm-1.20.5.tar.gz.1
-> 71ee9125414e5a4c3916c5f5f35ee76ca1397f9d gpm-1.20.5.tar.gz
-> 71ee9125414e5a4c3916c5f5f35ee76ca1397f9d gpm-1.20.5.tar.gz.1
[ OK ] MUST: The package MUST successfully compile and build into binary
rpms on at least one primary architecture.
[ N/A ] MUST: If the package does not successfully compile, build or work on
an architecture, then those architectures should be listed in the
spec in ExcludeArch. Each architecture listed in ExcludeArch MUST
have a bug filed in bugzilla, describing the reason that the package
does not compile/build/work on that architecture. The bug number MUST
be placed in a comment, next to the corresponding ExcludeArch line
[ OK ] MUST: All build dependencies must be listed in BuildRequires, except
for any that are listed in the exceptions section of the Packaging
Guidelines ; inclusion of those as BuildRequires is optional. Apply
common sense.
[ N/A ] MUST: The spec file MUST handle locales properly. This is done by
using the %find_lang macro. Using %{_datadir}/locale/* is strictly
forbidden.
[ OK ] MUST: Every binary RPM package (or subpackage) which stores shared
library files (not just symlinks) in any of the dynamic linker's
default paths, must call ldconfig in %post and %postun.
[ N/A ] MUST: If the package is designed to be relocatable, the packager
must state this fact in the request for review, along with the
rationalization for relocation of that specific package. Without
this, use of Prefix: /usr is considered a blocker
[ OK ] MUST: A package must own all directories that it cr
[Bug 225856] Merge Review: gpm
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: gpm https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Additional Comments From [EMAIL PROTECTED] 2008-01-03 13:55 EST --- After thinking and looking abit more at it, I don't have an idea on how the Build cross dependency could be avoided since gpm needs the ncurses headers, and ncurses needs the gpm headers (and libs). -- 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 225856] Merge Review: gpm
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: gpm https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Additional Comments From [EMAIL PROTECTED] 2008-01-03 13:18 EST --- (In reply to comment #3) > On http://invisible-island.net/ncurses/ncurses.faq.html#using_gpm_lib > there is > > With ncurses 5.5, the recommendation is still the same: build the GPM library > without the Gpm_Wgetch interface. ncurses 5.5 can dynamically load the GPM > library on Linux, and that eliminates any reason to have the ncurses library > built with an explicit dependency upon GPM. This seems wrong, however since at least libaa and w3m need the Gpm_Wgetch symbol... -- 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 225856] Merge Review: gpm
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: gpm https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Additional Comments From [EMAIL PROTECTED] 2008-01-03 05:00 EST --- Also there is a strange cross dependency on ncurses, see Bug 226188#13 -- 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 225856] Merge Review: gpm
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: gpm https://bugzilla.redhat.com/show_bug.cgi?id=225856 --- Additional Comments From [EMAIL PROTECTED] 2008-01-03 05:01 EST --- On http://invisible-island.net/ncurses/ncurses.faq.html#using_gpm_lib there is With ncurses 5.5, the recommendation is still the same: build the GPM library without the Gpm_Wgetch interface. ncurses 5.5 can dynamically load the GPM library on Linux, and that eliminates any reason to have the ncurses library built with an explicit dependency upon GPM. -- 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 225856] Merge Review: gpm
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: gpm https://bugzilla.redhat.com/show_bug.cgi?id=225856 [EMAIL PROTECTED] changed: What|Removed |Added Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora Version|devel |rawhide [EMAIL PROTECTED] changed: What|Removed |Added CC||[EMAIL PROTECTED] --- Additional Comments From [EMAIL PROTECTED] 2008-01-03 04:28 EST --- This spec file should be brought up to date, there are many easy items, it would be nice to do this before the review not to waste anyone time. But I have a concern about circular Build dependencies. Indeed, it seems to me that there is a cirular build loop texinfo -> ncurses-devel -> gpm-devel -> gpm -> install-info in texinfo. One way to avoid this loop would be to have a gpm-lib package. This only makes sense if the library are really independent from the executables. It may be, for example, that the library only works if the server is started. Another way could be to have a subpackage for the info files. Or don't care about it. -- 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
