backend plugin

[vLesk]

Implementing the backend plugin. Still under construction :)

Discussed in #2963

Basics

Check relevant points but please do not remove entries.
Do not describe the purpose of this PR in the PR description but:

• Short descriptions should be in the release notes (added as entry in
  doc/news/_preparation_next_release.md which contains _(my name)_)
  Please always add something to the the release notes.
• Longer descriptions should be in documentation or in design decisions.
• Describe details of how you changed the code in commit messages
  (first line should have module: short statement syntax)
• References to issues, e.g. close #X, should be in the commit messages.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.

• I added unit tests for my code
• I fixed all affected documentation
• I added code comments, logging, and assertions as appropriate (see Coding Guidelines)
• I updated all meta data (e.g. README.md of plugins and METADATA.ini)
• I mentioned every code not directly written by me in THIRD-PARTY-LICENSES

Review

Reviewers will usually check the following:

• Documentation is introductory, concise and good to read
• Examples are well chosen and understandable
• Code is conforming to our Coding Guidelines
• APIs are conforming to our Design Guidelines
• Code is consistent to our Design Decisions

Labels

• Add the "work in progress" label if you do not want the PR to be reviewed yet.
• Add the "ready to merge" label if the build server is happy and also you
  say that everything is ready to be merged.

[markus2330]

Thank you for creating this PR!

[markus2330]

You can link to the list plugin here.

[markus2330]

One important implication is that we need a hard-coded default backend, similar to the default storage and resolver we already have.

[vLesk]

I'm currently working on moving backendOpen in backend.c to elektraBackendOpen in the backend plugin, one notable function which is called there is elektraProcessPlugins, which builds up the plugin arrays. Since this is implementation specific for backends (correct me if I'm wrong), the idea would be to move the function to the backend plugin and call it there (modified to work with the new array structure). Since elektraPluginOpen and elektraProcessPlugin are exported, they can be called there as well.

Most of the arguments the function needs can be accessed from within the plugin as well:
-plugins: is the plugin array and is hence accessible
-reference plugins: given to the function as an empty keyset
-config & systemConfig: given to the plugin
-global: given to the plugin
-errorKey: given as argument to elektraBackendOpen

The only argument I'm not sure about is the KeySet modules, since it is neither given to the plugin nor as an argument, but is necessary to call elektraPluginOpen.

My idea would be to give the keyset to the plugin using the data field which is available in the plugin struct and make it possible to access the keyset that way. Is that a valid way to solve this or is this too much of a workaround?

[markus2330]

Since this is implementation specific for backends

exactly

the idea would be to move the function to the backend plugin and call it there

Maybe we should get rewrite of this code. If you add any number of plugins in the middle of this code, it might be very confusing.

Did you already design how the new system/elektra/mountpoints will look like?

My idea would be to give the keyset to the plugin using the data field which is available in the plugin struct and make it possible to access the keyset that way. Is that a valid way to solve this or is this too much of a workaround?

It is a good idea but use a new KeySet * modules in _Plugin and not the data field. Loading of modules is a common task for many plugins (multiresolver, language binding plugins, ...). So if the modules are somehow accessible from within plugins, it is a good thing.

[vLesk]

The new mountpoints will have the mountpoint key moved under config/mountpoint, like config/path, that way they can be assigned to the plugins and read out. The easiest solution would be to add the mountpoint variable to the plugin struct, but I'm not sure if it is useful enough for other plugins like the modules keyset.

Another thing that needs to be changed in the mountpoint configuration is the allocation of plugins in the arrays, since array numbers are currently being used to define a plugin's respective slot. I would like to change the numbers to the actual names of the roles. For example:

setplugins/#3#plugin#name

is changed to

setplugins/setstorage/#plugin#name.

That way it is possible to have more flexible numbers of plugins. This means that, among other things, I need to change the default configuration, as well as the configuration created when new backends are mounted. However, I am not sure where I can find these configurations, I was able to find elektra.ecf in /etc, but I'm not sure where it is created from.

[markus2330]

Thank you, can you please write this in the documentation?

setplugins/setstorage/#plugin#name.

Do we need the # here? I think that it would be better if we have # only for arrays, e.g. setplugins/presetstorage/#0/plugin?

This means that, among other things, I need to change the default configuration, as well as the configuration created when new backends are mounted. However, I am not sure where I can find these configurations, I was able to find elektra.ecf in /etc, but I'm not sure where it is created from.

It is created from the kdb tool which uses the libtools. In the source it is src/libs/tools/src/backend.cpp

With the tests it should be easy to understand what the code does. outputGTest allows you to generate regression tests. Current regression tests (you will need to fix them) are in src/libs/tools/testtool_backend.cpp

[vLesk]

Thank you for the answers! I agree that the # should be used only for arrays, I can implement the configuration that way. What I need to do in that case is find a new place for plugin labels. A good solution in my opinion would be something like setplugins/presetstorage/#0/plugin/label.

[markus2330]

Can you write up everything including a full example? It is easier to see how it looks like then.

[vLesk]

I'll use the mount point configuration from doc/dev/architecture.md as an example. The changed configuration would then look as follows:

system/elektra/mountpoints
system/elektra/mountpoints/fstab
system/elektra/mountpoints/fstab/config
system/elektra/mountpoints/fstab/config/mountpoint
system/elektra/mountpoints/fstab/config/path=fstab
system/elektra/mountpoints/fstab/config/struct=list FStab
system/elektra/mountpoints/fstab/config/struct/FStab
system/elektra/mountpoints/fstab/config/struct/FStab/device
system/elektra/mountpoints/fstab/config/struct/FStab/dumpfreq
system/elektra/mountpoints/fstab/config/struct/FStab/mpoint
system/elektra/mountpoints/fstab/config/struct/FStab/options
system/elektra/mountpoints/fstab/config/struct/FStab/passno
system/elektra/mountpoints/fstab/config/struct/FStab/type
system/elektra/mountpoints/fstab/errorplugins
system/elektra/mountpoints/fstab/errorplugins/rollback
system/elektra/mountpoints/fstab/errorplugins/rollback/#0
system/elektra/mountpoints/fstab/errorplugins/rollback/#0/label (="resolver")
system/elektra/mountpoints/fstab/errorplugins/rollback/#0/name (="resolver")
system/elektra/mountpoints/fstab/getplugins
system/elektra/mountpoints/fstab/getplugins/getresolver
system/elektra/mountpoints/fstab/getplugins/getresolver/#0
system/elektra/mountpoints/fstab/getplugins/getresolver/#0/ref (="resolver")
system/elektra/mountpoints/fstab/getplugins/getstorage
system/elektra/mountpoints/fstab/getplugins/getstorage/#0
system/elektra/mountpoints/fstab/getplugins/getstorage/#0/label (="fstab")
system/elektra/mountpoints/fstab/getplugins/getstorage/#0/name (="fstab")
system/elektra/mountpoints/fstab/setplugins
system/elektra/mountpoints/fstab/setplugins/commit
system/elektra/mountpoints/fstab/setplugins/commit/#0
system/elektra/mountpoints/fstab/setplugins/commit/#0/ref (="resolver")
system/elektra/mountpoints/fstab/setplugins/presetstorage
system/elektra/mountpoints/fstab/setplugins/presetstorage/#0
system/elektra/mountpoints/fstab/setplugins/presetstorage/#0/label (="struct")
system/elektra/mountpoints/fstab/setplugins/presetstorage/#0/name (="struct")
system/elektra/mountpoints/fstab/setplugins/presetstorage/#1
system/elektra/mountpoints/fstab/setplugins/presetstorage/#1/label (="type")
system/elektra/mountpoints/fstab/setplugins/presetstorage/#1/name (="type")
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2/label (="path")
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2/name (="path")
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2/config
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2/config/path/allow=proc tmpfs none
system/elektra/mountpoints/fstab/setplugins/setresolver
system/elektra/mountpoints/fstab/setplugins/setresolver/#0
system/elektra/mountpoints/fstab/setplugins/setresolver/#0/ref (="resolver")
system/elektra/mountpoints/fstab/setplugins/setstorage
system/elektra/mountpoints/fstab/setplugins/setstorage/#0
system/elektra/mountpoints/fstab/setplugins/setstorage/#0/ref (="fstab")

The notable changes are the following:

• The key containing the mountpoint has been moved from system/elektra/mountpoints/backendname/mountpoint to system/elektra/mountpoints/backendname/config/mountpoint. That way, the mountpoint of the backend can still be read out in the core from the backend plugin's plugin configuration.
• Plugin roles are no longer displayed as array slots, but actually by their names. For example, system/elektra/mountpoints/fstab/getplugins/#0 is now system/elektra/mountpoints/fstab/getplugins/getresolver.
• Each plugin role consists of a linked list containing the plugins fulfilling this role. In the configuration, the position of the plugin in the linked list is shown using an array. For example, the key system/elektra/mountpoints/fstab/setplugins/presetstorage/#2 means that the plugin (in this case path) belongs to the second position of the linked list belonging to the presetstorage role.
• The name, reference name and the label of a plugin are now stored in separate keys to avoid using the # symbol for something other than arrays. An example from the path plugin:

system/elektra/mountpoints/fstab/setplugins/presetstorage/#2
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2/label
system/elektra/mountpoints/fstab/setplugins/presetstorage/#2/name

That way, the path plugin is opened and stored for later use with the defined label. If it were to be used later, it would be referenced by adding ref instead of label and name.

[vLesk]

I made a rough implementation of elektraBackendOpen. I am currently working on implementing the other plugin functions, as well as writing appropriate tests and documentation.

[markus2330]

Can you put all the text you put here directly as text into the docu? It is much easier to review then (can also be in a separate PR).

system/elektra/mountpoints/fstab

fstab is not a valid mountpoint. It must be \/fstab, user\/fstab or similar (at least one \/ must be present to be valid)

system/elektra/mountpoints/fstab/config/path=fstab

Why is =fstab here not in ()?

struct

The struct plugin was removed and fstab is not finished, so better you use some other example (e.g. hosts with glob).

That way, the mountpoint of the backend can still be read out in the core from the backend plugin's plugin configuration.

Very nice idea.

Plugin roles are no longer displayed as array slots, but actually by their names. For example, system/elektra/mountpoints/fstab/getplugins/#0 is now system/elektra/mountpoints/fstab/getplugins/getresolver.

Maybe we can get rid of some redundancy here. system/elektra/mountpoints/fstab/get/resolver should be enough (getplugins -> get and getresolver -> resolver).

The name, reference name and the label of a plugin are now stored in separate keys to avoid using the # symbol for something other than arrays.

This is a big step forward. Did you also consider to completely separate the definitions of plugins and their use in the positions?

it would be referenced by adding ref instead of label and name.

I think @sanssecours will prefer reference instead of ref here 😉

@kodebach do you also want to share some thoughts here?

[kodebach]

Everything here seems good to me.

One important implication is that we need a hard-coded default backend, similar to the default storage and resolver we already have.

We should also ensure in CMake that there is no way to build Elektra without the default storage, resolver and backend plugins. I think right now, you could exclude them from the plugin list and get a broken build. At the least there should be an obvious warning from CMake.

[markus2330]

Yes, I agree that CMake should not succeed until the default-backend plugin is included. For resolver/storage this should already be the case, see src/plugins/CMakeLists.txt lines 67 and 76. Please open an issue if you find a way how to fail during compilation.

[kodebach]

For resolver/storage this should already be the case, see src/plugins/CMakeLists.txt lines 67 and 76. Please open an issue if you find a way how to fail during compilation.

Those checks are only applied during cmake configure, not during make. They ensure that a make (or make all) works, but a make kdb will not (re)build the default storage/resolver, since there is no dependency in the kdb target for this.

I think adding something like elektra-${DEFAULT_STORAGE} and elektra-resolver here:

libelektra/src/tools/kdb/CMakeLists.txt

Lines 30 to 33 in 4db328f

	add_dependencies (kdb
	kdberrors_generated
	elektra_error_codes_generated
	kdb_gen_templates_generated)

(and a few lines below for kdb-full and kdb-static) might be enough.

IMO make kdb should also build a functional Elektra. In fact it should build the absolute minimal version of Elektra.

[markus2330]

IMO make kdb should also build a functional Elektra. In fact it should build the absolute minimal version of Elektra.

I would not spent too much time on the build system. The sub-targets are for expert use only as we do not mention or encourage them anywhere.

But of course you are free to send patches 😉

[kodebach]

The sub-targets are for expert use only as we do not mention or encourage them anywhere.

The CLion tutorial kind of mentions them. But yes, its a minor issue.

[markus2330]

Thank you, I was not aware of that. Any specific reason why this is mentioned there?

[kodebach]

Any specific reason why this is mentioned there?

It actually just says that you can run individual tests by executing "any of the testmod_* or testkdb_*targets". In most cases that should work fine (especially testmod_*), at least after running make once to build everything. The changed parts should be rebuilt by CLion automatically.

[markus2330]

It actually just says that you can run individual tests by executing "any of the testmod_* or testkdb_*targets".

At least on the command-line it is not like this. When I execute make testmod_mini it builds the target testmod_mini but it does not run the test.

In most cases that should work fine (especially testmod_*), at least after running make once to build everything. The changed parts should be rebuilt by CLion automatically.

For me there is no noticeable difference between building everything vs. building a single target if everything was already build before. So why not simply always call the all target. Does someone use such a slow computer that this matters?

[kodebach]

When I execute make testmod_mini it builds the target testmod_mini but it does not run the test.

CLion automatically creates run configurations for all (executable) CMake targets. For target X these run configurations build X (i.e. make X) and then run the resulting executable. In the case of testmod_mini this is more or less like running make testmod_mini && ./bin/testmod_mini.

For me there is no noticeable difference between building everything vs. building a single target if everything was already build before.

Depends what you modified. Modifications to some CMake files for example invalidate a big chunk of the project. If you use make testmod_mini only testmod_mini and its dependencies are built, make all meanwhile would rebuild everything that is out dated including entirely unrelated plugins.

So why not simply always call the all target.

As stated above, the CLion run configurations are created automatically. You would have to manually change them to do that (or not use them).

Does someone use such a slow computer that this matters?

Even on my Ryzen 7 3700X system (using make -j 16), I see a small difference between make all and make kdb, if for example I change something in kdbGet().

Besides that, using e.g. make testmod_mini instead of make all may uncover unknown dependencies.

[markus2330]

I moved the discussion to #3055.

@vLesk do you already have some docu about the new mountpoint configs so that we can review that?

[lgtm-com]

This pull request introduces 1 alert when merging ba43e5a into ed6fd9b - view on LGTM.com

new alerts:

• 1 for FIXME comment

[vLesk]

Simple kdb get and kdb set calls seem to be working as well

[kodebach]

I now reimplemented that aspect so that the lack of a resolver is not considered to be an error by itself. Could this be an issue in certain cases?

Most plugins probably expect that the parentKey contains a resolved filename.
If it works, it would be better to add the noresolver plugin as an resolver in the version backend.

So far, things seem to be working fine with that implementation. Later on, I get the following error from the cache plugin:

That is actually a somewhat known bug, see #3299. The bug itself is almost certainly unrelated to your changes. It can appear, if certain tests fail.

As a workaround you can delete the elektra.ecf file with the broken data (as described in #3299). Then run make run_all again to see which test first produces these errors. Either this test or the last one that touched the KDB before that fails. Then you can disable the problematic test to get proper results from the other ones (how to disable depends on the test).

Alternatively, you can run tests separately with ctest --output-on-failure -R <REGEX> where <REGEX> is a regular expression that will be matched against the test names. For ctest --output-on-failure -R testmod_ will run all containing testmod_ i.e. all plugin tests.

You can also use ctest calls to narrow down which test causes the problem.

If you can't find the problematic test, try to fix the other ones first maybe the bug goes away in the process.

[mpranj]

Postcondition of backend was violated: drop key system/elektra/version not belonging to "system/elektra/version" but instead to "system/elektra/modules/cache" because it is hidden by other mountpoint

Yes, please ignore this at least until #3299 is resolved. If it goes away by disabling the cache, simply disable the cache for now. It is actually very likely that the cache does not work as expected with such huge changes to the backends, even if we disregard #3408

If you want I can push changes to this branch which should definitely disable the cache, and we will take a look together once everything else is resolved. I think this makes the most sense. It will be very hard to debug things if there are errors in the core (or backend implementation) and in the cache as well. I think it's better to focus on the core and backend plugin right now.

[vLesk]

@mpranj that would make things a lot easier, thank you! That way I can clear up the errors on my side.

[mpranj]

I disabled it now as it would be disabled on the master branch. I don't know if there are any other changes which I am not aware of.

Whenever you are unsure, you can also use -DPLUGINS="ALL;-cache" when running cmake, which is an alternative approach to what I pushed.

[vLesk]

Thank you very much!

[lgtm-com]

This pull request introduces 1 alert when merging 2be8b40 into ed6fd9b - view on LGTM.com

new alerts:

• 1 for FIXME comment

[vLesk]

I was held up in traffic on my way back from Slovenia, so I won't be able to make it to the meeting today, unfortunately.

The plugin is working now, I can do basic kdb commands, the default backend is also mounted correctly. The only issue that persists is the issue with the version backend we talked about. It works normally if I do not throw an error if a backend does not have a resolver, but if I use the noresolver plugin as @kodebach suggested, the Slot containing it is null when carrying out kdbGet, so it gets lost somewhere along the way. Resolvers in other backends work properly otherwise.

I am working on getting all the tests running, and I noticed that the testshell tests do not work, and after running them, the other tests as well as kdb no longer work until I clear default.ecf. Where can I find the testshell tests so that I can fix them?

I work full time from November onwards, so it will be difficult for me to find time for Elektra after that. That is why I will take next week and the weekend off from work to focus on the project and get as much work done as possible. I intend to finish my work during that week, but it could also happen that unforeseen issues pop up and I cannot finish all the work in time. In November, I will be unable to invest greater amounts of time into the project, but I will be available to help out in case of questions and provide minor fixes.

However, I do intend to finish my work next week, and I hope that this won't be an issue in the first place.

[markus2330]

Thank you, yes, I also think there is enough time in October. Would be amazing to get this merged soon! 💖

[markus2330]

@vLesk how is it going on? Do you need some specific help?

[vLesk]

@vLesk how is it going on? Do you need some specific help?

@markus2330 thanks for asking! I have a question, kdb seems to be working, but the testshell tests produce configuration which breaks it, leading to the following error which persists until I clear default.ecf:

Sorry, module dump issued the error C01100:
Resource: Could not open configuration file  for reading. Reason: No such file or directory

fatal: Could not export system config

My question in order to find and fix the problem is, where can I find the testshell tests and how do they work?

[kodebach]

My question in order to find and fix the problem is, where can I find the testshell tests and how do they work?

testshell is used as a prefix for a few different tests. The most common case is testshell_markdown_[plugin]. These are generated from code blocks in the README.md files of plugins. For example testshell_markdown_type is generated from src/plugins/type/README.md. The tests run all code blocks that are marked as sh. For type this means theses blocks:

libelektra/src/plugins/type/README.md

Lines 179 to 291 in 4146061

	## Example
	```sh
	#Mount the plugin
	sudo kdb mount typetest.dump user/tests/type dump type
	#Store a character value
	kdb set user/tests/type/key a
	#Only allow character values
	kdb meta-set user/tests/type/key type char
	kdb get user/tests/type/key
	#> a
	#If we store another character everything works fine
	kdb set user/tests/type/key b
	kdb get user/tests/type/key
	#> b
	#If we try to store a string Elektra will not change the value
	kdb set user/tests/type/key 'Not a char'
	# RET:5
	# ERROR:C03200
	kdb get user/tests/type/key
	#> b
	#Undo modifications to the database
	kdb rm user/tests/type/key
	sudo kdb umount user/tests/type
	```
	For enums:
	```sh
	# Backup-and-Restore:/tests/enum
	sudo kdb mount typeenum.ecf user/tests/type dump type
	# valid initial value + setup valid enum list
	kdb set user/tests/type/value middle
	kdb meta-set user/tests/type/value check/enum '#2'
	kdb meta-set user/tests/type/value 'check/enum/#0' 'low'
	kdb meta-set user/tests/type/value 'check/enum/#1' 'middle'
	kdb meta-set user/tests/type/value 'check/enum/#2' 'high'
	kdb meta-set user/tests/type/value type enum
	# should succeed
	kdb set user/tests/type/value low
	# should fail with error C03200
	kdb set user/tests/type/value no
	# RET:5
	# ERROR:C03200
	```
	Or with multi-enums:
	```sh
	# valid initial value + setup array with valid enums
	kdb set user/tests/type/multivalue middle_small
	kdb meta-set user/tests/type/multivalue check/enum/#0 small
	kdb meta-set user/tests/type/multivalue check/enum/#1 middle
	kdb meta-set user/tests/type/multivalue check/enum/#2 large
	kdb meta-set user/tests/type/multivalue check/enum/#3 huge
	kdb meta-set user/tests/type/multivalue check/enum/delimiter _
	kdb meta-set user/tests/type/multivalue check/enum "#3"
	kdb meta-set user/tests/type/multivalue type enum
	# should succeed
	kdb set user/tests/type/multivalue small_middle
	# should fail with error C03200
	kdb set user/tests/type/multivalue all_small
	# RET:5
	# ERROR:C03200
	# cleanup
	kdb rm -r user/tests/type
	sudo kdb umount user/tests/type
	```
	For booleans:
	```sh
	# Mount plugin
	sudo kdb mount config.ecf user/tests/type dump type
	# By default the plugin uses `1` (true) and `0` (false) to represent boolean values
	kdb set user/tests/type/truthiness false
	kdb meta-set user/tests/type/truthiness type boolean
	kdb get user/tests/type/truthiness
	#> 0
	# The plugin does not change ordinary values
	kdb set user/tests/type/key value
	kdb get user/tests/type/key
	#> value
	# Undo changes
	kdb rm -r user/tests/type
	sudo kdb umount user/tests/type
	```
	```sh
	sudo kdb mount config.ecf user/tests/type dump type
	kdb set user/tests/type/truthiness 0
	kdb meta-set user/tests/type/truthiness type boolean
	kdb set user/tests/type/truthiness yes
	# RET: 0
	# Undo changes
	kdb rm -r user/tests/type
	sudo kdb umount user/tests/type
	```

The tests are quite hard to debug. But if all of them fail, there is probably a quite severe bug. You should be able to narrow things down by manually executing the shell commands one after another.

The exact format used in the comments of these shell scripts is documented in tests/shell/shell_recorder/tutorial_wrapper/README.md

[vLesk]

Thank you @kodebach! It seems that the issue is not with the tests themselves, but with mounting in general, the config does not seem to be stored correctly, I'm looking into it.

[markus2330]

@vLesk any update? Can you describe us what does not work? 💖

[markus2330]

I hope we can get this into 0.9.5. @vLesk do you have any status update? Can you find some time? Now would be a good time for a rebase, we upgraded to clang-format-11.

[kodebach]

I'm closing this PR, since it takes for every to load on GitHub. I will open a new PR with a rebased version of this, once #3651 is merged.

[markus2330]

Thank you so much! 💖

I just read yesterday in #3461 that it would be great to have this backend plugin finally fixed.