This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 199183 - Review Request: e2tools - Manipulate files in unmounted ext2/ext3 filesystems
Review Request: e2tools - Manipulate files in unmounted ext2/ext3 filesystems
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-07-17 15:55 EDT by Andreas Thienemann
Modified: 2007-11-30 17:11 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-01 08:55:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Andreas Thienemann 2006-07-17 15:55:55 EDT
Spec URL: http://home.bawue.de/~ixs/e2tools/e2tools.spec
SRPM URL: http://home.bawue.de/~ixs/e2tools/e2tools-0.0.16-4.src.rpm
Description: 
A simple set of utilities to read, write, and manipulate files in an
ext2/ext3 filesystem directly using the ext2fs library. This does not
require any of

  - root access
  - the filesystem to be mounted
  - the kernel to support ext2

The utilities are: e2cp e2ln e2ls e2mkdir e2mv e2rm e2tail
Comment 1 Michał Bentkowski 2006-07-18 06:13:41 EDT
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?
Comment 2 Parag AN(पराग) 2006-07-19 02:18:34 EDT
== 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.
Comment 3 Andreas Thienemann 2006-07-19 12:05:10 EDT
(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.
Comment 4 Jason Tibbitts 2006-07-20 21:06:54 EDT
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
Comment 5 Chris Weyl 2006-07-21 02:29:05 EDT
(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? 

Comment 6 Jason Tibbitts 2006-07-21 09:41:44 EDT
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.
Comment 7 Hans Ulrich Niedermann 2006-07-21 10:11:50 EDT
The expression in question is

(void *)
(verbose) ? &verbose : NULL

That should almost certainly read

(void *) (
(verbose) ? &verbose : NULL )

instead.
Comment 9 Andreas Thienemann 2006-07-31 07:13:14 EDT
Okay, new upload at http://home.bawue.de/~ixs/e2tools/e2tools.spec fixing the
x86_64 build issue and incorporating Uli's fixes.
Comment 10 Jason Tibbitts 2006-10-05 23:04:09 EDT
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
Comment 11 Jason Tibbitts 2006-10-29 01:01:06 EST
Ping?  This has been approved for three weeks now; any reason it hasn't been
checked in yet?
Comment 12 Hans Ulrich Niedermann 2006-10-30 07:06:53 EST
Andreas, if you're OK with it, I can take it over.

Note You need to log in before you can comment on or make changes to this bug.