mbj4668 / pyang

An extensible YANG validator and converter in python
ISC License
535 stars 344 forks source link

--feature doesn't work as expected with augmented nodes? #780

Open wlupton opened 2 years ago

wlupton commented 2 years ago

With the YANG shown below, the current pyang master behaves as follows (disabling all features prunes foo from the local augment but not from the /if:interfaces/if:interface augment):

% pyang ../bugs/feature-bug.yang -f tree
module: feature-bug
  +--rw root
     +--rw foo?   string {has-leaf-foo}?

  augment /if:interfaces/if:interface:
    +--rw foo?   string {has-leaf-foo}?
% pyang ../bugs/feature-bug.yang -f tree -F feature-bug:
module: feature-bug
  +--rw root

  augment /if:interfaces/if:interface:
    +--rw foo?   string {has-leaf-foo}?

I don't think this is the expected behaviour? FWIW yanger does prune /if:interfaces/if:interface/foo.

This change to Statement.prune() appears to fix the problem. Is there indeed a problem and (if so) is this an appropriate fix? If so, I can create a PR.

% git diff
diff --git a/pyang/statements.py b/pyang/statements.py
index e6f94b7..14e1542 100644
--- a/pyang/statements.py
+++ b/pyang/statements.py
@@ -3145,6 +3145,8 @@ class ModSubmodStatement(Statement):
                     for d in deletes:
                         idx = n.i_children.index(d)
                         del n.i_children[idx]
+            for a in n.search('augment'):
+                p(a)
         p(self)

 class AugmentStatement(Statement):

YANG

module feature-bug {
  yang-version 1.1;
  namespace "urn:feature-bug";
  prefix feature-bug;

  import ietf-interfaces {
    prefix if;
  }

  feature has-leaf-foo;

  grouping common {
    leaf foo {
      if-feature has-leaf-foo;
      type string;
    }
  }

  container root {
  }

  augment /root {
    uses common;
  }

  augment /if:interfaces/if:interface {
    uses common;
  }
}
wlupton commented 2 years ago

@mbj4668, any thoughts on this? Thanks

mbj4668 commented 2 years ago

Yes this looks like a bug. I agree it should be pruned in both cases, so please open a PR with the fix (and the test)!

wlupton commented 2 years ago

Thanks. Will do.

wlupton commented 2 years ago

Oh sorry, PR #790 doesn't include the test. I'll add it... done.