Codegen updates #2927

kodebach · 2019-09-02T19:14:53Z

Re-created from #2805

Contains some updates for the code-generation. For progress see #2680.

Fixes #2866

Basics

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

I added unit tests
I ran all tests locally and everything went fine
affected documentation is fixed
I added code comments, logging, and assertions (see Coding Guidelines)
meta data is updated (e.g. README.md of plugins and METADATA.ini)

Review

Reviewers will usually check the following:

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

lgtm-com · 2019-09-02T19:59:46Z

This pull request introduces 1 alert when merging 29dc818 into be9b67f - view on LGTM.com

new alerts:

1 for Empty branch of conditional

kodebach · 2019-09-03T14:45:15Z

@markus2330 Once a7 is online again and Jenkins completes all build jobs, this PR can be merged. (At least from my point of view.)

markus2330 · 2019-09-04T08:22:07Z

Thank you!

@bauhaus93: here is the new PR. Please try out the tutorials.

also fix bugs from earlier commits

This reverts commit cda9269.

bauhaus93 · 2019-09-04T12:01:53Z

src/plugins/specload/README.md

-
-  However, because the default `resolver` is not compatible with plugins that produce a KeySet without a file present, you can only use
-  `noresolver`. This means that you cannot set any values. (You will get an error about a non-existent file, if you try.)
+- The plugin must be mounted using `noresolver` or another resolver that always calls the storage plugin. To work arround


Suggested change

- The plugin must be mounted using `noresolver` or another resolver that always calls the storage plugin. To work arround

- The plugin must be mounted using `noresolver` or another resolver that always calls the storage plugin. To work around

kodebach · 2019-09-04T17:15:37Z

jenkins build libelektra please

kodebach · 2019-09-04T19:52:37Z

@markus2330 Any idea why testscr_check_formatting fails in debian-stretch-full-mmap-asan but not in any other job using the same docker image?

sanssecours · 2019-09-05T08:27:58Z

Any idea why testscr_check_formatting fails in debian-stretch-full-mmap-asan but not in any other job using the same docker image?

This seems to be a transient error. As far as I can tell, git diff --quiet does report code differences in line 69 of the code below, while it does not detect a difference in line 72. As to why this happens, I have no idea.

libelektra/tests/shell/check_formatting.sh

Lines 69 to 76 in 4e0d346

    
           git diff --quiet 
        
           succeed_if "$error_message" 
        
           git diff --quiet || { 
        
           	printf '\n\n————————————————————————————————————————————————————————————\n\n' 
        
           	git diff -p 
        
           	printf '\n\n————————————————————————————————————————————————————————————\n\n' 
        
           }

Anyway, I opened pull request #2929, which hopefully gets rid of this error.

kodebach · 2019-09-05T08:33:41Z

But then the same thing would occur on all the other build jobs as well, wouldn't it? (At least the ones using the same docker image)

sanssecours · 2019-09-05T08:55:11Z

But then the same thing would occur on all the other build jobs as well, wouldn't it? (At least the ones using the same docker image)

Unfortunately, since this is a transient error (it only happens in rare cases), I think the answer to this question is no.

Currently one option to get rid of the error is to restart the build. Since this is a rather bad “solution” – especially considering the stability of the Jenkins build servers –, I opened PR #2929. At least the new version of the test should provide more information on what the specific problem is.

Anyway, I am sorry for the false positive. I am pretty I wrote the code that causes these problems.

markus2330 · 2019-09-06T05:07:51Z

@bauhaus93 do you have any troubles reviewing the PR?

kodebach · 2019-09-06T13:02:30Z

We need an update to the debian branch, since I removed kdbconfig.h from the installed files. Should I update it first, or do we want to merge the PR before updating debian?

markus2330 · 2019-09-06T14:02:29Z

You can update the debian branch by directly pushing to it (to remove the one file).

kodebach · 2019-09-06T14:15:32Z

I made the required change to the debian branch.

@markus2330 Is it possible for you (when you are logged in) to restart the Jenkins build from the "Build artifacts" stage? It would be a bit of a waste to repeat all the successful builds, when nothing inside the PR has changed.

sanssecours · 2019-09-06T14:29:43Z

Is it possible for you (when you are logged in) to restart the Jenkins build from the "Build artifacts" stage?

Unfortunately, as far as I know that is not possible.

It would be a bit of a waste to repeat all the successful builds, when nothing inside the PR has changed.

I think so too. I restarted the whole Jenkins build for now.

kodebach · 2019-09-06T14:40:42Z

Unfortunately, as far as I know that is not possible.

That issue is marked as "Resolved" and the release notes, mention that it was release as part of version 1.3. I am not sure which version we are using.

sanssecours · 2019-09-06T14:51:13Z

That issue is marked as "Resolved" and the release notes, mention that it was release as part of version 1.3. I am not sure which version we are using.

As far as I know, we do not use the declarative syntax for pipeline stages, which means we are out of luck according to the issue description:

Out of scope

Automatically stash/unstash the workspace

Scripted pipeline (technical limitation)

.

kodebach · 2019-09-06T22:03:34Z

@markus2330 I suggest we merge this huge PR now, since all build jobs succeeded. If there is anything else that needs to change, we can always create a new PR.

markus2330

Amazing PR.

markus2330 · 2019-09-07T04:25:04Z

cmake/ElektraConfig.cmake.in

@@ -2,32 +2,29 @@
 #
 # Variables for the Elektra library:
 #
-#  Elektra_INCLUDE_DIRS  - where to find kdb.h, etc.
-#  Elektra_LIBRARIES     - List of libraries when using Elektra.
+# Elektra_INCLUDE_DIRS  - where to find kdb.h, etc. Elektra_LIBRARIES     - List of libraries when using Elektra.


This formatting got broken.

@sanssecours any idea how to avoid this?

The easiest option is probably using a fence:

# ~~~

before and after the comment block, just like we did here:

libelektra/cmake/ElektraCache.cmake

Lines 1 to 9 in bc9254c

# ~~~

# CACHE

#

# Here all cache variables are set

#

#

# If you add something here, make sure to also add it in

# src/plugins/constants/

# ~~~

.

markus2330 · 2019-09-07T04:25:39Z

.travis.yml

@@ -82,7 +82,6 @@ matrix:
          -multifile;\
          -network;\
          -ni;\
-          -noresolver;\


Why the change?

It is required for specload and therefore needed for some of the codegen tests.

Then maybe make it mandatory? (src/plugins/CMakeLists.txt)

It is not actually mandatory, since specload isn't mandatory. Also specload can work with other resolvers, it is just that right now noresolver is the only one that works. Other resolvers don't call the storage plugin, if no storage file exists.

markus2330 · 2019-09-07T04:27:50Z

cmake/ElektraConfig.cmake.in

+#                  INPUT <plugin> <file>
+#                  OUTPUT_DIR <output_dir>
+#                  KDB <kdb>
+#                  PARAMS <params>...


Isn't PARAMS optional?

Yes, and OPTIONS, KDB and OUTPUT_DIR too. I'll make that more clear.

markus2330 · 2019-09-07T04:28:02Z

cmake/ElektraConfig.cmake.in

+#
+# This will call `<kdb> gen -F <plugin>=<file> <options>... template parent_key output_name <params>...` in the
+# directory <output_dir>.
+# 


Can you also give an example of how to execute it?

I can, but does it really help that much, if I add e.g.:

elektra_kdb_gen (highlevel "/sw/org/app/#0/current" elektragen ni spec.ini)

That is all you need to do. CMake will then know which files are produced by kdb gen and call it, if one of the files is required by a target.

I think even this example might help. A full usage example might help even more (how to compile the generated files).

Full examples can be found in the examples directory.

Then adding links to the examples directories is all what is needed.

How should I link to the examples? With a Github link? I don't think the external examples show up on the homepage.

The website automatically turns all internal links that are not in the website to github links.

markus2330 · 2019-09-07T04:39:13Z

doc/THIRD-PARTY-LICENSES

@@ -13,3 +13,4 @@ src/tools/website-frontend/resources/assets/skin/base/fonts/font-awesome/font-aw
 src/tools/website-frontend/resources/assets/skin/base/util/space.less: mit
 src/tools/web/client/public/Roboto-Light.ttf: apache-2.0
 src/tools/web/client/public/Roboto-Regular.ttf: apache-2.0
+src/tools/kdb/gen/mustache.hpp: BSL-1.0


Thank you for maintaining! I made it now mandatory in f256c0d

markus2330 · 2019-09-07T04:45:34Z

doc/news/_preparation_next_release.md

+While the new `kdb gen` was already included in the last release, it is now fully functional and ready for use. To get started take a look
+at the new man-page for [`kdb-gen(1)`](https://www.libelektra.org/manpages/kdb-gen). _(Klemens Böswirth)_
+
+If specifically want to use it with the High-Level API take a look at [this tutorial](https://www.libelektra.org/tutorials/high-level-api).


markus2330 · 2019-09-07T04:45:52Z

doc/news/_preparation_next_release.md

+While the new `kdb gen` was already included in the last release, it is now fully functional and ready for use. To get started take a look
+at the new man-page for [`kdb-gen(1)`](https://www.libelektra.org/manpages/kdb-gen). _(Klemens Böswirth)_
+
+If specifically want to use it with the High-Level API take a look at [this tutorial](https://www.libelektra.org/tutorials/high-level-api).



Also write about cmake integration.

markus2330 · 2019-09-07T04:46:07Z

doc/news/_preparation_next_release.md

-### <<HIGHLIGHT1>>
+### Code Generation
+
+While the new `kdb gen` was already included in the last release, it is now fully functional and ready for use. To get started take a look


productive use?

markus2330 · 2019-09-07T04:47:18Z

doc/news/_preparation_next_release.md

@@ -150,7 +157,7 @@ you up to date with the multi-language support provided by Elektra.

 ### CMake

- <<TODO>>
+- `kdbtypes.h` is now generated directly via a CMake `configure_file` call. _(Klemens Böswirth)_


Not really important. Better write more about the highlight.

markus2330 · 2019-09-07T04:48:48Z

doc/tutorials/application-integration.md

@@ -12,10 +12,15 @@ In Elektra different forms of application integrations are possible:
   - [Intercept File System](/src/bindings/intercept/fs/README.md)
 3. Integration where applications directly use Elektra to read and
   store settings.
+   1. Using the low-level API.


Is it possible to call it "i" and "ii" here?

No, markdown only recognises Arabic numbers as list items. GitHub is quite unique in how it renders nested numbered lists. Most other markdown renderers I tried render all levels with Arabic numerals.

I don't know what we use for the website, but I'll change the references in the text below to 3.1 and 3.2. IMO it is easier to understand that 3.1 references 3.i than the other way around.

Yes, I agree. Only Arabic numbers is easier.

kodebach mentioned this pull request Sep 2, 2019

Codegen updates #2805

Closed

14 tasks

kodebach requested a review from markus2330 September 3, 2019 14:42

markus2330 mentioned this pull request Sep 4, 2019

Link checker: deduplicate links #2923

Merged

14 tasks

markus2330 requested a review from bauhaus93 September 4, 2019 08:21

kodebach added 23 commits September 4, 2019 11:32

quickdump: add noparent mode

1fc2430

specload: allow noparent mode in sendSpec

0b659d8

codegen: various small improvements

6b9bb92

codegen: make tag prefix configurable

3415044

codegen: remove static help key

50179d2

highlevel: elektraOpen: take defaults values from metadata

6559dc4

codegen: de-duplicate spec

8823abc

also fix bugs from earlier commits

codegen: add no setter mode

190d3e7

quickdump: fix memleak

35a8b6a

codegen: specLocation and defaultsHandling options

c621403

type: fix major bug

3cdc47b

spec: fix return value for ignored conflicts

98260fa

core: add ensure global remount test

236b269

list: fix bug

a107e9c

highlevel: merge elektraEnsure into new version of elektraOpen

15e53f0

highlevel: fix bug in elektraOpen and update usages

42ec9ca

codegen: update examples READMEs

bc19196

tests: fix kdbEnsure test

be35c54

highlevel: try to fix symbol versioning

19935a7

highlevel: try to get symver working

9e8d643

type: fix broken kdbSet

0f2e2ff

highlevel: document pointer lifetimes

354b186

codegen: improve documentation

087f14e

kodebach added 5 commits September 4, 2019 11:34

pluginprocess: include stdlib.h in test (for EXIT_SUCCESS)

1c7feb8

ease: add tests for conversion functions

02f8d6e

codegen: use better float value in tests

d612e08

codegen: fix possible segfault in example

d1bf6c1

Revert "Documentation: Fix broken link"

1166a9f

This reverts commit cda9269.

kodebach force-pushed the highlevel_codegen branch from ce93c24 to 1166a9f Compare September 4, 2019 09:36

bauhaus93 reviewed Sep 4, 2019

View reviewed changes

doc: fix typos

c84845a

This comment has been minimized.

Sign in to view

kodebach added the ready to merge label Sep 6, 2019

markus2330 approved these changes Sep 7, 2019

View reviewed changes

markus2330 merged commit 79408a3 into ElektraInitiative:master Sep 7, 2019

kodebach deleted the highlevel_codegen branch September 7, 2019 12:09

This was referenced Sep 7, 2019

Minor changes to codegen #2933

Merged

Minor documentation updates #2942

Merged

sanssecours mentioned this pull request Nov 2, 2019

Jenkins: retry problematic stages #3146

Closed

	- The plugin must be mounted using `noresolver` or another resolver that always calls the storage plugin. To work arround
	- The plugin must be mounted using `noresolver` or another resolver that always calls the storage plugin. To work around

	# ~~~
	# CACHE
	#
	# Here all cache variables are set
	#
	#
	# If you add something here, make sure to also add it in
	# src/plugins/constants/
	# ~~~

Codegen updates #2927

Codegen updates #2927

Conversation

kodebach commented Sep 2, 2019 • edited

Basics

Checklist

Review

lgtm-com bot commented Sep 2, 2019

kodebach commented Sep 3, 2019

markus2330 commented Sep 4, 2019

Choose a reason for hiding this comment

kodebach commented Sep 4, 2019

kodebach commented Sep 4, 2019

sanssecours commented Sep 5, 2019

kodebach commented Sep 5, 2019

sanssecours commented Sep 5, 2019

This comment has been minimized.

markus2330 commented Sep 6, 2019

kodebach commented Sep 6, 2019

markus2330 commented Sep 6, 2019

kodebach commented Sep 6, 2019

sanssecours commented Sep 6, 2019

kodebach commented Sep 6, 2019

sanssecours commented Sep 6, 2019 • edited

Out of scope

kodebach commented Sep 6, 2019

markus2330 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodebach Sep 7, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodebach commented Sep 2, 2019 •

edited

sanssecours commented Sep 6, 2019 •

edited

kodebach Sep 7, 2019 •

edited