[Bug 581181] Review Request: scalpel - Forensic tool for file carving from disk images

[bugzilla at redhat.com]

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=581181

--- Comment #2 from Michal Ambroz <rebus at seznam.cz> 2010-08-24 13:06:26 EDT ---
Hello Pavel,
thank you for picking review of this package. The review is great and I will
try to fix the issues.


>[+] MUST: rpmlint
>scalpel.src: E: unknown-key (MD5
Package is signed with my gpg id and rpmlint reports this as error, this shuold
be gone when rebuilt by koji.

>[-] MUST: The package must meet the Packaging Guidelines.
>1) Tarball contain binary windows files in %prep.
fixed - deleted in prep phase

>2) For what you are include scalpel.conf also in doc?
For nothing. Original idea was to include sample config, but then I decided it
would be better to have one usable global configuration out of the box so I
patched the code for opening the configuration file

>3) Lines #BuildRequires: #Requires: is garbage.
Garbage out

>4) You include patch without comment with link to bugtracker.
There is no bugtracker for the package. 
I have sent now the patch to author to consider include it in the mainstream
version and added comment.


>[-] MUST: The License field in the package spec file must match the actual license.
>Source tarball contain two files prioque.h and prioque.c with other author than
>scalpel and without licence mention. It require clarification.
I believe author of all files is the same - just he made Ph. D. (congratz
Golden G. Richard III ! :) . Have you found it by some automated tool?
I will contact the author to consider putting the license to the files, but I
assume it is not a problem right now.

>[-] MUST: Permissions on files must be set properly. Executables should be set
>%attr specification is ambiguous in:
>%attr(755,root,root) %{_bindir}/%{name} 
I do not understand what you mean - this executable will be set with 755
permissions ownership by root.
How ambiguous is that?

>Some more things:
>1) It is ambiguous explicit archiving man page because it will be done
>automatically.
Ok ... removed

>2) If you do not plan maintain it for EPEL 4-5 (guess by presented builds) tag
>BuildRoot is ambiguous.
I do plan to maintain for EPEL as well

>3) I have not completely understand what you are doing wit config on sed. Can
>you describe slightly?
I have put description to comments
In distribution configuration everything is commented out.
Sed will enable most of the file extensions to be found.

http://rebus.webz.cz/download.php?get=scalpel.spec
http://rebus.webz.cz/download.php?get=scalpel-1.60-2.fc13.src.rpm

Best regards
Michal Ambroz

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.