[Bug 225660] Merge Review: crash
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=225660 manuel wolfshant wo...@nobugconsulting.ro changed: What|Removed |Added AssignedTo|wo...@nobugconsulting.ro|nob...@fedoraproject.org -- 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 225660] Merge Review: crash
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=225660 manuel wolfshant wo...@nobugconsulting.ro changed: What|Removed |Added Status|ASSIGNED|NEW Flag|fedora-review? | --- Comment #9 from manuel wolfshant wo...@nobugconsulting.ro 2009-04-10 21:18:15 EDT --- I am releasing the review. Due to $JOB issues I no longer have the time and the willingness to watch over the shoulder of an unresponsive maintainer. -- 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 225660] Merge Review: crash
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=225660 Jason Tibbitts ti...@math.uh.edu changed: What|Removed |Added Flag||fedora-review? -- 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 225660] Merge Review: crash
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=225660 --- Comment #8 from manuel wolfshant wo...@nobugconsulting.ro 2009-03-07 21:58:32 EDT --- It would have been nice if a mention of the existence of a new version in the CVS + a link to the new spec and src.rpm would have been provided here. Back where I come from this is called courtesy. Current issues, as of version crash-4_0-8_7_2_fc11 1. $ rpmlint *rpm crash.src: E: summary-too-long Kernel analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles crash.x86_64: W: spurious-executable-perm /usr/share/doc/crash-4.0/COPYING crash.x86_64: E: summary-too-long Kernel analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles crash-devel.x86_64: W: no-documentation crash-devel.x86_64: W: summary-not-capitalized kernel crash analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles crash-devel.x86_64: E: summary-too-long kernel crash analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles 3 packages and 0 specfiles checked; 3 errors, 3 warnings. The no documentation for -devel is ignorable. The error messages are easy fixable by shortening the summary lines. 2. Source should be http://people.redhat.com/anderson/crash-4.0-7.7.tar.gz; (full URL to the source should be provided, not just a filename) 3. ppc is excluded from the list of architectures. However, ppc64 is included (and koji builds happily for it) so I have to ask: is excluding ppc really intended or just an oversight? 4. The package uses its private copies of several programs (gdb, binutils, readline) without providing any reason for that. At least a note should be included in the spec, explaining the reason. And I think that an exception must be requested from the FPC. 5. The package ignores the compiler flags which are mandatory according to the guidelines. Once again, no reason is provided. On top of that, the flags are not consistently used (gdb gets -c -DHAVE_CONFIG_H -g -O2 -I. -I./../include -W -Wall -Wtraditional -pedantic, readline gets -c -DHAVE_CONFIG_H -I. -I. -DRL_LIBRARY_VERSION='4.3' -g -O2, while crash itself is built using gcc -c -g -O2-I. -I. -I./config -DLOCALEDIR=\/usr/local/share/locale\ -DCRASH_MERGE -DHAVE_CONFIG_H -I./../include/opcode -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../intl -I./../intl -DMI_OUT=1 -DTUI=1 -Wimplicit -Wreturn-type -Wcomment -Wtrigraphs -Wformat -Wparentheses -Wpointer-arith -Wuninitialized -Wformat-nonliteral -Wunused-label -Wunused-function 6. Parallel make (https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make ) is not used. Once again, no reason provided. -- 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 225660] Merge Review: crash
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=225660 --- Comment #3 from Dave Anderson ander...@redhat.com 2009-01-23 12:11:22 EDT --- I'm not sure why the review was made of the crash.spec file from the upstream package instead of the Fedora version? When updating the Fedora package, the upstream spec file is not pulled into Fedora. In any case, I'm presuming that the relevant issues brought up re: the upstream spec file should be addressed in the Fedora version if they haven't already. The Fedora crash utility NVR scheme has (up until now) simply mirrored the upstream version upon which it was based, but I can see the validity in encapsulating the upstream NVR into the Fedora version. It's been an annoyance in the past when somebody has come in (unbeknownst to me) and bumped up the release number, which in turn screwed up the upstream NVR relationship. With respect to the CVE's, both 1704 and 4146 are highly unlikely to be issues given that the crash utility does a number of checks on the vmlinux object file prior to the embedded gdb module ever being invoked. (i.e., using the supplied sample hand-carved object files, they get rejected by the crash code as not being legitimate vmlinux files) Nonetheless, the patches for both of those issues are reasonable and safe to add, and I have done so. I've also incorporated the patch for the .gdbinit-related 1705 issue. When I update the upstream version, I will follow up with a Fedora update -- after I figure out how to check into my account given that I haven't been there since the Fedora break-in. Thanks, Dave Anderson -- 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 225660] Merge Review: crash
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=225660 --- Comment #5 from manuel wolfshant wo...@nobugconsulting.ro 2009-01-23 12:23:29 EDT --- I meant here relying on a bundled version of an existing application -- 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 225660] Merge Review: crash
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=225660 --- Comment #4 from manuel wolfshant wo...@nobugconsulting.ro 2009-01-23 12:22:30 EDT --- (In reply to comment #3) I reviewed the upstream version because by definition so to say Fedora wants to have the latest version of the software, and the content in the cvs is very old. I assumed it will be easier for you to maintain a single or at least very similar spec for both places. I apologize if I made an error by this assumption. Let me know what is the URL of the files that I should have reviewed and I will do it. And yes, the relevant issues brought up re: the upstream spec file must be addressed in the Fedora version. And no, I did not touch the spec exactly because I knew that there is an active maintainer who might have a well defined agenda. As of relying on a bundled application, this is not usually permitted in Fedora and therefore they should be discussed. -- 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 225660] Merge Review: crash
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=225660 --- Comment #6 from Dave Anderson ander...@redhat.com 2009-01-23 14:06:25 EDT --- As of relying on a bundled application, this is not usually permitted in Fedora and therefore they should be discussed. The crash utility package is essentially built around, on-top-of, wrapped-around, or however you want to put it, and it's designed that way on purpose. There's no way it's ever going to be unbundled. -- 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 225660] Merge Review: crash
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=225660 --- Comment #7 from Dave Anderson ander...@redhat.com 2009-01-23 14:20:56 EDT --- I reviewed the upstream version because by definition so to say Fedora wants to have the latest version of the software, and the content in the cvs is very old. I assumed it will be easier for you to maintain a single or at least very similar spec for both places. Not really. The upstream src.rpm (and its minimal .spec file) was added as a convenience to those users who prefer that format to the tar.gz file. I apologize if I made an error by this assumption. Let me know what is the URL of the files that I should have reviewed and I will do it. The content in the cvs (Fedora) is currently based upon the upstream 4.0-6.3 release from last April, and its .spec file updated when Tom Calloway rebuilt it as 4.0-7 this past August. Anyway, the .spec file in the devel branch was -- as I recall -- carried over from a RHEL package version some years ago. Anyway, that's the only .spec file I've ever touched w/respect to Fedora, and that's the one that I would have presumed would be the one that would be review-able. -- 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 225660] Merge Review: crash
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=225660 Lubomir Rintel lkund...@v3.sk changed: What|Removed |Added CC||lkund...@v3.sk --- Comment #2 from Lubomir Rintel lkund...@v3.sk 2009-01-18 16:45:00 EDT --- (In reply to comment #1) - license seems to be GPLv2+. A lot of files are GPL+, some are GPLv2+, some have no license at all. A cleanup of those would be nice Certain files (xen_hyper*) use GPLv2 (only), spot already fixed this in CVS. Other problems (fixed) There are yet more: - You use Revision tag to mark upstream release, which is wrong. It is meant to be used to version the SPEC file. Given you (package owner, crash group, seem to be upstream, you can definitely fix this by changing the versioning scheme. (e.g use 4.0.8 instead of 4.0-8)) - The bundled gdb is old and has issues. It is likely that some of older GDB security problems affect it: http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2005-1704 http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2005-1705 http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2006-4146 Please address those, if they are relevant. Notify your SRT that you bundle GDB code and communicate with GDB upstream (or people involved in Archer, your colleagues) to avoid having to bundle GDB in longer run. -- 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 225660] Merge Review: crash
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=225660 manuel wolfshant wo...@nobugconsulting.ro changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|nob...@fedoraproject.org|wo...@nobugconsulting.ro --- Comment #1 from manuel wolfshant wo...@nobugconsulting.ro 2009-01-09 22:27:09 EDT --- Created an attachment (id=328610) -- (https://bugzilla.redhat.com/attachment.cgi?id=328610) revised spec according to fedora guidelines I've taken a look at the current version (http://people.redhat.com/anderson/crash-4.0-7.6.src.rpm) and did a local mock rebuild. I am attaching a slightly revised spec which fixes part of the problems. The major issues (I have not tried to fix any of these because I am not familiar with the requirements of this special package) are that 1. the package uses it's own versions of several utilities/libs (at least gdb and readline) which contradicts the guidelines. However, according to http://people.redhat.com/anderson/crash_whitepaper/ this is intended. 2.the package builds a static binary. Once again, this is intended 3.fedora compile flags are completely ignored. This also seems to be intended, each of the libraries is built with a different set of flags. Easily fixable stuff (not done): - SMP_FLAGS are not used (I did not know if it's OK to use them ) - the source files (included in -debuginfo) have the exec bit set, which makes rpmlint unhappy - license seems to be GPLv2+. A lot of files are GPL+, some are GPLv2+, some have no license at all. A cleanup of those would be nice Other problems (fixed) - usage of forbidden tags (packager, vendor) or incorrect values (buildroot, source0) - some aesthetic warnings from rpmlint - missing license file from the binary rpm - there was no changelog. I have added one but it should be properly populated - timestamps (for docs/man) were not preserved - defattr was missing in %files -- 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
