| Message ID | 20200327123608.198650-1-peter.stadler@student.uibk.ac.at |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [OpenWrt-Devel] netifd: fix 14_migrate-dhcp-release script | expand |
Or 'uci_commit network' … On 27.03.20 13:36, Peter Stadler wrote: > prepend 'uci' to 'commit network' > > Signed-off-by: Peter Stadler <peter.stadler@student.uibk.ac.at> > --- > .../netifd/files/etc/uci-defaults/14_migrate-dhcp-release | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release b/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > index 651c437cb2..f1b384eecc 100644 > --- a/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > +++ b/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > @@ -18,6 +18,6 @@ migrate_release() { > > config_load network > config_foreach migrate_release interface > -commit network > +uci commit network > > exit 0
Hi, On Fri, Mar 27, 2020 at 1:36 PM Peter Stadler < peter.stadler@student.uibk.ac.at> wrote: > prepend 'uci' to 'commit network' > Can you explain why the prepend of 'uci' is required ? In other words what is not working as I fail to spot the issue Hans > > Signed-off-by: Peter Stadler <peter.stadler@student.uibk.ac.at> > --- > .../netifd/files/etc/uci-defaults/14_migrate-dhcp-release | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git > a/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > b/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > index 651c437cb2..f1b384eecc 100644 > --- > a/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > +++ > b/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release > @@ -18,6 +18,6 @@ migrate_release() { > > config_load network > config_foreach migrate_release interface > -commit network > +uci commit network > > exit 0 > -- > 2.24.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
>> prepend 'uci' to 'commit network' > Can you explain why the prepend of 'uci' is required ? > In other words what is not working as I fail to spot the issue I think the author is right. "commit" is no command but an argument to the uci command. Best Adrian
Hi On Sat, Apr 4, 2020 at 8:46 PM <mail@adrianschmutzler.de> wrote: > >> prepend 'uci' to 'commit network' > > Can you explain why the prepend of 'uci' is required ? > > In other words what is not working as I fail to spot the issue > > I think the author is right. "commit" is no command but an argument to the > uci command. > I've tested the script and it works fine for me .. So I want to understand what is failing ... Hans > > Best > > Adrian >
>>> prepend 'uci' to 'commit network' >> Can you explain why the prepend of 'uci' is required ? >> In other words what is not working as I fail to spot the issue > > I think the author is right. "commit" is no command but an argument to the uci command. > I've tested the script and it works fine for me .. > So I want to understand what is failing ... uci-defaults "scripts" are implemented here: https://github.com/openwrt/openwrt/blob/master/package/base-files/files/lib/functions.sh#L256-L263 As you see, after all of them have been sourced a global "uci commit" is called anyway. So, after all, the "uci commit something" statements in uci-defaults files are actually not strictly necessary. However, many authors prefer to have committed what they touch directly in the uci-defaults script. This is handled relatively inconsistently across OpenWrt main/packages repos, I once had a look into it and found that it was about 50:50 back then (having a commit in the file vs. not having it). However, I'd still expect the "commit network" to produce some error anywhere. But instead of adding the "uci" in front, another working solution would be to just drop the line entirely. Best Adrian
On Sat, Apr 4, 2020 at 9:04 PM <mail@adrianschmutzler.de> wrote: > >>> prepend 'uci' to 'commit network' > >> Can you explain why the prepend of 'uci' is required ? > >> In other words what is not working as I fail to spot the issue > > > > I think the author is right. "commit" is no command but an argument to > the uci command. > > I've tested the script and it works fine for me .. > > So I want to understand what is failing ... > > uci-defaults "scripts" are implemented here: > > https://github.com/openwrt/openwrt/blob/master/package/base-files/files/lib/functions.sh#L256-L263 > > As you see, after all of them have been sourced a global "uci commit" is > called anyway. > > So, after all, the "uci commit something" statements in uci-defaults files > are actually not strictly necessary. However, many authors prefer to have > committed what they touch directly in the uci-defaults script. > This is handled relatively inconsistently across OpenWrt main/packages > repos, I once had a look into it and found that it was about 50:50 back > then (having a commit in the file vs. not having it). > I observed the same inconsistency across the code base which makes it hard to refer to a default way of working > > However, I'd still expect the "commit network" to produce some error > anywhere. > I did not see an issue when doing a quick test as the global 'uci commit' covered up the issue for me > > But instead of adding the "uci" in front, another working solution would > be to just drop the line entirely. > Agree and I would prefer this solution Thx, Hans > > Best > > Adrian >
Removing the line should be good :-) Beside the error message that `commit` is not found, I experienced no other issue. Thank you Peter
diff --git a/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release b/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release index 651c437cb2..f1b384eecc 100644 --- a/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release +++ b/package/network/config/netifd/files/etc/uci-defaults/14_migrate-dhcp-release @@ -18,6 +18,6 @@ migrate_release() { config_load network config_foreach migrate_release interface -commit network +uci commit network exit 0
prepend 'uci' to 'commit network' Signed-off-by: Peter Stadler <peter.stadler@student.uibk.ac.at> --- .../netifd/files/etc/uci-defaults/14_migrate-dhcp-release | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)