[pmg-devel] [PATCH pmg-api 1/2] Use ucf to handle template overrides.

[Fabian Grünbichler]

On March 25, 2020 7:42 pm, Stoiko Ivanov wrote:
> ucf(1) is a utility to track changes in config files which are not shipped in
> the debian package (but e.g. get generated through the postinst script)
> 
> While the template overriding mechanism of PMG does not directly write those
> config files - users who override a config-file currently need to manually
> compare the templates shipped in '/var/lib/pmg/templates' on every upgrade.
> 
> By selectively registering the existing template overrides with ucf, users get
> asked once upon the next upgrade regarding their changes, and then will
> always get prompted when the shipped default template changes.
> 
> The alternative of unconditionally registering all templates with ucf, as done
> by dh_ucf (1), would copy all templates to /etc/pmg/templates, which I deemed
> less elegant.

The net effect would be the same, modulo that existence of the file in 
/etc now implies some manual/explicit action by the admin, and would not 
in the other variant - correct?

Both sound okay to me.

> 
> The changes in the maintainer scripts are based on the generated ones from
> dh_ucf (1), with the replacement of `which ucf` by the (more portable)
> `command -v ucf` in the postrm script.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>  debian/postinst | 20 +++++++++++++++++++-
>  debian/postrm   | 26 ++++++++++++++++++++++++++
>  src/Makefile    |  1 +
>  3 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 debian/postrm
> 
> diff --git a/debian/postinst b/debian/postinst
> index 5e4db3a..9118dff 100644
> --- a/debian/postinst
> +++ b/debian/postinst
> @@ -3,6 +3,17 @@
>  set -e
>  
>  
> +ucf_register_templates() {
> +    PMG_TEMPLATES=""

it's very non-obvious that this is a placeholder that gets set via make.
we usually add '.in' to the filename, and use some placeholder syntax 
like

PMG_TEMPLATES="@PMG_TEMPLATE_FILES@"

> +
> +    for template in ${PMG_TEMPLATES}; do
> +	if [ -e "/etc/pmg/templates/${template}" ]; then
> +	    ucf "/var/lib/pmg/templates/${template}" "/etc/pmg/templates/${template}"
> +	    ucfr pmg-api "/etc/pmg/templates/${template}"
> +	fi
> +    done

but I am not sure we need that here - couldn't we just

for template in /etc/pmg/templates/*; do
  if [ -e "/var/lib/pmg/templates/$(basename $template)" ]; then
    ... ucf thing ...
  else
    echo "template '$(basename $template)' no longer shipped by pmg-api, consider removing it.."
    ucf unregister/purge ... (possibly? not sure :-P whether that makes sense or not)
  fi
done

it's even less work in the default case :-P the semantics are not quite 
the same though, so maybe I am missing something.

> +}
> +
>  case "$1" in
>      triggered)
>  
> @@ -24,6 +35,8 @@ case "$1" in
>  
>  	    pmgconfig init
>  	    pmgdb init
> +
> +	    ucf_register_templates
>  	    pmgconfig sync --restart
>  
>  	    if [ -z "$2" ]; then
> @@ -36,13 +49,18 @@ case "$1" in
>  	    pmgdb update  >/dev/null 2>&1 &
>  
>  	    update-mime-database /usr/share/mime
> +
>  	else
>  	    # rewrite banner
>  	    pmgbanner || true
>  	fi
>      ;;
>  
> -    abort-upgrade|abort-remove|abort-deconfigure)
> +    abort-upgrade)
> +	ucf_register_templates
> +    ;;
> +
> +    abort-remove|abort-deconfigure)
>      ;;
>  
>      *)
> diff --git a/debian/postrm b/debian/postrm
> new file mode 100644
> index 0000000..1c14ce7
> --- /dev/null
> +++ b/debian/postrm
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +
> +set -e
> +
> +if [ "$1" = "purge" ]; then
> +    PMG_TEMPLATES=""

same here, and 

> +
> +    for template in ${PMG_TEMPLATES}; do
> +	for ext in .ucf-new .ucf-old .ucf-dist ""; do
> +	    rm -f "/etc/pmg/templates/${template}${ext}"

here as well - these could just be globbed/'find .. -delete'd? we want 
to remove them, no matter whether they are for templates which were 
shipped by the last installed version?

> +	done
> +	if command -v ucf >/dev/null 2>&1; then
> +	    ucf --purge "/etc/pmg/templates/${template}"
> +	fi
> +	if command -v ucfr >/dev/null 2>&1; then
> +	    ucfr --purge pmg-api "/etc/pmg/templates/${template}"

according to "man 1 ucrf", the recommended way to do this is (with a 
slight fixup, since the example does not purge :-P)

    ucfq -w pmg-api | cut -d : -f 1 | while read cfile ; do ucfr --purge $cfile ; done

and since all templates in /etc that ucf knows about should be 
registered, the loop can be extended to call 'ucf --purge' as well, and 
the postrm script does not need to hardcode the template file names.


> +	fi
> +    done
> +fi
> +
> +# dh_installdeb will replace this with shell code automatically
> +# generated by other debhelper scripts.
> +
> +#DEBHELPER#
> +
> +
> diff --git a/src/Makefile b/src/Makefile
> index b5e4c9f..3617f05 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -192,6 +192,7 @@ install: ${SOURCES} $(addsuffix .service-bash-completion, ${SERVICES}) $(addsuff
>  	install -d -m 0755 ${DESTDIR}/lib/systemd/system
>  	for i in ${TIMER_UNITS}; do install -m 0644 $$i ${DESTDIR}/lib/systemd/system/; done
>  	install -D -m 0644 pmg-initramfs.conf ${DESTDIR}/etc/initramfs-tools/conf.d/pmg-initramfs.conf
> +	sed -ri 's/^    PMG_TEMPLATES=.*$$/PMG_TEMPLATES="${TEMPLATES}"/' debian/postinst debian/postrm

this would then not be needed anymore AFAICT ?

>  
>  .PHONY: check
>  check:
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
>