Bug 1636169

Summary:	Review Request: libxmlb - Library for querying compressed XML metadata
Product:	[Fedora] Fedora	Reporter:	Richard Hughes <rhughes>
Component:	Package Review	Assignee:	Kalev Lember <klember>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	klember, package-review
Target Milestone:	---	Flags:	klember: fedora-review+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2020-06-08 08:51:16 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 Richard Hughes 2018-10-04 16:00:47 UTC

Spec URL: https://people.freedesktop.org/~hughsient/temp/libxmlb.spec
SRPM URL: https://people.freedesktop.org/~hughsient/temp/libxmlb-0.1.0-1.fc29.src.rpm
Fedora Account System Username: rhughes
Description:

XML is slow to parse and strings inside the document cannot be memory mapped as
they do not have a trailing NUL char. The libxmlb library takes XML source, and
converts it to a structured binary representation with a deduplicated string
table -- where the strings have the NULs included.

This allows an application to mmap the binary XML file, do an XPath query and
return some strings without actually parsing the entire document. This is all
done using (almost) zero allocations and no actual copying of the binary data.

rpmlint: $ rpmlint SPECS/libxmlb.spec SRPMS/libxmlb-0.1.0-1.fc29.src.rpm RPMS/libxmlb-0.1.0-1.fc29.x86_64.rpm 
libxmlb.src: W: spelling-error %description -l en_US deduplicated -> reduplicated, duplicated, quadruplicated
libxmlb.src: W: spelling-error %description -l en_US mmap -> map, m map, mamma
libxmlb.x86_64: W: spelling-error %description -l en_US deduplicated -> reduplicated, duplicated, quadruplicated
libxmlb.x86_64: W: spelling-error %description -l en_US mmap -> map, m map, mamma
2 packages and 1 specfiles checked; 0 errors, 4 warnings.

Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=30048160 (all architectures green)

I'm planning to use this library in fwupd and gnome-software in Fedora 30, but it would be good to include it in Fedora 29 too for easy testing. Comments welcome, thanks.

Comment 1 Kalev Lember 2018-10-04 16:05:18 UTC

Taking for review.

Comment 2 Kalev Lember 2018-10-04 16:18:35 UTC

Fedora review libxmlb-0.1.0-1.fc29.src.rpm 2018-10-04

$ rpmlint libxmlb-0.1.0-1.fc29.src.rpm \
          libxmlb \
          libxmlb-devel

libxmlb.src: W: spelling-error %description -l en_US deduplicated -> reduplicated, duplicated, quadruplicated
libxmlb.src: W: spelling-error %description -l en_US mmap -> map, m map, mamma
libxmlb.x86_64: W: spelling-error %description -l en_US deduplicated -> reduplicated, duplicated, quadruplicated
libxmlb.x86_64: W: spelling-error %description -l en_US mmap -> map, m map, mamma
3 packages and 0 specfiles checked; 0 errors, 4 warnings.

+ OK
! needs attention

+ rpmlint warnings are harmless and can be ignored
+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The license text (LICENSE) is included in %license
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match the sources in the srpm
  SHA512 (libxmlb-0.1.0.tar.xz) = d09e4ac822fee6c2509f2d667e404ad7c959c1fb87b5676776d3a8255087b80814a6b29107432616f3dce76f10c1b0117e406c9959da06dbba4d8e9a677d88d0
  SHA512 (Download/libxmlb-0.1.0.tar.xz) = d09e4ac822fee6c2509f2d667e404ad7c959c1fb87b5676776d3a8255087b80814a6b29107432616f3dce76f10c1b0117e406c9959da06dbba4d8e9a677d88d0
+ Package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a locale handling
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
! Package doesn't own all the directories it creates:
  %{_libdir}/girepository-1.0 and %{_datadir}/gir-1.0 and %{_datadir}/gtk-doc/html and %{_datadir}/gtk-doc are unowned
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect the runtime of application
n/a Static libraries should be in -static
+ Development files should be in -devel
+ -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Proper .desktop file handling
+ Doesn't own files or directories already owned by other packages
+ Filenames are valid UTF-8

Looks nice and clean. Just a small nit about directory ownership:

The package creates %{_libdir}/girepository-1.0 and %{_datadir}/gir-1.0 and %{_datadir}/gtk-doc/html and %{_datadir}/gtk-doc directories, but they aren't listed in the spec file so they don't get removed when uninstalling the package.

Can you add:
%dir %{_libdir}/girepository-1.0
%dir %{_datadir}/gir-1.0
%dir %{_datadir}/gtk-doc
%dir %{_datadir}/gtk-doc/html

... in the right spots in the spec file please?

Beyond that, looks good to me!

APPROVED

Comment 3 Richard Hughes 2018-10-04 16:26:59 UTC

> Can you add ... in the right spots in the spec file please?

Done! Thanks for the super speedy review. I'm waiting for Paquire now.

Comment 4 Gwyn Ciesla 2018-10-04 16:36:41 UTC

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libxmlb