Re: [PATCH 5/6] tests/boot_linux_console: add extract_from_rpm method

[Philippe Mathieu-Daudé]

On Tue, Feb 4, 2020 at 2:34 PM Liam Merwick <address@hidden> 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
>