highlevel: detect if spec properly mounted and unmodified #4047
Conversation
|
Mentioning @markus2330 as discussed. |
… "kdb spec-mount" and improve error message
…ounted() to better reflect its purpose
See inline comments for the rationale of this algorithm.
qwepoizt
force-pushed
the
3998-detect-spec-mounted
branch
from
d3ccd5b
to
ba142c2
Compare
…the caller can still use their ks afterwards.
|
@markus2330: I'm very happy to report, that - from my point of view - this is now ready to merge! I've fixed the problems with arrays and some others reported by the build servers. The 4 unsuccessful checks are caused by:
|
qwepoizt
changed the title
qwepoizt
changed the title
…pace, not all namespaces.
|
@qwepoizt In libelektra/tests/shell/gen/highlevel/commands.expected.c Lines 42 to 61 in 28bbf3e So for your solution, you need to remove the parentKey prefix as well as the namespace. However, I think there is much easier solution. You can simply duplicate the KeySet during |
…es not ignore key namespaces for sha256 calculation
…ace is not ignored anymore in calculateSpecificationToken()
|
@markus2330 @kodebach Thank you for your hints! I've fixed this and am waiting for the checks to run through. |
src/libs/ease/hash.c
Outdated
| /** | ||
| * Ignore array parents for token calculation. | ||
| * Rationale: There is a bug in the spec plugin that is triggered on changing the size of an array. | ||
| * It leads to array parents vanishing from the spec namespace and thus a different token. | ||
| * See https://github.com/ElektraInitiative/libelektra/issues/4061 | ||
| */ | ||
| size_t baseNameSize = keyGetBaseNameSize (currentKey); | ||
| char * kBaseName = elektraMalloc (baseNameSize * sizeof (char)); | ||
| keyGetBaseName (currentKey, kBaseName, baseNameSize); | ||
| if (strcmp (kBaseName, "#") == 0) | ||
| { | ||
| continue; | ||
| } | ||
| elektraFree (kBaseName); |
There was a problem hiding this comment.
Shouldn't this much simpler snippet work here too:
if (strcmp (keyBaseName (currentKey), "#") == 0)
{
continue;
}There was a problem hiding this comment.
Well spotted, thanks! I wasn't aware this function existed. I've replaced it accordingly!
| * For token calculation, only keys from the spec namespace are relevant. | ||
| * Thus, the parentKeyForTokenCalculation must be in spec namespace. | ||
| */ | ||
| Key parentKeyForTokenCalculation (parentKeyName, KEY_END); |
There was a problem hiding this comment.
I think you can avoid all of this parentKeyForTokenCalculation stuff, by simply requiring that parentKeyName starts with spec:/. We already check in highlevel.cpp that it starts with spec:/ or / (and then generate the other variant). Theoretically other templates might not have this requirement, but for now kdb gen only makes sense for a spec:/ parent, so we might as well enforce this properly.
There was a problem hiding this comment.
Thanks for your suggestion! Unfortunately, I don't have time to implement and test this change.
I propose to merge this PR and make any further changes in a new one!
There was a problem hiding this comment.
Yeah, merging as is is totally fine for me.
Fixes #3998
Basics
These points need to be fulfilled for every PR:
(added as entry in
doc/news/_preparation_next_release.mdwhichcontains
_(my name)_)Please always add something to the release notes.
(first line should have
module: short statementsyntax)close #X, are in the commit messages.doc/news/_preparation_next_release.mdscripts/dev/reformat-allIf 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.
(not in the PR description)
Review
Reviewers will usually check the following:
Labels
If you are already Elektra developer:
say that everything is ready to be merged.