On 04/03/13 23:32, Andrew Gregory wrote: > 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 <[email protected]> >>> --- >>> >>> 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? >
We should have an error if the path starts with "/" so it would be great if you could add that. The testsuite does wired things if you do use patch starting with / anyway. The rest of your explanation makes sense, so no need to change that.
