* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 13:35 [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources Hosgor, Tolga (CT RDA DS EU TR MTS)
@ 2018-12-04 14:11 ` Jan Kiszka
2018-12-04 14:23 ` Claudius Heine
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Jan Kiszka @ 2018-12-04 14:11 UTC (permalink / raw)
To: Hosgor, Tolga (CT RDA DS EU TR MTS), isar-users
You should also add a
From: ...
line here when you are using a different transport email account.
On 04.12.18 14:35, Hosgor, Tolga (CT RDA DS EU TR MTS) wrote:
> Building 'isar-bootstrap-target' fails when there are HTTPS URIs in
> the distro APT sources and the first URI is not HTTPS.
> The first URI in the APT sources is passed to 'debootstrap' and
> the distro APT sources file is written to the isar bootstrap rootfs.
> Then, following 'apt-get update' fails due to apt-transport-https,
> ca-certificates being missing. In a normal workflow, 'debootstrap'
> automatically includes them if the MIRROR is HTTPS.
>
> Signed-off-by: Hosgor, Tolga (CT RDA DS EU TR MTS) <tolga.hosgor@siemens.com>
> ---
> .../isar-bootstrap/isar-bootstrap.inc | 28 +++++++++++++++++--
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> index cfad136..33ad8e8 100644
> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> @@ -109,11 +109,15 @@ def aggregate_aptsources_list(d, file_list, file_out):
> out_fd.write("\n".encode())
> out_fd.write("\n".encode())
>
> -def get_distro_primary_source_entry(d, is_host=False):
> +def get_aptsources_list(d, is_host=False):
> if is_host:
> apt_sources_list = (d.getVar("HOST_DISTRO_APT_SOURCES", True) or "").split()
> else:
> apt_sources_list = (d.getVar("DISTRO_APT_SOURCES", True) or "").split()
> + return apt_sources_list
> +
> +def get_distro_primary_source_entry(d, is_host=False):
> + apt_sources_list = get_aptsources_list(d, is_host)
> for entry in apt_sources_list:
> entry_real = bb.parse.resolve_file(entry, d)
> with open(entry_real, "r") as in_fd:
> @@ -125,6 +129,18 @@ def get_distro_primary_source_entry(d, is_host=False):
> return parsed[2:]
> return ["", "", ""]
>
> +def get_distro_have_https_source(d, is_host=False):
> + for entry in get_aptsources_list(d, is_host):
> + entry_real = bb.parse.resolve_file(entry, d)
> + with open(entry_real, "r") as in_fd:
> + for line in in_fd:
> + parsed = parse_aptsources_list_line(line)
> + if parsed:
> + parsed = get_apt_source_mirror(d, parsed)
Minor, but I would use different variable names here to make things more readable.
> + if parsed[2].startswith("https://"):
> + return True
> + return False
> +
> def get_distro_source(d, is_host):
> return get_distro_primary_source_entry(d, is_host)[0]
>
> @@ -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"
> + else:
> + return "locales"
> +
> do_generate_keyring[dirs] = "${DL_DIR}"
> do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> do_generate_keyring() {
> @@ -183,7 +205,7 @@ isar_bootstrap() {
> if [ ${IS_HOST} ]; then
> ${DEBOOTSTRAP} --verbose \
> --variant=minbase \
> - --include=locales \
> + --include="${@get_debootstrap_includes(d, True)}" \
> ${@get_distro_components_argument(d, True)} \
> ${DEBOOTSTRAP_KEYRING} \
> "${@get_distro_suite(d, True)}" \
> @@ -194,7 +216,7 @@ isar_bootstrap() {
> "${DEBOOTSTRAP}" --verbose \
> --variant=minbase \
> --arch="${DISTRO_ARCH}" \
> - --include=locales \
> + --include="${@get_debootstrap_includes(d, False)}" \
> ${@get_distro_components_argument(d, False)} \
> ${DEBOOTSTRAP_KEYRING} \
> "${@get_distro_suite(d, False)}" \
>
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 13:35 [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources Hosgor, Tolga (CT RDA DS EU TR MTS)
2018-12-04 14:11 ` Jan Kiszka
@ 2018-12-04 14:23 ` Claudius Heine
2018-12-04 14:28 ` Jan Kiszka
2018-12-05 17:12 ` Henning Schild
2018-12-11 7:17 ` Henning Schild
3 siblings, 1 reply; 14+ messages in thread
From: Claudius Heine @ 2018-12-04 14:23 UTC (permalink / raw)
To: Hosgor, Tolga (CT RDA DS EU TR MTS), isar-users
Hi,
On 04/12/2018 14.35, Hosgor, Tolga (CT RDA DS EU TR MTS) wrote:
> Building 'isar-bootstrap-target' fails when there are HTTPS URIs in
> the distro APT sources and the first URI is not HTTPS.
> The first URI in the APT sources is passed to 'debootstrap' and
> the distro APT sources file is written to the isar bootstrap rootfs.
> Then, following 'apt-get update' fails due to apt-transport-https,
> ca-certificates being missing. In a normal workflow, 'debootstrap'
> automatically includes them if the MIRROR is HTTPS.
>
> Signed-off-by: Hosgor, Tolga (CT RDA DS EU TR MTS) <tolga.hosgor@siemens.com>
> ---
> .../isar-bootstrap/isar-bootstrap.inc | 28 +++++++++++++++++--
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> index cfad136..33ad8e8 100644
> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> @@ -109,11 +109,15 @@ def aggregate_aptsources_list(d, file_list, file_out):
> out_fd.write("\n".encode())
> out_fd.write("\n".encode())
>
> -def get_distro_primary_source_entry(d, is_host=False):
> +def get_aptsources_list(d, is_host=False):
> if is_host:
> apt_sources_list = (d.getVar("HOST_DISTRO_APT_SOURCES", True) or "").split()
> else:
> apt_sources_list = (d.getVar("DISTRO_APT_SOURCES", True) or "").split()
> + return apt_sources_list
> +
> +def get_distro_primary_source_entry(d, is_host=False):
> + apt_sources_list = get_aptsources_list(d, is_host)
> for entry in apt_sources_list:
> entry_real = bb.parse.resolve_file(entry, d)
> with open(entry_real, "r") as in_fd:
> @@ -125,6 +129,18 @@ def get_distro_primary_source_entry(d, is_host=False):
> return parsed[2:]
> return ["", "", ""]
>
> +def get_distro_have_https_source(d, is_host=False):
> + for entry in get_aptsources_list(d, is_host):
> + entry_real = bb.parse.resolve_file(entry, d)
> + with open(entry_real, "r") as in_fd:
> + for line in in_fd:
> + parsed = parse_aptsources_list_line(line)
> + if parsed:
> + parsed = get_apt_source_mirror(d, parsed)
> + if parsed[2].startswith("https://"):
> + return True
> + return False
Maybe you can use the any [1] statement with a generator, to simplify
the code here.
[1] https://docs.python.org/3/library/functions.html#any
> +
> def get_distro_source(d, is_host):
> return get_distro_primary_source_entry(d, is_host)[0]
>
> @@ -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.
Claudius
> + else:
> + return "locales"
> +
> do_generate_keyring[dirs] = "${DL_DIR}"
> do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> do_generate_keyring() {
> @@ -183,7 +205,7 @@ isar_bootstrap() {
> if [ ${IS_HOST} ]; then
> ${DEBOOTSTRAP} --verbose \
> --variant=minbase \
> - --include=locales \
> + --include="${@get_debootstrap_includes(d, True)}" \
> ${@get_distro_components_argument(d, True)} \
> ${DEBOOTSTRAP_KEYRING} \
> "${@get_distro_suite(d, True)}" \
> @@ -194,7 +216,7 @@ isar_bootstrap() {
> "${DEBOOTSTRAP}" --verbose \
> --variant=minbase \
> --arch="${DISTRO_ARCH}" \
> - --include=locales \
> + --include="${@get_debootstrap_includes(d, False)}" \
> ${@get_distro_components_argument(d, False)} \
> ${DEBOOTSTRAP_KEYRING} \
> "${@get_distro_suite(d, False)}" \
>
--
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 14:23 ` Claudius Heine
@ 2018-12-04 14:28 ` Jan Kiszka
2018-12-04 14:46 ` Claudius Heine
0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2018-12-04 14:28 UTC (permalink / raw)
To: [ext] Claudius Heine, Hosgor, Tolga (CT RDA DS EU TR MTS), isar-users
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.
>
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.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 14:28 ` Jan Kiszka
@ 2018-12-04 14:46 ` Claudius Heine
2018-12-05 10:28 ` Tolga Hoşgör
0 siblings, 1 reply; 14+ messages in thread
From: Claudius Heine @ 2018-12-04 14:46 UTC (permalink / raw)
To: Jan Kiszka, Hosgor, Tolga (CT RDA DS EU TR MTS), isar-users
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.
>>
>
> 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.
Not sure about those names, but in principle yes. Package names are
distro specific and should not be part of isar-bootstrap IMO.
Not sure why the 'locales' package made it through review, but that
might just be because its just one package that is probably named like
this in every distribution, past and future.
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 14:46 ` Claudius Heine
@ 2018-12-05 10:28 ` Tolga Hoşgör
2018-12-05 12:27 ` Claudius Heine
0 siblings, 1 reply; 14+ messages in thread
From: Tolga Hoşgör @ 2018-12-05 10:28 UTC (permalink / raw)
To: claudius.heine.ext; +Cc: jan.kiszka, isar-users
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.
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?
"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.
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.
> >>
> >
> > 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.
> Not sure about those names, but in principle yes. Package names are
How about BOOTSTRAP_PACKAGES as the variable name?
> distro specific and should not be part of isar-bootstrap IMO.
> Not sure why the 'locales' package made it through review, but that
> might just be because its just one package that is probably named like
> this in every distribution, past and future.
>
> 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
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-05 10:28 ` Tolga Hoşgör
@ 2018-12-05 12:27 ` Claudius Heine
2018-12-05 12:55 ` Claudius Heine
2018-12-05 17:09 ` Henning Schild
0 siblings, 2 replies; 14+ messages in thread
From: Claudius Heine @ 2018-12-05 12:27 UTC (permalink / raw)
To: Tolga Hoşgör; +Cc: jan.kiszka, isar-users
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-05 12:27 ` Claudius Heine
@ 2018-12-05 12:55 ` Claudius Heine
2018-12-05 12:58 ` Jan Kiszka
2018-12-05 17:09 ` Henning Schild
1 sibling, 1 reply; 14+ messages in thread
From: Claudius Heine @ 2018-12-05 12:55 UTC (permalink / raw)
To: Tolga Hoşgör; +Cc: jan.kiszka, isar-users
On 05/12/2018 13.27, [ext] Claudius Heine wrote:
>>
>> 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.
Maybe I should be a bit more explicit here.
The distro config does *not* know if somewhere else (some other
configuration, file potentially from another layer) adds an additional
repository that uses https. Since isar-bootstrap already does some
parsing of the source lists and calls debootstrap, it would be the right
point to handle that.
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-05 12:55 ` Claudius Heine
@ 2018-12-05 12:58 ` Jan Kiszka
2018-12-05 13:04 ` Claudius Heine
0 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2018-12-05 12:58 UTC (permalink / raw)
To: Claudius Heine, Tolga Hoşgör; +Cc: isar-users
On 05.12.18 13:55, Claudius Heine wrote:
> On 05/12/2018 13.27, [ext] Claudius Heine wrote:
>>>
>>> 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.
>
> Maybe I should be a bit more explicit here.
>
> The distro config does *not* know if somewhere else (some other configuration,
> file potentially from another layer) adds an additional repository that uses
> https. Since isar-bootstrap already does some parsing of the source lists and
> calls debootstrap, it would be the right point to handle that.
Do we really need to model that with overrides? It brings some complexity of its
own when changing that dynamically.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-05 12:58 ` Jan Kiszka
@ 2018-12-05 13:04 ` Claudius Heine
0 siblings, 0 replies; 14+ messages in thread
From: Claudius Heine @ 2018-12-05 13:04 UTC (permalink / raw)
To: Jan Kiszka, Tolga Hoşgör; +Cc: isar-users
Hi Jan,
On 05/12/2018 13.58, Jan Kiszka wrote:
> On 05.12.18 13:55, Claudius Heine wrote:
>> On 05/12/2018 13.27, [ext] Claudius Heine wrote:
>>>>
>>>> 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.
>>
>> Maybe I should be a bit more explicit here.
>>
>> The distro config does *not* know if somewhere else (some other
>> configuration, file potentially from another layer) adds an additional
>> repository that uses https. Since isar-bootstrap already does some
>> parsing of the source lists and calls debootstrap, it would be the
>> right point to handle that.
>
> Do we really need to model that with overrides? It brings some
> complexity of its own when changing that dynamically.
Of course we don't *need* to. I just don't like introducing new
variables all the time. At some point converging to some small set of
variables is desirable. (IMO we have to many of them in isar as is)
IMO that is a trade-off and there is no right solution here. I just
wanted to point that out that this might be possible and then let the
person writing the patch decides.
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-05 12:27 ` Claudius Heine
2018-12-05 12:55 ` Claudius Heine
@ 2018-12-05 17:09 ` Henning Schild
1 sibling, 0 replies; 14+ messages in thread
From: Henning Schild @ 2018-12-05 17:09 UTC (permalink / raw)
To: [ext] Claudius Heine; +Cc: Tolga Hoşgör, jan.kiszka, isar-users
Am Wed, 5 Dec 2018 13:27:34 +0100
schrieb "[ext] 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"
I like that idea. Because what Isar asks for is "https-support" and it
should not directly carry what that means ... in terms of packages to
install.
Looking at the debootstrap scripts we will end up with the two packages
mentioned already ... for all distros. But we are much cleaner and
ready for another distro to rename those packages.
Henning
> # 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
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 13:35 [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources Hosgor, Tolga (CT RDA DS EU TR MTS)
2018-12-04 14:11 ` Jan Kiszka
2018-12-04 14:23 ` Claudius Heine
@ 2018-12-05 17:12 ` Henning Schild
2018-12-11 7:17 ` Henning Schild
3 siblings, 0 replies; 14+ messages in thread
From: Henning Schild @ 2018-12-05 17:12 UTC (permalink / raw)
To: Hosgor, Tolga (CT RDA DS EU TR MTS); +Cc: isar-users
Very good idea and thanks for looking into that. Please make sure
to add a CI test. Would be simple as changing one of our .list files to
have http in the first line and at least one https in another. And
maybe comment on the weird mix inside the file. I would suggest the
buster one.
Henning
Am Tue, 4 Dec 2018 16:35:44 +0300
schrieb "Hosgor, Tolga (CT RDA DS EU TR MTS)" <tlghosgor@gmail.com>:
> Building 'isar-bootstrap-target' fails when there are HTTPS URIs in
> the distro APT sources and the first URI is not HTTPS.
> The first URI in the APT sources is passed to 'debootstrap' and
> the distro APT sources file is written to the isar bootstrap rootfs.
> Then, following 'apt-get update' fails due to apt-transport-https,
> ca-certificates being missing. In a normal workflow, 'debootstrap'
> automatically includes them if the MIRROR is HTTPS.
>
> Signed-off-by: Hosgor, Tolga (CT RDA DS EU TR MTS)
> <tolga.hosgor@siemens.com> ---
> .../isar-bootstrap/isar-bootstrap.inc | 28
> +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> cfad136..33ad8e8 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -109,11
> +109,15 @@ def aggregate_aptsources_list(d, file_list, file_out):
> out_fd.write("\n".encode()) out_fd.write("\n".encode())
>
> -def get_distro_primary_source_entry(d, is_host=False):
> +def get_aptsources_list(d, is_host=False):
> if is_host:
> apt_sources_list = (d.getVar("HOST_DISTRO_APT_SOURCES",
> True) or "").split() else:
> apt_sources_list = (d.getVar("DISTRO_APT_SOURCES", True) or
> "").split()
> + return apt_sources_list
> +
> +def get_distro_primary_source_entry(d, is_host=False):
> + apt_sources_list = get_aptsources_list(d, is_host)
> for entry in apt_sources_list:
> entry_real = bb.parse.resolve_file(entry, d)
> with open(entry_real, "r") as in_fd:
> @@ -125,6 +129,18 @@ def get_distro_primary_source_entry(d,
> is_host=False): return parsed[2:]
> return ["", "", ""]
>
> +def get_distro_have_https_source(d, is_host=False):
> + for entry in get_aptsources_list(d, is_host):
> + entry_real = bb.parse.resolve_file(entry, d)
> + with open(entry_real, "r") as in_fd:
> + for line in in_fd:
> + parsed = parse_aptsources_list_line(line)
> + if parsed:
> + parsed = get_apt_source_mirror(d, parsed)
> + if parsed[2].startswith("https://"):
> + return True
> + return False
> +
> def get_distro_source(d, is_host):
> return get_distro_primary_source_entry(d, is_host)[0]
>
> @@ -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"
> + else:
> + return "locales"
> +
> do_generate_keyring[dirs] = "${DL_DIR}"
> do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> do_generate_keyring() {
> @@ -183,7 +205,7 @@ isar_bootstrap() {
> if [ ${IS_HOST} ]; then
> ${DEBOOTSTRAP} --verbose \
> --variant=minbase \
> - --include=locales \
> +
> --include="${@get_debootstrap_includes(d, True)}" \
> ${@get_distro_components_argument(d, True)} \ ${DEBOOTSTRAP_KEYRING} \
> "${@get_distro_suite(d, True)}" \
> @@ -194,7 +216,7 @@ isar_bootstrap() {
> "${DEBOOTSTRAP}" --verbose \
> --variant=minbase \
> --arch="${DISTRO_ARCH}" \
> - --include=locales \
> +
> --include="${@get_debootstrap_includes(d, False)}" \
> ${@get_distro_components_argument(d, False)} \ ${DEBOOTSTRAP_KEYRING}
> \ "${@get_distro_suite(d, False)}" \
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-04 13:35 [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources Hosgor, Tolga (CT RDA DS EU TR MTS)
` (2 preceding siblings ...)
2018-12-05 17:12 ` Henning Schild
@ 2018-12-11 7:17 ` Henning Schild
2018-12-11 17:08 ` Tolga Hoşgör
3 siblings, 1 reply; 14+ messages in thread
From: Henning Schild @ 2018-12-11 7:17 UTC (permalink / raw)
To: Hosgor, Tolga (CT RDA DS EU TR MTS); +Cc: isar-users
Hi,
will you send a v2?
Henning
Am Tue, 4 Dec 2018 16:35:44 +0300
schrieb "Hosgor, Tolga (CT RDA DS EU TR MTS)" <tlghosgor@gmail.com>:
> Building 'isar-bootstrap-target' fails when there are HTTPS URIs in
> the distro APT sources and the first URI is not HTTPS.
> The first URI in the APT sources is passed to 'debootstrap' and
> the distro APT sources file is written to the isar bootstrap rootfs.
> Then, following 'apt-get update' fails due to apt-transport-https,
> ca-certificates being missing. In a normal workflow, 'debootstrap'
> automatically includes them if the MIRROR is HTTPS.
>
> Signed-off-by: Hosgor, Tolga (CT RDA DS EU TR MTS)
> <tolga.hosgor@siemens.com> ---
> .../isar-bootstrap/isar-bootstrap.inc | 28
> +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> cfad136..33ad8e8 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -109,11
> +109,15 @@ def aggregate_aptsources_list(d, file_list, file_out):
> out_fd.write("\n".encode()) out_fd.write("\n".encode())
>
> -def get_distro_primary_source_entry(d, is_host=False):
> +def get_aptsources_list(d, is_host=False):
> if is_host:
> apt_sources_list = (d.getVar("HOST_DISTRO_APT_SOURCES",
> True) or "").split() else:
> apt_sources_list = (d.getVar("DISTRO_APT_SOURCES", True) or
> "").split()
> + return apt_sources_list
> +
> +def get_distro_primary_source_entry(d, is_host=False):
> + apt_sources_list = get_aptsources_list(d, is_host)
> for entry in apt_sources_list:
> entry_real = bb.parse.resolve_file(entry, d)
> with open(entry_real, "r") as in_fd:
> @@ -125,6 +129,18 @@ def get_distro_primary_source_entry(d,
> is_host=False): return parsed[2:]
> return ["", "", ""]
>
> +def get_distro_have_https_source(d, is_host=False):
> + for entry in get_aptsources_list(d, is_host):
> + entry_real = bb.parse.resolve_file(entry, d)
> + with open(entry_real, "r") as in_fd:
> + for line in in_fd:
> + parsed = parse_aptsources_list_line(line)
> + if parsed:
> + parsed = get_apt_source_mirror(d, parsed)
> + if parsed[2].startswith("https://"):
> + return True
> + return False
> +
> def get_distro_source(d, is_host):
> return get_distro_primary_source_entry(d, is_host)[0]
>
> @@ -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"
> + else:
> + return "locales"
> +
> do_generate_keyring[dirs] = "${DL_DIR}"
> do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> do_generate_keyring() {
> @@ -183,7 +205,7 @@ isar_bootstrap() {
> if [ ${IS_HOST} ]; then
> ${DEBOOTSTRAP} --verbose \
> --variant=minbase \
> - --include=locales \
> +
> --include="${@get_debootstrap_includes(d, True)}" \
> ${@get_distro_components_argument(d, True)} \ ${DEBOOTSTRAP_KEYRING} \
> "${@get_distro_suite(d, True)}" \
> @@ -194,7 +216,7 @@ isar_bootstrap() {
> "${DEBOOTSTRAP}" --verbose \
> --variant=minbase \
> --arch="${DISTRO_ARCH}" \
> - --include=locales \
> +
> --include="${@get_debootstrap_includes(d, False)}" \
> ${@get_distro_components_argument(d, False)} \ ${DEBOOTSTRAP_KEYRING}
> \ "${@get_distro_suite(d, False)}" \
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] isar-bootstrap: debootstrap --include flag to support https:// sources
2018-12-11 7:17 ` Henning Schild
@ 2018-12-11 17:08 ` Tolga Hoşgör
0 siblings, 0 replies; 14+ messages in thread
From: Tolga Hoşgör @ 2018-12-11 17:08 UTC (permalink / raw)
To: Henning Schild; +Cc: Tolga Hoşgör, isar-users
[-- Attachment #1: Type: text/plain, Size: 4277 bytes --]
Hi,
I would like to when I have the time. :)
I am convinced to the tidyness of the "https-support" with overrides idea.
On Tue, Dec 11, 2018, 08:20 Henning Schild <henning.schild@siemens.com
wrote:
> Hi,
>
> will you send a v2?
>
> Henning
>
> Am Tue, 4 Dec 2018 16:35:44 +0300
> schrieb "Hosgor, Tolga (CT RDA DS EU TR MTS)" <tlghosgor@gmail.com>:
>
> > Building 'isar-bootstrap-target' fails when there are HTTPS URIs in
> > the distro APT sources and the first URI is not HTTPS.
> > The first URI in the APT sources is passed to 'debootstrap' and
> > the distro APT sources file is written to the isar bootstrap rootfs.
> > Then, following 'apt-get update' fails due to apt-transport-https,
> > ca-certificates being missing. In a normal workflow, 'debootstrap'
> > automatically includes them if the MIRROR is HTTPS.
> >
> > Signed-off-by: Hosgor, Tolga (CT RDA DS EU TR MTS)
> > <tolga.hosgor@siemens.com> ---
> > .../isar-bootstrap/isar-bootstrap.inc | 28
> > +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> > cfad136..33ad8e8 100644 ---
> > a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> > b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -109,11
> > +109,15 @@ def aggregate_aptsources_list(d, file_list, file_out):
> > out_fd.write("\n".encode()) out_fd.write("\n".encode())
> >
> > -def get_distro_primary_source_entry(d, is_host=False):
> > +def get_aptsources_list(d, is_host=False):
> > if is_host:
> > apt_sources_list = (d.getVar("HOST_DISTRO_APT_SOURCES",
> > True) or "").split() else:
> > apt_sources_list = (d.getVar("DISTRO_APT_SOURCES", True) or
> > "").split()
> > + return apt_sources_list
> > +
> > +def get_distro_primary_source_entry(d, is_host=False):
> > + apt_sources_list = get_aptsources_list(d, is_host)
> > for entry in apt_sources_list:
> > entry_real = bb.parse.resolve_file(entry, d)
> > with open(entry_real, "r") as in_fd:
> > @@ -125,6 +129,18 @@ def get_distro_primary_source_entry(d,
> > is_host=False): return parsed[2:]
> > return ["", "", ""]
> >
> > +def get_distro_have_https_source(d, is_host=False):
> > + for entry in get_aptsources_list(d, is_host):
> > + entry_real = bb.parse.resolve_file(entry, d)
> > + with open(entry_real, "r") as in_fd:
> > + for line in in_fd:
> > + parsed = parse_aptsources_list_line(line)
> > + if parsed:
> > + parsed = get_apt_source_mirror(d, parsed)
> > + if parsed[2].startswith("https://"):
> > + return True
> > + return False
> > +
> > def get_distro_source(d, is_host):
> > return get_distro_primary_source_entry(d, is_host)[0]
> >
> > @@ -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"
> > + else:
> > + return "locales"
> > +
> > do_generate_keyring[dirs] = "${DL_DIR}"
> > do_generate_keyring[vardeps] += "DISTRO_APT_KEYS"
> > do_generate_keyring() {
> > @@ -183,7 +205,7 @@ isar_bootstrap() {
> > if [ ${IS_HOST} ]; then
> > ${DEBOOTSTRAP} --verbose \
> > --variant=minbase \
> > - --include=locales \
> > +
> > --include="${@get_debootstrap_includes(d, True)}" \
> > ${@get_distro_components_argument(d, True)} \ ${DEBOOTSTRAP_KEYRING} \
> > "${@get_distro_suite(d, True)}" \
> > @@ -194,7 +216,7 @@ isar_bootstrap() {
> > "${DEBOOTSTRAP}" --verbose \
> > --variant=minbase \
> > --arch="${DISTRO_ARCH}" \
> > - --include=locales \
> > +
> > --include="${@get_debootstrap_includes(d, False)}" \
> > ${@get_distro_components_argument(d, False)} \ ${DEBOOTSTRAP_KEYRING}
> > \ "${@get_distro_suite(d, False)}" \
>
>
[-- Attachment #2: Type: text/html, Size: 5982 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread