launchpad-reviewers team mailing list archive

[Julian Andres Klode <mp+473136@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lpbuildd/target/apt.py b/lpbuildd/target/apt.py
> index 3f9ddca..d99e041 100644
> --- a/lpbuildd/target/apt.py
> +++ b/lpbuildd/target/apt.py
> @@ -13,6 +14,66 @@ from lpbuildd.target.operation import Operation
>  logger = logging.getLogger(__name__)
>  
>  
> +def split_options(raw):
> +    table = str.maketrans({
> +        "[": None,
> +        "]": None
> +    })
> +    options = raw.translate(table).split(' ')
> +
> +    return options
> +
> +
> +def prepare_source(line):
> +    pattern = re.compile(

That subset may be correct, but I can't fully read this. [] could appear in other places too, I only think cdrom urls right now, though - nothing launchpad needs to concern itself with.

> +        r'^(?: *(?P<type>deb|deb-src)) +'
> +        r'(?P<options>\[.+\] ?)*'
> +        r'(?P<uri>\w+:\/\/\S+) +'
> +        r'(?P<suite>\S+)'
> +        r'(?: +(?P<components>.*))?$'
> +    )
> +    matches = re.match(pattern, line)
> +    if matches is not None:
> +        options = {}
> +        if matches.group('options'):
> +            for option in split_options(matches['options']):
> +                if "=" in option:
> +                    key, value = option.split("=")
> +                    options[key] = value
> +        source = {
> +            "Types": {matches['type']},
> +            "URIs": matches['uri'],
> +            "Enabled": "yes",
> +        }
> +        if matches.group('suite'):
> +            source["Suites"] = set(matches['suite'].split(' '))
> +        if matches.group('components'):
> +            source["Components"] = set(
> +                matches['components'].split(' ')
> +            )
> +        if "arch" in options:
> +            if "Architectures" in source:
> +                source["Architectures"].append(options["arch"])
> +            else:
> +                source["Architectures"] = {options["arch"]}
> +        if "signed-by" in options:
> +            if "Signed-by" in source:
> +                source["Signed-by"].append(options["signed-by"])
> +            else:
> +                source["Signed-by"] = {options["signed-by"]}
> +        if "lang" in options:
> +            if "Languages" in source:
> +                source["Languages"].append(options["lang"])
> +            else:
> +                source["Languages"] = {options["lang"]}
> +        if "target" in options:
> +            if "Targets" in source:
> +                source["Targets"].append(options["target"])
> +            else:
> +                source["Targets"] = {options["target"]}
> +    return source
> +
> +
>  class OverrideSourcesList(Operation):
>      description = "Override sources.list in the target environment."
>  
> @@ -28,12 +89,28 @@ class OverrideSourcesList(Operation):
>  
>      def run(self):
>          logger.info("Overriding sources.list in build-%s", self.args.build_id)
> -        with self.backend.open(
> -            "/etc/apt/sources.list", mode="w+"
> -        ) as sources_list:
> -            for archive in self.args.archives:
> -                print(archive, file=sources_list)
> -            os.fchmod(sources_list.fileno(), 0o644)
> +        if os.path.exists("/etc/apt/sources.list.d"):

Well sources.list.d always exists.

> +            with self.backend.open(
> +                "/etc/apt/sources.list.d/ubuntu.sources", mode="w+"

Should this write lp-buildd.sources and delete an existing ubuntu.sources?

> +            ) as sources_list:
> +                for archive in self.args.archives:
> +                    source = prepare_source(archive)
> +                    for key, value in source.items():
> +                        if isinstance(value, str):
> +                            sources_list.write("{}: {}\n".format(key, value))
> +                        else:
> +                            sources_list.write(
> +                                "{}: {}\n".format(key, ' '.join(value))
> +                            )
> +                    sources_list.write("\n")
> +                os.fchmod(sources_list.fileno(), 0o644)
> +        else:
> +            with self.backend.open(
> +                "/etc/apt/sources.list", mode="w+"
> +            ) as sources_list:
> +                for archive in self.args.archives:
> +                    print(archive, file=sources_list)
> +                os.fchmod(sources_list.fileno(), 0o644)
>          with self.backend.open(
>              "/etc/apt/apt.conf.d/99retries", mode="w+"
>          ) as apt_retries_conf:


-- 
https://code.launchpad.net/~pelpsi/launchpad-buildd/+git/launchpad-buildd/+merge/473136
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad-buildd:deb822-support into launchpad-buildd:master.