Skip to content

printer sid BUGFIX: include nodes augmented into other modules - #2560

Closed
manoe wants to merge 113 commits into
CESNET:masterfrom
nokia:fix/sid-augment-coverage
Closed

printer sid BUGFIX: include nodes augmented into other modules#2560
manoe wants to merge 113 commits into
CESNET:masterfrom
nokia:fix/sid-augment-coverage

Conversation

@manoe

@manoe manoe commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

printer sid BUGFIX: include nodes augmented into other modules

Summary

The RFC 9595 SID generator does not assign SIDs to data nodes that a module
contributes to other modules via augment. For any module that places its
data by augmenting another module, lys_sid_gen()/lys_sid_update() (and
yanglint -g) produce a .sid file containing only the module item and no
data items
. This PR makes the generator collect those augmented-in nodes.

Problem

sid_collect_items() gathered data-namespace items only from the processed
module's own compiled tree:

LY_CHECK_RET((rc = lysc_module_dfs_full(module, collect_data_cb, callback_data)), rc);

Nodes that module augments into another module live in the target module's
compiled tree (their node->module still points back to the augmenting
module), so lysc_module_dfs_full(module, ...) never visits them. As a result
they receive no SID.

Minimal example — a module whose only data is an augment into another module:

module a1 {
  import b1 { prefix b1; }
  augment "/b1:cont" {
    leaf x { type string; }
  }
}

Before this PR, a1's generated .sid contains a single item (module a1) and
0 data items; the augmented-in /b1:cont/a1:x is missing.

Fix

Collect all nodes defined by the processed module, wherever they are grafted
in the compiled schema:

  • collect_data_cb() now attributes each node to its defining module and skips
    nodes whose node->module differs from the module being processed. This both
    (a) lets us safely traverse other modules' trees and pick only the
    augmented-in nodes, and (b) fixes a latent mis-attribution where a module's own
    .sid could wrongly include nodes augmented into it by others.
  • After walking the module's own tree, sid\_collect\_items() also walks every
    context module whose augmented\_by references the processed module (guarded on
    ->compiled), using the same callback.

libyang already implements augment target modules when the augmenting module is
implemented (lys_precompile_augments_deviations()), so the target trees are
present during generation. The existing choice/case handling and top-level
extension-data (yang-data/structure) traversal are unchanged.

Backward compatibility

Output is unchanged for modules that do not augment other modules. The only
behavioral change is correct attribution: augmented-in nodes now appear in the
augmenting module's .sid, and are no longer (incorrectly) included in the
target module's .sid.

Notes

  • Deviation-introduced nodes (deviated_by) are a separate RFC 9595 concern and
    are intentionally out of scope here.

michalvasko and others added 30 commits June 25, 2026 09:19
... with quoted values with both ' and "
characters.
Required by proper schema-mount data parsing.

Fixes sysrepo/sysrepo#3772
michalvasko and others added 29 commits August 7, 2026 14:34
Add instructions for installing on Mac OS using homebrew
The CBOR printer kept a single "currently open array" pointer
(cborpr_ctx.array) shared across all nesting levels, while tracking the
open nodes on a proper stack (cborpr_ctx.open). When a leaf-list or list
was nested inside a list, printing the inner array overwrote that pointer
and then reset it to NULL, so the enclosing list subsequently pushed into
a NULL array, causing a segfault in cbor_array_push().

Replace the single array pointer with a stack (cborpr_ctx.arrays) kept
parallel to the open-node stack, add cbor_current_array() to fetch the
innermost open array, and update cbor_print_leaf_list()/cbor_print_opaq()
and their callers accordingly. The stack is freed on all exit paths of
cbor_print_data().

Add a regression test (test_nested_list) covering a leaf-list nested in a
list, a list nested in a list, and mixed multi-instance nesting, plus the
supporting list-nested node in the cbor-test module.

Co-authored-by: Cursor <cursoragent@cursor.com>
The SID generator collected data-namespace items only from the processed
module's own compiled tree (lysc_module_dfs_full(module, ...)). Nodes that
the module contributes to other modules by augmentation live in the target
modules' trees, so they were never collected and got no SID. For models
that place feature data via augments (e.g. augmenting a common root module),
the generated .sid file ended up with just the module item and no data
items.

Collect all nodes defined by the processed module wherever they are grafted:
after walking the module's own tree, also walk every context module whose
augmented_by references the processed module. collect_data_cb now attributes
each node to its defining module (node->module), so a foreign module's own
nodes are skipped while traversing its tree, and a module's .sid no longer
wrongly includes nodes augmented into it by others.

Add test_augment covering both directions (the augmenting module's .sid
contains the augmented-in node; the base module's .sid does not).

Co-authored-by: Cursor <cursoragent@cursor.com>
@manoe manoe closed this Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.