[pacman-dev] [PATCHv2] pmpkg: add missing directories to test packages

[Allan McRae]

On 04/03/13 09:54, Andrew Gregory wrote:
> Several tests require complete file lists in order to provide accurate
> results.  These can be non-obvious.  Adding missing parent directories
> helps insure the integrity of tests against human error.  Filling in
> parent directories also allows us to check that file lists are actually
> valid.
> 
> There didn't seem to be a good place to do this that was always
> guaranteed to be run, so this adds a finalize() function to packages
> that will always be run before the package is actually used to allow for
> this type of tidying.
> 
> Fixes FS#30723
> 
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---
> 
> Not quite as simple as I had hoped...
> 
>  test/pacman/pmdb.py   |  2 +-
>  test/pacman/pmpkg.py  | 49 ++++++++++++++++++++++++++++---------------------
>  test/pacman/pmtest.py |  3 +++
>  3 files changed, 32 insertions(+), 22 deletions(-)
> 
> diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py
> index b694dff..3e9d305 100644
> --- a/test/pacman/pmdb.py
> +++ b/test/pacman/pmdb.py
> @@ -219,7 +219,7 @@ def db_write(self, pkg):
>          # files and install
>          if self.is_local:
>              data = []
> -            make_section(data, "FILES", pkg.full_filelist())
> +            make_section(data, "FILES", pkg.filelist())
>              make_section(data, "BACKUP", pkg.local_backup_entries())
>              entry["files"] = "\n".join(data)
>  
> diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py
> index c0c9f13..1e019c8 100644
> --- a/test/pacman/pmpkg.py
> +++ b/test/pacman/pmpkg.py
> @@ -69,6 +69,7 @@ def __init__(self, name, version = "1.0-1"):
>              "post_upgrade": "",
>          }
>          self.path = None
> +        self.finalized = False
>  
>      def __str__(self):
>          s = ["%s" % self.fullname()]
> @@ -182,27 +183,33 @@ def install_package(self, root):
>              if os.path.isfile(path):
>                  os.utime(path, (355, 355))
>  
> -    def full_filelist(self):
> -        """Generate a list of package files.
> -
> -        Each path is decomposed to generate the list of all directories leading
> -        to the file.
> -
> -        Example with 'usr/local/bin/dummy':
> -        The resulting list will be:
> -            usr/
> -            usr/local/
> -            usr/local/bin/
> -            usr/local/bin/dummy
> -        """
> -        file_set = set()
> -        for name in self.files:
> -            name = self.parse_filename(name)
> -            file_set.add(name)
> -            while "/" in name:
> -                name, tmp = name.rsplit("/", 1)
> -                file_set.add(name + "/")
> -        return sorted(file_set)
> +    def filelist(self):
> +        """Generate a list of package files."""
> +        return sorted([self.parse_filename(f) for f in self.files])
> +
> +    def finalize(self):
> +        """Perform any necessary operations to ready the package for use."""
> +        if self.finalized:
> +            return
> +
> +        # add missing parent dirs to file list
> +        # use bare file names so trailing ' -> ', '*', etc don't throw off the
> +        # checks for existing files
> +        file_names = self.filelist()
> +        for name in file_names:
> +            name = os.path.dirname(name.rstrip("/"))
> +
> +            while name and name != "/":
> +                if name in file_names:
> +                    # path exists as both a file and a directory
> +                    raise ValueError("Path '%s' duplicated in filelist." % name)
> +                elif name + "/" not in file_names:
> +                    file_names.append(name + "/")
> +                    self.files.append(name + "/")
> +                name = os.path.dirname(name)
> +        self.files.sort()
> +

Is there are reason not to use the old full_filelist function here?  I
said ti was the better way in my last email...  I see the error for
conflicting files/directories has been added - so that is good.  But in
pactests can not start with a "/", so you should use "while "/" in name"

> +        self.finalized = True
>  
>      def local_backup_entries(self):
>          return ["%s\t%s" % (self.parse_filename(i), util.mkmd5sum(i)) for i in self.backup]
> diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py
> index 6dc0ee6..2eafe68 100644
> --- a/test/pacman/pmtest.py
> +++ b/test/pacman/pmtest.py
> @@ -147,8 +147,11 @@ def generate(self, pacman):
>          vprint("    Creating package archives")
>          for pkg in self.localpkgs:
>              vprint("\t%s" % os.path.join(util.TMPDIR, pkg.filename()))
> +            pkg.finalize()
>              pkg.makepkg(tmpdir)
>          for key, value in self.db.iteritems():
> +            for pkg in value.pkgs:
> +                pkg.finalize()
>              if key == "local" and not self.createlocalpkgs:
>                  continue
>              for pkg in value.pkgs:
>