launchpad-reviewers team mailing list archive

[Yuliy Schwartzburg <mp+492120@xxxxxxxxxxxxxxxxxx>]

Really great work finding all this out. It doesn't look trivial at all. Just left a few comments of `gatherResults()` that I don't really understand.

Diff comments:

> diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
> index 5ce2cd8..ffaafd0 100644
> --- a/lpbuildd/oci.py
> +++ b/lpbuildd/oci.py
> @@ -185,6 +194,9 @@ class OCIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
>                      )
>                      symlinks.append(file)
>                      continue
> +                if oci_layout_detected:
> +                    tar.extract(file, extract_path)

Are we extracting twice unnecessarily?

> +                    continue
>                  if current_dir and file.name.endswith("layer.tar"):
>                      # This is the actual layer data.
>                      # Instead of adding the layer.tar to a gzip directory
> @@ -203,9 +217,11 @@ class OCIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
>                              gzip_layer.write(byte)
>                              byte = fileobj.read(1)
>                  elif current_dir and file.name.startswith(current_dir):
> -                    # Other files that are in the layer directories,
> -                    # we don't care about
> -                    continue
> +                    if current_dir.startswith("blobs"):
> +                        tar.extract(file, extract_path)

This is also still the same `file` and `extract_path` right? Is this yet a 3rd extract into the same place?

> +                    else:
> +                        # Legacy layout: ignore other files
> +                        continue
>                  else:
>                      # If it's not in a directory, we need that
>                      tar.extract(file, extract_path)


-- 
https://code.launchpad.net/~artemstreltsov/launchpad-buildd/+git/launchpad-buildd/+merge/492120
Your team Launchpad code reviewers is requested to review the proposed merge of ~artemstreltsov/launchpad-buildd:add_docker26.x_support into launchpad-buildd:master.