New API preview

[kodebach]

DO NOT MERGE

---

Preview of new API (DO NOT MERGE)

So far only the typedefs for Key, KeySet and KDB have been renamed and it only compiles with -DBINDINGS=cpp. Also this is based on new-backend to reduce the need for rebases to keep the branch current.

Some notes on what had to be done manually (the rest was done with my not-yet-published tool based on tree-sitter).

Notes

Macro problems

tree-sitter has problems with some uses of macros, in particular va_arg, macros used in concatenated strings and any macro that is a wrapper around __attribute__. To work around this, we ignore errors from tree-sitter (reported as warning), as long as the script doesn't edit anything within an ERROR node.

First attempt: rename_structs

#ifdef __cplusplus

The above caused problems with parsing. Essentially the whole file was in an ERROR node. Fix: temporarily comment out

Some plugins et al. use something similar to

KeySet * contract =
#include "contract.h"

This also causes ERROR nodes around the KeySet * variable declaration. Fix: temporarily replace #include with ksNew(0);

gtest-elektra.h is a C++ header, but was detected as C (because of simple file extension based search)

template.h from pythongen had errors, but was ignored because it's deprecated.

testmod_base64.c used function declarations like this

static void test_init (void)
#ifdef __llvm__
	__attribute__ ((annotate ("oclint:suppress[high ncss method]")))
#endif

Fix: temporarily comment out the #ifdef.

Extra fixes (to make it compile)

Note: compiled with only -DBINDINGS=cpp

Some manual replacements had to be done in

• kdberrors.h
• kdbnotification.h
• kdbnotificationinternal.h
• kdbmacros.h
• mmapstorage/internal.h
• macros/type_create_to_value.h
• macros/add_type.h
• elektra_value.c
• elektra_array_value.c
• errors.c
• tests_plugin.h
• tests.h
• testmod_reference.c
• testmod_toml.c
• testmod_spec.c
• kdbos.h
• testmod_matchcheck.c
• test_opts.c
• test_conversion.c

because Key/KeySet/KDB was used within a macro definition.

keyset.c, key.c, keyhelpers.c needed a manual fix, because of sizeof(KeySet)/sizeof(Key).

augeas.c required a manual fix, because of a macro in string concatenation. This wasn't reported as an error, but it caused a declaration to run much further than it should.

gpgme.c required manual fixes because of ELEKTRA_UNUSED. This wasn't reported as an error, but instead ELEKTRA_UNUSED was treated as the type and Key * was put into an ERROR node.

testmod_hosts.c, testmod_xerces.c, testmod_reference.c needed a fix, because of a missing ; (was inside macro, also broke formatting).

To make C++ code work,

#ifdef __cplusplus
using KDB = ckdb::ElektraKdb;
using Key = ckdb::ElektraKey;
using KeySet = ckdb::ElektraKeyset;
#endif

was added to kdb.h.in

using ckdb::Key;
using ckdb::KeySet;

was added to directoryvalue.cpp and in various places e.g. using Key = ckdb::Key was adapted.

---

DO NOT MERGE

[markus2330]

@kodebach Do you already have renames of the functions? (I see they are not yet pushed here.) @lawli3t has currently very progress with the rust-reimplementation and otherwise would need to write a wrapper to be compatible to the old function names.

[kodebach]

No, this is currently all I've got. I will be on holiday for the next week, but after that I'll have more time to work more on this.

[lawli3t]

It should be fine to just write a simple translation layer to map the old function names to the new ones, so don't worry too much about it

Klemens Böswirth @.***> schrieb am Mo., 15. Aug. 2022, 13:34:

No, this is currently all I've got. I will be on holiday for the next week, but after that I'll have more time to work more on this.

— Reply to this email directly, view it on GitHub https://github.com/ElektraInitiative/libelektra/pull/4426#issuecomment-1214912896, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFR66CV3WRSAEWXAS35FFTVZITMVANCNFSM55HX75DQ . You are receiving this because you were mentioned.Message ID: @.***>

[kodebach]

Update

I have added a new feature to my tree-surgeon tool. It is now much easier to add a prefix to a list of identifiers. I used this to update the names of various constants (e.g. KEY_*, KDB_O_*, etc.) by adding the ELEKTRA_ prefix.

[kodebach]

Update

I have now further extended tree-surgeon and created a script to rename the functions from libelektra-core and libelektra-kdb. To be more precise, the script covers the key*, ks* and kdb* functions listed in src/libs/elektra/symbols.map.

I didn't have time to do the required manual updates of macros and other places where the script failed, so this PR currently doesn't compile.

How to proceed

@markus2330 I propose that we limit this PR to renaming (i.e. no removal of functions, no changing of functionality, etc.). This should make reviewing much easier, since the PR shouldn't actually change any semantics.

The more complex changes will then happen in new PRs. This way reviewing should still be pretty okay, since some PRs may be big, but all changes in a PR would follow the same idea.

[markus2330]

@markus2330 I propose that we limit this PR to renaming (i.e. no removal of functions, no changing of functionality, etc.). This should make reviewing much easier, since the PR shouldn't actually change any semantics. The more complex changes will then happen in new PRs. This way reviewing should still be pretty okay, since some PRs may be big, but all changes in a PR would follow the same idea.

Yes, very good proposal. Please also make sure to always give information&statistics what you manually did and what the tool changed. This information is also essential for your thesis.

Please notify me when you also did the manual changes etc. (i.e. the PR is "ready to merge")

[kodebach]

Please also make sure to always give information&statistics what you manually did and what the tool changed. This information is also essential for your thesis.

I always made a commit with the results from the tool and then separate one(s) for the manual fixes. (the latest rebase splits the automated changes out of the first commit)

For now the notes above are mostly accurate. There are a handful of files that always cause problems, because tree-sitter produces large error nodes for them. Some files (e.g. tests/data/data_ns.c) are not valid syntax at all, other times tree-sitter gets confused by some code that is actually excluded via #idefs. These issue can only be fixed, by changing our code to avoid the parse errors. There is, however, only a handful of files with such problems so I don't see this as a big priority. Manually fixing these is fine.

The second category of manual fixes is macros. Any code within the definition of macro is not processed by tree-sitter at all. These issues are more of a problem, since macros are used a lot, especially in tests. For the structs and consts it wasn't too big a deal. But before I fix the next batch, I may look into getting tree-sitter to parse the macro definitions to avoid this problem in future.

Please notify me when you also did the manual changes etc. (i.e. the PR is "ready to merge")

Will do. We should also mention this PR in the meeting today, since it will severely impact all the people working on the new-backend branch. Either their code must be updated to match the new function names, or they merge first and I update it with this PR.

[markus2330]

or they merge first

Yes, better we do not hinder the work on the new-backend branch. So let us merge this shortly before you also plan to reintegrate new-backend to master. I hope the big renames get done before the term starts...