Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 187609
Summary: | Review Request: tre - POSIX compatible regexp library with approximate matching | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dominik 'Rathann' Mierzejewski <dominik> |
Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | jima |
Target Milestone: | --- | Flags: | wtogami:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-08-04 22:51:22 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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 187610 |
Description
Dominik 'Rathann' Mierzejewski
2006-04-01 18:39:43 UTC
Greetings. I plan on reviewing this package... before I do the full review however, it appears that upstream has released a new version (0.7.3) and changed the license to LGPL. Can you update to the new version and change the license tag in the spec? You may also consider (not a requirement or blocker) using a dist tag in your package, see: http://fedoraproject.org/wiki/DistTag I'll wait for the new version to do a full review. Dropping FE-NEEDSPONSOR. Tom Callaway offered sponsorship in bug 177235 (and bugzilla change-several-bugs-at-once feature requires me to add/edit something). uhm, bugzilla is broken :( Ping Dominik: Are you still interested in packaging this? If I don't hear back from you in a bit more, I will close this review request. Still here, sorry for the long silence. I'll try to update all of my requests over the weekend. Any further progress on an updated submission? There is interest in the crm114 submission that requires this package... Hey Dominik. Any chance of an updated submission of tre? If you don't have time/interest, we can close this submission and see if someone else is able to submit/maintain it. Adding myself to CC list due to above-mentioned interest in crm114 (for a LUG colleague). It appears that tre 0.7.4 (released a couple months ago) may resolve the issues with x86_64 which caused the original delays with this package moving forward (which I learned of by discussing this package with Dominik on IRC). Alas, I lack access to an x86_64 buildhost (well, for scratch builds), so I can't verify this. If Dominik doesn't have the time to continue this (which was mentioned), I'd be happy to pick up the pieces he put together and maintain them. http://rpm.greysector.net/extras/tre.spec http://rpm.greysector.net/extras/tre-0.7.4-1.src.rpm But I'm afraid the 64bit issues are still there, i.e. it still crashes on me. Also, the license changed to LGPL. Sorry for my delay now. I was out of town. I will try and do a full review in the next few days... Perhaps it's worth reporting the 64bit crash upstream? I see that the 0.7.4 release claims to have a bunch of x86_64 build fixes, but it doesn't say that it fully works there yet. Well, I'm in no position to complain about delays, so no worries. ;) Thanks for taking the time. I'll try reporting the crash this week. Actually, it's crm114 that triggers the crash. ok, here's a review: OK - Package name OK - Spec file matches base package name. OK - Meets Packaging Guidelines. OK - License (LGPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: 8b4bfb078f2cc9e01f37d3d251672f75 tre-0.7.4.tar.bz2 8b4bfb078f2cc9e01f37d3d251672f75 tre-0.7.4.tar.bz2.1 OK - Package compiles and builds on at least one arch. See below - Package needs ExcludeArch OK - BuildRequires correct OK - Spec handles locales/find_lang OK - Spec has needed ldconfig in post and postun n/a - Package is relocatable and has a reason to be. OK - Package owns all the directories it creates. OK - Package has no duplicate files in %files. OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Spec has consistant macro usage. OK - Package is code or permissible content. n/a - -doc subpackage needed/used. OK - Packages %doc files don't affect runtime. OK - Headers/static libs in -devel subpackage. OK - .pc files in -devel subpackage. OK - .so files in -devel subpackage. See below - -devel package Requires: %{name} = %{version}-%{release} OK - .la files are removed. n/a - Package is a GUI app and has a .desktop file OK - Package doesn't own any directories other packages own. See below - No rpmlint output. Issues: 1. Not a blocker, but you might consider using a dist tag: http://fedoraproject.org/wiki/Packaging/DistTag 2. The devel package should Requires: %{name} = %{version}-%{release} 3. rpmlint has some output: (all warnings, but should be cleaned up) W: agrep summary-ended-with-dot Approximate grep utility. W: tre macro-in-%changelog doc W: tre macro-in-%changelog post W: tre mixed-use-of-spaces-and-tabs W: tre-devel summary-ended-with-dot Development files for use with the tre package. W: tre-devel no-documentation (the last one should be ignored) 4. You mention crashes with the x86_64 version. Perhaps the package should be ExcludeArch'ed for now until the bug is found/fixed? I don't have a x86_64 or ppc test machine here, perhaps someone else would like to give it a try on those arches before approval? http://rpm.greysector.net/extras/tre.spec http://rpm.greysector.net/extras/tre-0.7.4-2.src.rpm Fixed all of the above. Thanks for the review! Two issues left that I see: 1. rpmlint still complains about W: tre mixed-use-of-spaces-and-tabs The top part of your tre.spec file uses tabs instead of spaces. This isn't a blocker, but might be nice to replace those tabs with spaces for consistancy. 2. Instead of ExclusiveArch, you should use ExcludeArch on x64... Even though there has been no testing on ppc, it should probibly be included until there is a report of it not working, it shouldn't be excluded just because you aren't able to test it. From the package review guidelines: - 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 needs to have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number should then be placed in a comment, next to the corresponding ExcludeArch line. New packages will not have bugzilla entries during the review process, so they should put this description in the comment until the package is approved, then file the bugzilla entry, and replace the long explanation with the bug number. (Extras Only) The bug should be marked as blocking one (or more) of the following bugs to simplify tracking such issues: FE-ExcludeArch-x86, FE-ExcludeArch-x64, FE-ExcludeArch-ppc http://rpm.greysector.net/extras/tre.spec http://rpm.greysector.net/extras/tre-0.7.4-3.src.rpm Fixed. Everything looks fixed up and therefore this package is APPROVED. Don't forget to close this bug as NEXTRELEASE once it's been imported and built. Also, don't forget to submit a bug against the package once it's got a bugzilla component about the breakage on x64. Package imported and devel build successful. FC-4 and FC-5 branches requested. FC-4 and FC-5 built, closing as NEXTRELEASE. Package Change Request ====================== Package Name: tre New Branches: EL-5 |