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

Fix LYD_NEW_PATH_OUTPUT issue to support libyang v3.x #16511

Merged
merged 1 commit into from
Aug 10, 2024

Conversation

oplklum
Copy link
Contributor

@oplklum oplklum commented Aug 1, 2024

There will be a LYD_NEW_PATH_OUTPUT undeclared error if using the latest libyang V3.x version. It was reported in #16287 before.
I fixed the error to support that. And it's compatible with old version.

lib/yang.c: In function ‘yang_dnode_rpc_output_add’:
lib/yang.c:674:7: error: ‘LYD_NEW_PATH_OUTPUT’ undeclared (first use in this function); did you mean ‘LYD_NEW_VAL_OUTPUT’?
674 | LYD_NEW_PATH_OUTPUT | LYD_NEW_PATH_UPDATE, NULL);
| ^~~~~~~~~~~~~~~~~~~
| LYD_NEW_VAL_OUTPUT
lib/yang.c:674:7: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:10793: lib/yang.lo] Error 1
make[1]: Leaving directory '/opt/frr'
make: *** [Makefile:6576: all] Error 2

@donaldsharp
Copy link
Member

Please let's follow our style conventions for commit messages. Read doc/developer/workflow.rst for the how to.

@oplklum
Copy link
Contributor Author

oplklum commented Aug 1, 2024

Please let's follow our style conventions for commit messages. Read doc/developer/workflow.rst for the how to.

Thanks for your reminding👌

@choppsv1
Copy link
Contributor

choppsv1 commented Aug 5, 2024

let's use #16514 instead it keeps the libyang3 conditionals together and also deals with the change not being correct (i guess early libyang v3 didn't have/require this change?)

@riw777
Copy link
Member

riw777 commented Aug 6, 2024

Aug 4, 2024

does this need to be closed?

@choppsv1
Copy link
Contributor

choppsv1 commented Aug 6, 2024

@oplklum we could also use your PR if you'd like to update your change to match #16514. I hadn't seen your fix when I submitted mine.

@vjardin
Copy link
Contributor

vjardin commented Aug 6, 2024

does this need to be closed?

Let's give an opportunity to @oplklum to have a contribution on FRR, but this code needs to be updated as it was done by @choppsv1 .

@oplklum
Copy link
Contributor Author

oplklum commented Aug 10, 2024

does this need to be closed?

Let's give an opportunity to @oplklum to have a contribution on FRR, but this code needs to be updated as it was done by @choppsv1 .

Thanks very much to give me the opportunity to contribute FRR, @choppsv1 @vjardin. I feel so happy and respected😁😁😁. Thank you all, great team.

I change the code following @choppsv1's way and I add the define in condition (LY_VERSION_MAJOR < 3). According to the below comment, I think the code can still be compiled fine if we remove the old compatible code in future.

/* Safe to remove after libyang 2.2.8 */
#if (LY_VERSION_MAJOR < 3)

@oplklum oplklum closed this Aug 10, 2024
@oplklum oplklum reopened this Aug 10, 2024
@oplklum oplklum force-pushed the master branch 2 times, most recently from 533aa13 to 763dd64 Compare August 10, 2024 00:31
Fix the LYD_NEW_PATH_OUTPUT undeclared error to support the latest libyang v3.x version,
and also compatible with old version.

Signed-off-by: Lu Mao <[email protected]>
@choppsv1 choppsv1 merged commit 64ac03c into FRRouting:master Aug 10, 2024
9 checks passed
@choppsv1
Copy link
Contributor

choppsv1 commented Sep 9, 2024

https://github.com/Mergifyio backport stable/10.1

Copy link

mergify bot commented Sep 9, 2024

backport stable/10.1

✅ Backports have been created

ton31337 added a commit that referenced this pull request Sep 10, 2024
Fix LYD_NEW_PATH_OUTPUT issue to support libyang v3.x (backport #16511)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants