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

Fix handling of array parents in spec plugin #4061

Closed
qwepoizt opened this issue Sep 21, 2021 · 9 comments · Fixed by #4918
Closed

Fix handling of array parents in spec plugin #4061

qwepoizt opened this issue Sep 21, 2021 · 9 comments · Fixed by #4918
Assignees
Labels

Comments

@qwepoizt
Copy link
Contributor

The spec plugin changes which keys of an array it gets/sets, when a previously empty array becomes non-empty.

Expected behavior

  1. cd into .../libelektra/examples/codegen/econf/
  2. kdb mount `pwd`/spec.ini "spec:/sw/example/econf/#0/current" ni
  3. kdb spec-mount "/sw/example/econf/#0/current"
  4. kdb ls "spec:/sw/example/econf/#0" yields:
spec:/sw/example/econf/#0/current
spec:/sw/example/econf/#0/current/format/#
spec:/sw/example/econf/#0/current/format/#/charset
spec:/sw/example/econf/#0/current/format/#/eofnewline
spec:/sw/example/econf/#0/current/format/#/eol
spec:/sw/example/econf/#0/current/format/#/indent/size
spec:/sw/example/econf/#0/current/format/#/indent/style
spec:/sw/example/econf/#0/current/format/#/linelength
spec:/sw/example/econf/#0/current/format/#/pattern
spec:/sw/example/econf/#0/current/format/#/tabwidth
spec:/sw/example/econf/#0/current/format/#/trim
spec:/sw/example/econf/#0/current/root
  1. kdb meta-set "user:/sw/example/econf/#0/current/format" array "#2"
  2. kdb ls "spec:/sw/example/econf/#0" yields:
spec:/sw/example/econf/#0/current
spec:/sw/example/econf/#0/current/format/#
spec:/sw/example/econf/#0/current/format/#/charset
spec:/sw/example/econf/#0/current/format/#/eofnewline
spec:/sw/example/econf/#0/current/format/#/eol
spec:/sw/example/econf/#0/current/format/#/indent/size
spec:/sw/example/econf/#0/current/format/#/indent/style
spec:/sw/example/econf/#0/current/format/#/linelength
spec:/sw/example/econf/#0/current/format/#/pattern
spec:/sw/example/econf/#0/current/format/#/tabwidth
spec:/sw/example/econf/#0/current/format/#/trim
spec:/sw/example/econf/#0/current/root

Actual behavior:

Steps 1. to 5. like expected
6. Yields

spec:/sw/example/econf/#0/current
spec:/sw/example/econf/#0/current/format/#/charset
spec:/sw/example/econf/#0/current/format/#/eofnewline
spec:/sw/example/econf/#0/current/format/#/eol
spec:/sw/example/econf/#0/current/format/#/indent/size
spec:/sw/example/econf/#0/current/format/#/indent/style
spec:/sw/example/econf/#0/current/format/#/linelength
spec:/sw/example/econf/#0/current/format/#/pattern
spec:/sw/example/econf/#0/current/format/#/tabwidth
spec:/sw/example/econf/#0/current/format/#/trim
spec:/sw/example/econf/#0/current/root

In the actual behavior, spec:/sw/example/econf/#0/current/format/# is missing!

Fix ideas

  • If I replace the if condition here with 1 == 1, the spec:/sw/example/econf/#0/current/format/# is not missing anymore.

if (keyGetMeta (specKey, "internal/spec/array") == NULL && keyGetMeta (specKey, "internal/spec/remove") == NULL)
{
ksAppendKey (returned, specKey);
}

  • But, now spec will also add each array child to the spec namespace:
kdb ls "spec:/sw/example/econf/#0"
spec:/sw/example/econf/#0/current
spec:/sw/example/econf/#0/current/format/#
spec:/sw/example/econf/#0/current/format/#/charset
spec:/sw/example/econf/#0/current/format/#/eofnewline
spec:/sw/example/econf/#0/current/format/#/eol
spec:/sw/example/econf/#0/current/format/#/indent/size
spec:/sw/example/econf/#0/current/format/#/indent/style
spec:/sw/example/econf/#0/current/format/#/linelength
spec:/sw/example/econf/#0/current/format/#/pattern
spec:/sw/example/econf/#0/current/format/#/tabwidth
spec:/sw/example/econf/#0/current/format/#/trim
spec:/sw/example/econf/#0/current/format/#0
spec:/sw/example/econf/#0/current/format/#0/charset
spec:/sw/example/econf/#0/current/format/#0/eofnewline
spec:/sw/example/econf/#0/current/format/#0/eol
spec:/sw/example/econf/#0/current/format/#0/indent/size
spec:/sw/example/econf/#0/current/format/#0/indent/style
spec:/sw/example/econf/#0/current/format/#0/linelength
spec:/sw/example/econf/#0/current/format/#0/pattern
spec:/sw/example/econf/#0/current/format/#0/tabwidth
spec:/sw/example/econf/#0/current/format/#0/trim
spec:/sw/example/econf/#0/current/format/#1
spec:/sw/example/econf/#0/current/format/#1/charset
spec:/sw/example/econf/#0/current/format/#1/eofnewline
spec:/sw/example/econf/#0/current/format/#1/eol
spec:/sw/example/econf/#0/current/format/#1/indent/size
spec:/sw/example/econf/#0/current/format/#1/indent/style
spec:/sw/example/econf/#0/current/format/#1/linelength
spec:/sw/example/econf/#0/current/format/#1/pattern
spec:/sw/example/econf/#0/current/format/#1/tabwidth
spec:/sw/example/econf/#0/current/format/#1/trim
spec:/sw/example/econf/#0/current/format/#2
spec:/sw/example/econf/#0/current/format/#2/charset
spec:/sw/example/econf/#0/current/format/#2/eofnewline
spec:/sw/example/econf/#0/current/format/#2/eol
spec:/sw/example/econf/#0/current/format/#2/indent/size
spec:/sw/example/econf/#0/current/format/#2/indent/style
spec:/sw/example/econf/#0/current/format/#2/linelength
spec:/sw/example/econf/#0/current/format/#2/pattern
spec:/sw/example/econf/#0/current/format/#2/tabwidth
spec:/sw/example/econf/#0/current/format/#2/trim
spec:/sw/example/econf/#0/current/root
@qwepoizt
Copy link
Contributor Author

This effectively means that the spec changes when an initially empty array is later non-empty.

The token will then not match anymore.

@qwepoizt
Copy link
Contributor Author

Interestingly, making the array empty again via kdb meta-rm "user:/sw/example/econf/#0/current/format" "array", does not make kdb ls include spec:/sw/example/econf/#0/current/format/# again.

Therefore:

  1. Turn empty array into non-empty array
  2. Turn non-empty array back into empty array

is not idempotent.
(Edit: "idempotent" is probably not the correct term here, I hope one can still get the idea.)

@qwepoizt
Copy link
Contributor Author

kodebach wrote:


If I replace the if condition here with 1 == 1, the spec:/sw/example/econf/#0/current/format/# is not missing anymore.

You're on the right track here, but 1 == 1 is not the correct condition. You need to look at where internal/spec/array and internal/spec/remove are added. Then you need to remove the line that marks the # array spec keys (like spec:/sw/example/econf/#0/current/format/#), or add an additional marker to ignore them later on.

Interestingly, making the array empty again via kdb meta-rm "user:/sw/example/econf/#0/current/format" "array", does not make kdb ls include spec:/sw/example/econf/#0/current/format/# again.

I assume this is related to the long standing bug that spec doesn't correctly remove the metadata it copied during kdbSet(). In other words, some or all of the copied metadata is persisted into the config files of other namespaces. This then influences the spec plugin in future kdbGet() calls. This bug will hopefully be fixed after #3693 is done.

@qwepoizt
Copy link
Contributor Author

@kodebach Thanks for your helpful input!

@markus2330 I tried to fix this all day, but I can't come up with a working solution.
However, I have a working workaround for #4047: Ignore array parent items (i.e. .../format/#) during specification token calculation.
I believe this is sensible as:

  1. redshift does not use arrays
  2. this bug is hard to fix and the time is probably better spent on other issues, especially Backend Plugin #3693.

What do you think?

@markus2330
Copy link
Contributor

Yes, better do some final touches on the HL API and the other issues you already work on. spec needs a reimplementation, the problem you found is unlikely to be the only one.

Thank you for the long bug description, let us leave this issue open for insights of how to redesign the spec plugin.

@markus2330 markus2330 added the bug label Sep 22, 2021
@stale
Copy link

stale bot commented Sep 28, 2022

I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot added the stale label Sep 28, 2022
@kodebach kodebach removed the stale label Sep 28, 2022
@kodebach
Copy link
Member

@atmaxinger maybe you can have a quick look at this (don't waste too much time) in #4495

@markus2330
Copy link
Contributor

@tmakar please write a test case to verify that this problem is gone.

@tmakar tmakar linked a pull request May 2, 2023 that will close this issue
19 tasks
tmakar added a commit to tmakar/libelektra that referenced this issue May 2, 2023
tmakar added a commit to tmakar/libelektra that referenced this issue May 3, 2023
@tmakar
Copy link
Contributor

tmakar commented May 8, 2023

@markus2330 PR exists in #4918

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants