-
Notifications
You must be signed in to change notification settings - Fork 295
sonic-swss-common: port to libyang3 #973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
bradh352
wants to merge
2
commits into
sonic-net:master
Choose a base branch
from
bradh352:bradh352/libyang3
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use typedef to avoid these changes?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its not clear to me what you are asking. Libyang1 and libyang3 are not API compatible, lys_node no longer exists. There were significant changes between libyang1 and libyang2, then again between libyang2 and libyang3. SONiC is way behind the curve here.
Are you asking for me to do something to try to make this compile against both libyang1 and libyang3 rather than just a straight port to a new version? If so, what would be the advantage and why would it be worth the effort? There are over a dozen linked PRs to upgrading to libyang3 at this point and all will need to be merged en masse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean if it's possible to use typedef/macro to encapsulate the data type and function in swss-common, which may avoid such scattered modification, and not sure this gonna be changed again if later upgrade to another version. Thanks for your effort for these PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being on the outside from libyang development, I can't really predict what changes they may make in the future that are not backwards compatible. But I do know that libyang1 -> libyang2 was supposedly a complete rewrite to differentiate uncompiled vs compiled schema trees hence
lys_
was split intolysp_
andlysc_
data types. libyang2 -> libyang3 was a less substantial change. I don't think trying to abstract the changes here would help, it would probably make it harder down the road.They do maintain transition manuals here:
https://netopeer.liberouter.org/doc/libyang/master/html/transition1_2.html
https://netopeer.liberouter.org/doc/libyang/master/html/transition2_3.html
It "feels" like their API is stabilizing, its just SONiC started with a really early version so there are lots of changes needed.