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=781687 Hans de Goede <hdegoede@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hdegoede@xxxxxxxxxx --- Comment #3 from Hans de Goede <hdegoede@xxxxxxxxxx> 2012-01-18 10:07:42 EST --- Hi Brendan, Orcan. Orcan, Brendan asked me to take a look at this as part of a review swap, but it seems you're already reviewing this one, so I guess I'll tackle the next one. But since I'm here anyways and I've already taken a quick peek already I would like to throw in my 2 cents: My comments are based on: http://bsjones.fedorapeople.org/lv2/spec/lv2-ui-2.4-2.fc16.src.rpm My main concern with the current package is that there are several directory ownership issues. First of all looking at the main package I see: %files %doc NEWS %dir %{_libdir}/lv2/ui.lv2 %{_libdir}/lv2/ui.lv2/manifest.ttl %{_libdir}/lv2/ui.lv2/ui.ttl %{_libdir}/lv2/ui.lv2/%{name}.doap.ttl But who owns %{_libdir}/lv2 ? The answer to that is probably lv2core, but if that is the case then the main package should have a Requires: lv2core, because otherwise this package may end up getting installed without lv2core, and then on removal the %{_libdir}/lv2 will stay behind. Then we have the %files section for the -devel package: %files devel %dir %{_includedir}/lv2/lv2plug.in/ns/extensions %{_includedir}/lv2/lv2plug.in/ns/extensions/ui %{_libdir}/lv2/ui.lv2/ui.h %{_libdir}/pkgconfig/lv2-lv2plug.in-ns-extensions-ui.pc Which brings many questions with it, first of all which package owns %{_includedir}/lv2/lv2plug.in/ns ? The answer seems to be lv2core-devel, which means that the -devel sub package should have a Requires: lv2core-devel. Then comes the question, should we have all extensions owning %dir %{_includedir}/lv2/lv2plug.in/ns/extensions I don't think that is a good idea, I think that instead lv2core-devel should own this, simply add a: mkdir $RPM_BUILD_ROOT%{_includedir}/lv2/lv2plug.in/ns/extensions to its %build and a %dir %{_includedir}/lv2/lv2plug.in/ns/extensions to its "%files devel" Looking into the includes a bit further I noticed some weird things with lv2core: # rpm -ql lv2core | grep lv2.h /usr/lib64/lv2/lv2core.lv2/lv2.h # rpm -ql lv2core-devel | grep lv2.h /usr/include/lv2.h These are 2 copies of the same file, installed by different (sub) packages, this seems wrong I think it would be better to make one of them a symlink. I'm also wondering what a C header file is doing in the main package of lv2core? -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review