ddns-scripts: fix regression added by version 2.8.0

[feckert]

Maintainer: me / @Ansuel
Compile tested: not needed only script changes
Run tested: x86_64, APU3, openwrt master

Description:

• Rename package for services installation ddns-scripts: move providers into json #13402 (comment)
• Rename packages for special services installation ddns-scripts: move providers into json #13402 (comment)
• Remove installation list file for custom providers this file gets installed by the new ddns script
• Refactoring new ddns script to save additional providers into custom directory
• Fix installation json files for special providers

The following issues should get resolved with this P/R.
#13495
https://lists.openwrt.org/pipermail/openwrt-devel/2020-September/031507.html

@Ansuel I have not add the dependency ddns-scripts-services to ddns-scripts for now. When I do that the menuconfig does not look clean! Could you verify this.

[Ansuel]

What problem do you have with the deps?

define Package/ddns-scripts
  $(call Package/ddns-scripts/Default)
  TITLE:=Dynamic DNS Client scripts (with IPv6 support)
  DEPENDS:=+ddns-scripts-services
endef

define Package/ddns-scripts/description
  Dynamic DNS Client scripts (with IPv6 support)
  A highly configurable set of scripts for doing dynamic dns updates.
  - IPv6 support
  - DNS server support
  - Glue Record support (require BIND host or KNOT host)
  - DNS requests via TCP
  - Proxy server support
  - log file support
  - support to run once
  Version: $(PKG_VERSION)-$(PKG_RELEASE)
  Info   : https://openwrt.org/docs/guide-user/services/ddns/client
endef

define Package/ddns-scripts/conffiles
/etc/config/ddns
endef


define Package/ddns-scripts-services
  $(call Package/ddns-scripts/Default)
  TITLE:=Common ddns providers
#   DEPENDS:=ddns-scripts
endef

[Ansuel]

I was thinking of renaming the 'services' dir to 'default'. This would make it more clear that in that dir there are json installed by the opkg and should not be touched...
Aside from this. Can we keep the list file on the repo but not install it?

[feckert]

I was thinking of renaming the 'services' dir to 'default'. This would make it more clear that in that dir there are json installed by the opkg and should not be touched...

Sounds good 👍. I will change that.

Aside from this. Can we keep the list file on the repo but not install it?

This should fit. I have only removed the installation call and the file is still in the repo

I have not add the dependency ddns-scripts-services to ddns-scripts for now. When I do that the menuconfig does not look clean! Could you verify this.

forget it. That has been cleared up I have rebuilt and then it worked. But thanks

[Ansuel]

This should fit. I have only removed the installation call and the file is still in the repo

Didn't notice... Will update the luci app pr with the default dir so we can merge both... What do you think?

[feckert]

@Ansuel I have to update this P/R with another commit to read also the custom json values.
For now the read function does only read the files from the services directory (In the future this directory is changed in this P/R to default, as you suggested).

But we could not read the custom directory if we have installed a provider in this directory for now.
Any solution to fix this?

My suggestions are:

• Read also in another call the values from the custom directory if we do not find the json file in the default directory
• Check first the custom directory and then the default directory

An if we find no json then we gave up?

I would use the second suggest. So we have always use the newest json information if we try to read first the custom directory.

[Ansuel]

I would use the second suggest.

Yes the default json should be the fallback if custom is not provided. Maybe add a comment somewhere to make this clear (that any custom json have priority than default?)

[snakwu]

@feckert @Ansuel
什么时候可以合并？

[neheb]

 In net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_updater.sh line 48:
	printf %s\n "$MYPROG: $@" >&2
                               ^-- SC2145: Argument mixes string and array. Use * or separate argument.

[feckert]

 In net/ddns-scripts/files/usr/lib/ddns/dynamic_dns_updater.sh line 48:
	printf %s\n "$MYPROG: $@" >&2
                               ^-- SC2145: Argument mixes string and array. Use * or separate argument.

@neheb Thanks for your advice. But I don't want to deal with the shellcheck things in this PR. Now I have to make sure that the JSONs work properly. I want to update some other things in the package. The restructuring of the package is not yet complete. There is still a lot to be done.

[feckert]

@Ansuel I would merge the changes now. If it works for you, too.

[Ansuel]

Sorry for the delay... I tested now and it seems to work all correctly. Did you test the luci app?

[ivanich]

Not sure how but this bricks my edgerouterx, had to recovery via tftp. When I remove this commit from packages repo it works fine

[feckert]

@ivanich That are only script changes. So what did you exactly do. This changes were recently committed so i do not think this has already picked up by the buildbot and that this package are available in the snapshot for installation.

[ivanich]

@feckert I make my own builds from master for 3 routers including edgerouter for a quite long time. As I said before build with this commit reverted works fine. I even tried to disable all ddns staff in the config but it still wont boot.

[feckert]

@ivanich sorry i can not help you. I need more information what did you exactly do.