batman-adv, batctl, mesh11sd, nodogsplash, opennds: fix defects found while reviewing the move to openwrt/packages - #1199
Conversation
8fed34a to
7206d1e
Compare
|
@ecsv @simonwunderlich — heads-up on the batman-adv commit in this PR: the problem is not only on the OpenWrt side.
#if LINUX_VERSION_IS_LESS(7, 0, 0) && \
!(LINUX_VERSION_IS_GEQ(6, 18, 33) && LINUX_VERSION_IS_LESS(6, 19, 0))
That range only carves out the 6.18.33 stable backport, but CI made it concrete: exactly the four targets on This PR switches the guards to Since the GitHub mirror has issues and pull requests disabled, I take it patches go to |
[...]
Upstream batman-adv handles it via:
It is just a read-only mirror. It is not officially support and might be completely out-of-date. Please use https://git.open-mesh.org/batman-adv.git/ as reference
Yes, see https://www.open-mesh.org/projects/open-mesh/wiki/Contribute#Submitting-patches
No, I would like to keep the way upstream way for now. You cannot assume that it will stay a define. We can talk about the |
ecsv
left a comment
ecsv
left a comment
There was a problem hiding this comment.
For the batctl + batman-adv: Looks mostly ok. But I would prefer when you would keep the outer check to completely disable it for kernels >= 7.0 (#if LINUX_VERSION_IS_LESS(7, 0, 0)). Will make it easier to avoid problems when these are no longer defines or to directly see when these compat defines can finally dropped from this file.
Since you did all the rest, I will handle this part EDIT: https://git.open-mesh.org/batman-adv.git/commit/?id=96a26bababe5e9c9b9f9226e7cdbe60f49f57b71 |
7206d1e to
16bacd7
Compare
16bacd7 to
fc02609
Compare
get_current_setup() runs unconditionally before any argument is parsed, so even `mesh11sd -v` collects the whole runtime environment first. That includes refresh_bridgemac(), which calls wait_for_interface() for br-lan; when that interface is not up, the poll loop runs for the full interface_timeout, ten seconds by default. On a running router br-lan is up and the loop exits on its first iteration, so the delay is invisible. It shows up wherever the interface is absent or still coming up. Add a patch which skips the setup pass for the informational options, which need only $version and $tmpdir. Verified against the unpatched script that all seven informational invocations (no argument, -h, --help, help, -v, --version, version) keep their exact output and exit status while returning immediately. Upstream took a different route for the same problem and moved the get_current_setup() call after the version and help handling. That change sits in the private 7.2.0 beta, so the patch can be dropped once the package is bumped to that release. Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Three problems in the generated configuration: - The error message for a missing configuration file prints $file, which is never set, instead of $val, which holds the path. - The warning for the unsupported FAS options prints the value of the option instead of its name, so the message does not say which option is unsupported. - gatewayinterface is emitted twice: once resolved to a device via network_get_device() as "GatewayInterface", and once more by the generic option loop with the raw UCI value. When the value names a network section rather than a device, the second line overrides the resolved device with a name nodogsplash cannot use. Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Upstream spells its repository openNDS/openNDS. The Makefile already depends on that casing, since PKG_BUILD_DIR expects the openNDS- tarball root; the lowercase URL only resolves through GitHub's case-insensitive redirect and is the string shown to users in menuconfig and the package metadata. Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…script /etc/config/batman-adv is registered as a conffile although the package never installs it: files/ only ships the uci-defaults migration script and the netifd proto handlers. The migration script itself removes /etc/config/batman-adv as its last step once the settings have been moved into /etc/config/network, so the entry is left over from the layout that predates batadv_hardif. The same script uses continue to skip a section, but there is no loop in the function itself. POSIX leaves continue without an enclosing loop in the current function unspecified; ash walks the dynamic call stack and lands on the loop inside config_foreach, which happens to produce the intended behaviour. Use return, which expresses the intent directly. Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
- PKG_BUILD_DIR was set to exactly the value package.mk derives by default for a package with build variants, so the override had no effect. - The CONFIG_* switches were assembled into a newline separated string and passed to the build through a shell variable via shexport. They can simply be listed on the make command line; the result is identical, since later assignments override earlier ones, exactly as the "n first, then y" order of the old string relied on. - "managment" is user visible in the description of all three variants. - The MAKE_VARS and MAKE_FLAGS continuation lines were indented with spaces while every other multi-line list in the file uses a tab. Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
fc02609 to
2076ee5
Compare
openwrt
Bot
added
the
not following guidelines
Formality Check: FailedWe checked this pull request against the contribution guidelines. Here is what needs your attention: 🛑 CRITICAL ERRORS
Tip Do not close this pull request to make corrections. Instead, modify your existing commits (e.g. Something broken? Consider reporting an issue. |
openwrt
Bot
added
the
release/25.12
These are defects that surfaced while preparing the move of these packages to openwrt/packages (#184). They are all pre-existing in this feed, so they are worth fixing here rather than only in the migration PRs — those may take a while to land, and some of this affects users today.
One commit per package:
/etc/config/batman-advis registered as a conffile although nothing installs it; the shipped migration script deletes that file as its last step. The same script usescontinueto skip a section although there is no loop in the function, which only works because ash walks the dynamic call stack intoconfig_foreach.PKG_BUILD_DIRduplicated the defaultpackage.mkderives for a package with build variants; theCONFIG_*switches went through ashexportshell variable although listing them on the make command line is equivalent (~2 KB for the largest variant); plus a user-visible typo and inconsistent indentation.mesh11sd -vwaits forbr-lanthroughwait_for_interface()before printing anything, for the fullinterface_timeout(10 s) when the interface is not up. Patch also submitted upstream as Fix - do not wait for interfaces when printing version or help openNDS/mesh11sd#204.gatewayinterfacetwice, so the raw UCI value can override the device resolved vianetwork_get_device().URLuses a casing that only resolves through GitHub's redirect.Each package that changes content gets a
PKG_RELEASEbump.Maintainers: @simonwunderlich (batman-adv, batctl), @bluewavenet (mesh11sd, opennds), @mwarning (nodogsplash)
[allow cherry-pick]