The merge framework contains some TODOs and some parts need more extensive testing

Hey,
would it be possible for you to add a dummy storage plugin that simply passes the data to the xfconf tool. This way, i a user is running Xfce, they would be able to use there default settings system if available.

Thanks and greetings
Leo (from Debienna)

"none" fs entries
kdb-fstab-set does not work on dump or similar
path: UUID= LABEL= rechner:/...

encoding of spaces not supported

TARGET_TOOL_EXEC_FOLDER is currently hard doded to "lib/elektra/tool_exec".
That ignores LIB_SUFFIX, which points currently pkg-config and library installation to optional lib(64) directories.
The issue here is, that it appears strange that test files hang around in /usr/lib while most files go into /usr/lib64 during installation.

Many plugins do not use add_plugin but have duplicated code.

Many build combinations are currently broken:
http://build.libelektra.org:8080/job/elektra-multiconfig-gcc47-cmake-options/

There are 8192 possible cmake flag combinations even though many of those do not make sense. E.g. BUILD_PDF only makes sense if BUILD_DOCUMENTATION is active and ENABLE_TESTING only makes sense if BUILD_TESTING is active.

Concrete suggestion how we can reduce flags can be suggested here.

use enum instead of the boolean flags above

Unrelated to the issue, but to improve consistency we will also introduce a BINDINGS cmake variable that works like PLUGINS and TOOLS.

Several workflows could be simplified (most recent example is the elektra-merge script) if there was a tool to identify the Elektra path where a file is mounted by giving only the filename. Example:

> kdb file-path /etc/hosts
system/hosts

Currently this can only be done by either scanning the mount configuration (in code) or parsing the output of kdb mount (in scripts).

Notice that this issue is closely related to #70 because reverse mapping can only happen unambiguously if there is a one to one relationship from files to Elektra paths.

The lua binding file, kdb.so installs to /usr/local/lib/ instead of /usr/lib like it should. The following line shows the error:

-- Installing: /home/ian/Development/GSoC/libelektra/debian/tmp/usr/local/lib/lua/5.2/kdb.so

The expected behavior would be to install kdb.so to /home/ian/Development/GSoc/libelektra/debian/tmp/usr/lib/lua/5.2/kdb.so

The cmake file for the lua bindings should be updated to fix this error.

The lines that need to be fixed are Line 23 and Line 36. When running those commands on my system, the terminal returns /usr/local/lib/lua/5.2/ which is not the right directory to install the bindings.

Please use the same style of external lib than for nickel and gtest, the folders are called: gtest-1.7.0 and nickel-1.1.0 which makes obvious which upstream version was imported. It also makes it easy to search for all external libs, folders with -:

find . -type d -name '*-*'

While kdb-static does not have a dependency to elektra it still have many other dependencies [0] and so it is actually not static as the name would suggest.

[0] ldd /usr/bin/kdb-static
linux-vdso.so.1 => (0x00007fffb17dc000)
libyajl.so.2 => /usr/lib/x86_64-linux-gnu/libyajl.so.2 (0x00007fc42ed4a000)
libdbus-1.so.3 => /lib/x86_64-linux-gnu/libdbus-1.so.3 (0x00007fc42eb04000)
libxml2.so.2 => /usr/lib/x86_64-linux-gnu/libxml2.so.2 (0x00007fc42e7a3000)
libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fc42e49c000)
libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fc42e21a000)
libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fc42e003000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fc42dc78000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fc42da5c000)
librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fc42d853000)
libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fc42d64f000)
libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fc42d438000)
liblzma.so.5 => /lib/x86_64-linux-gnu/liblzma.so.5 (0x00007fc42d214000)
/lib64/ld-linux-x86-64.so.2 (0x00007fc42ef78000)

• changing key names?
• changing owner?
• add or drop support for owner of keys?

terminate called after throwing an instance of 'kdb::KDBException'
what(): 1 Warning was issued:
Warning number: 2
Description: could not get pointer to factory, dlsym failed
Ingroup: modules
Module: dl
At:/home/libelektra/src/libloader/dl.c:80
Reason: of module: libelektra-resolver.so, because: /usr/local/lib/elektra/libelektra-resolver.so: undefined symbol: elektraPluginSymbol
Error (#40) occurred!
Description: Failed to open default backend (see warnings for more information)
Ingroup: kdb
Module:
At: /home/libelektra/src/libelektra/kdb.c:149
Reason: could not open default backend

Aborted (core dumped)

Is this a local issue or a bug?

Currently plugins handle the recreation of formatting information during the set operation (e.g. whitespaces) very differently, if at all. This could be very annoying for users because sometimes configuration files are very hard to read without any type of formatting. Any suggestions for handling this issue in a uniform way?

gcc 4.7.2 warns in src/plugins/keytometa/keytometa.c line 303 col 17

    warning: ‘result’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Do you find a way to suppress the warning by changing the code or do we need a compiler directive to suppress it?

Installation of files into /etc makes installations without root impossible, which is a regression.
Patch below (hope formatting is ok):
diff --git a/cmake/ElektraCache.cmake b/cmake/ElektraCache.cmake
index a98223b..3d740b7 100644
--- a/cmake/ElektraCache.cmake
+++ b/cmake/ElektraCache.cmake
@@ -374,6 +374,8 @@ set (TARGET_TEMPLATE_FOLDER
"This folder (below prefix) will be used to install templates"
)

+option (INSTALL_SYSTEM_FILES "Install to system directories" ON)
+

Misc.

diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt
index 3e94d9f..daf914d 100644
--- a/scripts/CMakeLists.txt
+++ b/scripts/CMakeLists.txt
@@ -3,9 +3,11 @@ install (PROGRAMS elektra-mount DESTINATION ${TARGET_TOOL_EXEC_FOLDER})
install (PROGRAMS elektra-umount DESTINATION ${TARGET_TOOL_EXEC_FOLDER})

IF (IS_DIRECTORY /etc/bash_completion.d)

•   install (FILES kdb-bash-completion
  

•   IF( INSTALL_SYSTEM_FILES )
  

•           install (FILES kdb-bash-completion
                  DESTINATION /etc/bash_completion.d
                  RENAME kdb)
  

•   ENDIF( INSTALL_SYSTEM_FILES )
  

  ENDIF()

  configure_file(

@pinotree wrote:

there's another occurrence of this error in tests/ctest/test_trie.c, but changing it was causing the test to fail, and I'm not too familiar with elektra's internals.

kdb get only outputs the first line of a multiline value. This issue may also be bash related because kdb get works fine in the Eclipse console.

Currently the the ini plugin uses a #define to specify whether multiline values are supported or not. This could easily be changed to a config option.

Many plugins still use the old-style contract and not README.md

Please fix the plugins so that they use README.md and also add more documentation what the plugins are for and how they can be used.

Extend libtools to build a backend out from a specification.

kdb run_all does not find all resource files (e.g. example config files)
resources need to be installed with CMake.

Commit 3df8fc9 splits python bindings into two explicit configure options (python2 and python3). However it's not possible to enable both at the same time, as cmake doesn't support this. This is highly confusing for people not familiar with our build environment.

Additional there won't be separate python plugins which makes this commit and the outcome even more confusing.

Building bindings (and the upcoming plugin) with a specific python version is documented in https://github.com/ElektraInitiative/libelektra/blob/master/src/bindings/swig/README.md
Switching python versions in an already configured build environment is also possible that way.

[] throws an exception when a key does not exist:

    Traceback (most recent call last):
      File "example_kdb.py", line 10, in <module>
        key = ks["user/MyApp/mykey"]
      File "kdb.py", line 1172, in __getitem__
        raise KeyError(str(key))
    KeyError: 'user/MyApp/mykey'

When this is the intended behaviour, then the example should be changed to

    key = ks.lookup("user/MyApp/mykey")

Otherwise, [] should be changed.

• simpleini
• ni
• tcl
• dump
• yajl with cascading keys
• xmltool (not really relevant, is superseded by xerces plugin)

I am currently having a problem with kdb import. When I try to import a file:
cat backup.ecf | sudo kdb import system/restore
None of the keys are added from backup.ecf (which is in dump format).

If I have a file such as backup.txt in line format,
cat backup.txt | sudo kdb import system/restore line
Works as expected.

Can anybody confirm?

The user should have a uniform behaviour for any backend.
So all test cases should also pass for:

• ini
• yajl
• csvstorage

One problem these plugins have is that they "directories" cannot store values.

So a plugin should be developed that transform:

user/a = dirvalue
user/a/b = leafvalue

to:

user/a = 
user/a/___dirdata = dirvalue
user/a/b = leafvalue

(filesys used %%dirdata for the same thing)
Could also be named directoryvalue

the testcases supplied with keytometa have a bunch of memleaks

valgrind detects them, but I can post them on request

dlopen'ing fails on symbol load:
compiz: symbol lookup error: /opt/local/lib64/elektra/libelektra-resolver.so: undefined symbol: elektraPluginExport

Further on Windows all symbols need to be resolved at link time, including for dll's.

The below patch adds the mandatory libelektra dependency to all plugins using the add_plugin() macro. Thus the elektraPluginExport is resolved. With the below patch the dlopen error goes away.

diff --git a/cmake/Modules/LibAddPlugin.cmake b/cmake/Modules/LibAddPlugin.cmake
index 4c3217e..ba6cc45 100644
--- a/cmake/Modules/LibAddPlugin.cmake
+++ b/cmake/Modules/LibAddPlugin.cmake
@@ -92,6 +92,7 @@ function(add_plugin PLUGIN_SHORT_NAME)

    if (BUILD_SHARED)
            add_library (${PLUGIN_NAME} MODULE ${ARG_SOURCES})

•           target_link_libraries (${PLUGIN_NAME} elektra)
          target_link_libraries (${PLUGIN_NAME}
                  ${ARG_LINK_LIBRARIES})
          install (TARGETS ${PLUGIN_NAME} DESTINATION
  

Pino Toscano wrote:

• 'make test' after build passes almost fully; the only issue seems
  to
  be in the exception handling within the Python binding: in
  particular,

in test_key.py, Key::test_properties:

   with self.assertRaises(kdb.KeyInvalidName):
         k.name = "foo"

the above causes the crash of the Python (3.4) interpreter (!),
not even a graceful failure.

Bash treats the \ as escape character and eliminates them. This causes wrong key completions.

For example the key name system/contracttest/system\/elektra\/modules\/_/description is completed to system/contracttest/system/elektra/modules/_/description.

with 4f2b0c3

cmake configure finish by error:

http://sl1pkn07.wtf/paste/view/1c628dcf

and when run make:

http://sl1pkn07.wtf/paste/view/03931d29

also, 'git describe --long --tags' (in master branch) gets

0.8.1-1321-g4f2b0c3

instead of

0.8.8-xxxx-g4f2b0c3

greetings

hosts plugin is currently broken when the same entry exists for ipv4 and ipv6.

We would need sub-hierarchies for ipv4 and ipv6 to make the canonical hosts really canonical.

@markus2330 after your changes to merge.cpp in c5162c3 the -b option no longer does what it says. The behaviour of merge command as you implemented it is much more intuitive than it initally was, but the option is no longer descriptive. Maybe we could entirely remove the option and use -f instead? I can think of many tools that use -f to indicate that something will be overwritten. What do you think?

In Debian Unstable there is no more journal-dev, but instead libsystemd-dev seems to be used. cmake/Modules/FindSystemdJournal.cmake seems to not work with the new version of systemd:

CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
LIBSYSTEMD_JOURNAL_LIBRARIES (ADVANCED)
  linked by target "elektra-journald" in directory /home/markus/libelektra/src/plugins/journald
  linked by target "elektra-full" in directory /home/markus/libelektra/src/libelektra
  linked by target "elektra-static" in directory /home/markus/libelektra/src/libelektra

I recently sumbled over the following problem a couple of times. I am not quite sure yet however if this is actually a bug or something on my system is broken.

My guess: if two mountpoints are created where the name of the one is a prefix of the other, the shorter mountpoint is no longer accessible. Example:

sudo kdb mount /etc/hosts system/foo hosts
kdb ls system/foo

Up to now it works as expected. Now i do

sudo kdb mount /etc/hosts system/foo-hosts hosts
kdb ls system/foo-hosts

This still works as expected. But now a kdb ls system/foo won't return any results anymore. A look into system/elektra/mountpoints reveals, that the key system/elektra/mountpoints/foo and its subkeys are not sorted continously anymore. Instead the order is

system/elektra/mountpoints/foo
system/elektra/mountpoints/foo-hosts
system/elektra/mountpoints/foo-hosts/subkeys
system/elektra/mountpoints/foo/subkeys

I suspect that this causes the problem, but I have not looked into the mount code in detail yet.

Since the added _Pragma("GCC diagnostic pop") in b02ec92, clang emitts a very annoying warning several times (thise hides potentially dangerous warnings):

warning: pragma diagnostic pop could not pop, no matching push [-Wunknown-pragmas]

GCC ignores this situation. However, I think disabling this warning in clang is not an option because complaining about unknown pragmas is important.

It seems like I can no longer do a fresh build after a47c5e0. I realised it when trying to do a build from scratch on a different machine (for #38). Cmake aborts with the following error

CMake Error in src/libelektra/CMakeLists.txt:
  Cannot find source file "$<TARGET_OBJECTS:elektra-resolver-objects>".
  Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp
  .hxx .in .txx

Building on f137b99 works fine.
Can somebody reproduce this?

errors should be transformed to warnings

add Testcases for non-successful kdbOpen()

Why does a call to kdb.get(result, parentKey) return a lot more keys than just the keys below parentKey? Regarding to the documentation it should return the keys below parentKey.

• ini has some code review comments
• testcases currently fail
• please properly integrate ini (in src/plugins/README.md and cmake/ElektraCache.cmake).

Implementation of a plugin that is able to rebase keys passing through to a new parent

In c++ calling

key.setMeta("metaname", NULL)

stores the string "0" instead of deleting the meta key with the name "metaname"

Try to build whole Elektra with C++ (even the C files) to support VC++ (that does not support C99)

Compiling with clang yields

src/libtools/src/backend.cpp:31:25: error: addition of default argument on redeclaration makes this constructor a default constructor:

Backend::Backend(string name_ = "", string mp_ = "") :
                        ^       ~~

contract broken + needs README.md adoption
hard coded limit of characters in simpleset?

The -s option was added to kdb import but it's description is confusing.
-s --strategy the strategy which should be used on conflicts
For merging:
preserve .. fail on conflict (default)
ours .. use ours for conflicts
theirs .. use theirs for conflicts
base .. use base for conflicts
Otherwise:
preserve .. no old key is overwritten (default)
overwrite .. overwrite keys with same name
cut .. completely cut at rootkey to make place for new keys

It should just show the options for merging on the command kdb merge and the other options on other commands (such as kdb import)

keyAddBaseName(key, "..") currently is not supported.

Currently the C++ bindings of KeySet provides, among the others, two contructors with varargs:

    inline explicit KeySet(size_t alloc, va_list ap);
    inline explicit KeySet(size_t alloc, ...);

the problem is that on some architectures va_list is represented as void* as public type, which makes the two constructor ambiguous when called with alloc greater than 0 and no additional arguments. In such case, the va_list version gets used, causing ksVNew to try to read arguments from a null (or garbage) ap.

The following patch for src/bindings/cpp/tests/testcpp_ks.cpp

@@ -821,6 +821,18 @@ void test_duplicate()
        // ks4.toStream(stdout, 0);
 }

+void test_alloc_no_keys()
+{
+       cout << "testing test_alloc_no_keys" << endl;
+
+       KeySet ks (5,
+               KS_END);
+       succeed_if (ks.size() == 5, "size not correct");
+
+       KeySet ks2 (ks);
+       succeed_if (ks2.size() == 5, "size not correct");
+}
+

 int main()
 {
@@ -844,6 +856,7 @@ int main()
        test_ksrelease();
        test_lookuppop();
        test_duplicate();
+       test_alloc_no_keys();

        cout << endl;
        cout << "test_key RESULTS: " << nbTest << " test(s) done. " << nbError << " error(s)." << endl;

is enough to trigger the issue on e.g. mips(el) or hppa. Currently this is exposed by the lua and python bindings, since all do:

  KeySet(size_t alloc) {
    return new kdb::KeySet(alloc, KS_END);
  }

In those it can be easily worked around by adding the KS_END marker a second time (making them really as vararg), but of course it wouldn't fix the issue with the C++ contructors of KeySet.

Currently the same file mounted multiple times lead to undefined behaviour.

Despite the current testcases, keyIsInactive should consider a key below an inactive key as inactive too (as mentioned in #9). Example: user/.hidden/key should be inactive, but currently is not.

As marked by TODO, there is a memleak in
testtool_metamergestrategy.cpp:46

Also document the ownership of memory in add_strategies.

please also remove the lines

 set_property(TEST testtool_metamergestrategy PROPERTY LABELS memleak)
 set_property(TEST testmod_keytometa PROPERTY LABELS memleak)

after you fixed the issues