[Bug 1368855] Review Request: radare2 - The reverse engineering framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1368855



--- Comment #8 from Michal Ambroz <rebus@xxxxxxxxx> ---
SPEC URL: https://rebus.fedorapeople.org/SPECS/radare2.spec
SRPM URL: https://rebus.fedorapeople.org/SRPMS/radare2-1.3.0-1.fc24.src.rpm

>Please update to the 1.1.0 release tarball.
Updated to 1.3.0 release.

>> Release:        1%{?dist}
>> Summary:        The %{name} reverse engineering framework
> Please consider *not* using macro expansion for every single occurrence of a substring.
Sure ... was mistake. Fixed.

>> Group:          Applications/Engineering
>Probably not the right category. Development/Tools is more suitable (or Development/Debuggers).
Agreed.

>> License:        LGPLv3
>Some parts of the package use a different license; e.g. shlr/grub is apparently GPLv3+, 
>shlr/qnx is probably GPL+, shlr/zip looks like BSD, etc.
Changed to GPLv3+ due to its viral nature, although majority of the package is
meant to be licensed LGPL.
This would definitely deserve some second look.

>> URL:            http://radare.org/
>> #URL:           https://github.com/radare/radare2
>Drop a useless comment please.
No. I consider it useful when updating the package. While the browser friendly
is the radare.org, when updating the package it is more handy the link to
github.


>> Source0:        https://github.com/%{gituser}/%{gitname}/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz
>> Source1:        https://github.com/%{gituser}/%{sdbgitname}/archive/%{sdbcommit}/%{sdbgitname}-%{version}-%{sdbshort}.tar.gz
>This source is not used at all.
True. Not used for the release packing. Source0 used when packing from git
directly (in case of patches preventing build on fedora).


>> %build
>> %configure --with-sysmagic --with-syszip --with-syscapstone
>You don't enable openssl. Why? (no idea what is it used for)
Option is in the configure, but it is not working in radare for anything now.
At least that was answer from Pancake last time I asked about that (see
sys/*.sh for the recommended build path - doesn't contain this option).


>> CFLAGS="%{optflags} -fPIC -I../include" make %{?_smp_mflags} LIBDIR=%{_libdir} PREFIX=%{_prefix} DATADIR=%{DATADIR}
>%{DATADIR}?
fixed

>> %install
>> rm -rf %{buildroot}
>Cleaning buildroot is not needed anymore.
dropped

>> NOSUDO=1 make install DESTDIR=%{buildroot} LIBDIR=%{_libdir} PREFIX=%{_prefix}
>> cp shlr/sdb/src/libsdb.a %{buildroot}/%{_libdir}/libsdb.a
>No static libraries please; drop this one.
dropped

>> %files
>> %doc AUTHORS.md CONTRIBUTING.md DEVELOPERS.md README.md TODO.md doc/*
>> %doc %{_datadir}/doc/%{name}
>Drop this one; it's no longer needed or allowed.
I do not understand your request - please explain why the %files and %doc
should be dropped.

>> %post -n %{name}-devel -p /sbin/ldconfig
>> %postun -n %{name}-devel -p /sbin/ldconfig
>Why? You're not supposed to ship libraries in -devel packages; and you most likely are not.
OK .. gone

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]