run_progress's has_error detects warnings as error

toni-patroni commented

2021-05-26 12:01:06 +00:00

Hi,

I wanted to switch over a debootstrap use to mmdebstrap, mainly to get the "real" apt dependency resovling.

The debootstrap use case includes some tooling to pull together various packages from different (verified) sources and put them in a single local unsigned repo.

With debootstrap I just had to add the --no-check-gpg parameter and be done, as mmdebstrap doesn't have a equivalent of that option I wen for passing --aptopt 'Acquire::AllowInsecureRepositories "1"', which itself works, as apt doesn't errors but only warns about that, for example an output like:

I: 1655215 1883 running apt-get update...
D: 1655215 633 run_progress: exec apt-get update -oAPT::Status-Fd=${FD} -oDpkg::Use-Pty=false
Get:1 file://root/sources/infra/builder/tmp/bootstraprepo bullseye InRelease
Ign:1 file://root/sources/infra/builder/tmp/bootstraprepo bullseye InRelease
Get:2 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release [1470 B]
Get:2 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release [1470 B]
Get:3 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release.gpg
Ign:3 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release.gpg
Get:4 file://root/sources/infra/builder/tmp/bootstraprepo bullseye/main amd64 Packages [190 kB]
Ign:4 file://root/sources/infra/builder/tmp/bootstraprepo bullseye/main amd64 Packages
Get:4 file://root/sources/infra/builder/tmp/bootstraprepo bullseye/main amd64 Packages [537 kB]
Reading package lists...
W: The repository 'file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release' is not signed.
E: apt-get update -oAPT::Status-Fd=<$fd> -oDpkg::Use-Pty=false failed

The $has_error helper detects such warnings as errors, which is IMO wrong, as a warning cannot be an error by definition.

To be honest, I required far longer than I'd like to admit to correlate the warning with the error from mmdebstrap, only once I read the source I got it.

So maybe mmdebstrap could allow warnings in updates, they are not an inherent sign of a broken setup after all.

Possibly, mmdebstrap could even do away with parsing the output and use the --error-on=any option for the apt-get update call, which will be available with bullseye.

Hi, I wanted to switch over a debootstrap use to mmdebstrap, mainly to get the "real" apt dependency resovling. The debootstrap use case includes some tooling to pull together various packages from different (verified) sources and put them in a single local unsigned repo. With debootstrap I just had to add the `--no-check-gpg` parameter and be done, as mmdebstrap doesn't have a equivalent of that option I wen for passing `--aptopt 'Acquire::AllowInsecureRepositories "1"'`, which itself works, as apt doesn't errors but only warns about that, for example an output like: ``` I: 1655215 1883 running apt-get update... D: 1655215 633 run_progress: exec apt-get update -oAPT::Status-Fd=${FD} -oDpkg::Use-Pty=false Get:1 file://root/sources/infra/builder/tmp/bootstraprepo bullseye InRelease Ign:1 file://root/sources/infra/builder/tmp/bootstraprepo bullseye InRelease Get:2 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release [1470 B] Get:2 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release [1470 B] Get:3 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release.gpg Ign:3 file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release.gpg Get:4 file://root/sources/infra/builder/tmp/bootstraprepo bullseye/main amd64 Packages [190 kB] Ign:4 file://root/sources/infra/builder/tmp/bootstraprepo bullseye/main amd64 Packages Get:4 file://root/sources/infra/builder/tmp/bootstraprepo bullseye/main amd64 Packages [537 kB] Reading package lists... W: The repository 'file://root/sources/infra/builder/tmp/bootstraprepo bullseye Release' is not signed. E: apt-get update -oAPT::Status-Fd=<$fd> -oDpkg::Use-Pty=false failed ``` The $has_error helper detects such warnings as errors, which is IMO wrong, as a warning cannot be an error by definition. To be honest, I required far longer than I'd like to admit to correlate the warning with the error from mmdebstrap, only once I read the source I got it. So maybe mmdebstrap could allow warnings in updates, they are not an inherent sign of a broken setup after all. Possibly, mmdebstrap could even do away with parsing the output and use the `--error-on=any` option for the `apt-get update` call, which will be available with bullseye.

toni-patroni referenced this issue

2021-05-26 12:54:16 +00:00

josch commented

2021-05-26 13:33:32 +00:00

Owner

This is a known problem and you will have to bring it up with the apt maintainers. The underlying problem is, that there is currently no way for mmdebstrap to tell apt: "Fail in these cases but not in these others". Have a look at apt-pkg/acquire-item.cc from the apt sources and you will see that apt throws warnings for many things that mmdebstrap absolutely should treat as errors. Since we cannot distinguish one warning from the other because the error messages are human readable strings and differ between different apt versions, mmdebstrap will fail for all warnings of "apt-get update". The solution would be to improve the scripting interface of apt and make its output machine readable. See Debian bugs #778357, #776152, #696335, and #745735.

We have multiple ways forward:

improve the apt scripting interface -- this would solve more than this particular problem
change apt so that it doesn't issue a warning when it's told to allow insecure repositories -- after all, we told it that it's fine, so why warn about it?
sign your repositories with a dummy key

This is a known problem and you will have to bring it up with the apt maintainers. The underlying problem is, that there is currently no way for mmdebstrap to tell apt: "Fail in these cases but not in these others". Have a look at `apt-pkg/acquire-item.cc` from the apt sources and you will see that apt throws warnings for many things that mmdebstrap absolutely should treat as errors. Since we cannot distinguish one warning from the other because the error messages are human readable strings and differ between different apt versions, mmdebstrap will fail for all warnings of "apt-get update". The solution would be to improve the scripting interface of apt and make its output machine readable. See Debian bugs [#778357](https://bugs.debian.org/778357), [#776152](https://bugs.debian.org/776152), [#696335](https://bugs.debian.org/696335), and [#745735](https://bugs.debian.org/745735). We have multiple ways forward: 1. improve the apt scripting interface -- this would solve more than this particular problem 2. change apt so that it doesn't issue a warning when it's told to allow insecure repositories -- after all, we told it that it's fine, so why warn about it? 3. sign your repositories with a dummy key

toni-patroni commented

2021-05-26 14:55:42 +00:00

Author

improve the apt scripting interface -- this would solve more than this particular problem

agreed, wouldn't the mentioned --error-on=any help already?
Seems to cover the network errors which apt called "temporary errors" already:
https://salsa.debian.org/apt-team/apt/-/merge_requests/150/diffs

change apt so that it doesn't issue a warning when it's told to allow insecure repositories -- after all, we told it that it's fine, so why warn about it?

I get where they come from, warning about disabling any authentication should be visible to an user/log, so that it's clear that the trust about using that repo need to come through something else, could be a Note or the like though.

sign your repositories with a dummy key

yeah thought so..

I actually could get away with it through reworking the previous steps to do more directly from already signed repos; the "recreate new single repo" was mostly done due to limitations of debootstrap after all. But I'd have liked to have both pluggable for comparing results and to be able to switch back if running into other issues in our specific and slightly weird use case later.

For now I'll just locally patch mmdebstrap to not care about warnings, not catching those that can be produced at all in our controlled environment is not an issue at all at that point anyway.

Any how, much thanks for your response!

> improve the apt scripting interface -- this would solve more than this particular problem agreed, wouldn't the mentioned `--error-on=any` help already? Seems to cover the network errors which apt called "temporary errors" already: https://salsa.debian.org/apt-team/apt/-/merge_requests/150/diffs > change apt so that it doesn't issue a warning when it's told to allow insecure repositories -- after all, we told it that it's fine, so why warn about it? I get where they come from, warning about disabling any authentication should be visible to an user/log, so that it's clear that the trust about using that repo need to come through something else, could be a Note or the like though. > sign your repositories with a dummy key yeah thought so.. I actually could get away with it through reworking the previous steps to do more directly from already signed repos; the "recreate new single repo" was mostly done due to limitations of debootstrap after all. But I'd have liked to have both pluggable for comparing results and to be able to switch back if running into other issues in our specific and slightly weird use case later. For now I'll just locally patch mmdebstrap to not care about warnings, not catching those that can be produced at all in our controlled environment is not an issue at all at that point anyway. Any how, much thanks for your response!

josch commented

2021-05-26 17:27:31 +00:00

Owner

agreed, wouldn't the mentioned --error-on=any help already?
Seems to cover the network errors which apt called "temporary errors" already:
https://salsa.debian.org/apt-team/apt/-/merge_requests/150/diffs

The problem with --error-on=any is, that the meaning of any may change in the future when more values are supported as argument for the --error-on option. So what we need would be --error-on=transient or something similar.

I get where they come from, warning about disabling any authentication should be visible to an user/log, so that it's clear that the trust about using that repo need to come through something else, could be a Note or the like though.

Yes, quote from jak: "There is no way to get rid of those, by design"

For now I'll just locally patch mmdebstrap to not care about warnings, not catching those that can be produced at all in our controlled environment is not an issue at all at that point anyway.

There is another way forward. mmdebstrap has the --skip option which is the "allow to shoot yourself in the foot" option that allows deviation from a bunch of safe defaults. We could add something like --skip=update/aptwarnings which would instruct mmdebstrap to ignore any W: lines printed by apt.

> agreed, wouldn't the mentioned --error-on=any help already? Seems to cover the network errors which apt called "temporary errors" already: https://salsa.debian.org/apt-team/apt/-/merge_requests/150/diffs The problem with `--error-on=any` is, that the meaning of `any` may change in the future when more values are supported as argument for the `--error-on` option. So what we need would be `--error-on=transient` or something similar. > I get where they come from, warning about disabling any authentication should be visible to an user/log, so that it's clear that the trust about using that repo need to come through something else, could be a Note or the like though. Yes, quote from jak: "There is no way to get rid of those, by design" > For now I'll just locally patch mmdebstrap to not care about warnings, not catching those that can be produced at all in our controlled environment is not an issue at all at that point anyway. There is another way forward. mmdebstrap has the `--skip` option which is the "allow to shoot yourself in the foot" option that allows deviation from a bunch of safe defaults. We could add something like `--skip=update/aptwarnings` which would instruct mmdebstrap to ignore any `W:` lines printed by apt.

josch commented

2021-05-31 07:05:38 +00:00

Owner

I talked to the apt developers. Indeed --error-on=any is doing exactly what I want (error out on transient network errors) and is supposed to keep that semantic in future apt versions. I'm going to implement this so that users of apt 2.1.16 and later can make use of it. Thanks for the suggestion!

I talked to the apt developers. Indeed `--error-on=any` is doing exactly what I want (error out on transient network errors) and is supposed to keep that semantic in future apt versions. I'm going to implement this so that users of apt 2.1.16 and later can make use of it. Thanks for the suggestion!

josch referenced this issue from a commit

2021-05-31 09:18:05 +00:00

since apt 2.1.16 we can use --error-on=any and do not anymore need to error out on all W: lines (closes: #6)

josch closed this issue

2021-05-31 09:18:05 +00:00

run_progress's has_error detects warnings as error #6