Keyname overhaul #3555

kodebach · 2020-11-12T17:45:58Z

Continuation of #3447 (commits squashed and rebased)

Implements #3102

Closes #3050
Closes #3084
Closes #2698
Closes #3082

Basics

These points need to be fulfilled for every PR:

Short descriptions of your changes are in the release notes
(added as entry in doc/news/_preparation_next_release.md which
contains _(my name)_)
Please always add something to the release notes.
Details of what you changed are in commit messages
(first line should have module: short statement syntax)
References to issues, e.g. close #X, are in the commit messages.
The buildservers are happy.
The PR is rebased with current master.

If you have any troubles fulfilling these criteria, please write
about the trouble as comment in the PR. We will help you.
But we cannot accept PRs that do not fulfill the basics.

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 fully described what my PR does in the documentation
(not in the PR description)
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, good to read and describes everything what the PR does
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

kodebach · 2020-11-12T17:50:41Z

@markus2330 Please check, if the new Python reference implementation and the documentation matches the decisions in #3549.

The changes can be found in this commit: ae05d31

If everything matches, I will update the C implementation so that this PR can be merged ASAP. Then we can start on the other API changes (keyNew, removal of escaped name, etc.) in separate PRs.

markus2330 · 2020-11-15T14:31:34Z

I looked at ae05d31, I would simplify it and give away this special handling about # (make it pure conventional).

markus2330 · 2020-11-15T14:32:11Z

The part with . and .. looks perfect to me now! 🥇

kodebach · 2020-11-15T15:04:11Z

I looked at ae05d31, I would simplify it and give away this special handling about # (make it pure conventional).

I already explained it a bit, but here is a more in depth explanation:

We decided that we want a 1:1 relation between escaped and unescaped forms.
We also want that #123 works as an array index, i.e. it turns into #__123 automatically.

Just based 2 we already need at least some form of escaping mechanism, or keyAddBaseName (k, "#123") results in a Key that has no escaped name. We could just use .../#123 as the escaped name and say that you can construct such a name only via keyAddBaseName. But that has another problem. It is very common to do keyNew (keyName(k), KEY_END) to create a Key with the same name as k, but nothing else in common. This wouldn't work anymore, since it would result in a key with name .../#__123.

To have the 1:1 relation the \# can only be valid, when # has an effect. Otherwise we would have some unescaped names with two escaped forms. For example \#abc and #abc would both be unescaped into #abc. This can be solved, making either \#abc or #abc illegal in the escaped name. Here I decided to make the first one illegal. In the previous version #abc was illegal, but we decided that was to much "semantics".

I actually just noticed that the check for \# is not correct. \# should only be allowed for the case were _ will be inserted. So line 90 should actually be the much simpler:

"#": lambda part: re.match(r"^\\#[1-9][0-9]{1,18}$", part) and (len(part) < 21 or part[2:] <= "9223372036854775807"),

markus2330 · 2020-11-15T15:28:19Z

Thank you, I was just missing what you write here in the docu! With this fixed the commit looks fine.

Is there any other specific part which needs a closer look?

kodebach · 2020-11-15T15:51:21Z

Is there any other specific part which needs a closer look?

No, apart from ae05d31 everything is unchanged compared to #3447. I'll fix the Python code and add the details about arrays and then I'll start updating the C code. With a bit of luck this PR can be merged by the end of the week.

Squashed (and fixed) commit of the following: commit 541e404 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Fri Oct 23 17:50:26 2020 +0200 proposal and demo for new api commit 29fb355 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Oct 20 21:11:01 2020 +0200 add TODO commit b169534 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Mon Oct 12 17:25:38 2020 +0200 fix footnote commit 60c508b Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Oct 11 16:55:20 2020 +0200 add TODO commit 7da783d Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Oct 11 16:44:46 2020 +0200 Remove keyIsInactive and plugin 'hidden' commit 2f167d5 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Fri Oct 9 14:38:56 2020 +0200 Update release notes commit 1429765 Author: kodebach <23529132+kodebach@users.noreply.github.com> Date: Mon Oct 12 17:25:24 2020 +0200 Apply suggestions from code review Co-authored-by: markus2330 <markus2330@users.noreply.github.com> commit 2fdb2dc Author: kodebach <23529132+kodebach@users.noreply.github.com> Date: Sun Oct 11 20:28:12 2020 +0200 Apply suggestions from code review Co-authored-by: markus2330 <markus2330@users.noreply.github.com> commit 3ae0fb7 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat Oct 3 21:32:05 2020 +0200 fix spelling commit 6feba9c Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat Oct 3 20:51:13 2020 +0200 Finish new documentation and extend release notes commit a026717 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat Oct 3 14:08:19 2020 +0200 Add Key Name documentation - Add a Python reference implementation - Start documentation README commit 6120185 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat Oct 3 00:57:21 2020 +0200 ccode: fix oclint issue commit 495410c Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Fri Oct 2 20:34:50 2020 +0200 fix bug in keyname validation commit 0616290 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Fri Oct 2 18:36:12 2020 +0200 ccode: fix en-/decoding bug commit abe9dd7 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Fri Oct 2 15:41:35 2020 +0200 Update shell test data commit 8b47125 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 29 20:21:29 2020 +0200 Update release notes commit f90ec81 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 29 20:11:44 2020 +0200 fix swig compile flags commit 4403f12 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Mon Sep 28 20:13:22 2020 +0200 Update Keynames proposal commit 76c5529 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Wed Sep 23 20:19:30 2020 +0200 update some bits of the C++ binding commit 6edcb87 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Wed Sep 23 18:53:56 2020 +0200 update TODOs commit f42102b Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Wed Sep 23 18:29:12 2020 +0200 remove various deprecated things commit 3a63e42 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Wed Sep 23 18:12:56 2020 +0200 fix website build commit 151f770 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Wed Sep 23 00:53:18 2020 +0200 fix oclint errors commit bb90d37 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 23:38:20 2020 +0200 fix oclint errors commit e5a0611 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 22:28:17 2020 +0200 fix oclint errors (again) commit b9a5021 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 22:21:38 2020 +0200 Revert "Revert "adapt ruby code to work with ruby 2.3"" This reverts commit 2e8f936. commit 25a788e Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 22:21:35 2020 +0200 Revert "fix oclint errors" This reverts commit 8703b05. commit 5865fa6 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 22:21:32 2020 +0200 Revert "fix ccode" This reverts commit ffebe18. commit 7e7de68 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 21:37:04 2020 +0200 Revert "adapt ruby code to work with ruby 2.3" This reverts commit eebd51a. commit 4703b86 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 21:02:52 2020 +0200 adapt ruby code to work with ruby 2.3 commit ca687a9 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 20:57:08 2020 +0200 fix ccode commit d54ed1a Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 20:54:09 2020 +0200 fix oclint errors commit 8eb9717 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 18:50:29 2020 +0200 fix JNA keyCmp commit 2c1850e Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 18:09:55 2020 +0200 remove TODO commit b6894d4 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 18:03:51 2020 +0200 fix disabled pythongen commit 291dded Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 18:01:29 2020 +0200 remove debian-stretch-full-ini Jenkins job commit e8e6d4b Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 18:00:05 2020 +0200 add ELEKTRA_NO_SANITIZE_ADDRESS for gcc commit 0e7890c Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 17:55:31 2020 +0200 fully disable pythongen commit 85d7224 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Tue Sep 22 17:50:12 2020 +0200 disable more santizers in opmphmHashfunction commit 63c29ca Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 22:22:19 2020 +0200 fix kdb gen commit 1e47869 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 22:08:29 2020 +0200 update man page commit be60437 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 21:52:59 2020 +0200 fix more keynames commit 6d9be23 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 21:22:40 2020 +0200 fix .gitignore and add missing files commit 74d7229 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 21:07:26 2020 +0200 fix javascript formatting commit e59e9a5 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 20:50:02 2020 +0200 update some files missed in rebase commit d0102d7 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 20:00:58 2020 +0200 remove some files commit c9f0b99 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 20:00:28 2020 +0200 Revert "undo changes to kdb-global-umount test" This reverts commit 3716959. commit 6521996 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 19:15:11 2020 +0200 fix testcpp_contextual_update commit 350ad25 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 18:48:43 2020 +0200 undo changes to kdb-global-umount test commit 8390278 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 18:21:55 2020 +0200 update TODOs commit 529c2ec Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 18:02:11 2020 +0200 fix kdb-global-umount commit d93027f Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sun Sep 20 17:27:35 2020 +0200 fix various memory problems commit 1bf60ee Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat Sep 5 16:57:55 2020 +0200 java: fix after keyname overhaul commit 33c2c53 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat Sep 5 16:35:51 2020 +0200 fix formatting commit 0486d04 Author: Klemens Böswirth <k.boeswirth+git@gmail.com> Date: Sat May 30 14:34:43 2020 +0200 Keyname overhaul Squashed version of 22ff913..3645f79

Fixes 1:1 mapping by removing collision between keys like / and /%

kodebach · 2020-11-20T17:59:29Z

@mpranj This PR should finally be ready this weekend. It would be nice, if don't merge any PRs until the meeting on Monday (too avoid more rebases, which are quite annoying in such a big PR).

@markus2330 Not sure, how much you actually want to review. Everything should™ be the same as the Python code and documentation you already reviewed. However, I had to make a small change (see also 5839dd4): An escaped key name that is just (namespace +) /% is now illegal, because the unescaped version would collide with the root keys (/, user:/ etc.), which already use an empty part.

kodebach · 2020-11-21T00:06:09Z

Not sure what's wrong with the pdflatex build (TBH not sure why we even have that build job), but since the macOS jobs are blocked by #3559 I won't fix it for now. Apart from these two build issues the PR is done IMO.

markus2330

Incredible work!

I only looked through https://github.com/ElektraInitiative/libelektra/pull/3555/files/7f767a442377cdb7c80db71cd52f8b3b35e99864..99e0766a86212a3ca0041e7f40165a0341da03c2

Even if someone finds further problems in this PR, I think it is better if we fix them in follow-up PRs. This already worked quite well with @kodebach several times in the past.

So I agree we should merge it before any other PR, @kodebach has already rebased often enough. As master is now broken anyway, we can also merge right away and fix master afterwards. Then it is easier to help @kodebach with the remaining minor issues (like the pdflatex build).

kodebach · 2020-11-21T13:37:10Z

As master is now broken anyway, we can also merge right away and fix master afterwards. Then it is easier to help @kodebach with the remaining minor issues (like the pdflatex build).

In that case, I will have another look at the pdflatex stuff today, but if I can't find a fix quickly we can merge this PR.

kodebach · 2020-11-21T15:23:02Z

@markus I got as far as figuring out that something about doc/keynames/README.md is wrong, but not what exactly. If you know about markdown things that break doxygen's latex/pdf generation, please check if you spot any...

markus2330 · 2020-11-21T17:29:34Z

Some UTF-8 chars, which are not declared for LaTeX (in doc/markdownlinkconverter/elektraSpecialCharacters.sty) break the pdf build. Maybe it is the (r) sign?

kodebach · 2020-11-21T17:51:19Z

Some UTF-8 chars, which are not declared for LaTeX (in doc/markdownlinkconverter/elektraSpecialCharacters.sty) break the pdf build. Maybe it is the (r) sign?

Hm, yes that could be that case. It would be an explanation why the build works locally on my machine. Probably differnt LaTeX settings... I'll try replacing it with the HTML entity ®

markus2330 · 2020-11-21T18:12:01Z

You can simply declare ® in doc/markdownlinkconverter/elektraSpecialCharacters.sty, I think this is the better solution.

kodebach · 2020-11-21T20:03:20Z

Seems like it didn't help

mpranj

@kodebach thank you so much for this exceptional work! Thanks as well for fixing all the little problems unrelated to the main topic of this PR. I can't imagine how painful rebasing was, it took me three hours just to scroll throught the changes.

Most importantly:
I think there is an error with clear_bit/test_bit in src/libs/elektra/keyname.c.

All other comments are just suggestions, so feel free to ignore the rest.

Additionally, can these empty files be removed:

src/bindings/swig/python2/tests/testpy2_kdb.py
doc/todo/PROPOSALS?

doc/VERSION.md

doc/help/kdb-meta-show.md

doc/keynames/README.md

src/bindings/swig/python/tests/test_key.py

src/libs/elektra/kdb.c

src/libs/elektra/keyname.c

mpranj · 2020-11-21T19:55:50Z

src/plugins/mmapstorage/mmapstorage.c

+		}
+
+		// move Key unescaped name
+		if (curMeta->ukey)


Is ukey always allocated and freed with the key name? Or would we actually need a third mmap flag for the ukey?

Yes, ukey and key should be allocated and freed together. I even made sure that a Key is not modified by e.g. keySetName unless the operation will be successful (apart from malloc failures).

Co-authored-by: Mihael Pranjić <mpranj@limun.org>

kodebach · 2020-11-21T22:53:50Z

it took me three hours just to scroll throught the changes.

I'm amazed you did it that quickly...

The clear_bit thing was definitely an error. Let's see, if the builds still work. If so, let's merge this PR and fix everything else in a follow up. Since merging this will break anything that depends on master, don't think anyone will mind if it also introduces 1 or 2 small bugs ;)

mpranj

Excellent work! Again, a huge thank you to @kodebach for making this a reality. 🥇

kodebach added the work in progress label Nov 12, 2020

kodebach self-assigned this Nov 12, 2020

kodebach mentioned this pull request Nov 12, 2020

[Old] Keyname overhaul #3447

Closed

16 tasks

PhilippGackstatter mentioned this pull request Nov 13, 2020

Remove kdbproposal #3201

Merged

16 tasks

kodebach added 10 commits November 20, 2020 18:04

remove rejected proposals

7f767a4

New keyname docu and Python reference implementation

cdf8e31

fixes and improvements from review

e5b3966

reduce capitalization

4ca8a2c

Implement new key name decision

36db1e6

Update key name decision

5839dd4

Fixes 1:1 mapping by removing collision between keys like / and /%

Update documentation

8fdf75b

Reorder functions

3832c37

fixes after rebase

ab37756

kodebach force-pushed the keyname_overhaul_v3 branch from f9b7ac4 to ab37756 Compare November 20, 2020 17:30

fix memory leak

99e0766

kodebach requested a review from markus2330 November 20, 2020 17:56

kodebach added 2 commits November 20, 2020 19:01

fix links

5f71e83

update man pages

a67fd13

kodebach marked this pull request as ready for review November 21, 2020 00:06

markus2330 approved these changes Nov 21, 2020

View reviewed changes

replace (R) signs

51eb9b6

add ® to special chars

815f9dc

mpranj suggested changes Nov 21, 2020

View reviewed changes

kodebach and others added 3 commits November 21, 2020 23:40

Apply suggestions from code review

9d159e9

Co-authored-by: Mihael Pranjić <mpranj@limun.org>

fix rebase error

be38f81

remove empty files

42e33e3

mpranj approved these changes Nov 22, 2020

View reviewed changes

mpranj merged commit 1630723 into ElektraInitiative:master Nov 22, 2020

This was referenced Nov 25, 2020

make namespace enum more future proof #3567

Closed

output of more than 10 warnings broken #3577

Closed

keynames: fix addWarning #3579

Merged

kodebach mentioned this pull request Dec 5, 2020

keyNew overhaul #3529

Closed

kodebach pushed a commit to kodebach/go-elektra that referenced this pull request Dec 10, 2020

Various fixes related to ElektraInitiative/libelektra#3555

d126f06

kodebach added a commit to kodebach/go-elektra that referenced this pull request Dec 10, 2020

Various fixes related to ElektraInitiative/libelektra#3555

abb90aa

This was referenced Dec 10, 2020

Keyname overhaul ElektraInitiative/go-elektra-archive#10

Merged

qtgui: update for keyname overhaul #3589

Merged

keyname overhaul finishing touches #3596

Merged

kodebach mentioned this pull request May 13, 2021

JNI Test Fix #3825

Merged

20 tasks

markus2330 mentioned this pull request Jun 20, 2021

toml plugin in 0.9.6 eats the first character of each key #3896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keyname overhaul #3555

Keyname overhaul #3555

kodebach commented Nov 12, 2020

kodebach commented Nov 12, 2020

markus2330 commented Nov 15, 2020

markus2330 commented Nov 15, 2020

kodebach commented Nov 15, 2020

markus2330 commented Nov 15, 2020

kodebach commented Nov 15, 2020

kodebach commented Nov 20, 2020

kodebach commented Nov 21, 2020

markus2330 left a comment

kodebach commented Nov 21, 2020

kodebach commented Nov 21, 2020

markus2330 commented Nov 21, 2020

kodebach commented Nov 21, 2020

markus2330 commented Nov 21, 2020

kodebach commented Nov 21, 2020

mpranj left a comment

mpranj Nov 21, 2020

kodebach Nov 21, 2020

kodebach commented Nov 21, 2020

mpranj left a comment

Keyname overhaul #3555

Keyname overhaul #3555

Conversation

kodebach commented Nov 12, 2020

Basics

Checklist

Review

kodebach commented Nov 12, 2020

markus2330 commented Nov 15, 2020

markus2330 commented Nov 15, 2020

kodebach commented Nov 15, 2020

markus2330 commented Nov 15, 2020

kodebach commented Nov 15, 2020

kodebach commented Nov 20, 2020

kodebach commented Nov 21, 2020

markus2330 left a comment

Choose a reason for hiding this comment

kodebach commented Nov 21, 2020

kodebach commented Nov 21, 2020

markus2330 commented Nov 21, 2020

kodebach commented Nov 21, 2020

markus2330 commented Nov 21, 2020

kodebach commented Nov 21, 2020

mpranj left a comment

Choose a reason for hiding this comment

mpranj Nov 21, 2020

Choose a reason for hiding this comment

kodebach Nov 21, 2020

Choose a reason for hiding this comment

kodebach commented Nov 21, 2020

mpranj left a comment

Choose a reason for hiding this comment