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 --- 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() + + 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: -- 1.8.1.5

On 03/04/13 at 05:51pm, Allan McRae wrote:

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 ---

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"

full_filelist couldn't be used because it discarded file type, mode, etc. information. At this stage in the test that rule against leading "/" in paths has not been enforced, so an absolute path would cause an endless loop. I could add an explicit error for that and reduce the loop to 'while name' but 'while "/" in name" won't work without rearranging the loop because the top directory won't have "/" in it. Aside from that, the only real difference I see is that full_filelist used direct string manipulation rather than os.path, is there a preference for that? apg

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 --- Changes from v2: * check for absolute paths * copy the name list so we don't modify the list we're looping over * move to the next manually entered file if the parent directory already exists test/pacman/pmdb.py | 2 +- test/pacman/pmpkg.py | 54 +++++++++++++++++++++++++++++++-------------------- test/pacman/pmtest.py | 3 +++ 3 files changed, 37 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..988c73f 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,38 @@ 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 list(file_names): + if os.path.isabs(name): + raise ValueError("Absolute path in filelist '%s'." % name) + + name = os.path.dirname(name.rstrip("/")) + while name: + if name in file_names: + # path exists as both a file and a directory + raise ValueError("Duplicate path in filelist '%s'." % name) + elif name + "/" in file_names: + # path was either manually included or already processed + break + else: + file_names.append(name + "/") + self.files.append(name + "/") + name = os.path.dirname(name) + self.files.sort() + + 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: -- 1.8.1.5