curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+408391@xxxxxxxxxxxxxxxxxx>]

Review: Approve

Nice, thanks for splitting this work up into all those previous branches, makes this much easier to review! Just a couple of comments, nothing deep, no further review required.

Diff comments:

> diff --git a/curtin/commands/apt_config.py b/curtin/commands/apt_config.py
> index ef4a34c..032273d 100644
> --- a/curtin/commands/apt_config.py
> +++ b/curtin/commands/apt_config.py
> @@ -282,6 +282,32 @@ def disable_suites(disabled, src, release):
>      return output
>  
>  
> +def disable_components(disabled, entries):
> +    """reads the config for components to be disabled and remove those
> +       from the entries"""
> +    if not disabled:
> +        return entries
> +
> +    # purposefully skip disabling the main component
> +    comps_to_disable = {comp for comp in disabled if comp != 'main'}
> +
> +    output = []
> +    for entry in entries:
> +        if not entry.disabled and comps_to_disable.intersection(entry.comps):
> +            # handle commenting ourselves - it handles lines with
> +            # options better
> +            comment = SourceEntry('# ' + str(entry))

This little sequence is duplicated in at least one other place, is it worth making a helper? (maybe not)

> +            output.append(comment)
> +
> +            entry.comps = [comp for comp in entry.comps
> +                           if comp not in comps_to_disable]
> +            if entry.comps:
> +                output.append(entry)
> +        else:
> +            output.append(entry)
> +    return output
> +
> +
>  def update_dist(entries, release):
>      for entry in entries:
>          entry.dist = util.render_string(entry.dist, {'RELEASE': release})
> diff --git a/examples/apt-source.yaml b/examples/apt-source.yaml
> index 695c696..a3ff8b0 100644
> --- a/examples/apt-source.yaml
> +++ b/examples/apt-source.yaml
> @@ -45,12 +45,26 @@ apt:
>    # There is no harm in specifying a suite to be disabled that is not found in
>    # the source.list file (just a no-op then)
>    #
> -  # Note: Lines don’t get deleted, but disabled by being converted to a comment.
>    # The following example disables all usual defaults except $RELEASE-security.
>    # On top it disables a custom suite called "mysuite"
>    disable_suites: [$RELEASE-updates, backports, $RELEASE, mysuite]
>  
> -  # 1.3 primary/security
> +  # 1.3 disable_components
> +  #
> +  # This is an empty list by default, so nothing is disabled.
> +  #
> +  # If given, those components are removed from sources.list after all other
> +  # modifications have been made.
> +  # Components are even disabled if no other modification was made,
> +  # but not if is preserve_sources_list is active.

This should mention that main cannot be disabled?

> +  #
> +  # There is no harm in specifying a component to be disabled that is not found
> +  # in the source.list file (just a no-op then)
> +  #
> +  # The following example disables all usual default components except main.
> +  disable_components: [restricted, universe, multiverse]
> +
> +  # 1.4 primary/security
>    #
>    # define a custom (e.g. localized) mirror that will be used in sources.list
>    # and any custom sources entries for deb / deb-src lines.


-- 
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/408391
Your team curtin developers is subscribed to branch curtin:master.