Re: [OE-core] [PATCH v2 1/5] wic: Ensure internal workdir is not reused

["Quentin Schulz" <quentin.schulz@streamunlimited.com>]

Hi Paul,

On Tue, Jan 19, 2021 at 04:26:06PM +0000, Paul Barker wrote:
> If a path is specified for the internal wic working directory using
> the -w/--workdir argument then it must not already exist. Re-using a
> previous workdir could easily result in rootfs and intermediate files
> from a previous build being added to the current image.
> 
> Signed-off-by: Paul Barker <pbarker@konsulko.com>
> ---
>  scripts/lib/wic/plugins/imager/direct.py | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
> index b329568c7a..f107e60089 100644
> --- a/scripts/lib/wic/plugins/imager/direct.py
> +++ b/scripts/lib/wic/plugins/imager/direct.py
> @@ -62,7 +62,7 @@ class DirectPlugin(ImagerPlugin):
>  
>          self.name = "%s-%s" % (os.path.splitext(os.path.basename(wks_file))[0],
>                                 strftime("%Y%m%d%H%M"))
> -        self.workdir = options.workdir or tempfile.mkdtemp(dir=self.outdir, prefix='tmp.wic.')
> +        self.workdir = self.setup_workdir(options.workdir)
>          self._image = None
>          self.ptable_format = self.ks.bootloader.ptable
>          self.parts = self.ks.partitions
> @@ -78,6 +78,16 @@ class DirectPlugin(ImagerPlugin):
>          self._image = PartitionedImage(image_path, self.ptable_format,
>                                         self.parts, self.native_sysroot)
>  
> +    def setup_workdir(self, workdir):
> +        if workdir:
> +            if os.path.exists(workdir):
> +                raise WicError("Internal workdir '%s' specified in wic arguments already exists!" % (workdir))
> +
> +            os.makedirs(workdir)
> +            return workdir

os.makedirs already raises a FileExistsError if the directory exists, so
probably:

try:
    os.makedirs(workdir)
    return workdir
except FileExistsError:
    raise WicError("Internal workdir '%s' specified in wic arguments already exists!" % (workdir))

That being said, you could even not catch the exception? Don't know how
"normal" exceptions are handled by Yocto and the loggers though.

c.f. https://docs.python.org/3/library/os.html#os.makedirs the exist_ok
parameter.

> +        else:
> +            return tempfile.mkdtemp(dir=self.outdir, prefix='tmp.wic.')
> +

Since the if condition returns in all cases, the indentation here is not
needed.

All nitpicks so not a blocker.

Cheers,
Quentin