Skip to content

Bslawomir330 patch 1#1044

Closed
bslawomir330 wants to merge 1 commit into
openembedded:scarthgapfrom
bslawomir330:bslawomir330-patch-1
Closed

Bslawomir330 patch 1#1044
bslawomir330 wants to merge 1 commit into
openembedded:scarthgapfrom
bslawomir330:bslawomir330-patch-1

Conversation

@bslawomir330
Copy link
Copy Markdown

@bslawomir330 bslawomir330 commented Apr 7, 2026

FRR frr-reload.py path update to /usr/libexec/frr/

Summary

This pull request updates the FRR (FRRouting) recipe to use the correct path for frr-reload.py in /usr/libexec/frr/ instead of /usr/lib/frr/.

Changes

  • Added new patch file: 0002-tools-update-frr-reload.py-path-to-usr-libexec-frr.patch
    • Updates tools/frr-reload script to look for frr-reload.py in /usr/libexec/frr/
    • Ensures proper execution path for the FRR reload utility
  • Updated frr_9.1.3.bb recipe to include the new patch in the build process

Details

  • Recipe: meta-networking/recipes-protocols/frr/frr_9.1.3.bb
  • FRR Version: 9.1.3
  • Branch: stable/9.1

Testing

Please verify that:

  • The FRR package builds successfully with this patch
  • The frr-reload.py script is correctly located and executable in /usr/libexec/frr/
  • Existing functionality is not affected

@OldManYellsAtCloud
Copy link
Copy Markdown
Contributor

OldManYellsAtCloud commented Apr 7, 2026 •
edited
Loading

The FRR package builds successfully with this patch

I am not confident that this was really tested - looking at the change, I don't think that this can end in anything else but a fatal QA error. (Edit: I missed that it's scarthgap, which shouldn't end with error, but there should be a warning about the same issue - regardless, please fix)

After fixing that, could you please squash these commits, add a descriptive commit message about the issue this is solving, and prefix the commit subject with the recipe name?

As a somewhat related question, if this is about the location of files, is it really a bug in the frr-reload script, and not a problem with the recipe that installs it to an incorrect location? (The patch essentially might be correct as it is, I'm just looking for some clarity)

@bslawomir330
Copy link
Copy Markdown
Author

bslawomir330 commented Apr 7, 2026

I do not understand the claim regarding a fatal QA error.

The location of executable files in /usr/libexec/frr is appropriate; after all, it concerns scripts, not libraries.

This is not a bug in the frr-reload.sh script, but rather a misalignment with the Autotools configuration process.
I believe it is a waste of time to address this at the source of the problem.
Patching is intended, among other things, to correct such issues.

@OldManYellsAtCloud
Copy link
Copy Markdown
Contributor

OldManYellsAtCloud commented Apr 7, 2026

I'm referring to the Upstream-Status tag, which is missing from the patch: https://docs.yoctoproject.org/scarthgap/contributor-guide/recipe-style-guide.html#patch-upstream-status

It might be a waste of time to fix it other way, but it would be useful to add some note about it in the commit message and maybe in the patch also: what's the actual problem, and why it is better to solve it like that. It doesn't need to be a long essay, but leaving it out will just people scratching their head.

OldManYellsAtCloud
OldManYellsAtCloud reviewed Apr 7, 2026
View reviewed changes
Comment thread ...king/recipes-protocols/frr/frr/0002-tools-update-frr-reload.py-path-to-usr-libexec-frr.patch Outdated
-if test -e /usr/lib/frr/frr-reload.py; then
- exec /usr/lib/frr/frr-reload.py --reload /etc/frr/frr.conf
+if test -e /usr/libexec/frr/frr-reload.py; then
+ exec /usr/lib/frrexec/frr-reload.py --reload /etc/frr/frr.conf
Copy link
Copy Markdown
Contributor

@OldManYellsAtCloud OldManYellsAtCloud Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be /usr/libexec/frr/frr-reload.py also just like the path in the test one line above also?

Copy link
Copy Markdown
Author

@bslawomir330 bslawomir330 Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is an error.

@bslawomir330
frr: Update frr-reload.py path to /usr/libexec/frr/
65b21c6 
Update tools/frr-reload to reference frr-reload.py in the correct
location (/usr/libexec/frr/ instead of /usr/lib/frr/).

Add corresponding patch to the recipe build process.

Signed-off-by: Slawomir Blaszczyk <bslawomir330>
@bslawomir330 bslawomir330 force-pushed the bslawomir330-patch-1 branch from a8a07a0 to 65b21c6 Compare April 7, 2026 12:23
@bslawomir330
Copy link
Copy Markdown
Author

bslawomir330 commented Apr 7, 2026

Thank you for your patience. I believe I have implemented the requested adjustments.

@OldManYellsAtCloud
Copy link
Copy Markdown
Contributor

OldManYellsAtCloud commented Apr 13, 2026

@anujm1 ping

@anujm1
Copy link
Copy Markdown
Contributor

anujm1 commented Apr 14, 2026

I do not understand the claim regarding a fatal QA error.

The location of executable files in /usr/libexec/frr is appropriate; after all, it concerns scripts, not libraries.

This is not a bug in the frr-reload.sh script,

It seems like a bug in that script to me. We pass sbindir to a different value and the script isn't respecting it and hardcoding a presumed value.

but rather a misalignment with the Autotools configuration process. I believe it is a waste of time to address this at the source of the problem. Patching is intended, among other things, to correct such issues.

It's not a waste of time because such patches then need to be maintained for the entire duration of the release and are best fixed upstream when possible.

anujm1
anujm1 reviewed Apr 14, 2026
View reviewed changes
Comment thread ...king/recipes-protocols/frr/frr/0002-tools-update-frr-reload.py-path-to-usr-libexec-frr.patch
-if test -e /usr/lib/frr/frr-reload.py; then
- exec /usr/lib/frr/frr-reload.py --reload /etc/frr/frr.conf
+if test -e /usr/libexec/frr/frr-reload.py; then
+ exec /usr/frrexec/frr-reload.py --reload /etc/frr/frr.conf
Copy link
Copy Markdown
Contributor

@anujm1 anujm1 Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like this has been tested.

anujm1
anujm1 reviewed Apr 14, 2026
View reviewed changes
Comment thread ...king/recipes-protocols/frr/frr/0002-tools-update-frr-reload.py-path-to-usr-libexec-frr.patch
Date: Tue, 07 Apr 2026 13:54:00 +0200
Subject: [PATCH] tools: update frr-reload.py path to /usr/libexec/frr/

Upstream-Status: Submitted [https://github.com/openembedded/meta-openembedded/pull/1044]
Copy link
Copy Markdown
Contributor

@anujm1 anujm1 Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submitted is for patches submitted upstream. This is a valid issue and the status should be Pending. At the very least, it should be reported upstream. This would also need your Signed-off-by.

@anujm1
Copy link
Copy Markdown
Contributor

anujm1 commented Apr 14, 2026

Kindly also note that the preferred way to submit patches is through mailing list so others can also chime in.

kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request May 13, 2026
@wangmingyu84 @kraj
hunspell: upgrade 1.7.2 -> 1.7.3
dad06c7 
Changelog:
===========
- Fix stack-buffer-overflow in Hunzip::getline
- Fix stack overflow in compound_check on Hungarian dictionaries
  under certain conditions
- Fix UB when SFX condition starts with '^' (#1095)
- Bounds-check continuation bytes in u8_u16 (#1110)
- oss-fuzz timeout/OOM hardenings
- Fix openembedded#715 CHECKCOMPOUNDCASE considers digits uppercase
- Fix openembedded#748 hzip: cannot write file
- Fix openembedded#1024 std::string bounds check
- Fix openembedded#1044 tools/analyze crash
- Fix #1076 flags 65520/65521 wrongly rejected
- Fix #1058 don't suggest the input word as its own correction
- Fix openembedded#1002 exact word marked as a near miss
- Fix tdf#125600 dotted-I regression
- Partial Unicode table refresh for Mc combining marks (#1057)
- Add Hunspell_add_with_flags / Hunspell::add_with_flags
- New SPELL_BEST_SUG flag, MAXBREAKDEPTH limit
- Replace clock() with std::chrono for suggestion time limits (openembedded#716)
- Improve exception safety (openembedded#587)
- Document analyze/stem/generate requirements (openembedded#554)
- Report iconv direction on private dic load failures (openembedded#619)
- Show dic load errors unconditionally (openembedded#1012)
- Rename es_EU to eu (#1113)
- Build fixes: out-of-tree, Windows ARM64, MSVC hzip tmpfile (openembedded#919),
  --disable-shared with mingw32 (openembedded#698), iconv on msys2 (openembedded#723),
  ncurses with separate tinfo
- New fuzzers: hzfuzzer, persdicfuzzer, parserfuzzer, affdicfuzzer
- Coverity-flagged fixes
- Merge in weblate translations

Signed-off-by: Wang Mingyu <wangmy@fujitsu.com>
Signed-off-by: Khem Raj <khem.raj@oss.qualcomm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@anujm1 anujm1 anujm1 left review comments

@OldManYellsAtCloud OldManYellsAtCloud OldManYellsAtCloud left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bslawomir330 @OldManYellsAtCloud @anujm1