Issue 6430 - implement read-only bdb #6431
Conversation
| #include "bdb_layer.h" | ||
| #include <robdb.h> | ||
|
|
||
| #define DEBUG 1 |
There was a problem hiding this comment.
Shouldn't it be 0 by default
There was a problem hiding this comment.
should not even be defined at all (since we are using #ifdef)
| { | ||
| int rc = bdbreader_cur_lookup(db->cur->impl, key->data, key->size); | ||
| if (rc == DB_SUCCESS) { | ||
| rc = dbc_get(db->cur, key, data, flags); |
There was a problem hiding this comment.
Shouldn't it call db->cur->c_get than dbc_get ?
There was a problem hiding this comment.
db->cur->c_get == dbc_get so it does not really matter.
But to respect the layered approach since we are in the glue code that maps bdb API to robdb API,
bdb_bdbreader_glue.c code should use either internal or librobdb function rather than the bdb API
(So dbc_get looks good.)
| @@ -397,6 +397,7 @@ const char *dblayer_op2str(dbi_op_t op) | |||
| return str[idx]; | |||
| } | |||
|
|
|||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
Probably a leftover from a merge.
There was a problem hiding this comment.
Indeed: bad conflict resolution when rebasing to main ...
|
|
||
| int dbc_close(DBC *dbc) | ||
| { | ||
| bdbreader_bdb_close(dbc->impl); |
There was a problem hiding this comment.
Did you mean bdbreader_cur_close ?
|
Fixed review cooments about DEBUG, bdbreader_cur_close, and rebase conflict issue |
rpm/389-ds-base.spec.in
Outdated
| @@ -540,11 +582,11 @@ if ! make DESTDIR="$RPM_BUILD_ROOT" check; then cat ./test-suite.log && false; f | |||
|
|
|||
| %post | |||
| if [ -n "$DEBUGPOSTTRANS" ] ; then | |||
| output=$DEBUGPOSTTRANS | |||
There was a problem hiding this comment.
Indentation was removed here and below, is this intentional?
lib/librobdb/robdb.spec
Outdated
| Summary: Provide basic functions to search and read Berkeley Database records | ||
|
|
||
| License: GPL-2.0-or-later | ||
| URL: https://github.com/389ds/389-ds-base/lib/librobdb |
There was a problem hiding this comment.
That is expected:
Right now, lib/librobdb does not exists yet on main branch, but it will exist once the PR get merged.
There was a problem hiding this comment.
Then it would be something like this (note the tree and main branch in the URL):
https://github.com/389ds/389-ds-base/tree/main/lib/librobdb
| %doc %{_defaultdocdir}/%{name}-devel/README.md | ||
| %{_libdir}/*.so | ||
| %{_includedir}/* | ||
|
|
There was a problem hiding this comment.
Since there is test.c for robdb, maybe we should run it as part of %check section?
There was a problem hiding this comment.
Added a make localtest to be able to run the test without having to install the rpm first
and added a %check section to use it
|
Pushed commit about Viktor's review comments |
|
Pushed new commit to fix the URL |
This PR:
Issue: #6430
Reviewed by: @tbordaz and @vashirov (Thanks)