Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

ini plugin makes hierarchies flat #138

Closed
markus2330 opened this issue Nov 6, 2014 · 18 comments
Closed

ini plugin makes hierarchies flat #138

markus2330 opened this issue Nov 6, 2014 · 18 comments
Assignees
Labels

Comments

@markus2330
Copy link
Contributor

E.g. I have:

kdb ls system/passwd

system/passwd/yacy/uid
system/passwd/zeroinst
system/passwd/zeroinst/gid

Actual outcome (excerpt):
kdb export system/passwd ini

...
shell = /bin/false
uid = 114
yacy = 
gid = 265
home = /var/lib/yacy
...

Makes the hierachy flat.

Expected outcome:

[yacy]
uid=..
[zeroinst]
gid=..
@markus2330 markus2330 added the bug label Nov 6, 2014
@markus2330 markus2330 added this to the 0.8.10 milestone Nov 6, 2014
@fberlakovich
Copy link
Contributor

Currently only directories are serialised to sections. This is due to historical reasons (former treatment of keys with / in the name) and I will fix it.

@fberlakovich
Copy link
Contributor

What is the intended behaviour? With only inner keys becoming sections, empty sections would not be possible anymore. For example, what if the kdb contained something like this

system/passwd/yacy
system/passwd/zeroinst
system/passwd/zeroinst/gid

According to your example the expected outcome would be

[yacy]
[zeroinst]
gid=..

But due to the missing subkey, yacy is no longer an inner key, but a leaf key now.

@markus2330
Copy link
Contributor Author

Hello,

You wrote:

What is the intended behaviour?

Does INI support arbitrary depth?

If not its straight forward: keyDirectBelow parentKey are sections, others are
values.

With only inner keys becoming sections,
empty sections would not be possible anymore.
[..]
But due to the missing subkey, yacy is no longer an inner key, but a leaf
key now.

Thats the general topic of dir2leafvalue, see #106. One has to remember if a
section corresponends to an actual key and if it has a value. This information
needs to be stored in extra bits (e.g. in a subkey).

dir2leafvalue is a quite important topic: many serializations have these
problems.

best regards

@markus2330 markus2330 removed this from the 0.8.10 milestone Dec 3, 2014
markus2330 pushed a commit that referenced this issue Dec 31, 2014
- #138 now breaks test cases, temporarily commented out
- added everything needed for dir (untested)
@markus2330
Copy link
Contributor Author

Any progress here?

I removed keyIsDir (because the name is needed for the dir-namespace). So unit tests are now broken in ini, because it wrongly relied on keyIsDir. (I temporarily commented out the comparision between files)

Instead it should rely on the key's position below parentKey.

Can you fix it for the release 0.8.11?

@fberlakovich
Copy link
Contributor

What is the planned release date for 0.8.11?

@markus2330
Copy link
Contributor Author

Hello,

You wrote:

What is the planned release date for 0.8.11?

Asap, hopefully before the 5th.

@fberlakovich
Copy link
Contributor

First of all: sorry for the late response. I wrote an answer 2 days ago, but obviously something went wrong.

If not its straight forward: keyDirectBelow parentKey are sections, others are values.

This is also problematic because as far as I know INI may contain key value pairs without a section. E.g.

nosectionvalue = value
[section1]
sectionvalue = value1

The value could even be empty:

nosectionvalue = 
[section1]
sectionvalue = value1

So even using the value as a differentiator is not possible. We could simply reject parsing such INI files, but then we would have yet another incomplete INI parser implementation :(. Unfortunately INI is hardly (if at all) standardized. Another possibility would be to make the behaviour configurable by allowing the user to specify section strategies (e.g. all keys below the parent key are treated as sections). However, this increases the configuration burden and would also be a fair amount of work. Which option do you like best?

@markus2330
Copy link
Contributor Author

Hello,

you wrote:

This is also problematic because as far as I know INI may contain key value
pairs without a section.

The plugin "ni" solves this problem that keys without value are sections, with
value are key=value, e.g.:

abc = value
abc/x = value1

would be:

abc = value
[abc]
x = value1

w/o abc = value would be without first line.

The value could even be empty:

So even using the value as a differentiator is not possible. We could
simply reject parsing such INI files, but then we would have yet another
incomplete INI parser implementation :(. Unfortunately INI is hardly (if
at all) standardized. Another possibility would be to make the behaviour
configurable by allowing the user to specify section strategies (e.g. all
keys below the parent key are treated as sections). However, this
increases the configuration burden and would also be a fair amount of
work. Which option do you like best?

In Elektra empty value != null (without) value. So it should work nicely.
See keyValue() docu

Or you can check presence/absence of keys. Shouldn't that be enough already?
Just do not generate the key abc if there is a section but not a key with
(empty) value.

@fberlakovich
Copy link
Contributor

@markus2330 Is there currently any way to unescape a key name? My problem is that currently an ini key
test/key will produce an Elektra key like user/ini/test\/key. However, when writing the key back it will result in an ini key test\/key which is obviously not the same.

I would even more prefer to already have a way to set the key name unescaped. This would allow the ini file

[/settings/NRPE/server]
allow arguments = true

to result in the very intuitive Elektra keyset

user/settings/NRPE/server
user/settings/NRPE/server/allow arguments

(example taken from http://docs.nsclient.org/tutorial/core/index.html)

@markus2330
Copy link
Contributor Author

Hello,

you wrote:

@markus2330 Is there currently any way to unescape a key name? My problem
is that currently an ini key test/key will produce an Elektra key like
user/ini/test\/key. However, when writing the key back it will result in
an ini key test\/key which is obviously not the same.

keyAddBaseName is to add only one part and thus will escape slashes.
keyAddName (to be added in the API with the next release, but to be renamed
to keyChangeName) changes a key-name as given, without escaping any part.
keySetName also does not escape the name.

If you find the documentation confusing, please report where which information
should be added.

best regards

@markus2330
Copy link
Contributor Author

Thank you! There was a lot of progress on this issue. Unfortunately, the import/export shell tests (e.g. tests/shell/check_export.sh) still do not support ini.

In the export test case an ini file is generated which has two times the same key.

@markus2330
Copy link
Contributor Author

Problem still persists. E.g. for structural exporting ini is basically useless:

$ kdb export system/info/constants ini
cmake = ON
cmake = ON
cmake = ON
cmake = /usr
cmake = OFF
cmake = OFF
cmake = OFF
...

@fberlakovich
Copy link
Contributor

This plugin is getting nasty. Could you provie the file you have mounted at system/info/constants?

@markus2330
Copy link
Contributor Author

You can mount it with the script "mount-info", but here a different export with ni:

$ kdb export system/info/constants ni
version/version/KDB_VERSION = 0.8.11
compiler = Flags defined for compilers as defined in ElektraCompiling.cmake
cmake/TARGET_CMAKE_FOLDER = share/cmake-2.8/Modules
compiler/id = GNU
cmake = All cmake variables as defined in the file cmake/ElektraCache.cmake
cmake/ENABLE_CXX11 = OFF
cmake/GTEST_ROOT = 
macros/KDB_DIR_MODE = 0100
version/SO_VERSION = 4
cmake/ELEKTRA_VERBOSE_BUILD = OFF
cmake/BUILD_STATIC = ON
cmake/TOOLS = kdb\;gen\;race\;qt-gui
cmake/TARGET_TEST_DATA_FOLDER = share/libelektra-test/test-data
cmake/KDB_DB_USER = .config
cmake/TARGET_INCLUDE_FOLDER = elektra

ni scrambles the order, so ini would be a nice gain ;)

@markus2330
Copy link
Contributor Author

Btw. the release is most likely to happen today and this issue is not release-critical. I will merge it if its a small change though.

@fberlakovich
Copy link
Contributor

You can mount it with the script "mount-info", but here a different export with ni:

Thank you, I will have a look at it!

ni scrambles the order, so ini would be a nice gain ;)

No matter how nasty it is, I already invested so much work in ini, it would be a shame not to fix it :).

@fberlakovich
Copy link
Contributor

Btw. the release is most likely to happen today and this issue is not release-critical. I will merge it if its a small change though.

The plugin is fairly complex already (especially this issue), so I am afraid I won't make it today.

@fberlakovich
Copy link
Contributor

As discussed, here is a more detailed explanation of the issue:

Let me first explain what we would expect from the plugin just to make sure we have the same understanding of its (expected) behavior.
A call like kdb export system/info/constants ini should produce an ini file like this:

cmake = All cmake variables as defined in the file cmake/ElektraCache.cmake
cmake/BINDINGS = cpp
cmake/BUILD_FULL = ON
...

assuming that a call like kdb ls system/info/constants looks like this:

system/info/constants
system/info/constants/cmake
system/info/constants/cmake/BINDINGS
system/info/constants/cmake/BUILD_FULL
...

(values don't really matter for this issue). No sections are produced because none of the keys below system/info/constants qualifies as a section key and the autosections option is not enabled.

However, the actual output looks like this:

cmake = All cmake variables as defined in the file cmake/ElektraCache.cmake
cmake = cpp
cmake = ON

Now that the issue is fully described let me explain why it happens:

The bug is caused by the call of elektraUnescapeKeyName in the function getIniName. The function is responsible for constructing a correct ini name out of the supplied key and its parent key. This involves finding the correct section of the key (if any) and afterwards constructing the key name based on that information. The function calls elektraUnescapeKeyName because usually an ini name like
cmake/BINDINGS would result in a key system/info/constants/cmake\\BINDINGS. Unescaping this key and applying the transformations done in getIniName would correctly result in the name cmake/BINDINGS. However, if the function elektraUnescapeKeyName is applied to a string that doesn't contain any escaping it replaces all / by \0. So applying it to cmake/BINDINGS (as it is done during the export) results in cmake\0BINDINGS which in turn leads to the corruped output.

In order to fix this issue the ini plugin needs a way to detect whether unescaping needs to be done. The only possible way I could think of up to now is marking escaped keys with a special meta key during the GET phase. However, this would make the ini plugin stateful which I don't really like.

fberlakovich referenced this issue in tom-wa/libelektra Oct 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants