Skip to content
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

sonic-swss-common: port to libyang3 #973

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bradh352
Copy link

Port from libyang1 to libyang3. This depends on changes to sonic-buildimage.

Main Tracking PR here: sonic-net/sonic-buildimage#21679

Signed-off-by: Brad House (@bradh352)

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -86,17 +86,17 @@ struct TableInfoMultipleList : public TableInfoBase
class DefaultValueHelper
{
public:
static int BuildTableDefaultValueMapping(struct lys_node* table, TableDefaultValueMapping& tableDefaultValueMapping);
static int BuildTableDefaultValueMapping(const struct lysc_node* table, TableDefaultValueMapping& tableDefaultValueMapping);
Copy link
Contributor

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?

Copy link
Author

@bradh352 bradh352 Feb 17, 2025

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

Copy link
Contributor

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.

Copy link
Author

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 into lysp_ and lysc_ 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.

bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Feb 17, 2025
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Feb 25, 2025
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Feb 25, 2025
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Mar 1, 2025
Port from libyang1 to libyang3.  This depends on changes to
sonic-buildimage.

Signed-off-by: Brad House (@bradh352)
@bradh352 bradh352 force-pushed the bradh352/libyang3 branch from b08a922 to 42f329d Compare March 1, 2025 19:57
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Mar 3, 2025
bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Mar 6, 2025
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.

3 participants