Bug 894338

Summary: Review Request: distorm3 - A powerful disassembler library for x86/AMD64
Product: [Fedora] Fedora Reporter: Ramon de C Valle <rcvalle>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alex94puchades, bressers, bugs.michael, huzaifas, i, keesdejong+dev, notting, rcvalle, sparks, zebob.m
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-07-11 00:46:44 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: 201449    
Attachments:
Description Flags
remaining spec fixes none

Description Ramon de C Valle 2013-01-11 13:45:23 UTC
Spec URL: http://people.redhat.com/~rdecarva/libdistorm/libdistorm.spec
SRPM URL: http://people.redhat.com/~rdecarva/libdistorm/libdistorm-3.3-1.fc18.src.rpm
Description: 
A lightweight, easy-to-use and fast disassembler/decomposer library for
x86/AMD64. A decomposer means that you get a binary structure that describes
an instruction rather than textual representation.

Fedora Account System Username: rcvalle

Comment 1 Eric Christensen 2013-01-11 14:59:12 UTC
A few items need to be taken care of here.  I'll finish the review when the failures have been addressed.


Package Review
==============

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
[!]: Permissions on files are set properly.
     Note: See rpmlint output
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files directly in %_libdir.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages


===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.
[!]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files directly in %_libdir.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[ ]: Package contains no bundled libraries.
[ ]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[ ]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[ ]: Macros in Summary, %description expandable at SRPM build time.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package requires other packages for directories it uses.
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[!]: Permissions on files are set properly.
     Note: See rpmlint output
[x]: Fully versioned dependency in subpackages, if present.
[ ]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[ ]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v3 or later)", "Unknown or generated". 2 files have unknown
     license. Detailed output of licensecheck in
     /tmp/894338-libdistorm/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named using only allowed ASCII characters.
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: Package do not use a name that already exist
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Package must own all directories that it creates.
[ ]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[ ]: Package is not relocatable.
[ ]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[ ]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[ ]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[ ]: Useful -debuginfo package or justification otherwise.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[ ]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[x]: The placement of pkgconfig(.pc) files are correct.
[ ]: Scriptlets must be sane, if used.
[x]: SourceX tarball generation or download is documented.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Source0 (distorm3.zip)
[x]: SourceX is a working URL.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: libdistorm-3.3-1.fc16.src.rpm
          libdistorm-debuginfo-3.3-1.fc16.x86_64.rpm
          libdistorm-3.3-1.fc16.x86_64.rpm
          libdistorm-devel-3.3-1.fc16.x86_64.rpm
libdistorm.src: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm.src: W: summary-not-capitalized C diStorm
libdistorm.src: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm.src: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm.src: E: no-changelogname-tag
libdistorm-debuginfo.x86_64: E: no-changelogname-tag
libdistorm-debuginfo.x86_64: E: debuginfo-without-sources
libdistorm.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm.x86_64: W: summary-not-capitalized C diStorm
libdistorm.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm.x86_64: E: no-changelogname-tag
libdistorm.x86_64: W: no-soname /usr/lib64/libdistorm3.so
libdistorm.x86_64: W: no-documentation
libdistorm.x86_64: E: non-standard-executable-perm /usr/lib64/libdistorm3.so 0775L
libdistorm-devel.x86_64: E: no-changelogname-tag
libdistorm-devel.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 6 errors, 11 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint libdistorm-debuginfo libdistorm-devel libdistorm
libdistorm-debuginfo.x86_64: I: enchant-dictionary-not-found en_US
libdistorm-debuginfo.x86_64: E: no-changelogname-tag
libdistorm-debuginfo.x86_64: E: debuginfo-without-sources
libdistorm-devel.x86_64: E: no-changelogname-tag
libdistorm-devel.x86_64: W: no-documentation
libdistorm.x86_64: W: summary-not-capitalized C diStorm
libdistorm.x86_64: E: no-changelogname-tag
libdistorm.x86_64: W: no-soname /usr/lib64/libdistorm3.so
libdistorm.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libdistorm3.so linux-vdso.so.1
libdistorm.x86_64: W: no-documentation
libdistorm.x86_64: E: non-standard-executable-perm /usr/lib64/libdistorm3.so 0775L
3 packages and 0 specfiles checked; 5 errors, 5 warnings.
# echo 'rpmlint-done:'



Requires
--------
libdistorm-debuginfo-3.3-1.fc16.x86_64.rpm (rpmlib, GLIBC filtered):
    

libdistorm-3.3-1.fc16.x86_64.rpm (rpmlib, GLIBC filtered):
    
    /sbin/ldconfig  
    libc.so.6()(64bit)  
    rtld(GNU_HASH)  

libdistorm-devel-3.3-1.fc16.x86_64.rpm (rpmlib, GLIBC filtered):
    
    libdistorm = 3.3-1.fc16



Provides
--------
libdistorm-debuginfo-3.3-1.fc16.x86_64.rpm:
    
    libdistorm-debuginfo = 3.3-1.fc16
    libdistorm-debuginfo(x86-64) = 3.3-1.fc16

libdistorm-3.3-1.fc16.x86_64.rpm:
    
    libdistorm = 3.3-1.fc16
    libdistorm(x86-64) = 3.3-1.fc16
    libdistorm3.so()(64bit)  

libdistorm-devel-3.3-1.fc16.x86_64.rpm:
    
    libdistorm-devel = 3.3-1.fc16
    libdistorm-devel(x86-64) = 3.3-1.fc16



Unversioned so-files
--------------------
libdistorm-3.3-1.fc16.x86_64.rpm: /usr/lib64/libdistorm3.so

MD5-sum check
-------------
http://distorm.googlecode.com/files/distorm3.zip :
  CHECKSUM(SHA256) this package     : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582
  CHECKSUM(SHA256) upstream package : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-16-x86_64
Command line :/usr/bin/fedora-review -b 894338

Comment 2 Michael Schwendt 2013-01-13 21:21:33 UTC
> [!]: Development (unversioned) .so files in -devel subpackage, if present.
>      Note: Unversioned so-files directly in %_libdir.

Did you notice that the main package would be empty then?

Please always verify what fedora-review reports.

The src.rpm needs a lot of work.

Comment 3 Ramon de C Valle 2013-01-14 16:29:03 UTC
Actually not, because my files section was (*.so, and not *.so.*):

%files
%doc
%{_libdir}/*.so

(In reply to comment #2)
> > [!]: Development (unversioned) .so files in -devel subpackage, if present.
> >      Note: Unversioned so-files directly in %_libdir.
> 
> Did you notice that the main package would be empty then?
> 
> Please always verify what fedora-review reports.
> 
> The src.rpm needs a lot of work.

Comment 4 Michael Schwendt 2013-01-14 18:49:57 UTC
$ rpmls -p libdistorm-devel-3.3-1.fc18.x86_64.rpm 
-rw-r--r--  /usr/include/distorm.h
-rw-r--r--  /usr/include/mnemonics.h

$ rpmls -p libdistorm-3.3-1.fc18.x86_64.rpm 
-rwxr-xr-x  /usr/lib64/libdistorm3.so

qed

Comment 5 Ramon de C Valle 2013-01-14 19:17:27 UTC
I don't see any empty packages in your demo.

(In reply to comment #4)
> $ rpmls -p libdistorm-devel-3.3-1.fc18.x86_64.rpm 
> -rw-r--r--  /usr/include/distorm.h
> -rw-r--r--  /usr/include/mnemonics.h
> 
> $ rpmls -p libdistorm-3.3-1.fc18.x86_64.rpm 
> -rwxr-xr-x  /usr/lib64/libdistorm3.so
> 
> qed

Comment 6 Michael Schwendt 2013-01-14 19:31:46 UTC
That can only be because you misunderstand Eric's review in comment 1 and my comment 2.

More slowly then, okay. From comment 1, where fedora-review reported this packaging failure:

  [!]: Development (unversioned) .so files in -devel subpackage, if present.
       Note: Unversioned so-files directly in %_libdir.

This is a false positive. You could not move /usr/lib64/libdistorm3.so to the -devel package, because it is the only file in the base package. And it is a run-time library, not a development file.

[...]

I see you've updated the spec file silently. Please bump the "Release" version when you do that, and maintain the %changelog section, too.

The updated src.rpm still suffers from several issues. What you've changed silently with regard to the shared library doesn't make sense.

Do run "rpmlint -i" on both the src.rpm and the built rpms. Also try a simple "rpmbuild --rebuild" with your src.rpm. It cannot be built more than once because of the weird things you do in %prep:

$ rpmbuild --rebuild libdistorm-3.3-1.fc18.src.rpm 
Installing libdistorm-3.3-1.fc18.src.rpm
warning: user rcvalle does not exist - using root
warning: group rcvalle does not exist - using root
warning: user rcvalle does not exist - using root
warning: group rcvalle does not exist - using root
Executing(%prep): /bin/sh -e /home/ms18b/tmp/rpm/tmp/rpm-tmp.lelXoK
+ umask 022
+ cd /home/ms18b/tmp/rpm/BUILD
+ unzip /home/ms18b/tmp/rpm/SOURCES/libdistorm-3.3/distorm3.zip
Archive:  /home/ms18b/tmp/rpm/SOURCES/libdistorm-3.3/distorm3.zip
replace distorm3/COPYING? [y]es, [n]o, [A]ll, [N]one, [r]ename: 
^C

Comment 7 Ramon de C Valle 2013-01-14 19:52:59 UTC
(In reply to comment #6)
> That can only be because you misunderstand Eric's review in comment 1 and my
> comment 2.
> 
> More slowly then, okay. From comment 1, where fedora-review reported this
> packaging failure:
> 
>   [!]: Development (unversioned) .so files in -devel subpackage, if present.
>        Note: Unversioned so-files directly in %_libdir.
> 
> This is a false positive. You could not move /usr/lib64/libdistorm3.so to
> the -devel package, because it is the only file in the base package. And it
> is a run-time library, not a development file.
> 
> [...]
> 
> I see you've updated the spec file silently. Please bump the "Release"
> version when you do that, and maintain the %changelog section, too.
I didn't updated it "silently". I'm working with Eric and notified him about the update. And even before I updated it, none of the resulting packages were empty.

> 
> The updated src.rpm still suffers from several issues. What you've changed
> silently with regard to the shared library doesn't make sense.
> 
> Do run "rpmlint -i" on both the src.rpm and the built rpms. Also try a
> simple "rpmbuild --rebuild" with your src.rpm. It cannot be built more than
> once because of the weird things you do in %prep:
> 
> $ rpmbuild --rebuild libdistorm-3.3-1.fc18.src.rpm 
> Installing libdistorm-3.3-1.fc18.src.rpm
> warning: user rcvalle does not exist - using root
> warning: group rcvalle does not exist - using root
> warning: user rcvalle does not exist - using root
> warning: group rcvalle does not exist - using root
> Executing(%prep): /bin/sh -e /home/ms18b/tmp/rpm/tmp/rpm-tmp.lelXoK
> + umask 022
> + cd /home/ms18b/tmp/rpm/BUILD
> + unzip /home/ms18b/tmp/rpm/SOURCES/libdistorm-3.3/distorm3.zip
> Archive:  /home/ms18b/tmp/rpm/SOURCES/libdistorm-3.3/distorm3.zip
> replace distorm3/COPYING? [y]es, [n]o, [A]ll, [N]one, [r]ename: 
> ^C
Just type "A". In addition, it doesn't happen for the "silently" updated Spec file anymore because I added the lines for removing the unpacked sources from previous builds, if any. Can you enumerate which "weird" things I do on prep? Btw, are you planning helping with anything?

Comment 8 Michael Schwendt 2013-01-14 20:09:14 UTC
> And even before I updated it, none of the resulting packages were empty.

You still misunderstand it then.


> Just type "A".

Interactive builds are not acceptable.


> Can you enumerate which "weird" things I do on prep?

1) not starting in a clean/empty builddir
2) not building in a %{name}-%{version} namespace dir like thousands of other packages
3) unzipping the source manually instead of using %setup for that
4) waiting for keyboard input because of 1)


> Btw, are you planning helping with anything?

Depends on whether you are willing to learn. At least you've started asking questions. That's good. I would use this %prep section, which solves all the problems in your one:

%prep
%setup -q -c %{name}-%{version}
%setup -q -n %{name}-%{version}/distorm3/make/linux -D -T

Comment 9 Ramon de C Valle 2013-01-14 20:23:59 UTC
(In reply to comment #8)
> > And even before I updated it, none of the resulting packages were empty.
> 
> You still misunderstand it then.
Or maybe you're contradicting yourself or not being clear enough.

> 
> 
> > Just type "A".
> 
> Interactive builds are not acceptable.
Quoting myself from the previous comment:

"...it doesn't happen for the "silently" updated Spec file anymore because I added the lines for removing the unpacked sources from previous builds, if any."

> 
> 
> > Can you enumerate which "weird" things I do on prep?
> 
> 1) not starting in a clean/empty builddir
> 2) not building in a %{name}-%{version} namespace dir like thousands of
> other packages
> 3) unzipping the source manually instead of using %setup for that
> 4) waiting for keyboard input because of 1)
For 1 and 4 see my above answer, for 2 and 3, see below.

> 
> 
> > Btw, are you planning helping with anything?
> 
> Depends on whether you are willing to learn. At least you've started asking
> questions. That's good. I would use this %prep section, which solves all the
> problems in your one:
> 
> %prep
> %setup -q -c %{name}-%{version}
> %setup -q -n %{name}-%{version}/distorm3/make/linux -D -T
It seems redundant and also unnecessarily uses the %setup macro twice. Why is it better than:

rm -fr %{_builddir}/distorm3
unzip %{SOURCE0}
%setup -q -n distorm3/make/linux -D -T

Comment 10 Michael Schwendt 2013-01-14 20:43:04 UTC
> Or maybe you're contradicting yourself or not being clear enough.

Not at all. Eric will be able to explain it to you, because it is his review you misunderstood to begin with.


> "...it doesn't happen for the "silently" updated Spec file anymore

I've downloaded _two_ src.rpms from this ticket, and the second one still was suffering from the same problem. If you continue to publish updates silently in an attempt to fix issues reported to you, you need to accept that reviewers still refer to older files:

  $ md5sum libdistorm-3.3-1.fc18.src.rpm 
  beac57444a21349c4a65c76f0e81cebc  libdistorm-3.3-1.fc18.src.rpm
  Build Date: Mon 14 Jan 2013 05:26:20 PM CET

That's why it's common practice to update the Release tag *and* to maintain a %changelog section in the spec file.
https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes


> Why is it better than:
> 
> rm -fr %{_builddir}/distorm3
> unzip %{SOURCE0}
> %setup -q -n distorm3/make/linux -D -T

Nobody claimed anything would be "better". I only pointed out that your %prep section didn't work well and suggested a cleaner working one. Your latest one still isn't pretty, and the top builddir is still not related to %name-%version, but if it works and if you like it so much, nobody would object. ;-)


What's the status of the package here now?

Comment 11 Ramon de C Valle 2013-01-14 20:53:31 UTC
(In reply to comment #10)
> > Or maybe you're contradicting yourself or not being clear enough.
> 
> Not at all. Eric will be able to explain it to you, because it is his review
> you misunderstood to begin with.
> 
> 
> > "...it doesn't happen for the "silently" updated Spec file anymore
> 
> I've downloaded _two_ src.rpms from this ticket, and the second one still
> was suffering from the same problem. If you continue to publish updates
> silently in an attempt to fix issues reported to you, you need to accept
> that reviewers still refer to older files:
Maybe this is because I'm working with Eric to resolve the issues reported? Until now you haven't annouced yourself as a reviewer nor as a possible sponsor, so don't expect notifications or anything made specially for you.

> 
>   $ md5sum libdistorm-3.3-1.fc18.src.rpm 
>   beac57444a21349c4a65c76f0e81cebc  libdistorm-3.3-1.fc18.src.rpm
>   Build Date: Mon 14 Jan 2013 05:26:20 PM CET
> 
> That's why it's common practice to update the Release tag *and* to maintain
> a %changelog section in the spec file.
> https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes
I wouldn't change it or add a changelog entry until the package is ready for the initial release.

> 
> 
> > Why is it better than:
> > 
> > rm -fr %{_builddir}/distorm3
> > unzip %{SOURCE0}
> > %setup -q -n distorm3/make/linux -D -T
> 
> Nobody claimed anything would be "better". I only pointed out that your
> %prep section didn't work well and suggested a cleaner working one. Your
> latest one still isn't pretty, and the top builddir is still not related to
> %name-%version, but if it works and if you like it so much, nobody would
> object. ;-)
> 
> 
> What's the status of the package here now?
If you are going to review it, the latest version was just "silently" uploaded.

Comment 12 Michael Schwendt 2013-01-14 21:01:59 UTC
Then I'll silently wait for public activity/progress in this ticket and add my comments.

Comment 13 Ramon de C Valle 2013-01-16 11:43:43 UTC
Package Review
==============

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[ ]: Package contains no bundled libraries.
[ ]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[ ]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[ ]: Macros in Summary, %description expandable at SRPM build time.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package requires other packages for directories it uses.
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[ ]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[ ]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v3 or later)", "Unknown or generated". 2 files have unknown
     license. Detailed output of licensecheck in /home/rcvalle/review-
     libdistorm/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named using only allowed ASCII characters.
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: Package do not use a name that already exist
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Package must own all directories that it creates.
[ ]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[ ]: Package is not relocatable.
[ ]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[ ]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[ ]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[ ]: Useful -debuginfo package or justification otherwise.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[ ]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[x]: The placement of pkgconfig(.pc) files are correct.
[ ]: Scriptlets must be sane, if used.
[x]: SourceX tarball generation or download is documented.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Patch0 (distorm3.produce-debugging-information.patch) Source0
     (distorm3.zip)
[x]: SourceX is a working URL.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: libdistorm-devel-3.3-1.fc18.x86_64.rpm
          libdistorm-3.3-1.fc18.x86_64.rpm
          libdistorm-debuginfo-3.3-1.fc18.x86_64.rpm
          libdistorm-3.3-1.fc18.src.rpm
libdistorm-devel.x86_64: W: no-documentation
libdistorm.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm.x86_64: W: summary-not-capitalized C diStorm
libdistorm.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3
libdistorm.x86_64: W: no-documentation
libdistorm.src: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm.src: W: summary-not-capitalized C diStorm
libdistorm.src: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm.src: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
4 packages and 0 specfiles checked; 0 errors, 11 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint libdistorm libdistorm-devel libdistorm-debuginfo
libdistorm.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm.x86_64: W: summary-not-capitalized C diStorm
libdistorm.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3
libdistorm.x86_64: W: no-documentation
libdistorm-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 7 warnings.
# echo 'rpmlint-done:'



Requires
--------
libdistorm-devel-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    
    libdistorm = 3.3-1.fc18

libdistorm-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    
    /sbin/ldconfig
    libc.so.6()(64bit)
    rtld(GNU_HASH)

libdistorm-debuginfo-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    



Provides
--------
libdistorm-devel-3.3-1.fc18.x86_64.rpm:
    
    libdistorm-devel = 3.3-1.fc18
    libdistorm-devel(x86-64) = 3.3-1.fc18

libdistorm-3.3-1.fc18.x86_64.rpm:
    
    libdistorm = 3.3-1.fc18
    libdistorm(x86-64) = 3.3-1.fc18
    libdistorm3.so.3.3()(64bit)

libdistorm-debuginfo-3.3-1.fc18.x86_64.rpm:
    
    libdistorm-debuginfo = 3.3-1.fc18
    libdistorm-debuginfo(x86-64) = 3.3-1.fc18



MD5-sum check
-------------
http://distorm.googlecode.com/files/distorm3.zip :
  CHECKSUM(SHA256) this package     : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582
  CHECKSUM(SHA256) upstream package : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-18-x86_64
Command line :/usr/bin/fedora-review --rpm-spec -n rpmbuild/SRPMS/libdistorm-3.3-1.fc18.src.rpm

Comment 14 Ramon de C Valle 2013-01-21 15:31:52 UTC
Spec URL: http://people.redhat.com/~rdecarva/libdistorm3/libdistorm3.spec
SRPM URL: http://people.redhat.com/~rdecarva/libdistorm3/libdistorm3-3.3-1.fc18.src.rpm
Description: 
A lightweight, easy-to-use and fast disassembler/decomposer library for
x86/AMD64. A decomposer means that you get a binary structure that describes
an instruction rather than textual representation.

Fedora Account System Username: rcvalle

Comment 15 Ramon de C Valle 2013-01-21 15:42:22 UTC
Package Review
==============

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[ ]: Package contains no bundled libraries.
[ ]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[ ]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[ ]: Macros in Summary, %description expandable at SRPM build time.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package requires other packages for directories it uses.
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[ ]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[ ]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v3 or later)", "Unknown or generated". 2 files have unknown
     license. Detailed output of licensecheck in /home/rcvalle/review-
     libdistorm3/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named using only allowed ASCII characters.
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: Package do not use a name that already exist
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Package must own all directories that it creates.
[ ]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[ ]: Package is not relocatable.
[ ]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[ ]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[ ]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[ ]: Useful -debuginfo package or justification otherwise.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[ ]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[ ]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[x]: The placement of pkgconfig(.pc) files are correct.
[ ]: Scriptlets must be sane, if used.
[x]: SourceX tarball generation or download is documented.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Patch0 (distorm3.produce-debugging-information.patch) Source0
     (distorm3.zip)
[x]: SourceX is a working URL.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: libdistorm3-debuginfo-3.3-1.fc18.x86_64.rpm
          libdistorm3-3.3-1.fc18.x86_64.rpm
          libdistorm3-devel-3.3-1.fc18.x86_64.rpm
          libdistorm3-3.3-1.fc18.src.rpm
libdistorm3.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm3.x86_64: W: summary-not-capitalized C diStorm
libdistorm3.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm3.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm3.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3
libdistorm3.x86_64: W: no-documentation
libdistorm3-devel.x86_64: W: no-documentation
libdistorm3.src: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm3.src: W: summary-not-capitalized C diStorm
libdistorm3.src: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm3.src: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
4 packages and 0 specfiles checked; 0 errors, 11 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint libdistorm3 libdistorm3-devel libdistorm3-debuginfo
libdistorm3.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm3.x86_64: W: summary-not-capitalized C diStorm
libdistorm3.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm3.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm3.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3
libdistorm3.x86_64: W: no-documentation
libdistorm3-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 7 warnings.
# echo 'rpmlint-done:'



Requires
--------
libdistorm3-debuginfo-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    

libdistorm3-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    
    /sbin/ldconfig
    libc.so.6()(64bit)
    rtld(GNU_HASH)

libdistorm3-devel-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    
    libdistorm3 = 3.3-1.fc18



Provides
--------
libdistorm3-debuginfo-3.3-1.fc18.x86_64.rpm:
    
    libdistorm3-debuginfo = 3.3-1.fc18
    libdistorm3-debuginfo(x86-64) = 3.3-1.fc18

libdistorm3-3.3-1.fc18.x86_64.rpm:
    
    libdistorm3 = 3.3-1.fc18
    libdistorm3(x86-64) = 3.3-1.fc18
    libdistorm3.so.3.3()(64bit)

libdistorm3-devel-3.3-1.fc18.x86_64.rpm:
    
    libdistorm3-devel = 3.3-1.fc18
    libdistorm3-devel(x86-64) = 3.3-1.fc18



MD5-sum check
-------------
http://distorm.googlecode.com/files/distorm3.zip :
  CHECKSUM(SHA256) this package     : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582
  CHECKSUM(SHA256) upstream package : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-18-x86_64
Command line :/usr/bin/fedora-review --rpm-spec -n rpmbuild/SRPMS/libdistorm3-3.3-1.fc18.src.rpm

Comment 16 Michael Schwendt 2013-01-21 16:10:55 UTC
Filling in the many '[ ]' fields in the report would be interesting as there some issues are waiting to be discovered.
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

Comment 17 eric 2013-01-23 02:55:40 UTC
Package Review
==============

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package contains no bundled libraries.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[-]: Macros in Summary, %description expandable at SRPM build time.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[-]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[?]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[x]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[!]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v3 or later)", "Unknown or generated". 2 files have unknown
     license. Detailed output of licensecheck in
     /tmp/894338-libdistorm3/licensecheck.txt
[-]: License file installed when any subpackage combination is installed.
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named using only allowed ASCII characters.
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: Package do not use a name that already exist
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[x]: Package is not relocatable.
[x]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file is legible and written in American English.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[?]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[x]: The placement of pkgconfig(.pc) files are correct.
[-]: Scriptlets must be sane, if used.
[x]: SourceX tarball generation or download is documented.
[!]: SourceX / PatchY prefixed with %{name}.
     Note: Patch0 (distorm3.produce-debugging-information.patch) Source0
     (distorm3.zip)
[x]: SourceX is a working URL.
[!]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[?]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: libdistorm3-3.3-1.fc18.src.rpm
          libdistorm3-devel-3.3-1.fc18.x86_64.rpm
          libdistorm3-3.3-1.fc18.x86_64.rpm
          libdistorm3-debuginfo-3.3-1.fc18.x86_64.rpm
libdistorm3.src: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm3.src: W: summary-not-capitalized C diStorm
libdistorm3.src: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm3.src: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm3-devel.x86_64: W: no-documentation
libdistorm3.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm3.x86_64: W: summary-not-capitalized C diStorm
libdistorm3.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm3.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm3.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3
libdistorm3.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 11 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint libdistorm3 libdistorm3-devel libdistorm3-debuginfo
libdistorm3.x86_64: W: spelling-error Summary(en_US) diStorm -> distort, storm
libdistorm3.x86_64: W: summary-not-capitalized C diStorm
libdistorm3.x86_64: W: spelling-error %description -l en_US disassembler -> disassemble, disassembles, disassembled
libdistorm3.x86_64: W: spelling-error %description -l en_US decomposer -> recomposed, decompose, decomposes
libdistorm3.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3
libdistorm3.x86_64: W: no-documentation
libdistorm3-devel.x86_64: W: no-documentation
3 packages and 0 specfiles checked; 0 errors, 7 warnings.
# echo 'rpmlint-done:'



Requires
--------
libdistorm3-devel-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    
    libdistorm3 = 3.3-1.fc18

libdistorm3-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    
    /sbin/ldconfig
    libc.so.6()(64bit)
    rtld(GNU_HASH)

libdistorm3-debuginfo-3.3-1.fc18.x86_64.rpm (rpmlib, GLIBC filtered):
    



Provides
--------
libdistorm3-devel-3.3-1.fc18.x86_64.rpm:
    
    libdistorm3-devel = 3.3-1.fc18
    libdistorm3-devel(x86-64) = 3.3-1.fc18

libdistorm3-3.3-1.fc18.x86_64.rpm:
    
    libdistorm3 = 3.3-1.fc18
    libdistorm3(x86-64) = 3.3-1.fc18
    libdistorm3.so.3.3()(64bit)

libdistorm3-debuginfo-3.3-1.fc18.x86_64.rpm:
    
    libdistorm3-debuginfo = 3.3-1.fc18
    libdistorm3-debuginfo(x86-64) = 3.3-1.fc18



MD5-sum check
-------------
http://distorm.googlecode.com/files/distorm3.zip :
  CHECKSUM(SHA256) this package     : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582
  CHECKSUM(SHA256) upstream package : d311d232e108def8acac0d4f6514e7bc070e37d7aa123ab9a9a05b9322321582


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-18-x86_64
Command line :/usr/bin/fedora-review -b 894338

Comment 18 eric 2013-01-23 03:04:03 UTC
The only problem I see is that the COPYING file is not in %docs.  The COPYING file is the license text and should be included in the package.

Comment 19 Michael Schwendt 2013-01-23 10:22:42 UTC
Hopefully this comment catches all issues:


https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.[4

MUST: The License field in the package spec file must match the actual license. [3]

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

A complicated %prep section makes access to local %doc files more complicated, btw.


> [x]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "GPL (v3 or later)", "Unknown or generated". 2 files have unknown
>      license. Detailed output of licensecheck in
>      /tmp/894338-libdistorm3/licensecheck.txt

> License:        GPLv3

GPLv3+ actually.

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses


> [x]: %build honors applicable compiler flags or justifies otherwise.

It doesn't.


> [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.

Since %install automatically empties %buildroot, there is no need to "rm -fr %{buildroot}%{_libdir}" either.


> [x]: Package complies to the Packaging Guidelines

Not yet.


> Summary:        diStorm

This is a very bad summary. The summary should be a short and concise description of the package. https://fedoraproject.org/wiki/Packaging/Guidelines#summary


> [x]: Fully versioned dependency in subpackages, if present.

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package


> [x]: Package is named according to the Package Naming Guidelines.

Debatable. There is nothing that mandates adding the "lib" prefix or appending the "3". Upstream name is just "distorm". OpenSUSE have packaged it differently, just for reference:
http://rpmfind.net/linux/rpm2html/search.php?query=distorm


> [x]: Package does not generate any conflict.

It bears a risk though to install a very generic file name, such "mnemonics.h", directly into /usr/include instead of placing files like that in a subdir.

  $ rpmls -p libdistorm3-devel-3.3-1.fc19.x86_64.rpm 
  -rw-r--r--  /usr/include/distorm.h
  -rw-r--r--  /usr/include/mnemonics.h


> [x]: Final provides and requires are sane (rpm -q --provides and rpm -q
>      --requires).

Not yet. There still is no SONAME:

  libdistorm.x86_64: W: no-soname /usr/lib64/libdistorm3.so.3.3

And what the spec file tries to do about that in the %install section doesn't make sense and doesn't result in sane RPM dependencies either.


> [x]: Package functions as described.

Dubious. Without a libdistorm3.so it's somewhat inconvenient to compile/link with this library. Has it been tested?


> [!]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

This '!' seems to be fedora-review's warning that the summary is not English language, but just the name of this software.

Comment 20 Ramon de C Valle 2013-04-08 17:40:47 UTC
Hi Eric,

I've updated both the SPEC file and packages. I think it is OK for Clint to proceed.

Thanks,

Comment 21 Christopher Meng 2013-08-20 02:31:15 UTC
Your poor attitude made me sick. 

Michael is a nice guy since 2003 being active in Fedora community. He is also a sponsor, you prefer too much of Eric, however you still need sponsor now. 

I don't have interests in such review however this package is in my to-do list.

Comment 22 Ramon de C Valle 2013-08-20 04:02:17 UTC
(In reply to Christopher Meng from comment #21)
> Your poor attitude made me sick. 
> 
> Michael is a nice guy since 2003 being active in Fedora community. He is
> also a sponsor, you prefer too much of Eric, however you still need sponsor
> now. 
This submission was made in conjunction with Eric purposely for him to review. Michael just showed up, and when asked about his intentions about sponsoring it, he just omitted himself. And no, I don't need a sponsor since I've lost interest.

> 
> I don't have interests in such review however this package is in my to-do
> list.

Feel free to take it.

Comment 23 Michael Schwendt 2013-08-20 08:32:35 UTC
Ramon, the whole conversation is preserved in this ticket.

And both the Review Process and the Sponsorship Process are documented in the Wiki. I've added review comments like many other reviewers and have been available for questions all the time.

Comment 24 Ramon de C Valle 2013-08-20 17:34:16 UTC
(In reply to Michael Schwendt from comment #23)
> Ramon, the whole conversation is preserved in this ticket.
Yes, it is.

> 
> And both the Review Process and the Sponsorship Process are documented in
> the Wiki. I've added review comments like many other reviewers and have been
> available for questions all the time.
So, may I assume that you are willing to sponsor it now?

Comment 25 Michael Schwendt 2013-08-20 18:28:00 UTC
Packages aren't sponsored, people are.  I have been here to review and help you.

Before I could decide whether to approve this package, I would need to review its most recent update that would fix the issues that have been pointed out. That would be the minimum prerequisite for sponsorship. Many new package submitters need to contribute a few good reviews first (or submit multiple packages and show knowledge of packaging), and given your previous comments it would only be fair to expect a similar activity from you:

  https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

For Red Hat employees it's much easier to seek a sponsor inside Red Hat, btw.

Comment 26 Ramon de C Valle 2013-08-20 18:41:15 UTC
(In reply to Michael Schwendt from comment #25)
> Packages aren't sponsored, people are.  I have been here to review and help
> you.
> 
> Before I could decide whether to approve this package, I would need to
> review its most recent update that would fix the issues that have been
> pointed out. That would be the minimum prerequisite for sponsorship. Many
> new package submitters need to contribute a few good reviews first (or
> submit multiple packages and show knowledge of packaging), and given your
> previous comments it would only be fair to expect a similar activity from
> you:
The most recent update is in the same URL you downloaded the previous updates and hopefully addresses all aforementioned issues. If you find any other issues just let me know.

> 
>   https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
> 
> For Red Hat employees it's much easier to seek a sponsor inside Red Hat, btw.

Comment 27 Michael Schwendt 2013-08-20 19:38:27 UTC
It's probably the links in comment 14 then.

Changed on March 5th, but without maintaining the %changelog.

  https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes
  |
  | Increase the "Release" tag every time you upload a new package
  | to avoid confusion. The reviewer and other interested parties
  | probably still have older versions of your SRPM lying around
  | to check what has changed between the old and new packages;
  | those get confused when the revision didn't change. 


The fedora-review tool evaluates the "Spec URL:" and "SRPM URL:" lines in review tickets.

Running "fedora-review -b 894338" reports a few things (which I don't copy here, because it seems it's also confused, as it complains about javadoc files).


"rpmlint -i libdistorm3-3.3-1.fc18.src.rpm" also finds something.


The update fixes a few things.

Headers are in an own dir now. Good, but it would be even better, if the %files section were explicit about that directory, so it doesn't include arbitrary stuff:

  %{_includedir}/distorm/
instead of
  %{_includedir}/*


A SONAME is present now, albeit with a very strict major-minor version in addition to the major version in the libname. Has that choice been discussed anywhere?


How to link with the library? Where is libdistorm3.so?

  $ rpmls -p libdistorm3-devel-3.3-1.fc19.x86_64.rpm 
  drwxr-xr-x  /usr/include/distorm
  -rw-r--r--  /usr/include/distorm/distorm.h
  -rw-r--r--  /usr/include/distorm/mnemonics.h


Skimming over the spec file and build.log, a few mistakes remain:

* rpmlint's findings (except for the false positives!)

* https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

* https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

* https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

* Why is the library built with DISTORM_STATIC and not DISTORM_DYNAMIC?

* Fedora 20 will feature this:
  http://fedoraproject.org/wiki/Changes/UnversionedDocdirs

Comment 28 Ramon de C Valle 2013-08-20 21:02:35 UTC
(In reply to Michael Schwendt from comment #27)
> It's probably the links in comment 14 then.
> 
> Changed on March 5th, but without maintaining the %changelog.
> 
>   https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes
>   |
>   | Increase the "Release" tag every time you upload a new package
>   | to avoid confusion. The reviewer and other interested parties
>   | probably still have older versions of your SRPM lying around
>   | to check what has changed between the old and new packages;
>   | those get confused when the revision didn't change. 
I don't want to add any changelog entries until I've a good initial release, which is when I'll start maintaining it.

> 
> 
> The fedora-review tool evaluates the "Spec URL:" and "SRPM URL:" lines in
> review tickets.
> 
> Running "fedora-review -b 894338" reports a few things (which I don't copy
> here, because it seems it's also confused, as it complains about javadoc
> files).
> 
> 
> "rpmlint -i libdistorm3-3.3-1.fc18.src.rpm" also finds something.
Is there any action I should take?

> 
> 
> The update fixes a few things.
> 
> Headers are in an own dir now. Good, but it would be even better, if the
> %files section were explicit about that directory, so it doesn't include
> arbitrary stuff:
> 
>   %{_includedir}/distorm/
> instead of
>   %{_includedir}/*
I just changed it as you pointed out.

> 
> 
> A SONAME is present now, albeit with a very strict major-minor version in
> addition to the major version in the libname. Has that choice been discussed
> anywhere?
I just changed it to "distorm" as you pointed out in previous comments as well.

> 
> 
> How to link with the library? Where is libdistorm3.so?
> 
>   $ rpmls -p libdistorm3-devel-3.3-1.fc19.x86_64.rpm 
>   drwxr-xr-x  /usr/include/distorm
>   -rw-r--r--  /usr/include/distorm/distorm.h
>   -rw-r--r--  /usr/include/distorm/mnemonics.h
> 
> 
> Skimming over the spec file and build.log, a few mistakes remain:
> 
> * rpmlint's findings (except for the false positives!)
> 
> *
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
The patches included are for producing debugging information and setting the soname, so they don't have an upstream bug.

> 
> * https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
> 
> * https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
> 
> * Why is the library built with DISTORM_STATIC and not DISTORM_DYNAMIC?
This is explained at https://code.google.com/p/distorm/wiki/Build_Compilation_Environment#Helper_Macros_for_Compilation

> 
> * Fedora 20 will feature this:
>   http://fedoraproject.org/wiki/Changes/UnversionedDocdirs

Comment 29 Ramon de C Valle 2013-08-20 21:04:19 UTC
Spec URL: http://people.redhat.com/~rdecarva/distorm/distorm.spec
SRPM URL: http://people.redhat.com/~rdecarva/distorm/distorm-3.3-1.fc18.src.rpm
Description: 
A lightweight, easy-to-use and fast disassembler/decomposer library for
x86/AMD64. A decomposer means that you get a binary structure that describes
an instruction rather than textual representation.

Fedora Account System Username: rcvalle

Comment 30 Michael Schwendt 2013-08-20 21:33:05 UTC
Okay, we're talking past eachother. Severely. :-/

I've suggested you use
  %{_includedir}/distorm/
and instead you've used
  %{_includedir}/distorm/*

That doesn't include the distorm directory:
https://fedoraproject.org/wiki/Packaging:UnownedDirectories


I've pointed out the package doesn't use Fedora's compiler flags yet, you've ignored that without a comment.

Same for the base package dependency.
Same for the unversioned docdirs feature.


> rpmlint
>
> Is there any action I should take?

Run rpmlint (or rpmlint -i for more helpful output) on the src.rpm and all
built rpms. Feel free to ignore obvious false positives in the report, but fix
anything else. Preferably add a comment here about whether/when you think what
rpmlint reports is correct or incorrect.
https://fedoraproject.org/wiki/Common_Rpmlint_issues


It seems we also don't use the same terminology. For example, I've pointed out something about the library SONAME, and you modify the src.rpm name and spec file name instead. Well, no objections about that. It's okay to name the package "distorm" as the upstream project. But why don't you answer my questions? How to link with the library? Where is libdistorm3.so?


And once more, why is the dynamic library built with DISTORM_STATIC and not DISTORM_DYNAMIC?

The documentation you've linked, says:
 
| Eventually, for compiling diStorm as a static library,
| make sure DISTORM_STATIC is defined. For compiling diStorm
| as a dynamic library or shared object, make sure DISTORM_DYNAMIC
| is defined. 

What am I missing?

Comment 31 Ramon de C Valle 2013-08-20 23:00:44 UTC
(In reply to Michael Schwendt from comment #30)
> Okay, we're talking past eachother. Severely. :-/
> 
> I've suggested you use
>   %{_includedir}/distorm/
> and instead you've used
>   %{_includedir}/distorm/*
I just fixed this.

> 
> That doesn't include the distorm directory:
> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
> 
> 
> I've pointed out the package doesn't use Fedora's compiler flags yet, you've
> ignored that without a comment.
I added optflags and set seemingly not working _hardened_build macro as well.

> 
> Same for the base package dependency.
The devel package already requires the base package. Should I also add "Requires: %{name}%{?_isa} = %{version}-%{release}"? Can you explain?

> Same for the unversioned docdirs feature.
It seems rpmbuild already has support for this. Can you elaborate?

> 
> 
> > rpmlint
> >
> > Is there any action I should take?
> 
> Run rpmlint (or rpmlint -i for more helpful output) on the src.rpm and all
> built rpms. Feel free to ignore obvious false positives in the report, but
> fix
> anything else. Preferably add a comment here about whether/when you think
> what
> rpmlint reports is correct or incorrect.
> https://fedoraproject.org/wiki/Common_Rpmlint_issues
I'll go over it on the next iteration.

> 
> 
> It seems we also don't use the same terminology. For example, I've pointed
> out something about the library SONAME, and you modify the src.rpm name and
> spec file name instead. Well, no objections about that. It's okay to name
> the package "distorm" as the upstream project. But why don't you answer my
> questions? How to link with the library? Where is libdistorm3.so?
You do have complained about the package name in #c19. It's not libdistorm3.so.3.3 anymore, the major version was removed from the soname as you complained about it as well in #c27, it's libdistorm.so.3.3. I've also added a symlink to libdistorm.so.3.3 in base package to devel package named libdistorm.so as in https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages.

If it's incorrect, how about explaining why it is incorrect and how to fix it instead of making rhetorical questions?

> 
> 
> And once more, why is the dynamic library built with DISTORM_STATIC and not
> DISTORM_DYNAMIC?
> 
> The documentation you've linked, says:
>  
> | Eventually, for compiling diStorm as a static library,
> | make sure DISTORM_STATIC is defined. For compiling diStorm
> | as a dynamic library or shared object, make sure DISTORM_DYNAMIC
> | is defined. 
> 
> What am I missing?
I overlooked this. Thanks for pointing it out. I just changed it by adding a separate patch.

Comment 32 Michael Schwendt 2013-08-20 23:54:39 UTC
> set seemingly not working _hardened_build macro as well.

Why would you say that? It certainly appears in the build.log.


> Should I also add "Requires: %{name}%{?_isa} = %{version}-%{release}"?

Yes, as the guidelines say.


> Can you explain?

It is explained at
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

The reason we make dependencies arch-specific is that it helps package dependency resolvers. Perhaps you've seen Yum pulling in i686 packages on x86_64 in an attempt at trying to resolve non-arch-specific dependencies in a broken-dependency scenario.


> > Same for the unversioned docdirs feature.
> It seems rpmbuild already has support for this. Can you elaborate?

You don't use %doc. You install manually into a versioned docdir, so you don't benefit from the F20 UnversionedDocdirs feature. And you introduce an unowned docdir directory, btw.

The empty %doc lines in your spec file are superfluous. They don't do anything. Typically, one would delete unused lines in the spec file.


> You do have complained about the package name in #c19.

In January. It's taken some time before you would reply to that. ;-)


> It's not libdistorm3.so.3.3 anymore, 

In the differently packaged openSUSE package, it's libdistorm3.so, albeit inside the private Python module directory. The upstream zip archive also contains a "libdistorm3.so" target.

I don't complain, I mostly point out things. Perhaps you are in contact with upstream maintainers? Dunno. When deciding on a library SONAME and version, ABI/API stability/compatibility come into play. I don't know what is being planned. Hence the comment that a full ".3.3" version in the lib soname would be very strict, but I don't know the versioning scheme that will be used _downstream_ in the Fedora package.

Have you planned packaging anything that links with this library?


> I've also added a symlink to libdistorm.so.3.3 in base package
> to devel package named libdistorm.so 

> If it's incorrect, how about explaining why it is incorrect and
> how to fix it 

Well, rpmlint warns about "no-soname" since comment 1 of this ticket, but that and my comments about it have been ignored so far. The linked Common_Rpmlint_issues page in the Wiki explains a bit.

Have you tried installing your builds yet?
It doesn't work.

  $ rpm -qpR distorm-devel-3.3-1.fc19.x86_64.rpm|grep ^lib
  libdistorm.so()(64bit)
  $ rpm -qp --provides distorm-3.3-1.fc19.x86_64.rpm |grep ^lib
  libdistorm.so.3.3()(64bit)

If you're really stuck at this point, I might give it a closer look.


> instead of making rhetorical questions?

???

Comment 33 Ramon de C Valle 2013-08-21 14:58:07 UTC
(In reply to Michael Schwendt from comment #32)
> > set seemingly not working _hardened_build macro as well.
> 
> Why would you say that? It certainly appears in the build.log.
I had to set -fPIC manually, but it might be because I'm using F18. This may work on F19.

> 
> 
> > Should I also add "Requires: %{name}%{?_isa} = %{version}-%{release}"?
> 
> Yes, as the guidelines say.
I just added it as well.

> 
> 
> > Can you explain?
> 
> It is explained at
> https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
> 
> The reason we make dependencies arch-specific is that it helps package
> dependency resolvers. Perhaps you've seen Yum pulling in i686 packages on
> x86_64 in an attempt at trying to resolve non-arch-specific dependencies in
> a broken-dependency scenario.
Thanks for the explanation!

> 
> 
> > > Same for the unversioned docdirs feature.
> > It seems rpmbuild already has support for this. Can you elaborate?
> 
> You don't use %doc. You install manually into a versioned docdir, so you
> don't benefit from the F20 UnversionedDocdirs feature. And you introduce an
> unowned docdir directory, btw.
I just added it as well.

> 
> The empty %doc lines in your spec file are superfluous. They don't do
> anything. Typically, one would delete unused lines in the spec file.
I just removed them.

> 
> 
> > You do have complained about the package name in #c19.
> 
> In January. It's taken some time before you would reply to that. ;-)
But you do complained.

> 
> 
> > It's not libdistorm3.so.3.3 anymore, 
> 
> In the differently packaged openSUSE package, it's libdistorm3.so, albeit
> inside the private Python module directory. The upstream zip archive also
> contains a "libdistorm3.so" target.
Stop referring to the openSUSE package, these are the Python bindings with the shared library embedded for its use, not the library itself as a system-wide shared library.

> 
> I don't complain, I mostly point out things. Perhaps you are in contact with
> upstream maintainers? Dunno. When deciding on a library SONAME and version,
> ABI/API stability/compatibility come into play. I don't know what is being
> planned. Hence the comment that a full ".3.3" version in the lib soname
> would be very strict, but I don't know the versioning scheme that will be
> used _downstream_ in the Fedora package.
I'm not in contact with maintainers, but from what I know from the project, it doesn't break API between minor releases and also have maintained compatibility between major releases. I just changed the name of the package again and changed the soname to have just the major version. I think this should suffice.

> 
> Have you planned packaging anything that links with this library?
Yes, I'm planning to submit another package that depends on this. It's a Ruby gem that uses this library through FFI.

> 
> 
> > I've also added a symlink to libdistorm.so.3.3 in base package
> > to devel package named libdistorm.so 
> 
> > If it's incorrect, how about explaining why it is incorrect and
> > how to fix it 
> 
> Well, rpmlint warns about "no-soname" since comment 1 of this ticket, but
> that and my comments about it have been ignored so far. The linked
> Common_Rpmlint_issues page in the Wiki explains a bit.
It was fixed, then regressed when the optflags was added and CFLAGS reset. I've removed the unnecessary patches and set all CFLAGS directly in the SPEC file. You really haven't saw the distorm.set-soname-field.patch all this time?

> 
> Have you tried installing your builds yet?
> It doesn't work.
> 
>   $ rpm -qpR distorm-devel-3.3-1.fc19.x86_64.rpm|grep ^lib
>   libdistorm.so()(64bit)
>   $ rpm -qp --provides distorm-3.3-1.fc19.x86_64.rpm |grep ^lib
>   libdistorm.so.3.3()(64bit)
> 
> If you're really stuck at this point, I might give it a closer look.
Looks good to me now:

[rcvalle@ThinkPad ~]$ rpm -qp --requires /home/rcvalle/rpmbuild/RPMS/x86_64/distorm3-devel-3.3-1.fc18.x86_64.rpm | grep distorm
distorm3 = 3.3-1.fc18
distorm3(x86-64) = 3.3-1.fc18
[rcvalle@ThinkPad ~]$ rpm -qp --provides /home/rcvalle/rpmbuild/RPMS/x86_64/distorm3-3.3-1.fc18.x86_64.rpm | grep distorm
distorm3 = 3.3-1.fc18
distorm3(x86-64) = 3.3-1.fc18
libdistorm3.so.3()(64bit)

Comment 34 Ramon de C Valle 2013-08-21 15:00:14 UTC
Spec URL: http://people.redhat.com/~rdecarva/distorm3/distorm3.spec
SRPM URL: http://people.redhat.com/~rdecarva/distorm3/distorm3-3.3-1.fc18.src.rpm
Description: 
A lightweight, easy-to-use and fast disassembler/decomposer library for
x86/AMD64. A decomposer means that you get a binary structure that describes
an instruction rather than textual representation.

Fedora Account System Username: rcvalle

Comment 35 Michael Schwendt 2013-08-21 18:49:44 UTC
> $ rpm -qp --requires /home/rcvalle/rpmbuild/RPMS/x86_64/distorm3-devel-3.3-1.fc18.x86_64.rpm | grep distorm
> distorm3 = 3.3-1.fc18
> distorm3(x86-64) = 3.3-1.fc18

That output is an indication that there's something wrong with the -devel package. Typically, shared lib -devel packages depend on the library SONAME. A dependency added for the .so symlink. I guess it's broken, but I haven't checked the new package yet.

Comment 36 Ramon de C Valle 2013-08-26 17:00:43 UTC
Hi Michael,

Do you have any updates on this?

Comment 37 Michael Schwendt 2013-08-30 20:37:32 UTC
Created attachment 792316 [details]
remaining spec fixes

* drop wrong usage of %_pkgdocdir, because it doesn't work in Fedora 20 if file is installed to %{_docdir}/%{name}-%{version} always
* include license file COPYING via %doc magic
* don't end %summary with a dot
* omit leading article in %summary
* drop superfluous %_isa-less base package dep from -devel package
* remove superfluous stuff from %prep section
* don't use %install -c (what does -c do?)
* use %install -p to preserve permissions for prebuilt files
* fix .so symlink in -devel package
* fix bogus date in %changelog

Comment 38 Ramon de C Valle 2013-09-03 22:35:02 UTC
I just updated the submission with your changes. Thanks!

What are the next steps?

(In reply to Michael Schwendt from comment #37)
> Created attachment 792316 [details]
> remaining spec fixes
> 
> * drop wrong usage of %_pkgdocdir, because it doesn't work in Fedora 20 if
> file is installed to %{_docdir}/%{name}-%{version} always
> * include license file COPYING via %doc magic
> * don't end %summary with a dot
> * omit leading article in %summary
> * drop superfluous %_isa-less base package dep from -devel package
> * remove superfluous stuff from %prep section
> * don't use %install -c (what does -c do?)
> * use %install -p to preserve permissions for prebuilt files
> * fix .so symlink in -devel package
> * fix bogus date in %changelog

Comment 39 Ramon de C Valle 2013-09-11 17:24:25 UTC
Hi Michael,

Do you have any updates on this?

Comment 40 Michael Schwendt 2013-09-14 07:55:08 UTC
> What are the next steps?

comment 25


With the bunch of fixes applied, the package should pass review.

Except for the package name, which has been changed back and forth several times already. See comment 19 where I expressed my interpretation of the guidelines:
|
| There is nothing that mandates adding the "lib" prefix or appending
| the "3". Upstream name is just "distorm".

I'm waiting for the FPC to announce something related to the package naming guidelines clarification request in fpc trac ticket 336.

Comment 41 Kees de Jong 2018-03-28 13:39:45 UTC
Why is this package not in the Fedora repository? The package `volatility` requires this package, which is already available in the Fedora repository. Because I needed the package `volatility` I also created a quick and dirty spec file [1], which builds and runs fine on my system. Without the distorm library, `volatility` doesn't work.

The spec file [2] provided in this Bugzilla ticket isn't available anymore. Can I take over this package if the original author of this package-review request lost interest? Of course, I will improve my spec file further.

[1] https://fedorapeople.org/cgit/keesdejong/public_git/rpmbuild.git/tree/SPECS/distorm.spec?id=32f3ac4d5b7696875be4409aa7b43296303e8157
[2] https://people.redhat.com/~rdecarva/libdistorm/libdistorm.spec

Comment 42 Robert-André Mauchin 🐧 2018-03-28 14:27:16 UTC
Mark it ar dead review and open a new bug for your review

Comment 43 Package Review 2020-07-10 00:47:53 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Comment 44 Package Review 2020-07-11 00:46:44 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.