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

FRRouting + libyang build #2205

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

vjardin
Copy link
Contributor

@vjardin vjardin commented Mar 20, 2024

It can build using a set of well known verions of libyang, for some well known tags.

The purpose is to ensure that libyang and FRRouting keeps working together smoothly.

Currently, it is just about a compilation and link test.

In order to work with pre-set tags/versions, the fork should include the git tags. If they are missing, you can catch up those tags using: $ git remote -v
origin git@github.com:vjardin/libyang.git (fetch)
origin git@github.com:vjardin/libyang.git (push)
upstream git@github.com:CESNET/libyang.git (fetch) upstream git@github.com:CESNET/libyang.git (push)
$ git fetch --tags upstream
$ git tag -l
$ git push --tags origin

Run it once a day.

@vjardin vjardin force-pushed the frr_checks_scheduled branch from c585200 to f544497 Compare March 20, 2024 12:51
@vjardin
Copy link
Contributor Author

vjardin commented Mar 20, 2024

link with: #2203

@michalvasko
Copy link
Member

I am sorry but I still do not understand the purpose of testing a specific libyang version against any FRR version. The specific version code will never change so if it works once, it will always work. If FRR backports some fixes to older versions or they simply change for any reason, FRR should have an action that will check the compatibility with older libyang versions. Because even if this action fails at some point in the future, what can we do about it? It would be only because FRR changed.

@vjardin
Copy link
Contributor Author

vjardin commented Mar 20, 2024

Right, sorry, I forgot to drop "master" in order to have only "well known versions". This array for libyang and FRR can evolve over the time.

@michalvasko
Copy link
Member

Actually, I was referring to libyang v2.1.128 as the specific version, which will never change. The only reason why you would want to keep testing, for example, FRR 8.5.4 with libyang 2.1.128 is if either of them will change in the future. Like I said, that is impossible for libyang but if that happens for FRR, it is FRR that needs to check its compatibility with the specific libyang version so I see no reason why it should run in libyang CI, FRR CI is the better place for it.

So, I am not sure why we have trouble understanding each other but I do not think this PR should be merged.

@vjardin
Copy link
Contributor Author

vjardin commented Mar 23, 2024

note: fixup FRRouting/libyang3 compilation:
FRRouting/frr#15608

@michalvasko : I'd be interested to get your review on this FRR patch about supporting the migration from libyang2 to libyang3. big THANKS.

@michalvasko
Copy link
Member

I have taken a look and it seems fine, not that many changes. But I see that the biggest changes have not affected you and the ones I thought would not, did, too bad.

@vjardin
Copy link
Contributor Author

vjardin commented Mar 27, 2024

I have taken a look and it seems fine, not that many changes. But I see that the biggest changes have not affected you and the ones I thought would not, did, too bad.

Any chance you adjust libyang3 before it'll be released in order to optimise it ?

michalvasko added a commit that referenced this pull request Mar 28, 2024
@michalvasko
Copy link
Member

I have added some simple macros for better backwards compatibility.

@vjardin
Copy link
Contributor Author

vjardin commented Apr 9, 2024

I have added some simple macros for better backwards compatibility.

thanks @michalvasko : it is being used by the latest updates from FRRouting/frr#15608 ; your comments on this pull request 15608 would be welcomed @michalvasko please ?

cc: @idryzhov

@michalvasko
Copy link
Member

My feedback is the same as it was before, I am still confused by testing specific libyang versions against specific frr versions, seems like a wasted CI time. The only thing that makes sense is testing libyang devel against whatever frr versions you want to support (and should be in libyang CI) and, in case frr has a similar development branch that is updated often with latest changes, test that branch against whatever libyang versions you want to be compatible with (and should be in frr CI).

michalvasko and others added 6 commits April 12, 2024 11:10
It can build using a set of well known verions of libyang,
for some well known tags.

The purpose is to ensure that libyang and FRRouting keeps working
together smoothly.

Currently, it is just about a compilation and link test.

In order to work with pre-set tags/versions, the fork should include the
git tags. If they are missing, you can catch up those tags using:
$ git remote -v
origin	git@github.com:vjardin/libyang.git (fetch)
origin	git@github.com:vjardin/libyang.git (push)
upstream git@github.com:CESNET/libyang.git (fetch)
upstream git@github.com:CESNET/libyang.git (push)
$ git fetch --tags upstream
$ git tag -l
$ git push --tags origin
While libyang is bringing many evolutions, let's avoid running it.
The versions can be master/devel or the tag'd ones.
Add 1 basic test for libyang checks
Apply the guidelines from https://docs.frrouting.org/projects/dev-guide/en/latest/topotests.html
@vjardin vjardin force-pushed the frr_checks_scheduled branch from 86c3259 to 2c571e7 Compare April 13, 2024 13:59
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.

2 participants