Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources

[Claudius Heine <claudius.heine.ext@siemens.com>]

Hi Tolga,

On 05/12/2018 11.28, Tolga Hoşgör wrote:
> On Tue, Dec 4, 2018 at 5:46 PM Claudius Heine
> <claudius.heine.ext@siemens.com> wrote:
>>
>> Hi Jan,
>>
>> On 04/12/2018 15.28, Jan Kiszka wrote:
>>> On 04.12.18 15:23, [ext] Claudius Heine wrote:
>>>>> @@ -138,6 +154,12 @@ def get_distro_components_argument(d, is_host):
>>>>>        else:
>>>>>            return ""
>>>>> +def get_debootstrap_includes(d, is_host=False):
>>>>> +    if get_distro_have_https_source(d, is_host):
>>>>> +        return "locales,apt-transport-https,ca-certificates"
>>>>
>>>> Those package names should be configured in the distro config.
> 
> I partly agree. It makes sense that it is the responsibility of distro
> config but "debootstrap" does this automatically [1]. This
> intervention by "debootstrap" creates some erratic behavior.

debootstrap has subscripts for each distribution, most of them link to 
the same script, but that doesn't change the fact that they already have 
a mechanism in place to implement different logic depending on the 
distribution.

isar-bootstrap has to work with every possible debootstrap 
implementation, so in order to do that, it shouldn't contain distro 
specific stuff.

> 
> Scenario 1:
> - No special bootstrap config on distro.
> - Distro sources have an "https" and "http" in that order.
> 
> Scenario 2:
> - No special bootstrap config on distro.
> - Distro sources have an "http" and "https" in that order.
> 
> While we were happily building on scenario 1, with a minor change now
> the build magically fails on scenario 2.
> 
> An acceptable solution for consistency would be to disable this
> behavior [1] but "debootstrap" does not seem to have an option to
> disable this magic-package-adding behavior. How about explicitly
> `--exclude` the packages for consistency?

No explicit excluding of packages please. That would hard code our 
implementation to internal debootstrap stuff. But if we just add the 
required packages to debootstrap to handle https when we detect the 
requirement, then this should work in what ever order the repository 
sources come.

> "debootstrap" also did this in its distro-specific scripts [1], like
> your approach. But now it is on a common script "debian-common" [2].
> So now they think this is more or less a standard package I guess.

debootstrap should do what ever debootstrap does. What it does in detail 
shouldn't concern isar. (at least as long as everything works)

> Actually what is needed is something the output of:
> "DEBOOTSTRAP_DIR=/usr/share/debootstrap source
> /usr/share/debootstrap/scripts/<distro>; MIRRORS=<mirrors>
> work_out_debs && echo $base" but this is not publicly exposed in any
> way. Could call `debootstrap --print-debs <mirror>` for each mirror
> and sort/uniq the result but it would be ugly.

Don't go that way. That will just lead us implementing our own 
debootstrap and that is not currently the scope of isar.

>>> Means you would suggest to introduce two variables, e.g.
>>> DEBOOTSTRAP_INCLUDE and DEBOOTSTRAP_INCLUDE_HTTPS to encode those
>>> packages? Those could then be set in distro confs and simply used here.
>>
> 
> Now I am completely lost. I suppose there is no need to create the
> logic to to differentiate HTTP and HTTPS in ISAR if distro conf will
> set the packages. Distro configuration knows whether if it is using
> HTTP or HTTPS (or in theory something else) and it should be enough to
> define/extend _one_ variable accordingly.

I would suggest calling those variables like this:

DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = " 
apt-transport-https ca-certificates"

# OVERRIDE = "...:https-support:..." # when you detect that https is needed.

or similar. And then use those variables in isar-bootstrap with your logic.

> Do you agree on adding `--exclude=apt-transport-https,ca-certificates`
> and introducing a `BOOTSTRAP_PACKAGES` variable to be configured in
> distro?
> 
> [1] https://salsa.debian.org/installer-team/debootstrap/blob/08cfced48bdf7bc1240efb63f69cde7ed385038b/scripts/sid#L36-40
> [2] https://salsa.debian.org/installer-team/debootstrap/blob/4a4e7186de7237c58eae4f0b2de3ad670ad7436f/scripts/debian-common#L30-34

NACK for exclude
ACK for BOOTSTRAP_PACKAGES variables (or similar named, see above)

kind regards,
Claudius


-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de