On Tue, Feb 4, 2020 at 2:34 PM Liam Merwick <liam.merw...@oracle.com> wrote: > On 31/01/2020 15:02, Liam Merwick wrote: > > [... deleted ...] > > >> > >>>>> > >>>>> + :returns: path of the extracted file > >>>>> + """ > >>>>> + cwd = os.getcwd() > >>>>> + os.chdir(self.workdir) > >>>>> + process.run("rpm2cpio %s | cpio -id %s" % (rpm, path), > >>>>> shell=True) > >>>>> + os.chdir(cwd) > >>>>> + return self.workdir + '/' + path > >>>> ^ > >>>> Is the extra slash needed? (just because the extract_from_deb() > >>>> doesn't put it) > >>>> > >>> > >>> > >>> Yes, I needed to put it in there because the 'path' passed in for > >>> processing by cpio is a relative patch unlike the deb arg so it > >>> couldn't be just appended to 'self.workdir' which doesn't end in a '/'. > >> > >> > >> It is a good practice use the `os.path` module methods when dealing > >> with filesystem paths. So that can be replaced with: > >> > >> >>> os.path.normpath(os.path.join('/path/to/workdir', './file/in/rpm')) > >> '/path/to/workdir/file/in/rpm' > >> > > > > > > Will do. I'll add a patch to fix extract_from_deb() too. > > Using the exact same code didn't work with extract_from_deb() because > the callers set 'path' to an absolute path (so os.path.join() drops the > self.workdir part). I'll include a patch with the following change and > it can be dropped if people think using os.path.relpath() is too much of > a hack. > > --- a/tests/acceptance/boot_linux_console.py > +++ b/tests/acceptance/boot_linux_console.py > @@ -49,7 +49,12 @@ class BootLinuxConsole(Test): > process.run("ar x %s %s" % (deb, file_path)) > archive.extract(file_path, self.workdir) > os.chdir(cwd) > - return self.workdir + path > + # Return complete path to extracted file. We need to use > + # os.path.relpath() because callers specify 'path' with a leading > + # slash which causes os.path.join() to interpret it as an absolute > + # path and to drop self.workdir part. > + return os.path.normpath(os.path.join(self.workdir, > + os.path.relpath(path, '/')))
LGTM, so the next one modifying this code won't make a mistake. > > def extract_from_rpm(self, rpm, path): > """ > > Regards, > Liam >