filiparag / hetzner_ddns

Hetzner Dynamic DNS (DDNS, DynDNS) daemon
BSD 2-Clause "Simplified" License
60 stars 10 forks source link

Support hetzner_ddns on openwrt #17

Open LLdaniel opened 3 days ago

LLdaniel commented 3 days ago

Thanks for that awesome script! I like it because it is pragmatic and simply runs as a shell script with the option to demonize it - great!

General idea

Within this issue I wanted to ask if you are interested in supporting hetzner_ddns also on openwrt?

Deeper look

First I thought maybe I can simply package it as .ipk and I am done (btw: in the future it seems that openwrt will use .apk https://lists.openwrt.org/pipermail/openwrt-devel/2024-November/043378.html). But after a while I realized that this is not that easy and I would have to introduce a few changes due to the minimalistic nature of openwrt. These are:

Maybe point 2-3 can be solved within an extra Makefile similar to AlpineLinux. For point 1 I am not sure, but maybe replacing the man call with a short "help" string and for point 5 I have to test a bit.

Question

Do you think that these changes are too many in order to support openwrt and should I create a kind of special fork or do you think we could integrate it into the main branch somehow?

I am willing to try a PR, but only if you think it is a good idea in general. If not I also can understand that, because as I am currently writing this it seems nevertheless to be a bit of work/change though.

filiparag commented 2 days ago

First of all, thanks for suggesting such an improvement.

Within this issue I wanted to ask if you are interested in supporting hetzner_ddns also on openwrt?

I would be happy to have OpenWrt support for this utility, as it falls right into the tools one might use on a router device. Personally I don't use it, so feature testing would have to come from someone with some experience with that OS.

Maybe point 2-3 can be solved within an extra Makefile similar to AlpineLinux. For point 1 I am not sure, but maybe replacing the man call with a short "help" string and for point 5 I have to test a bit.

For point 1) I think the most portable solution would be to check if manpages are viewable on the system, and to fall back to a baked-in string if not. This string can be manually written or man page's roff renderd into plaintext.

Points 2) and 3) can be solved akin to AlpineLinux's APKBUILD as you suggested. This seemed like a least painful solution at the time, because it is easier to strip unneeded parts of the path during build, than to have complex logic inside the tool itself.

I am not sure what the problem is in the point 5) exactly in terms of building this tool, could you elaborate?

Do you think that these changes are too many in order to support openwrt and should I create a kind of special fork or do you think we could integrate it into the main branch somehow?

I am all for upstreaming your changes, as long as they don't introduce changes incompatible with other targets. I would suggest you try putting as much platform-specific stuff into release/OpenWrt as possible.

LLdaniel commented 2 days ago

Thanks for your prompt answer!

I would be happy to have OpenWrt support for this utility, as it falls right into the tools one might use on a router device. Personally I don't use it, so feature testing would have to come from someone with some experience with that OS.

Yes this is partly the reason why I suggested this issue: I would like to run it on my openwrt router. Thus I can also help out to test it.

For point 1) I think the most portable solution would be to check if manpages are viewable on the system, and to fall back to a baked-in string if not. This string can be manually written or man page's roff renderd into plaintext.

The reason is that openwrt tries to be slim as possible sometimes on smaller router there is simply not that much disk space. I probably go for a string, but maybe we can discuss this further once I have a PR draft.

Points 2) and 3) can be solved akin to AlpineLinux's APKBUILD as you suggested. This seemed like a least painful solution at the time, because it is easier to strip unneeded parts of the path during build, than to have complex logic inside the tool itself.

I see, I am totally fine if we put as much openwrt-specific logic into release/OpenWrt.

I am not sure what the problem is in the point 5) exactly in terms of building this tool, could you elaborate?

In order to compile for router cpu architectures one has to establish a cross-compilation toolchain. I also did that for now even though with the shell script there is not really much to compile. As far as I understand

OpenWrt will run the initscript in the host system during build (currently using actions “enable” or “disable”), and it must correctly deal with that special case without undue side-effects. Refer to the “enable and disable” section below for more on this pitfall.

only states one has to be careful with init scripts: They will be executed not only on the target system, but also once the package is build on the host system. And sometimes it could lead to issues on the host system, because everyone is thinking for the target system. So I think we can get this right.

I think this is all I wanted to know for now. I will try to create a PR, I just have to clean up my testing mess ;)

filiparag commented 1 day ago

Allright, I am looking forward to your pull request 🙃