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 199183
Summary: | Review Request: e2tools - Manipulate files in unmounted ext2/ext3 filesystems | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Andreas Thienemann <andreas> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | cweyl, j, mr.ecik, panemade, rhbugs |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-11-01 13:55:47 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 |
Description
Andreas Thienemann
2006-07-17 19:55:55 UTC
Hi! I'm not yet sponsored so this is not official review. * MUST items: - rpmlint doesn't show anything - package is named according to Packaging Naming Guidelines - the spec file name is correct - package meets Packaging Guidelines - package is licensed with an open-source license - GPL, license field match actual license and package contains file with text of license in %doc - spec file is written in American English and is legible - package successfully compile on i386 - package doesn't contain duplicate files in %files section - %files section includes %defattr(...) line - spec file contains proper %clean section - macros is used proper in spec file and all others 'must' doesn't concern this package. I think you don't need CPPFLAGS="-Wall -Werror" in %build section, because the build server has his own CPPFLAGS (I think so) and could you explain what %%check section exaclty does? == Not an official review as I'm not yet sponsored == Mock build for rawhide i386 is successfull. * MUST Items: - rpmlint shows no errors - dist tag is present. - The package is named according to the Package Naming Guidelines. - The spec file name matching the base package e2tools, in the format e2tools.spec. - This package meets the Packaging Guidelines. - The spec file for the package MUST be legible. - The package is licensed with an open-source compatible license GPL. - This package includes License file COPYING. - This source package includes the text of the license in its own file,and that file, containing the text of the license for the package is included in %doc. - The sources used to build the package matches the upstream source, as provided in the spec URL. md5sum is correct (1829b2b261e0e0d07566066769b5b28b e2tools-0.0.16.tar.gz) - This package successfully compiled and built into binary rpms for i386 architecture. - This package did not containd any ExcludeArch. - This package owns all directories that it creates. - This package did not contain any duplicate files in the %files listing. - This package have a %clean section, which contains rm -rf $RPM_BUILD_ROOT. - This package used macros. - Document files are included like README COPYING ChangeLog TODO AUTHORS. - Package did NOT contained any .la libtool archives. Also, * Source URL is present and working. * BuildRoot is correct BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * I did not test package. (In reply to comment #1) > I think you don't need CPPFLAGS="-Wall -Werror" in %build section, because > the build server has his own CPPFLAGS (I think so) and could you explain > what %%check section exaclty does? I wanted them explicitly. %check does a function test of the built binaries, making sure they do work. It's just another build-stage. Seems to have some 64-bit problems: cc1: warnings being treated as errors rm.c: In function 'e2rm': rm.c:248: warning: cast to pointer from integer of different size (In reply to comment #4) > Seems to have some 64-bit problems: > > cc1: warnings being treated as errors > rm.c: In function 'e2rm': > rm.c:248: warning: cast to pointer from integer of different size Is this a problem? It should be, e.g., a 32-bit integer -> 64-bit pointer? It's obviously a problem in that it fails the build due to -Wall. Whether it woulc actually cause any problems in the running program, I can't say since it didn't build. The expression in question is (void *) (verbose) ? &verbose : NULL That should almost certainly read (void *) ( (verbose) ? &verbose : NULL ) instead. Andreas, all, this should fix the compile problem mentioned: http://n-dimensional.de/software/e2tools/e2tools-fedora-fixes.patch http://n-dimensional.de/software/e2tools/e2tools.spec http://n-dimensional.de/software/e2tools/e2tools-0.0.16-5.src.rpm Okay, new upload at http://home.bawue.de/~ixs/e2tools/e2tools.spec fixing the x86_64 build issue and incorporating Uli's fixes. This seems to have dropped through the cracks. I grabbed the package from http://home.bawue.de/~ixs/e2tools/e2tools-0.0.16-5.src.rpm and it builds fine on x86_64. * source files match upstream: 1829b2b261e0e0d07566066769b5b28b e2tools-0.0.16.tar.gz * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. * license field matches the actual license. * license is open source-compatible. License text included in package. * latest version is being packaged. * BuildRequires are proper. * compiler flags are appropriate. * %clean is present. * package builds in mock (development, x86_64). * package installs properly * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: e2tools = 0.0.16-5.fc6 = libcom_err.so.2()(64bit) libext2fs.so.2()(64bit) * %check is present and all tests pass (as far as I can tell) * no shared libraries are added to the regular linker search paths. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no libtool .la droppings. * not a GUI app. APPROVED Ping? This has been approved for three weeks now; any reason it hasn't been checked in yet? Andreas, if you're OK with it, I can take it over. |