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

ubus-lime-utils place scripts in /etc/udhcpc.user.d/ #950

Merged
merged 3 commits into from Mar 27, 2023

Conversation

ilario
Copy link
Member

@ilario ilario commented Nov 29, 2022

Fix #927
Still to be tested. Specifically, it is not obvious that it works on OpenWrt 19.07, while it should work on 22.03 as that directory is already present there.

@ilario
Copy link
Member Author

ilario commented Dec 6, 2022

On OpenWrt 19.07 the content of /etc/udhcpc.user.d/ does not get executed and the /etc/udhcpc.user file does not exist ( https://github.com/openwrt/openwrt/tree/openwrt-19.07/package/network/config/netifd/files/etc ), but gets executed if it exists (
https://github.com/openwrt/openwrt/blob/openwrt-19.07/package/network/config/netifd/files/lib/netifd/dhcp.script#L108 ).

On OpenWrt 22.03 the content of /etc/udhcpc.user.d/ does get executed ( https://github.com/openwrt/openwrt/blob/openwrt-22.03/package/network/config/netifd/files/lib/netifd/dhcp.script#L116-L118 ) and the /etc/udhcpc.user file does exist ( https://github.com/openwrt/openwrt/blob/openwrt-22.03/package/network/config/netifd/files/etc/udhcpc.user ).

For having something that works on both we could:
add the /etc/udhcpc.user.d/50-client-wwan-watchping file as in this PR
and
create a uci-defaults script that checks if the /etc/udhcpc.user is already existing and otherwise creates it with some lines executing the /etc/udhcpc.user.d/ content.

Anyone has a better solution?

@codecov-commenter
Copy link

Codecov Report

Merging #950 (c4f4df8) into master (626e8b3) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #950      +/-   ##
==========================================
- Coverage   77.72%   77.70%   -0.03%     
==========================================
  Files          52       52              
  Lines        4359     4359              
==========================================
- Hits         3388     3387       -1     
- Misses        971      972       +1     
Impacted Files Coverage Δ
...kages/lime-system/files/usr/lib/lua/lime/utils.lua 85.50% <0.00%> (-0.30%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ilario
Copy link
Member Author

ilario commented Dec 8, 2022

Situation on OpenWrt 19.07:

Situation with OpenWrt 22.03

So in my opinion this can be merged.

@ilario ilario marked this pull request as ready for review December 8, 2022 20:17
@ilario
Copy link
Member Author

ilario commented Jan 18, 2023

Ping @spiccinini @G10h4ck @selankon I think this is ready to merge, can you review it?

@ilario ilario added this to the 2023.1 milestone Feb 23, 2023
@G10h4ck
Copy link
Member

G10h4ck commented Mar 20, 2023

let's leave this for after retargetting to OpenWrt development version

@ilario
Copy link
Member Author

ilario commented Mar 20, 2023

let's leave this for after retargetting to OpenWrt development version

Ok, makes sense.

@ilario
Copy link
Member Author

ilario commented Mar 26, 2023

Can we merge this now?

@aparcar
Copy link
Member

aparcar commented Mar 26, 2023

Please go ahead

@G10h4ck
Copy link
Member

G10h4ck commented Mar 27, 2023

Please just drop the OpenWrt 19 compatibility script

@G10h4ck G10h4ck merged commit 50d3234 into libremesh:master Mar 27, 2023
1 check failed
@ilario
Copy link
Member Author

ilario commented Mar 27, 2023

Thanks for removing the unneeded script!

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.

ubus-lime-utils file conflict when compiling with OpenWrt 22.03
4 participants