Bug 1174408
Summary: | Review Request: libblockdev - A library for low-level manipulation with block devices | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Vratislav Podzimek <vpodzime> |
Component: | Package Review | Assignee: | Bohuslav "Slavek" Kabrda <bkabrda> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | bkabrda, package-review, slukasik |
Target Milestone: | --- | Flags: | bkabrda:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-01-10 13:45:01 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: |
Description
Vratislav Podzimek
2014-12-15 19:34:44 UTC
Looks promising, I will take a look. First couple of ideas: - There is no documentation for each plug-in. It is not the must for me, but it would be great to have a few words about each plug-in. - name of library: libbd, I am afraid to use libbd as library name. Light search on web for libdb gives some results. What do you think about possible collisions? - package name is libblockdev - lib name is bd. - header files are in blockdev directory I am not sure what can break if those are not consistent. - Quoting the Fedora guidelines: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name}%{?_isa} - requires from libblockdev-plugins-all should imho also put the fully versioned dependency in. What if I want to install libblockdev-plugins-all.i686 on my box? - spell-check says that metapackage is not word, 'meta-package' will do it. What are your thoughts, Vraťo? One generic reply: I think this should be about the packaging issues not upstream (although it's me as well) decisions/improvements. And as for the suggestions: (In reply to Šimon Lukašík from comment #2) > First couple of ideas: > - There is no documentation for each plug-in. It is not the must for me, > but it would be great to have a few words about each plug-in. On my TODO list for future development. I plan to generate documentation with gtk-doc for the library and all the plugins. > - name of library: libbd, I am afraid to use libbd as library name. Light > search on web for libdb gives some results. What do you think about possible > collisions? I only found libbd.dll which is a library that is a part of the Internet Explorer. So no, I'm not afraid of the collisions here. > - package name is libblockdev > - lib name is bd. No it's not. The library is libblockdev.so, just the plugins are libbd_lvm.so and so on because libblockdev_lvm.so looks too long for me. > - header files are in blockdev directory Not an issue with proper documentation that will come soon, I think. And the blockdev.pc file already has this information. > I am not sure what can break if those are not consistent. I'm not aware of anything. > - Quoting the Fedora guidelines: > In the vast majority of cases, devel packages must require > the base package using a fully versioned dependency: > Requires: %{name}%{?_isa} I believe all the -devel packages require their base packages as fully versioned dependencies. Or is any missing it? > - requires from libblockdev-plugins-all should imho also put the fully > versioned dependency in. What if I want to install > libblockdev-plugins-all.i686 on my box? Good catch, thanks! Fixing. > - spell-check says that metapackage is not word, 'meta-package' will do it. And googlefight (http://www.googlefight.com/index.php?lang=en_GB&word1=meta-package&word2=metapackage) agrees, fixing. The two fixes from the comment above are applied (same URLs). Is there anything else I should fix for the package to be "good to go" in its initial version? I am sorry, I have been quite busy lately. I am unable to allocate time for this now. I'll return to here after Feb 8th. Until then, I'll de-assign myself to allow others to step-in. I am sorry, Vraťo. Taking this. Ok, so I was trying really hard to find a problem I could report, but I didn't manage :) This package looks very good, APPROVED. :o) New Package SCM Request ======================= Package Name: libblockdev Short Description: A library for low-level manipulation with block devices Upstream URL: https://github.com/vpodzime/libblockdev Owners: vpodzime Branches: f20 f21 epel7 InitialCC: Git done (by process-git-requests). libblockdev-0.1-1.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/libblockdev-0.1-1.fc21 libblockdev-0.2-1.fc21 has been pushed to the Fedora 21 stable repository. All plugin lib subpackages except -crypto depend on libbd_crypto.so.0 (subpackage -utils). A similar thing for the -devel subpackages and -utils-devel. They include <utils.h>. That makes splitting off the -utils and -utils-devel packages a questionable decision. src/lib/blockdev.c also doesn't list the -utils lib as a plugin! Explicit review and confirmation that the plugin libs are loaded via name.so.$MAJOR_VER and not just name.so would have been very good. > %files devel > %{_libdir}/libblockdev.so > %{_includedir}/blockdev/blockdev.h > %{_includedir}/blockdev/plugins.h > … Directory /usr/include/blockdev is not included anywhere. $ repoquery --whatprovides /usr/include/blockdev $ https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging:UnownedDirectories > %package lvm-devel > Summary: Development files for the libblockdev-lvm plugin/library > Requires: %{name}-lvm%{?_isa} = %{version}-%{release} > Requires: %{name}-utils-devel Better would have been to add %{?_isa} also to the -utils-devel Requires. Especially the linking step would need the arch-specific .so lib to be found. (In reply to Michael Schwendt (Fedora Packager Sponsors Group) from comment #13) > All plugin lib subpackages except -crypto depend on libbd_crypto.so.0 > (subpackage -utils). A similar thing for the -devel subpackages and > -utils-devel. They include <utils.h>. That makes splitting off the -utils > and -utils-devel packages a questionable decision. I think you mean libbd_utils.so.0 not libbd_crypto.so.0. The -utils and -utils-devel packages are subpackages because a 3rd party code could use them not because of the plugins. > > src/lib/blockdev.c also doesn't list the -utils lib as a plugin! The utils lib is not a plugin, it is a library that is not loaded with dlopen(). > > > > Explicit review and confirmation that the plugin libs are loaded via > name.so.$MAJOR_VER and not just name.so would have been very good. This is fixed in the recent version of the package. > > > > %files devel > > %{_libdir}/libblockdev.so > > %{_includedir}/blockdev/blockdev.h > > %{_includedir}/blockdev/plugins.h > > … > > Directory /usr/include/blockdev is not included anywhere. > > $ repoquery --whatprovides /usr/include/blockdev > $ > > https://fedoraproject.org/wiki/Packaging: > Guidelines#File_and_Directory_Ownership > https://fedoraproject.org/wiki/Packaging:UnownedDirectories Good catch, thanks! Fixing. > > > > %package lvm-devel > > Summary: Development files for the libblockdev-lvm plugin/library > > Requires: %{name}-lvm%{?_isa} = %{version}-%{release} > > Requires: %{name}-utils-devel > > Better would have been to add %{?_isa} also to the -utils-devel Requires. > Especially the linking step would need the arch-specific .so lib to be found. Another good catch, thanks! Also fixing. |