https://bugzilla.redhat.com/show_bug.cgi?id=1368855 --- Comment #23 from Lubomir Rintel <lkundrak@xxxxx> --- (In reply to Michal Ambroz from comment #22) >Bump to release 2.0.1 >https://rebus.fedorapeople.org/SPECS/radare2.spec >https://rebus.fedorapeople.org/SRPMS/radare2-2.0.1-1.fc25.src.rpm > >I have addressed some of the your findings (comments, license, changelog), but there is still more work to do on the area of minified JS code. Thank you. This indeed looks better. >The bundled JS libraries are not the core of the package functionality. These are various templates to generated beautified reports out of the binary code. I do not even know how to generate some of these in order to test the modified functionality. I understand the reasons, simply I need some more time for that. As I mentioned earlier, one option is to drop the affected functionality. I've said above that fixing this almost certainly needs upstream to take action. If you haven't talking to them about the way they track their third party bundles please do so. >> This is clearly a bug. Their SONAME is wrong: >I would not call it bug. It is just not the double linking as you would expect. > >You expect that there would be (compare for example with libzip.so.4.0.0): >libr_reg.so.2.0.1 with soname libr_reg.so.2 >libr_reg.so.2 link pointing to libr_reg.so.2.0.1 >and libr_reg.so in devel package pointing to libr_reg.so.2.0.1 > >While they have: >libr_reg.so.2.0.1 with soname libr_reg.so >libr_reg.so link pointing to libr_reg.so.2.0.1 > >The structure is the same, just one level shallower. They use the same link for pointing to the library for runtime and for devel. The usual structure allows changing devel link while keeping the runtime link operational with old version. >So you could be having trouble compiling new radare package on a machine where already is old one (not something we would need in fedora). > >We can think of changing that in future, but right now I do not know how to do that without breaking the stuff. Is this an upstream opinion? Did they actually do this on purpose? My guess is not. To clarify, this absolutely needs fixing before the package can be imported. Unless the ABI is kept stable (I presume it is not. But even if it were this is still utterly useless) neither should be the SONAME and such .so files should under no circumstances appear in libdir or in RPM's provides (otherwise, things would link to it and inevitably break which is a huge no-no). >> Also, please drop the last %changelog message. >> tito is arguably something different from this package. >Changing "tito" to "radare2" - originally I thought it is refering to some nickname. >The line came from the original radare2.spec from the project (and was presumably created for the project by Pavel Odvody) so I would not like to just drop it. He deserves credit as well. Well I'm probably just nitpicking here, but that %changelog entry doesn't reflect reality (perhaps Pavel would be surprised to learn that he did an initial radare2 package?). A comment ("# based on tito package by ...") might be a better way to credit the original author if his contributions are significant. There's also unanswered stuff from the last review: (In reply to Lubomir Rintel from comment #17) >>>> %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). > >Fair enough. I'm wondering if you add a comment explaining how do you determine the right configure arguments? This still requires action. >2.2.) Bundling of C libraries This is still a problem (perhaps rather easy) that needs to be addressed; in particular the "provides" tags. With the time slow turnaround time this review has I find it difficult to keep track. What's a good way to track the unresolved issues? Should we start an Etherpad? In any case, a word on two on each of the items would ensure me you're not ignoring them and they're not getting forgotten. Thanks, Lubo -- 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
