Bug 1208101 - Review Request: libbson - Building, parsing, and iterating BSON documents
Summary: Review Request: libbson - Building, parsing, and iterating BSON documents
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Daniel Kopeček
QA Contact: Fedora Extras Quality Assurance
URL: https://jira.mongodb.org/browse/CDRIV...
Whiteboard:
: 995974 (view as bug list)
Depends On: 1215182
Blocks: 1252638 1260254 1265033 pecl/mongodb
TreeView+ depends on / blocked
 
Reported: 2015-04-01 12:10 UTC by Petr Pisar
Modified: 2016-03-02 20:00 UTC (History)
10 users (show)

Fixed In Version: libbson-1.3.1-1.fc24
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-03-02 20:00:49 UTC
dkopecek: fedora-review+


Attachments (Terms of Use)
fedora-review output (8.28 KB, text/plain)
2015-04-14 08:34 UTC, Daniel Kopeček
no flags Details
fedora-review output (modified) (9.00 KB, text/plain)
2016-02-12 10:14 UTC, Daniel Kopeček
no flags Details


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 995974 None None None Never

Internal Links: 995974

Description Petr Pisar 2015-04-01 12:10:36 UTC
Spec URL: https://ppisar.fedorapeople.org/libbson/libbson.spec
SRPM URL: https://ppisar.fedorapeople.org/libbson/libbson-1.1.2-1.fc21.src.rpm
Description:
This is a library providing useful routines related to building, parsing,
and iterating BSON documents <http://bsonspec.org/>.

Fedora Account System Username: ppisar

Comment 1 Daniel Kopeček 2015-04-14 08:34:34 UTC
Created attachment 1014204 [details]
fedora-review output

I've identified only one major issue with the package. The package bundles the yajl library, but most of the source code is removed in the %prep phase. However, the header files still contain some code. Why is it needed?

Minor issue: latest release is 1.1.4, package contains 1.1.2

The package builds fine on f23: http://koji.fedoraproject.org/koji/taskinfo?taskID=9475098

Comment 2 Petr Pisar 2015-04-14 11:34:32 UTC
It keeps some YAJL header files because it needs to access a non-publib structure member to redefine a memory allocator called by YAJL.

Comment 3 Daniel Kopeček 2015-04-14 12:45:30 UTC
(In reply to Petr Pisar from comment #2)
> It keeps some YAJL header files because it needs to access a non-publib
> structure member to redefine a memory allocator called by YAJL.

Would it be possible to have a patch in the yajl package which would allow to redefine the allocator instead in doing it this way in libbson?

Comment 4 Petr Pisar 2015-04-14 13:24:02 UTC
That's what libbson upstream works on. At least they told me so. Follow a link at the patch. However I haven't seen any activity in the YAJL upstream regarding this area yet.

Comment 5 Daniel Kopeček 2015-04-16 14:08:32 UTC
Are you able to provide at least a patch for the yajl library shipped in Fedora? So that the bundled header can be removed from BSON. Waiting for libbson upstream to complete the work could take more time than to wait for a FPC bundling exception.

The approach of removing the most of the yajl library and leaving some of the otherwise inaccessible structs to redefine the allocators seems not promising from the point of view of ABI compatibility.

In this case, I would rather leave the whole library there and use it instead of the packaged one. But that'll be hard to pass by the FPC, I guess... The fastest option I see here is to patch the yajl library shipped in Fedora.

Comment 6 Remi Collet 2015-04-23 06:55:45 UTC
While having a copy of some private symbols in libbson is of course not a good solution, I really think it is acceptable as a temporary solution while upstream is working on this issue.

https://jira.mongodb.org/browse/CDRIVER-597
https://jira.mongodb.org/browse/CDRIVER-623

e.g. we have used this solution for a long time in PHP (for private libzip symbol, patch from spot, and it took ~2 years to fix everything upstream)

Probably a hard requirement on the libyajl version to detect possible change in this private API.

%global yajlver %(pkg-config --silence-errors --modversion yajl 2>/dev/null || echo 999)
Requires: yajl = %{yajlver}

But from upstream ticket
  "Also, we've modified it to support some error-recovery..."

So probably better to ask FPC for a temporary exception...


Notice: I also need this package for mongo-c-driver and php-pecl-mongodb

Comment 7 Petr Pisar 2015-04-24 13:58:08 UTC
I proposed an enhancement to yajl (bug #1215182) which allows to unbundle yajl completely.

Updated libbson package spec and SRPM files are on the same addresses.

Comment 8 Petr Pisar 2015-04-24 14:22:42 UTC
Fedora yajl maintainer denied the extension. So we can:

(1) Ask FPC for permanent bundling exception.
(2) Fork yajl in Fedora.
(3) Remove MongoDB from Fedora.

Comment 9 Daniel Kopeček 2015-04-25 17:17:33 UTC
(In reply to Petr Pisar from comment #8)
> Fedora yajl maintainer denied the extension. So we can:
> ...
> (2) Fork yajl in Fedora.

Fork and maintain it there under a different name or use an already existing fork? Like this one:

 https://github.com/likema/yajl

Renaming would be necessary anyway, I guess.

Comment 10 Remi Collet 2015-09-04 05:09:13 UTC
(In reply to Petr Pisar from comment #8)
> (1) Ask FPC for permanent bundling exception.

This seems the pragmatic solution.

Comment 11 A. Jesse Jiryu Davis 2015-09-04 17:55:02 UTC
Thanks for your effort on this. We're willing to make any changes needed in libbson, in the medium-term, to comply with your requirements.

I've written YAJL's original author Lloyd Hilaiel at every email address I can find for him, repeatedly over the last year pleading for your patch, Petr, with no response.

(This patch: https://github.com/lloyd/yajl/issues/161 )

There may be a way to preserve libbson's behavior using only public YAJL APIs, we'll investigate this when we can. Otherwise, I think the right approach long-term is to remove the libbson feature that first required us to access YAJL's private header.

Comment 12 Christopher Meng 2015-09-05 01:20:06 UTC
Thanks for working on this, I need this package as well.

Comment 13 Daniel Kopeček 2015-09-25 07:53:07 UTC
Looks like yajl upstream is alive:
https://github.com/lloyd/yajl/issues/161#issuecomment-143094168

Comment 14 Petr Pisar 2015-10-06 08:01:49 UTC
Current yajl upstream status:

The feature, adding public yajl_reset(), has been implemented in yajl_reset branch. Especially with:

commit 646b8b82ce5441db3d11b98a1049e1fcb50fe776
Author: Lloyd Hilaiel <lloyd@hilaiel.com>
Date:   Thu Sep 24 13:11:40 2015 -0600

    add a way to reset a yajl parser without full re-allocation

This has not yet been merged into master, nor new version including this change has been yet released.

libbson upstream tested the change against their developmental code successfully.

Comment 15 Christopher Meng 2015-10-07 11:19:59 UTC
*** Bug 995974 has been marked as a duplicate of this bug. ***

Comment 16 Remi Collet 2015-10-14 11:12:40 UTC
Version 1.2.0 is released \o/

Can you please consider updating to this new version ?
(required by the new PHP driver)

I think recent Fesco https://fedorahosted.org/fesco/ticket/1483 apply here.

Best we can do is done (mongo-c-driver can use system libbson, pecl/mongodb can use system libbson and system libmongoc).

And situation will also be better in the perl side (as, it is better to bundle yafl, than to bundle full libbson)

Using libyajl is just "not yet" supported, but will soon be.

In all case, we need a conditional build time flag, as yafl is in base RHEL/CentOS, so very probably, is not going to be updated to include required changes.

Comment 17 Petr Pisar 2015-10-15 11:56:54 UTC
I know. I will update the package once FPC updates packaging guidelines. There is none currently. FeSCO has not yet finished the policy.

Comment 18 Remi Collet 2015-12-08 06:49:34 UTC
FYI:

https://jira.mongodb.org/browse/CDRIVER-1039 issue with man pages
https://jira.mongodb.org/browse/CDRIVER-1040 private ABI changes

For now, pecl/mongodb is only compatible with version 1.2.x (because of this private API usage)

Comment 19 Petr Pisar 2015-12-09 15:20:31 UTC
Daniel, would accept the libbson with bundled modified yajl? Guidelines do not forbid bundling anymore, but FPC has not yet come with guidelines how to bundle.

Comment 20 Remi Collet 2015-12-16 07:03:24 UTC
For your information:
- pecl/mongodb 1.0.x is compatible with version 1.2.x
- pecl/mongodb 1.1.x is compatible with version 1.3.x

So I'm fine with either 1.2 or 1.3 (of course, latest seems better)

Comment 21 Daniel Kopeček 2015-12-17 08:31:06 UTC
(In reply to Petr Pisar from comment #19)
> Daniel, would accept the libbson with bundled modified yajl? Guidelines do
> not forbid bundling anymore, but FPC has not yet come with guidelines how to
> bundle.

Yes, I'll accept it.

Comment 22 Remi Collet 2016-01-21 10:26:06 UTC
Just FYI, with just released 1.3.1

man pages are no more generated / installed
https://jira.mongodb.org/browse/CDRIVER-1069

man pages installation broken
https://jira.mongodb.org/browse/CDRIVER-1068

Comment 23 Petr Pisar 2016-01-22 15:18:39 UTC
Here is 1.3.1 package for review with bundled yajl:

Spec URL: https://ppisar.fedorapeople.org/libbson/libbson.spec
SRPM URL: https://ppisar.fedorapeople.org/libbson/libbson-1.3.1-1.fc24.src.rpm

Comment 24 Petr Pisar 2016-02-11 15:39:59 UTC
Daniel, any progress? If you don't have time, Remi offered me doing the review.

Comment 25 Upstream Release Monitoring 2016-02-12 09:18:15 UTC
mildew's scratch build of libbson-1.3.1-1.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12949967

Comment 26 Daniel Kopeček 2016-02-12 09:21:05 UTC
(In reply to Petr Pisar from comment #24)
> Daniel, any progress? If you don't have time, Remi offered me doing the
> review.

Sorry, totally forgot.

Rawhide build is fine.

Comment 27 Daniel Kopeček 2016-02-12 10:13:41 UTC
Issues:
=======
- All build dependencies are listed in BuildRequires, except for any that
  are listed in the exceptions section of Packaging Guidelines.
  Note: These BR are not needed: gcc
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

Comment 28 Daniel Kopeček 2016-02-12 10:14:37 UTC
Created attachment 1123464 [details]
fedora-review output (modified)

Comment 29 Petr Šabata 2016-02-12 10:26:01 UTC
(In reply to Daniel Kopeček from comment #27)
> Issues:
> =======
> - All build dependencies are listed in BuildRequires, except for any that
>   are listed in the exceptions section of Packaging Guidelines.
>   Note: These BR are not needed: gcc
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

The list of exceptions you're referring to was removed ages ago.
Nothing is guaranteed nowadays.

Comment 30 Daniel Kopeček 2016-02-12 10:42:51 UTC
Ok, should be fine then.

Comment 31 Petr Pisar 2016-02-12 13:01:36 UTC
Many thanks to everyone involved.

Comment 32 Gwyn Ciesla 2016-02-12 13:05:32 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/libbson

Comment 33 Fedora Update System 2016-02-12 13:43:18 UTC
libbson-1.3.1-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-2b04670a06

Comment 34 Fedora Update System 2016-02-12 13:43:30 UTC
libbson-1.3.1-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-522cb996e2

Comment 35 Fedora Update System 2016-02-15 04:53:09 UTC
libbson-1.3.1-1.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-522cb996e2

Comment 36 Fedora Update System 2016-02-15 05:24:51 UTC
libbson-1.3.1-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-2b04670a06

Comment 37 Fedora Update System 2016-02-15 12:42:55 UTC
libbson-1.3.3-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-da055d5847

Comment 38 Fedora Update System 2016-02-15 12:43:11 UTC
libbson-1.3.3-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-3c755735b7

Comment 39 Fedora Update System 2016-02-16 07:11:31 UTC
libbson-1.3.3-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fb7f0eb1b4

Comment 40 Fedora Update System 2016-02-17 04:49:43 UTC
libbson-1.3.3-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fb7f0eb1b4

Comment 41 Fedora Update System 2016-02-17 06:27:13 UTC
libbson-1.3.3-1.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-3c755735b7

Comment 42 Fedora Update System 2016-02-17 06:29:21 UTC
libbson-1.3.3-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-da055d5847

Comment 43 Fedora Update System 2016-02-25 08:53:02 UTC
libbson-1.3.3-1.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 44 Fedora Update System 2016-02-26 07:23:34 UTC
libbson-1.3.3-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 45 Fedora Update System 2016-03-02 20:00:45 UTC
libbson-1.3.3-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.