lopper: correctly label compact (or sparse) nodes in --enhanced mode #622
Conversation
| lcount = 0 | ||
| pattern2 = re.compile( | ||
| r'^\s*?\w*?\s*?\:', re.DOTALL | ||
| ) |
There was a problem hiding this comment.
Although not directly used, this commit shouldn't touch parts of the code not part of the fix.
There was a problem hiding this comment.
Understood - if I revise the PR I will remove this part of the diff.
There was a problem hiding this comment.
(I've done this now - please feel free to "Resolve conversation")
| ) | ||
| pattern = re.compile( | ||
| r'^\s*?(\w*?)\s*?\:(.*?)$', re.DOTALL | re.MULTILINE | ||
| r'^\s*?(\w*?)\s*?\:([^{]*?)\s*\{', re.DOTALL | re.MULTILINE |
There was a problem hiding this comment.
I'm not a fan of reviewing regex changes (And I'm on holiday right now), so bear with me on this :) I also can't run my integration tests until next week, so I can't comment on side effects at the moment either.
The previous expression was intended to simply match to the end of the line after a label was intended (although with the re.MULTILINE, that is a bit nebulous in my memory right now), the new one does trigger on on the node opening so yes, it should place the enhanced property to maintain the label properly (there are checks for comments that are maintained the same way and they are thrown out, but it was never needed for labels before)
So the summary is that it looks good on inspection, but I will run some extra tests to be sure.
This code is all due to the changed with the recent schema work, but will be maintained as an alternate path for dtb support.
There was a problem hiding this comment.
Great - please enjoy your vacation and come back refreshed. Stop reading here. :)
Ultimately, this PR is intended to fix a failure in my SDT flow with a "compact" device tree. It took some time to trace the issue into Lopper, and I'm hoping to save the next SDT adoptee the same hassle. If the issue gets resolved some other way (schema work?) and this PR gets dropped, I'm still happy.
Once you're back, please let me know if you need anything else.
There was a problem hiding this comment.
I'm back. Sorry for the delay. The minimal footprint change is fine with me.
I've been spending more time with the schema as it is the way forward to make a whole set of previously "clunky" things in lopper be more robust and then add some new features.
But there's no reason to not do these fixes, since it will still be some time before I get those efforts complete and it's a reminder for me to ensure I keep that use case working.
There was a problem hiding this comment.
Understood. With the updated commit (a6568d9), the unrelated dead code has been restored and I think this PR is ready to go.
gsmecher
force-pushed
the
fix_compact_labels
branch
from
d6a7a23 to
a6568d9
Compare
Before this commit, the following device tree
/dts-v1/;
/ {
compact_label: compact_thing@0 { compatible = "baz"; };
less_compact_label: compact_thing@1 {
compatible = "baz"; };
normal_label: compact_thing@2 {
compatible = "baz";
};
sparse_label: compact_thing@3
{
compatible = "baz";
};
};
...resulted in mangled device-tree code (and hence a failing dtc
invocation). By matching the "{" inside the regular expression, we no
longer rely on whitespace as a marker for structure.
Signed-off-by: Graeme Smecher <[email protected]>
gsmecher
force-pushed
the
fix_compact_labels
branch
from
a6568d9 to
15a9c81
Compare
2 participants
Before this commit, the following device tree
...resulted in mangled device-tree code (and hence a failing dtc invocation). By matching the "{" inside the regular expression, we no longer rely on whitespace as a marker for structure.
Fixes #621.