Hi Ondrej,

On Wed, 18 May 2011, Ondřej Surý wrote:
> > I'm just not sure what the correct solution is. Instead of purge_dir maybe
> > the right answer is "manage-manual-conffile" and/or
> > "manage-manual-file". And it would drop the file on removal/purge and try
> > to remove the parent directories if they are no longer owned.
> >
> > For now, the "preinst/prerm/postinst" would be a no-op but the postrm
> > would do the removal. Later when we can tell dpkg that the package
> > installed some manual files, the registration would happen in postinst
> > and in the other scripts the command would become a no-op since
> > dpkg itself would do the removal.
> 
> Sounds good. The only problem I see is that it could be "files" (as in
> /var/log/<package>), so maybe a pattern is needed?
> 
> Something like:
> dpkg-maintscript-helper manage-manual-files /var/log/<package>/*.log*
> 
> Or maybe I am overcomplicating stuff and we already know how to handle
> logfiles and this is really for low-number of files.
> 
> Anyway I think that my patch would work just like that, just rename
> the purge_dir to what you like :)

Thinking a bit more about it, I believe it's more reasonable to go
with a "purge_related_files" command. It would indeed accept several
files and/or directories.

Patterns should be expanded by the shell so that the function itself
doesn't have to deal with patterns.

You patch is not really ready for this, I'm commenting below.

On Thu, 12 May 2011, Ondřej Surý wrote:
> I have implemented the command. It doesn't go with corresponding
> manpage section and probably needs some polishing, but it works in my
> case...

Yeah, it would be nice if you could finish the work including the required
documentation. See my other comments below:

> In fact, I needed to add
> dpkg-maintscript-helper purge_dir $phpini
> only to php5-cli.postrm to cleanup the cruft.

All dpkg-maintscript-helper commands must support being called in all
maintainer scripts and must ensure they are a no-op in maintainer scripts
where nothing should be done.

This is currently not the case for your new command.

For instance dh_installdeb makes use of this characteristic to generate
the same snippet in all maintainer scripts based on what the packager
provided in debian/package.maintscript.

> +purge_dir() {

Oviously renamed to purge_related_files. 

> +     local DIRECTORY="$1"
> +     local PACKAGE="$2"
> +     if [ "${PACKAGE}" = "--" -o -z "${PACKAGE}" ]; then
> +             PACKAGE="${DPKG_MAINTSCRIPT_PACKAGE}"
> +     fi

So we need to accept multiple parameters here. It probably means that we
should get rid of the PACKAGE parameter and just hope that
DPKG_MAINTSCRIPT_PACKAGE is set.

And the algorithm should be something like this:
for f in $files; do
    if [ -L $f ] || [ ! -d $f ]; then
        rm $f
        # add $(dirname $f) to a list of directories
    else
        # add $f to a list of directories
    fi
done

for d in $dirlist; do
    purge_dir $dir # what your current function does
done

I'm not sure what's the best way to handle the list of directories
to correctly deal with path that includes spaces... possibly a temporary
file.

> +     while [ "$DIRECTORY" != "/" ]; do
> +
> +             local OWNED_BY="$(dpkg-query -S "${DIRECTORY}" 2>/dev/null | 
> cut -f 1 -d :)"
> +
> +             if [ -z "${OWNED_BY}" -o "${OWNED_BY}" = "${PACKAGE}" ]; then
> +                     [ -d "${DIRECTORY}" ] && \
> +                         rmdir --ignore-fail-on-non-empty "${DIRECTORY}"
> +
> +                     # Directory was not removed
> +                     if [ -d "${DIRECTORY}" ]; then
> +                             echo "Unable to remove ${DIRECTORY}: directory 
> not empty"
> +                             return 1

We don't want the postrm to fail just because of this. Thus return 0.

> +                     else
> +                             DIRECTORY="$(dirname "${DIRECTORY}")"
> +                             purge_dir "${DIRECTORY}" "${PACKAGE}"
> +                             return $?

Thus it doesn't bring anything to forward the return value here.

> +                     fi
> +             else
> +                     debug "Preserving ${DIRECTORY}; still owned by 
> ${#OWNED_BY} package(s)"

Why ${#OWNED_BY} ? The string length is not equal to the number of
packages...

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)




--
To UNSUBSCRIBE, email to debian-dpkg-bugs-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to