From 60cb23d948e4af6612d9344724d7adf9ae0842d8 Mon Sep 17 00:00:00 2001 From: piegames Date: Sat, 11 Jun 2022 11:32:50 +0200 Subject: [PATCH 01/25] [RFC 0127] Nixpkgs issues and warnings (#127) --- rfcs/0127-issues-warnings.md | 154 +++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 rfcs/0127-issues-warnings.md diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md new file mode 100644 index 000000000..4e84c6bd5 --- /dev/null +++ b/rfcs/0127-issues-warnings.md @@ -0,0 +1,154 @@ +--- +feature: issues-warnings +start-date: 2022-06-11 +author: piegames +co-authors: (find a buddy later to help out with the RFC) +shepherd-team: (names, to be nominated and accepted by RFC steering committee) +shepherd-leader: (name to be appointed by RFC steering committee) +related-issues: https://github.com/NixOS/nixpkgs/pull/177272 +--- + +# RFC: Nixpkgs issues and warnings + +## Summary +[summary]: #summary + +Introduce an issue system into Nixpkgs, similar to broken and insecure, but with a custom per-package message. This will then be used to warn users about packages that are in need of maintenance. Packages that have an open issue for a long time should eventually be removed. + +## Motivation +[motivation]: #motivation + +Nixpkgs has the problem that it is often treated as "append-only", i.e. packages only get added but not removed. There are a lot of packages that are broken for a long time, have end-of-life dependencies with known security vulnerabilities or that are otherwise unmaintained. + +Let's take the end of life of Python 2 as an example. (This applies to other ecosystems as well, and will come up again and again in the future.) It has sparked a few bulk package removal actions by dedicated persons, but those are pretty work intensive and prone to burn out maintainers. A goal of this RFC is to provide a way to notify all users of a package about the outstanding issues. This will hopefully draw more attention to abandoned packages, and spread the work load. It can also help soften the removal of packages by providing a period for users to migrate away at their own pace. + +Apart from that, there is need for a general per-package warning mechanism in nixpkgs – one that is stronger than `builtins.trace`. + +## Detailed design +[design]: #detailed-design + +### Package issues + +A new attribute is added to the `meta` section of a package: `issues`. If present, it is a list of attrsets which each have the following fields: + +- `message`: Required. A string message describing the issue with the package. Linking issues and pull requests is encouraged. +- `kind`: Optional but recommended. If present, the resulting warning will be printed as `kind: message`. +- `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. + +Example values: + +```nix +meta.issues = [{ + kind = "deprecated"; + message = "This package depends on Python 2, which has reached end of life. #148779"; + date = "1970-01-01"; +} { + kind = "removal"; + message = "The application has been abandoned upstream, use libfoo instead"; + date = "1970-01-01"; +}]; +``` + +### nixpkgs integration + +Two new config options are added to nixpkgs, `ignoreWarningsPredicate` and `ignoreWarningsPackages`. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. Their semantic and implementation directly parallel the existing "insecure" package handling. + +Similarly to broken, insecure and unfree packages, evaluating a package with an issue fails evaluation. Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time. + +## Examples and Interactions +[examples-and-interactions]: #examples-and-interactions + +### Package removal + +There are two ways issues interact with the removal of packages: Either they get an issue because they are going to be removed, or they are removed because they have an open issue for a prolonged period of time. + +- Instead of removing a package directly, it should first get an issue announcing the planned removal. This will allow users to migrate away beforehand. `removal` must be used as `kind` (This will facilitate automation in the future). +- Before branch-off for a new release, all (leaf) packages with issues that predate the previous branch-off are deemed safe for removal (unless stated otherwise). If a package is removed based on its issue, that message becomes part of the new `throw` alias. + +### Propagation across transitive dependencies + +When a package that is depended on has an issue, all packages that depend on it will fail to evaluate until that package is ignored or the issue resolved. Sometimes, this is sufficient. + +When the issue requires actions on dependents however, it does not sufficiently inform about all packages that need action. Marking all dependents with that issue is not a good idea either though: it would require users to go through some potentially long dependency chains. Instead, only applications, leaf packages or packages with very few dependents should get the issue. + +As an example, take `gksu` with the `gksu` → `libgksu` → `libglade` → `python2` dependency chain (for the sake of the example, ignore that it also depends on EOL Gtk 2). Obviously, `python2` should get an issue. As a leaf/application, `gksu` should get one too (it could be the same, or with an adpated message). For the packages in between, it depends on whether they require individual action or not. + +### Backporting + +New issues generally should not be added to stable branches, and also not be backported to them, since this breaks evaluation. + +## Drawbacks +[drawbacks]: #drawbacks + +- People have voiced strong negative opionions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. +- There is a slight long-term maintenance burden. It is expected to be similar to or slightly greater than the maintenance of our deprecation aliases. +- Some of the example interactions are built on the premise that nixpkgs is under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. + +## Alternatives +[alternatives]: #alternatives + +An alternative design would be to have issues as a separate list (not part of the package). Instead of allowing individual packages, one could ignore individual warnings (they'd need an identifying number for that). The advantage of doing this is that one could have one issue and apply it for a lot of packages (e.g. "Python 2 is deprecated"). The main drawback there is that it is more complex. + +A few other sketches about how the declaration syntax might look like in different scenarios: + +```nix +{ + # As proposed in the RFC + meta.issues = [{ + message = "deprecation: Python 2 is EOL. #12345"; + # (Other fields omitted for brevity) + }]; + + # Issues are defined elsewhere in some nixpkgs-global table, only get referenced in packages + meta.issues = [ "1234-python-deprecation" ]; + + # Attempt to unify both approaches to allow both ad-hoc and cross-package declaration + meta.issues = { + "1234-python-deprecation" = { + message = "deprecation: Python 2 is deprecated #12345"; + }; + }; +} +``` + +## Unresolved questions +[unresolved]: #unresolved-questions + +- From above: "Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time". How could this be implemented, and efficiently? +- Should issues be a list or an attrset? +- A lot of bike shedding. (See below) + +## Future work +[future]: #future-work + +- Issues are designed in a way that they supersede a lot of our "insecure"/"unfree"/"unsupported" packages infrastructure. There is a lot of code duplication between them. In theory, we could migrate some of these to make use of issues. At the very least, we hope that issues are general enough so that no new similar features will have to be added in the future anymore. + - `meta.knownVulnerabilities` is the first candidate to go + - Unfree package handling will probably be out of scope, since we already have some custom filtering based on licences. +- Inspired by the automation of aliases, managing issues can be helped by tooling as well. This is deemed out of scope of this RFC because only real world usage will tell which actions will be worthwhile automating, but it should definitely considered in the future. + - There will likely be need for tooling that lists issues on all nixpkgs packages, filtered by kind or sorted chronologically. + - Automatically removing packages based on time will likely require providing more information whether it is safe to do so or not. +- > If the advisories were a list, and we also added them for modules, maybe we could auto-generate most release notes, and move release notes closer to the code they change. [[source]](https://discourse.nixos.org/t/pre-rfc-package-advisories/19509/4) + - Issues can certainly be automatically integrated into the release notes somehow. However, this alone would not allow us to move most of our release notes into the packages, because for many release entries breaking eval would be overkill. + +## Bike shedding + +Here are a few naming proposals, and how well they would be suited to describe different conditions. "broken" and "unsupported" must be acknowledged but – unlike the others – don't imply some required user action. "unfree" is somewhat out of scope because it is unlikely to be replaced anytime soon. + +| Kind | Currently | "Issue" | "Warning" | "Problem" | "Advisory" | +|-------------|-----------|---------|-----------|-----------|------------| +|insecure | `meta.knownVulnerabilities` |✅|✅|✅|✅| +|unmaintained | `meta.maintainers = []` |✅|✅|❓|❓| +|deprecated | n/a |✅|✅|✅|❓| +|removal | n/a |❓|✅|✅|❓| +||||||| +|broken | `meta.broken` |✅|❌|✅|❌| +|unsupported | `meta.platforms` |✅|❓|✅|❌| +||||||| +|unfree | `meta.license` |✅|✅|❌|❓| + + +"Advisory" was initially chosen based on the notion of security advisories, but was later dismissed as the project grew in scope. "Issue" and "Problem" are similar words, of which the former is a well-known technical term¹ which should be preferred here. + +"Issue" and "Warning" are both good candidates, of which the former implies some required action whereas the latter merely wants to inform. In the end, we decided that packages should have *issues* which should produce *warnings* that can be *ignored*. While this distinction may be a bit unintuitive, it will make it easier to generate warnings from things that are not explicitly marked as issues (e.g. missing maintainers). + +¹ Non-native speakers: look up the difference between "issue" and "problem" in English :) From ee72999168ca8abe6f1485209577b654e016e497 Mon Sep 17 00:00:00 2001 From: piegames Date: Wed, 15 Jun 2022 23:25:35 +0200 Subject: [PATCH 02/25] Add URLs as structured information --- rfcs/0127-issues-warnings.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 4e84c6bd5..77022f908 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -31,17 +31,19 @@ Apart from that, there is need for a general per-package warning mechanism in ni A new attribute is added to the `meta` section of a package: `issues`. If present, it is a list of attrsets which each have the following fields: -- `message`: Required. A string message describing the issue with the package. Linking issues and pull requests is encouraged. +- `message`: Required. A string message describing the issue with the package. - `kind`: Optional but recommended. If present, the resulting warning will be printed as `kind: message`. - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. +- `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. Example values: ```nix meta.issues = [{ kind = "deprecated"; - message = "This package depends on Python 2, which has reached end of life. #148779"; + message = "This package depends on Python 2, which has reached end of life."; date = "1970-01-01"; + urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; } { kind = "removal"; message = "The application has been abandoned upstream, use libfoo instead"; From c109c3128227946279ac76f63ddf7219712caf11 Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 12 Sep 2022 16:23:35 +0200 Subject: [PATCH 03/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Linus Heckemann --- rfcs/0127-issues-warnings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 77022f908..a4d625d8a 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -65,7 +65,7 @@ Similarly to broken, insecure and unfree packages, evaluating a package with an There are two ways issues interact with the removal of packages: Either they get an issue because they are going to be removed, or they are removed because they have an open issue for a prolonged period of time. - Instead of removing a package directly, it should first get an issue announcing the planned removal. This will allow users to migrate away beforehand. `removal` must be used as `kind` (This will facilitate automation in the future). -- Before branch-off for a new release, all (leaf) packages with issues that predate the previous branch-off are deemed safe for removal (unless stated otherwise). If a package is removed based on its issue, that message becomes part of the new `throw` alias. +- Before branch-off for a new release, all (leaf) packages with issues that predate the previous branch-off are deemed safe for removal (unless stated otherwise). If a package is removed based on its issue, the issue's message becomes part of the new `throw` alias. ### Propagation across transitive dependencies From ff2e2d412fcdaf06c5d161ccae33f8e6a67bb4c8 Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 12 Sep 2022 16:24:01 +0200 Subject: [PATCH 04/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Linus Heckemann --- rfcs/0127-issues-warnings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index a4d625d8a..6c188e62c 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -69,7 +69,7 @@ There are two ways issues interact with the removal of packages: Either they get ### Propagation across transitive dependencies -When a package that is depended on has an issue, all packages that depend on it will fail to evaluate until that package is ignored or the issue resolved. Sometimes, this is sufficient. +When a package has an issue, all packages that depend on it will fail to evaluate until that package is ignored or the issue resolved. Sometimes, this is sufficient. When the issue requires actions on dependents however, it does not sufficiently inform about all packages that need action. Marking all dependents with that issue is not a good idea either though: it would require users to go through some potentially long dependency chains. Instead, only applications, leaf packages or packages with very few dependents should get the issue. From f83067aaa2d46053d8c8aa7a24790d48023b596f Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 18:27:55 +0200 Subject: [PATCH 05/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Linus Heckemann --- rfcs/0127-issues-warnings.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 6c188e62c..0d680974f 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -84,6 +84,7 @@ New issues generally should not be added to stable branches, and also not be bac - People have voiced strong negative opionions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. - There is a slight long-term maintenance burden. It is expected to be similar to or slightly greater than the maintenance of our deprecation aliases. + - We expect that in the long term, having a defined process for removing unmaintained and obsolete packages, especially compared to deciding on a case-by-case basis, is likely to reduce the overall maintenance burden. - Some of the example interactions are built on the premise that nixpkgs is under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. ## Alternatives From 2daf978d0d6af8249acb947f73257e4159829eb2 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 18:28:30 +0200 Subject: [PATCH 06/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Linus Heckemann --- rfcs/0127-issues-warnings.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 0d680974f..15efcf352 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -85,7 +85,8 @@ New issues generally should not be added to stable branches, and also not be bac - People have voiced strong negative opionions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. - There is a slight long-term maintenance burden. It is expected to be similar to or slightly greater than the maintenance of our deprecation aliases. - We expect that in the long term, having a defined process for removing unmaintained and obsolete packages, especially compared to deciding on a case-by-case basis, is likely to reduce the overall maintenance burden. -- Some of the example interactions are built on the premise that nixpkgs is under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. +- Some of the example interactions are built on the premise that parts of nixpkgs are under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. + - We hope that drawing attention to packages in need of maintenance can encourage new maintainers -- both from the existing pool of nixpkgs contributors and from non-contributor users -- to step up. ## Alternatives [alternatives]: #alternatives From 2b634240b80bef8949870f53848c815187df527d Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 18:29:32 +0200 Subject: [PATCH 07/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Linus Heckemann --- rfcs/0127-issues-warnings.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 15efcf352..762218682 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -36,6 +36,8 @@ A new attribute is added to the `meta` section of a package: `issues`. If presen - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. +Other attributes are allowed. Their meanings may be kind-specific. + Example values: ```nix From e5c0e13eeba2a6d396e025472f287ac454e8d4f2 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 18:30:09 +0200 Subject: [PATCH 08/25] Update rfcs/0127-issues-warnings.md --- rfcs/0127-issues-warnings.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 762218682..ba80a77dc 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -84,7 +84,8 @@ New issues generally should not be added to stable branches, and also not be bac ## Drawbacks [drawbacks]: #drawbacks -- People have voiced strong negative opionions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. +- People have voiced strong negative opinions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. + - We do not want to encourage the use of unmaintained software likely to contain security vulnerabilities, and we do not have the bandwidth to maintain packages deprecated by upstream. Nothing is lost though, because we have complete binary cache coverage of old nixpkgs versions, providing a comparatively easy way to pin old very package versions. - There is a slight long-term maintenance burden. It is expected to be similar to or slightly greater than the maintenance of our deprecation aliases. - We expect that in the long term, having a defined process for removing unmaintained and obsolete packages, especially compared to deciding on a case-by-case basis, is likely to reduce the overall maintenance burden. - Some of the example interactions are built on the premise that parts of nixpkgs are under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. From 3212d31c88d9ba9133856f10124aeb22dff8f460 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 18:35:03 +0200 Subject: [PATCH 09/25] RFC 127 update shepherds --- rfcs/0127-issues-warnings.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index ba80a77dc..9a5ec87a9 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -2,9 +2,9 @@ feature: issues-warnings start-date: 2022-06-11 author: piegames -co-authors: (find a buddy later to help out with the RFC) -shepherd-team: (names, to be nominated and accepted by RFC steering committee) -shepherd-leader: (name to be appointed by RFC steering committee) +co-authors: — +shepherd-team: @lheckemann, @mweinelt, @fgaz +shepherd-leader: @mweinelt related-issues: https://github.com/NixOS/nixpkgs/pull/177272 --- From ecdbe2c9b8feeb3cbc07b9e0a7ebf8fe8838cf78 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 22:06:57 +0200 Subject: [PATCH 10/25] RFC 127 rework --- rfcs/0127-issues-warnings.md | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 9a5ec87a9..ae6dc166f 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -13,7 +13,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/177272 ## Summary [summary]: #summary -Introduce an issue system into Nixpkgs, similar to broken and insecure, but with a custom per-package message. This will then be used to warn users about packages that are in need of maintenance. Packages that have an open issue for a long time should eventually be removed. +Inspired by the derivation checks for broken and insecure packages, a new system called "warnings" is introduced. It is planned to eventually replace the previously mentioned systems, as well as the current "warnings" (which currently only prints a trace message for unmaintained packages). `meta.issues` is added to derivations, which can be used to let the evaluation of individual packages fail with a custom message. This will then be used to warn users about packages that are in need of maintenance. Packages that have an open issue for a long time should eventually be removed, although doing so is not part of the RFC. ## Motivation [motivation]: #motivation @@ -29,14 +29,14 @@ Apart from that, there is need for a general per-package warning mechanism in ni ### Package issues -A new attribute is added to the `meta` section of a package: `issues`. If present, it is a list of attrsets which each have the following fields: +A new attribute is added to the `meta` section of a package: `issues`. If present, it is a list of attrsets which each have at least the following fields: - `message`: Required. A string message describing the issue with the package. - `kind`: Optional but recommended. If present, the resulting warning will be printed as `kind: message`. - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. -Other attributes are allowed. Their meanings may be kind-specific. +Other attributes are allowed. Some message kinds may specify additional required attributes. Example values: @@ -55,10 +55,25 @@ meta.issues = [{ ### nixpkgs integration -Two new config options are added to nixpkgs, `ignoreWarningsPredicate` and `ignoreWarningsPackages`. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. Their semantic and implementation directly parallel the existing "insecure" package handling. +The following new config options are added to nixpkgs: `ignoreWarningsPredicate`, `ignoreWarningsPackages`, `traceIgnoredWarnings`. The option `showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. + +The semantic and implementation of `ignoreWarningsPredicate` and `ignoreWarningsPackages` directly parallels the existing "insecure" package handling. Similarly to broken, insecure and unfree packages, evaluating a package with an issue fails evaluation. Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time. +The previous warnings system in `check-meta.nix` is removed, together with `showDerivationWarnings`. Instead, ignored warnings are printed with `builtins.trace` depending on the new option `traceIgnoredWarnings`. + +### Warning kinds + +At the moment, the following values for the `kind` field of a warning are known: + +- `removal`: The package is scheduled for removal some time in the future. +- `deprecated`: The package has been abandoned upstream or has end of life dependencies. +- `maintainerless`: `meta.maintainers` is empty +- `insecure`: The package has some security vulnerabilities + +Not all values make sense as issues (i.e. within `meta.issues`): Some warnings may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. + ## Examples and Interactions [examples-and-interactions]: #examples-and-interactions @@ -79,7 +94,7 @@ As an example, take `gksu` with the `gksu` → `libgksu` → `libglade` → `pyt ### Backporting -New issues generally should not be added to stable branches, and also not be backported to them, since this breaks evaluation. +New issues generally should not be added to stable branches if possible, and also not be backported to them, since this breaks evaluation. The same rule applies to other changes to a pacakge's `meta` which may generate a warning and thus lead to evaluation failure too. A notable exception are warnings of kind `insecure`, if there is no fix that can be backported. ## Drawbacks [drawbacks]: #drawbacks @@ -115,6 +130,11 @@ A few other sketches about how the declaration syntax might look like in differe message = "deprecation: Python 2 is deprecated #12345"; }; }; + + # Proposal by @matthiasbeyer + meta.issues = [ + { transitive = pkgs.python2.issues } + ]; } ``` @@ -122,8 +142,10 @@ A few other sketches about how the declaration syntax might look like in differe [unresolved]: #unresolved-questions - From above: "Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time". How could this be implemented, and efficiently? -- Should issues be a list or an attrset? -- A lot of bike shedding. (See below) +- ~~Should issues be a list or an attrset?~~ + - We are using a list for now, there is always the possibility to also allow attrsets in the future. +- ~~A lot of bike shedding. (See below)~~ + - Packages may have "issues" that generate "warnings" that have to be "ignored". Issues are explicitly added to packages in `meta.issues`, whereas warnings can be generated automatically from other sources, like other `meta` attributes. ## Future work [future]: #future-work From c907adb784955821ae66d9d61e815fecb576bcc7 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Sep 2022 22:39:18 +0200 Subject: [PATCH 11/25] Point out that the previous warnings system was not documented --- rfcs/0127-issues-warnings.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index ae6dc166f..3ca7e657a 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -13,7 +13,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/177272 ## Summary [summary]: #summary -Inspired by the derivation checks for broken and insecure packages, a new system called "warnings" is introduced. It is planned to eventually replace the previously mentioned systems, as well as the current "warnings" (which currently only prints a trace message for unmaintained packages). `meta.issues` is added to derivations, which can be used to let the evaluation of individual packages fail with a custom message. This will then be used to warn users about packages that are in need of maintenance. Packages that have an open issue for a long time should eventually be removed, although doing so is not part of the RFC. +Inspired by the derivation checks for broken and insecure packages, a new system called "warnings" is introduced. It is planned to eventually replace the previously mentioned systems, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). `meta.issues` is added to derivations, which can be used to let the evaluation of individual packages fail with a custom message. This will then be used to warn users about packages that are in need of maintenance. Packages that have an open issue for a long time should eventually be removed, although doing so is not part of the RFC. ## Motivation [motivation]: #motivation @@ -55,13 +55,13 @@ meta.issues = [{ ### nixpkgs integration -The following new config options are added to nixpkgs: `ignoreWarningsPredicate`, `ignoreWarningsPackages`, `traceIgnoredWarnings`. The option `showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. +The following new config options are added to nixpkgs: `ignoreWarningsPredicate`, `ignoreWarningsPackages`, `traceIgnoredWarnings`. The undocumented option `showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. The semantic and implementation of `ignoreWarningsPredicate` and `ignoreWarningsPackages` directly parallels the existing "insecure" package handling. Similarly to broken, insecure and unfree packages, evaluating a package with an issue fails evaluation. Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time. -The previous warnings system in `check-meta.nix` is removed, together with `showDerivationWarnings`. Instead, ignored warnings are printed with `builtins.trace` depending on the new option `traceIgnoredWarnings`. +The previous undocumented warnings system in `check-meta.nix` is removed, together with `showDerivationWarnings`. Instead, ignored warnings are printed with `builtins.trace` depending on the new option `traceIgnoredWarnings`. ### Warning kinds From d232a2d3277e86409a1bc4de9ca08ca0c05633c8 Mon Sep 17 00:00:00 2001 From: piegames Date: Sat, 17 Sep 2022 11:52:33 +0200 Subject: [PATCH 12/25] Rework ignore mechanism --- rfcs/0127-issues-warnings.md | 48 ++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 3ca7e657a..d4e8ee0b7 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -31,35 +31,41 @@ Apart from that, there is need for a general per-package warning mechanism in ni A new attribute is added to the `meta` section of a package: `issues`. If present, it is a list of attrsets which each have at least the following fields: +- `kind`: Required. If present, the resulting warning will be printed as `kind: message`. - `message`: Required. A string message describing the issue with the package. -- `kind`: Optional but recommended. If present, the resulting warning will be printed as `kind: message`. +- `name`: Optional but recommended. Give the issue a custom name for more easy filtering - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. +- `resolved`: Optional. Indicate that the issue is resolved so that it does not generate a warning anymore. Other attributes are allowed. Some message kinds may specify additional required attributes. Example values: ```nix -meta.issues = [{ - kind = "deprecated"; - message = "This package depends on Python 2, which has reached end of life."; - date = "1970-01-01"; - urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; -} { - kind = "removal"; - message = "The application has been abandoned upstream, use libfoo instead"; - date = "1970-01-01"; -}]; +meta.issues = [ + { + name = "python2-eol"; + kind = "deprecated"; + message = "This package depends on Python 2, which has reached end of life."; + date = "1970-01-01"; + urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; + } + { + kind = "removal"; + message = "The application has been abandoned upstream, use libfoo instead"; + date = "1970-01-01"; + } +]; ``` ### nixpkgs integration -The following new config options are added to nixpkgs: `ignoreWarningsPredicate`, `ignoreWarningsPackages`, `traceIgnoredWarnings`. The undocumented option `showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. +The following new config options are added to nixpkgs: `ignoreWarnings` and `traceIgnoredWarnings`. The undocumented option `showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. -The semantic and implementation of `ignoreWarningsPredicate` and `ignoreWarningsPackages` directly parallels the existing "insecure" package handling. +`ignoreWarnings` is a list of string of the format `packageName.warningKind`. A wildcard '\*' may be used before or after the dot, to ignore all warnings of one kind or all warnings of a package. For issues that have a name, `packageName.issueName` is allowed too. If an issue spans multiple packages, it is recommended to use the same name everywhere so that one may ignore all with `*.issueName`. -Similarly to broken, insecure and unfree packages, evaluating a package with an issue fails evaluation. Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time. +Similarly to broken, insecure and unfree packages, evaluating a package with an unignored warning fails evaluation. Ignoring resolved warnings results in a trace warning at evaluation time. The previous undocumented warnings system in `check-meta.nix` is removed, together with `showDerivationWarnings`. Instead, ignored warnings are printed with `builtins.trace` depending on the new option `traceIgnoredWarnings`. @@ -71,6 +77,8 @@ At the moment, the following values for the `kind` field of a warning are known: - `deprecated`: The package has been abandoned upstream or has end of life dependencies. - `maintainerless`: `meta.maintainers` is empty - `insecure`: The package has some security vulnerabilities +- `broken`: The package is marked as broken +- `unsupported`: The package is not expected to build on this platform Not all values make sense as issues (i.e. within `meta.issues`): Some warnings may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. @@ -101,6 +109,7 @@ New issues generally should not be added to stable branches if possible, and als - People have voiced strong negative opinions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. - We do not want to encourage the use of unmaintained software likely to contain security vulnerabilities, and we do not have the bandwidth to maintain packages deprecated by upstream. Nothing is lost though, because we have complete binary cache coverage of old nixpkgs versions, providing a comparatively easy way to pin old very package versions. + - This change is a general improvement in the ecosystem even if we do not end up using it to remove any packages. - There is a slight long-term maintenance burden. It is expected to be similar to or slightly greater than the maintenance of our deprecation aliases. - We expect that in the long term, having a defined process for removing unmaintained and obsolete packages, especially compared to deciding on a case-by-case basis, is likely to reduce the overall maintenance burden. - Some of the example interactions are built on the premise that parts of nixpkgs are under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. @@ -109,7 +118,7 @@ New issues generally should not be added to stable branches if possible, and als ## Alternatives [alternatives]: #alternatives -An alternative design would be to have issues as a separate list (not part of the package). Instead of allowing individual packages, one could ignore individual warnings (they'd need an identifying number for that). The advantage of doing this is that one could have one issue and apply it for a lot of packages (e.g. "Python 2 is deprecated"). The main drawback there is that it is more complex. +An alternative design would be to have issues as a separate list (not part of the package). ~~Instead of allowing individual packages, one could ignore individual warnings (they'd need an identifying number for that). The advantage of doing this is that one could have one issue and apply it for a lot of packages (e.g. "Python 2 is deprecated"). The main drawback there is that it is more complex.~~ The advantages of that approach have been integrated while keeping the downsides small: Warnings are ignored with a per-kind granularity, but one may give some of them a name to allow finer control where necessary. A few other sketches about how the declaration syntax might look like in different scenarios: @@ -117,6 +126,8 @@ A few other sketches about how the declaration syntax might look like in differe { # As proposed in the RFC meta.issues = [{ + kind = "deprecated"; + name = "python2-eol"; message = "deprecation: Python 2 is EOL. #12345"; # (Other fields omitted for brevity) }]; @@ -141,7 +152,10 @@ A few other sketches about how the declaration syntax might look like in differe ## Unresolved questions [unresolved]: #unresolved-questions -- From above: "Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time". How could this be implemented, and efficiently? +- ~~From above: "Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time". How could this be implemented, and efficiently?~~ + - ~~More generally, how do we tell users that their ignored warning can be removed, so that they won't accidentally miss future warnings?~~ + - Issues have a `resolved` attribute that may be used for that purpose. + - The ignore mechanism has been refined so that there is less risk of missing future warnings. - ~~Should issues be a list or an attrset?~~ - We are using a list for now, there is always the possibility to also allow attrsets in the future. - ~~A lot of bike shedding. (See below)~~ @@ -161,6 +175,8 @@ A few other sketches about how the declaration syntax might look like in differe ## Bike shedding +*Note: the bike shedding phase is considered over.* + Here are a few naming proposals, and how well they would be suited to describe different conditions. "broken" and "unsupported" must be acknowledged but – unlike the others – don't imply some required user action. "unfree" is somewhat out of scope because it is unlikely to be replaced anytime soon. | Kind | Currently | "Issue" | "Warning" | "Problem" | "Advisory" | From ff6e33d6b4d9e60f11987b7292c31cb03ffff92a Mon Sep 17 00:00:00 2001 From: piegames Date: Fri, 14 Oct 2022 17:28:08 +0200 Subject: [PATCH 13/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Silvan Mosberger --- rfcs/0127-issues-warnings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index d4e8ee0b7..8a637784c 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -61,7 +61,7 @@ meta.issues = [ ### nixpkgs integration -The following new config options are added to nixpkgs: `ignoreWarnings` and `traceIgnoredWarnings`. The undocumented option `showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. +The following new config options are added to nixpkgs: `config.ignoreWarnings` and `config.traceIgnoredWarnings`. The undocumented option `config.showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. `ignoreWarnings` is a list of string of the format `packageName.warningKind`. A wildcard '\*' may be used before or after the dot, to ignore all warnings of one kind or all warnings of a package. For issues that have a name, `packageName.issueName` is allowed too. If an issue spans multiple packages, it is recommended to use the same name everywhere so that one may ignore all with `*.issueName`. From 6ffce6de613aa2e7c46d9453959fe4c16727e5ed Mon Sep 17 00:00:00 2001 From: piegames Date: Fri, 14 Oct 2022 17:44:20 +0200 Subject: [PATCH 14/25] Update rfcs/0127-issues-warnings.md Co-authored-by: Silvan Mosberger --- rfcs/0127-issues-warnings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 8a637784c..1c8b58b9c 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -90,7 +90,7 @@ Not all values make sense as issues (i.e. within `meta.issues`): Some warnings m There are two ways issues interact with the removal of packages: Either they get an issue because they are going to be removed, or they are removed because they have an open issue for a prolonged period of time. - Instead of removing a package directly, it should first get an issue announcing the planned removal. This will allow users to migrate away beforehand. `removal` must be used as `kind` (This will facilitate automation in the future). -- Before branch-off for a new release, all (leaf) packages with issues that predate the previous branch-off are deemed safe for removal (unless stated otherwise). If a package is removed based on its issue, the issue's message becomes part of the new `throw` alias. +- Before branch-off for a new release, all (leaf) packages with issues that predate the previous branch-off are deemed safe for removal via `throw` (unless stated otherwise). If a package is removed based on its issue, the issue's message becomes part of the new `throw` alias. ### Propagation across transitive dependencies From 24a8985edd37fbdd6d826869dbab0f1c1af80251 Mon Sep 17 00:00:00 2001 From: piegames Date: Sat, 15 Oct 2022 23:52:15 +0200 Subject: [PATCH 15/25] Remove "resolved" attribute again --- rfcs/0127-issues-warnings.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 1c8b58b9c..8485d526a 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -36,7 +36,6 @@ A new attribute is added to the `meta` section of a package: `issues`. If presen - `name`: Optional but recommended. Give the issue a custom name for more easy filtering - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. -- `resolved`: Optional. Indicate that the issue is resolved so that it does not generate a warning anymore. Other attributes are allowed. Some message kinds may specify additional required attributes. @@ -154,7 +153,8 @@ A few other sketches about how the declaration syntax might look like in differe - ~~From above: "Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time". How could this be implemented, and efficiently?~~ - ~~More generally, how do we tell users that their ignored warning can be removed, so that they won't accidentally miss future warnings?~~ - - Issues have a `resolved` attribute that may be used for that purpose. + - ~~Issues have a `resolved` attribute that may be used for that purpose.~~ + - Properly implementing this turned out to be non-trivial, so this feature was cut for the sake of simplicity as it was not of hight importance anyways. - The ignore mechanism has been refined so that there is less risk of missing future warnings. - ~~Should issues be a list or an attrset?~~ - We are using a list for now, there is always the possibility to also allow attrsets in the future. From 1b1424e36d2b4caaca1d55b4eec53a34d10b38ae Mon Sep 17 00:00:00 2001 From: piegames Date: Sat, 15 Oct 2022 23:53:12 +0200 Subject: [PATCH 16/25] Incorporate review feedback --- rfcs/0127-issues-warnings.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 8485d526a..1b732e202 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -156,8 +156,8 @@ A few other sketches about how the declaration syntax might look like in differe - ~~Issues have a `resolved` attribute that may be used for that purpose.~~ - Properly implementing this turned out to be non-trivial, so this feature was cut for the sake of simplicity as it was not of hight importance anyways. - The ignore mechanism has been refined so that there is less risk of missing future warnings. -- ~~Should issues be a list or an attrset?~~ - - We are using a list for now, there is always the possibility to also allow attrsets in the future. +- ~~Should issues be a list or an attrset?~~· + - We are using a list ~~for now, there is always the possibility to also allow attrsets in the future.~~ - ~~A lot of bike shedding. (See below)~~ - Packages may have "issues" that generate "warnings" that have to be "ignored". Issues are explicitly added to packages in `meta.issues`, whereas warnings can be generated automatically from other sources, like other `meta` attributes. From 1e6725ece081e4828883a418a1f9a1afbd4dcf55 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 13 Dec 2022 00:25:10 +0100 Subject: [PATCH 17/25] Rewrite (again) --- rfcs/0127-issues-warnings.md | 216 ++++++++++++++++++++++++----------- 1 file changed, 150 insertions(+), 66 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 1b732e202..5825b14af 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -8,12 +8,16 @@ shepherd-leader: @mweinelt related-issues: https://github.com/NixOS/nixpkgs/pull/177272 --- -# RFC: Nixpkgs issues and warnings +# RFC: Nixpkgs "problem" infrastructure ## Summary [summary]: #summary -Inspired by the derivation checks for broken and insecure packages, a new system called "warnings" is introduced. It is planned to eventually replace the previously mentioned systems, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). `meta.issues` is added to derivations, which can be used to let the evaluation of individual packages fail with a custom message. This will then be used to warn users about packages that are in need of maintenance. Packages that have an open issue for a long time should eventually be removed, although doing so is not part of the RFC. +Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problemHandler` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "throw" (fail evaluation), "trace" (print a warning message) or "ignore" (do nothing). + +Additionally, `meta.problems` is added to derivations, which can be used to manually declare that a package has a certain problem. This will then be used to inform users about packages that are in need of maintenance, for example security vulnerabilities or deprecated dependencies. + +Using the newly introduced features, we may create a process for removing packages from nixpkgs that is easier to maintain and friendlier to users than just replacing packages with `throw`. ## Motivation [motivation]: #motivation @@ -22,18 +26,21 @@ Nixpkgs has the problem that it is often treated as "append-only", i.e. packages Let's take the end of life of Python 2 as an example. (This applies to other ecosystems as well, and will come up again and again in the future.) It has sparked a few bulk package removal actions by dedicated persons, but those are pretty work intensive and prone to burn out maintainers. A goal of this RFC is to provide a way to notify all users of a package about the outstanding issues. This will hopefully draw more attention to abandoned packages, and spread the work load. It can also help soften the removal of packages by providing a period for users to migrate away at their own pace. -Apart from that, there is need for a general per-package warning mechanism in nixpkgs – one that is stronger than `builtins.trace`. +For some use cases, like for packages without maintainers, we do not want to break evaluation of a package and simply warn the user instead. We want the users to configure all these according to their needs and through a single standardized interface. ## Detailed design [design]: #detailed-design -### Package issues +### Package problems -A new attribute is added to the `meta` section of a package: `issues`. If present, it is a list of attrsets which each have at least the following fields: +A new attribute is added to the `meta` section of a package: `problems`. If present, it is a list of attrsets which each have at least the following fields: - `kind`: Required. If present, the resulting warning will be printed as `kind: message`. -- `message`: Required. A string message describing the issue with the package. -- `name`: Optional but recommended. Give the issue a custom name for more easy filtering +- `message`: Required. A string message describing the issue with the package. The value should: + - Start with the "This package", "The application" or equivalent, or simply with the package name. + - Be capitalized (unless it starts with the package name). + - Use a period at the end. +- `name`: Required if there are multiple values of the same `kind`. Give the issue a custom name for more easy filtering - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. @@ -42,7 +49,7 @@ Other attributes are allowed. Some message kinds may specify additional required Example values: ```nix -meta.issues = [ +meta.problems = [ { name = "python2-eol"; kind = "deprecated"; @@ -58,17 +65,7 @@ meta.issues = [ ]; ``` -### nixpkgs integration - -The following new config options are added to nixpkgs: `config.ignoreWarnings` and `config.traceIgnoredWarnings`. The undocumented option `config.showDerivationWarnings` will be removed. A new environment variable is defined, `NIXPKGS_IGNORE_WARNINGS`. - -`ignoreWarnings` is a list of string of the format `packageName.warningKind`. A wildcard '\*' may be used before or after the dot, to ignore all warnings of one kind or all warnings of a package. For issues that have a name, `packageName.issueName` is allowed too. If an issue spans multiple packages, it is recommended to use the same name everywhere so that one may ignore all with `*.issueName`. - -Similarly to broken, insecure and unfree packages, evaluating a package with an unignored warning fails evaluation. Ignoring resolved warnings results in a trace warning at evaluation time. - -The previous undocumented warnings system in `check-meta.nix` is removed, together with `showDerivationWarnings`. Instead, ignored warnings are printed with `builtins.trace` depending on the new option `traceIgnoredWarnings`. - -### Warning kinds +### Problem kinds At the moment, the following values for the `kind` field of a warning are known: @@ -79,44 +76,115 @@ At the moment, the following values for the `kind` field of a warning are known: - `broken`: The package is marked as broken - `unsupported`: The package is not expected to build on this platform -Not all values make sense as issues (i.e. within `meta.issues`): Some warnings may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. +Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems, and therefore also no need to give them a name in order to distinguish them. + +### Nixpkgs configuration + +The following new config option is added to nixpkgs: `config.problemHandler`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. + +Handler values can be either `"throw"`, `"trace"` or `"ignore"`. Future values may be added in the future. The key is of the form `packageName.problemKind` or `packageName.problemName`, where `"*"` is allowed on either level as a wildcard. + +```nix +problemHandler = { + "*" = { + "*" = "throw"; + alias = "trace"; + maintainerless = "ignore"; + }; + myPackage.foo = "ignore"; +}; +``` + +If multiple rules match a given problem of a package, the most specific handler will be called: + +1. `pkgName.problemName` +2. `pkgName.problemKind` +3. `pkgName."*"` +4. `"*".problemName` +5. `"*".problemKind` +6. `"*"."*"` + +So for example the value of `myPackage.*` would override the one in `*.maintainerless` if both matched, as it is more specific. + +The default value in nixpkgs might be something like: + +```nix +problemHandler."*" = { + "*" = "trace"; + removal = "throw"; + deprecated = "throw"; + maintainerless = "ignore"; +}; +``` + +It may also be expanded by values from other configuration options as part of a migration scheme from the other mechanisms. ## Examples and Interactions [examples-and-interactions]: #examples-and-interactions -### Package removal +### Propagation across transitive dependencies -There are two ways issues interact with the removal of packages: Either they get an issue because they are going to be removed, or they are removed because they have an open issue for a prolonged period of time. +When a package has a problem that `throw`s, all packages that depend on it will fail to evaluate until that problem is ignored or resolved. Most of the time, this is sufficient. -- Instead of removing a package directly, it should first get an issue announcing the planned removal. This will allow users to migrate away beforehand. `removal` must be used as `kind` (This will facilitate automation in the future). -- Before branch-off for a new release, all (leaf) packages with issues that predate the previous branch-off are deemed safe for removal via `throw` (unless stated otherwise). If a package is removed based on its issue, the issue's message becomes part of the new `throw` alias. +When the problem requires actions on dependents however, it does not sufficiently inform about all packages that need action. Multiple packages may be annotated with the same problem, in that case it should be given a name and the name should be the same across all instances. Other values like the message or the URL list do not need to be the same and may be adapted sensibly. -### Propagation across transitive dependencies +For the example of Python 2 deprecation, all problems would have `name = "python2-eol"` and then a user may set `config."*".python2-eol = "ignore";` to ignore them. -When a package has an issue, all packages that depend on it will fail to evaluate until that package is ignored or the issue resolved. Sometimes, this is sufficient. +### Backwards compatbility, Backporting and stable releases -When the issue requires actions on dependents however, it does not sufficiently inform about all packages that need action. Marking all dependents with that issue is not a good idea either though: it would require users to go through some potentially long dependency chains. Instead, only applications, leaf packages or packages with very few dependents should get the issue. +New problems generally should not be added to stable branches if possible, and also not be backported to them, since it may break evaluation. The same rule applies to other changes to a pacakge's `meta` which may generate a problem and thus lead to evaluation failure too. Scenarios where evaluation failure is a desired goal, for example with unfixable security issues, are obviously exempt from this. -As an example, take `gksu` with the `gksu` → `libgksu` → `libglade` → `python2` dependency chain (for the sake of the example, ignore that it also depends on EOL Gtk 2). Obviously, `python2` should get an issue. As a leaf/application, `gksu` should get one too (it could be the same, or with an adpated message). For the packages in between, it depends on whether they require individual action or not. +### Removal of packages -### Backporting +The plan is to eventually remove packages with long outstanding problems. The details will be part of future work, but at the very least a package must have a problem whose kind defaults to "throw" for at least one full release cycle (so that stable users have sufficient time to be warned and intervene). -New issues generally should not be added to stable branches if possible, and also not be backported to them, since this breaks evaluation. The same rule applies to other changes to a pacakge's `meta` which may generate a warning and thus lead to evaluation failure too. A notable exception are warnings of kind `insecure`, if there is no fix that can be backported. +If a package needs to be removed for some other reason, the problem kind `removal` should be used instead: + +```nix +meta.problems = [{ + kind = "removal"; + message = "We don't want this in nixpkgs anymore"; + date = "1970-01-01"; +}]; +``` ## Drawbacks [drawbacks]: #drawbacks +- Too much warnigns may cause Alarm fatigue + - One idea I had is to be more verbose on the unstable channels, and then tune down the noise after branch-off. + - New lints to packages should be introduced gradually, by making them "silent" by default on start and only going to "warn" after most problems in nixpkgs itself are resolved. - People have voiced strong negative opinions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. - We do not want to encourage the use of unmaintained software likely to contain security vulnerabilities, and we do not have the bandwidth to maintain packages deprecated by upstream. Nothing is lost though, because we have complete binary cache coverage of old nixpkgs versions, providing a comparatively easy way to pin old very package versions. - This change is a general improvement in the ecosystem even if we do not end up using it to remove any packages. -- There is a slight long-term maintenance burden. It is expected to be similar to or slightly greater than the maintenance of our deprecation aliases. - - We expect that in the long term, having a defined process for removing unmaintained and obsolete packages, especially compared to deciding on a case-by-case basis, is likely to reduce the overall maintenance burden. -- Some of the example interactions are built on the premise that parts of nixpkgs are under-maintained, and that most users are at least somewhat involved in the nixpkgs development process. At the time of writing this RFC this is most certainly true, but the effects on this in the future are unknown. - - We hope that drawing attention to packages in need of maintenance can encourage new maintainers -- both from the existing pool of nixpkgs contributors and from non-contributor users -- to step up. ## Alternatives [alternatives]: #alternatives +This project has gone through multiple iterations, and grown quite in scope during that. Therefore this section doubles a chronology of previous attempts of implementing it. + +Alternative to the current solution, we could just continue to add new systems to `check-meta.nix` when specific use cases arise. The downside is that they are mostly similar yet not 100% consistent, resulting in both code duplication and user confusion. + +The original proposal only wanted to deal with eval-breaking problems, and grew in scope since. Removing warnings from the equation makes the implementation simpler, however we would then miss out on things like tracing aliases. Maybe we should have a distinct mechanism for evaluation warnings. + +The following sections discuss alternative ways of implementing the current feature proposal. + +### Naming + +A lot of names have been considered so far for this feature, currently "problems" is the "least bad" proposal. + +- Problem: Extremely generic "kind of fits all use cases but not that well" word +- Issue: Colliding with GitHub issues +- Warning: People might expect that it would never break evaluation +- Error: This is too harsh of a word for things that are minor issues, like unmaintained packages +- Advisory: Too focused on security issues + +Consider that the name should work as well as possible for all of the following cases, even if there are no current plans to replace all of these with our new feature: "removal", "deprecated", "maintainerless", "insecure", "broken", "unsupported", "unfree". + +### Problem declaration + +*n.b.: the terminology of the feature has multiple times since the earlier proposals were made* + An alternative design would be to have issues as a separate list (not part of the package). ~~Instead of allowing individual packages, one could ignore individual warnings (they'd need an identifying number for that). The advantage of doing this is that one could have one issue and apply it for a lot of packages (e.g. "Python 2 is deprecated"). The main drawback there is that it is more complex.~~ The advantages of that approach have been integrated while keeping the downsides small: Warnings are ignored with a per-kind granularity, but one may give some of them a name to allow finer control where necessary. A few other sketches about how the declaration syntax might look like in different scenarios: @@ -148,52 +216,68 @@ A few other sketches about how the declaration syntax might look like in differe } ``` +Some more design options that were considered: + +```nix +meta.problems = { + "deprecated/python2-eol" = { + message = "This package depends on Python 2, which has reached end of life."; + date = "1970-01-01"; + urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; + }; + removal = { + message = "The application has been abandoned upstream, use libfoo instead"; + date = "1970-01-01"; + }; +}; + +meta.problems = { + "deprecated" = [{ + name = "python2-eol"; + message = "This package depends on Python 2, which has reached end of life."; + date = "1970-01-01"; + urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; + }]; + removal = { + message = "The application has been abandoned upstream, use libfoo instead"; + date = "1970-01-01"; + }; +}; +``` + +These have the advantage of enforcing the presence of a name if there are multiple problems of the same kind, at the cost of some additional nesting in those cases. + +### Problem resolution + +On the nixpkgs configuration side, the first iteration used a generic "predicate" system for ignoring packages, similar to `allowUnfreePredicate` and `allowInsecurePredicate`. This turned out to be both too flexible and not convenient enough to use, so this was complemented with a list of packages to ignore and "smart" default values generation. + +A second approach used a list type: `list of ("packageName.warningKind" or "packageName.*" or "*.warningKind" or "*.*")`. This was a binary choice (compared to the ternary value today), with an additional boolean `traceIgnoredWarnings` option. One downside is that it does not allow granular control over warnings, only evaluation failures. A bigger issue is that due to how the merge rules on lists work, it would have been difficult to provide good default values for the nixpkgs confinguration while keeping backwards compatibility. + ## Unresolved questions [unresolved]: #unresolved-questions - ~~From above: "Ignoring a package without issues (i.e. they have all been resolved) results in a warning at evaluation time". How could this be implemented, and efficiently?~~ - ~~More generally, how do we tell users that their ignored warning can be removed, so that they won't accidentally miss future warnings?~~ - ~~Issues have a `resolved` attribute that may be used for that purpose.~~ - - Properly implementing this turned out to be non-trivial, so this feature was cut for the sake of simplicity as it was not of hight importance anyways. + - Properly implementing this turned out to be non-trivial, so this feature was cut for the sake of simplicity as it was not of high importance anyways. - The ignore mechanism has been refined so that there is less risk of missing future warnings. - ~~Should issues be a list or an attrset?~~· - We are using a list ~~for now, there is always the possibility to also allow attrsets in the future.~~ -- ~~A lot of bike shedding. (See below)~~ - - Packages may have "issues" that generate "warnings" that have to be "ignored". Issues are explicitly added to packages in `meta.issues`, whereas warnings can be generated automatically from other sources, like other `meta` attributes. +- Currently, many of the relevant nixpkgs configuration options can also be set impurely via environment variables. The `config.problemHandler` option does however not easily map to some environment variable. + - When merging existing features into the problems system, existing environment variable will keep working in the future. + - Maybe using less environment variables is all for the better? + - We may always add specific environment variables for specific use cases where needed without having to expose the full expression power of `config.problemHandler`. ## Future work [future]: #future-work -- Issues are designed in a way that they supersede a lot of our "insecure"/"unfree"/"unsupported" packages infrastructure. There is a lot of code duplication between them. In theory, we could migrate some of these to make use of issues. At the very least, we hope that issues are general enough so that no new similar features will have to be added in the future anymore. - - `meta.knownVulnerabilities` is the first candidate to go - - Unfree package handling will probably be out of scope, since we already have some custom filtering based on licences. -- Inspired by the automation of aliases, managing issues can be helped by tooling as well. This is deemed out of scope of this RFC because only real world usage will tell which actions will be worthwhile automating, but it should definitely considered in the future. - - There will likely be need for tooling that lists issues on all nixpkgs packages, filtered by kind or sorted chronologically. +- The actual process of removing packages is only sketched out here to show how the new infrastructure may improve the situation. It is intentionally left as vague as possible, and details should be figured out in a follow-up discussion. +- The problems system is designed in a way that it supersedes a lot of our "insecure"/"unfree"/"unsupported" packages infrastructure. There is a lot of code duplication between them. In theory, we could migrate some of these to make use of problems. At the very least, we hope that problems are general enough so that no new similar features will have to be added in the future anymore. + - Migrating the existing systems may end up being tricky due to backwards compatibility issues. +- Inspired by the automation of aliases, we could build tooling for this as well. This is deemed out of scope of this RFC because only real world usage will tell which actions will be worthwhile automating, but it should definitely be considered in the future. + - There will likely be need for tooling that lists problems on all nixpkgs packages, filtered by kind or sorted chronologically. - Automatically removing packages based on time will likely require providing more information whether it is safe to do so or not. - > If the advisories were a list, and we also added them for modules, maybe we could auto-generate most release notes, and move release notes closer to the code they change. [[source]](https://discourse.nixos.org/t/pre-rfc-package-advisories/19509/4) - Issues can certainly be automatically integrated into the release notes somehow. However, this alone would not allow us to move most of our release notes into the packages, because for many release entries breaking eval would be overkill. +- There has been discussion about introducing "tracing aliases", aliases that don't `throw` by default. Such an implementation may make use of the problem infrastructure. -## Bike shedding - -*Note: the bike shedding phase is considered over.* - -Here are a few naming proposals, and how well they would be suited to describe different conditions. "broken" and "unsupported" must be acknowledged but – unlike the others – don't imply some required user action. "unfree" is somewhat out of scope because it is unlikely to be replaced anytime soon. - -| Kind | Currently | "Issue" | "Warning" | "Problem" | "Advisory" | -|-------------|-----------|---------|-----------|-----------|------------| -|insecure | `meta.knownVulnerabilities` |✅|✅|✅|✅| -|unmaintained | `meta.maintainers = []` |✅|✅|❓|❓| -|deprecated | n/a |✅|✅|✅|❓| -|removal | n/a |❓|✅|✅|❓| -||||||| -|broken | `meta.broken` |✅|❌|✅|❌| -|unsupported | `meta.platforms` |✅|❓|✅|❌| -||||||| -|unfree | `meta.license` |✅|✅|❌|❓| - - -"Advisory" was initially chosen based on the notion of security advisories, but was later dismissed as the project grew in scope. "Issue" and "Problem" are similar words, of which the former is a well-known technical term¹ which should be preferred here. - -"Issue" and "Warning" are both good candidates, of which the former implies some required action whereas the latter merely wants to inform. In the end, we decided that packages should have *issues* which should produce *warnings* that can be *ignored*. While this distinction may be a bit unintuitive, it will make it easier to generate warnings from things that are not explicitly marked as issues (e.g. missing maintainers). - -¹ Non-native speakers: look up the difference between "issue" and "problem" in English :) From 81564e67693d2cefe2be361fe041b1a5bae1baf8 Mon Sep 17 00:00:00 2001 From: piegames Date: Sat, 21 Jan 2023 11:28:36 +0100 Subject: [PATCH 18/25] Rename throw->error, trace->warn --- rfcs/0127-issues-warnings.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 5825b14af..d0b22caea 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -13,11 +13,11 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/177272 ## Summary [summary]: #summary -Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problemHandler` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "throw" (fail evaluation), "trace" (print a warning message) or "ignore" (do nothing). +Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problemHandler` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "error" (fail evaluation), "warn" (print a trace message) or "ignore" (do nothing). Additionally, `meta.problems` is added to derivations, which can be used to manually declare that a package has a certain problem. This will then be used to inform users about packages that are in need of maintenance, for example security vulnerabilities or deprecated dependencies. -Using the newly introduced features, we may create a process for removing packages from nixpkgs that is easier to maintain and friendlier to users than just replacing packages with `throw`. +Using the newly introduced features, we may create a process for removing packages from nixpkgs that is easier to maintain and friendlier to users than just replacing packages with `throw` directly. ## Motivation [motivation]: #motivation @@ -82,13 +82,13 @@ Not all values make sense for declaration in `meta.problems`: Some may be automa The following new config option is added to nixpkgs: `config.problemHandler`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. -Handler values can be either `"throw"`, `"trace"` or `"ignore"`. Future values may be added in the future. The key is of the form `packageName.problemKind` or `packageName.problemName`, where `"*"` is allowed on either level as a wildcard. +Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Future values may be added in the future. The key is of the form `packageName.problemKind` or `packageName.problemName`, where `"*"` is allowed on either level as a wildcard. ```nix problemHandler = { "*" = { - "*" = "throw"; - alias = "trace"; + "*" = "error"; + alias = "warn"; maintainerless = "ignore"; }; myPackage.foo = "ignore"; @@ -110,9 +110,9 @@ The default value in nixpkgs might be something like: ```nix problemHandler."*" = { - "*" = "trace"; - removal = "throw"; - deprecated = "throw"; + "*" = "warn"; + removal = "error"; + deprecated = "error"; maintainerless = "ignore"; }; ``` @@ -136,7 +136,7 @@ New problems generally should not be added to stable branches if possible, and a ### Removal of packages -The plan is to eventually remove packages with long outstanding problems. The details will be part of future work, but at the very least a package must have a problem whose kind defaults to "throw" for at least one full release cycle (so that stable users have sufficient time to be warned and intervene). +The plan is to eventually remove packages with long outstanding problems. The details will be part of future work, but at the very least a package must have a problem whose kind defaults to "error" for at least one full release cycle (so that stable users have sufficient time to be warned and intervene). If a package needs to be removed for some other reason, the problem kind `removal` should be used instead: From c5c64ecbf71fabb3a9c1fb1cd9aa708004a81134 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 31 Jan 2023 21:13:50 +0100 Subject: [PATCH 19/25] Make meta.problems an attrset --- rfcs/0127-issues-warnings.md | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index d0b22caea..fb2351d2b 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -33,36 +33,38 @@ For some use cases, like for packages without maintainers, we do not want to bre ### Package problems -A new attribute is added to the `meta` section of a package: `problems`. If present, it is a list of attrsets which each have at least the following fields: +A new attribute is added to the `meta` section of a package: `problems`. If present, it is a an attribute set of attrsets which each have at least the following fields: -- `kind`: Required. If present, the resulting warning will be printed as `kind: message`. +- `kind`: Required. The resulting warning will be printed as `kind: message`. Defaults to the attrset key if absent. - `message`: Required. A string message describing the issue with the package. The value should: - Start with the "This package", "The application" or equivalent, or simply with the package name. - Be capitalized (unless it starts with the package name). - Use a period at the end. -- `name`: Required if there are multiple values of the same `kind`. Give the issue a custom name for more easy filtering +- `name`: Inferred from the attrset key if `kind` is specified. Give the issue a custom name for more easy filtering. - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. Other attributes are allowed. Some message kinds may specify additional required attributes. +The keys in the attribute set are used to infer the `name` or `kind` values within, depending on which is present. Example values: ```nix -meta.problems = [ - { - name = "python2-eol"; +meta.problems = { + # This one will have the name "python2-eol" + python2-eol = { kind = "deprecated"; message = "This package depends on Python 2, which has reached end of life."; date = "1970-01-01"; urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; - } - { - kind = "removal"; + }; + # This one will have no name + removal = { + # kind = "removal"; # Inferred from attribute key message = "The application has been abandoned upstream, use libfoo instead"; date = "1970-01-01"; - } -]; + }; +}; ``` ### Problem kinds @@ -191,7 +193,7 @@ A few other sketches about how the declaration syntax might look like in differe ```nix { - # As proposed in the RFC + # Using a list instead of attribute set. Slightly less complexity, but also slightly more verbose. meta.issues = [{ kind = "deprecated"; name = "python2-eol"; From 2a78241fda56d40dcea21b2c44c89a8251183d2d Mon Sep 17 00:00:00 2001 From: piegames Date: Fri, 28 Apr 2023 23:26:23 +0200 Subject: [PATCH 20/25] Rewrite *again*, most change is in the configuration options --- rfcs/0127-issues-warnings.md | 85 +++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index fb2351d2b..c4b6a6f03 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -13,7 +13,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/177272 ## Summary [summary]: #summary -Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problemHandler` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "error" (fail evaluation), "warn" (print a trace message) or "ignore" (do nothing). +Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problemHandler` and `problemHandlerDefaultMatchers` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "error" (fail evaluation), "warn" (print a trace message) or "ignore" (do nothing). Additionally, `meta.problems` is added to derivations, which can be used to manually declare that a package has a certain problem. This will then be used to inform users about packages that are in need of maintenance, for example security vulnerabilities or deprecated dependencies. @@ -82,45 +82,47 @@ Not all values make sense for declaration in `meta.problems`: Some may be automa ### Nixpkgs configuration -The following new config option is added to nixpkgs: `config.problemHandler`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. +The following new config options are added to nixpkgs: `config.problemHandler` and `config.problemHandlerDefaultMatchers`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. -Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Future values may be added in the future. The key is of the form `packageName.problemKind` or `packageName.problemName`, where `"*"` is allowed on either level as a wildcard. +Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Future values may be added in the future. + +`config.problemHandlers` is the simple and most user-facing configuration value. It is a simple doubly-nested attribute set, of style `pkgName.problemName`. ```nix -problemHandler = { - "*" = { - "*" = "error"; - alias = "warn"; - maintainerless = "ignore"; - }; - myPackage.foo = "ignore"; -}; +config.problemHandlers = { + myPackage.maintainerless = "warn"; + otherPackage.CVE1234 = "ignore"; +} ``` -If multiple rules match a given problem of a package, the most specific handler will be called: - -1. `pkgName.problemName` -2. `pkgName.problemKind` -3. `pkgName."*"` -4. `"*".problemName` -5. `"*".problemKind` -6. `"*"."*"` - -So for example the value of `myPackage.*` would override the one in `*.maintainerless` if both matched, as it is more specific. - -The default value in nixpkgs might be something like: +Some times, there is the need to set the handler for multiple problems on a package, or for one problem kind across all packages. For this, `config.problemHandlerDefaultMatchers` exists. It is a list of matchers (currently package name, problem name or problem kind), with an associated handler. If multiple matchers match a single package, the one with the highest level (error > warn > ignore) will be used. Since the user configuration is merged with the default configuration, this means that one can not decrease the handler level below the default value. This is to protect users against accidentally disabling entire classes of notifications. Values from `config.problemHandlers` take precedence. ```nix -problemHandler."*" = { - "*" = "warn"; - removal = "error"; - deprecated = "error"; - maintainerless = "ignore"; -}; +config.problemHandlerDefaultMatchers = [ + { # Wildcard matcher for everything + handler = "warn"; + } + { # Match all security warnings on "hello" + package = "hello"; + kind = "insecure"; + handler = "error"; + } + { # Match all packages affected by the python2 deprecation + name = "python2-eol"; + handler = "error"; + } + { # Has no effect: the default value is higher + kind = "insecure"; + handler = "ignore"; + } + { # Disallowed: use problemHandlers instead + pkgName = "hello"; + name = "CVE1234"; + handler = "ignore"; + } +] ``` -It may also be expanded by values from other configuration options as part of a migration scheme from the other mechanisms. - ## Examples and Interactions [examples-and-interactions]: #examples-and-interactions @@ -130,8 +132,6 @@ When a package has a problem that `throw`s, all packages that depend on it will When the problem requires actions on dependents however, it does not sufficiently inform about all packages that need action. Multiple packages may be annotated with the same problem, in that case it should be given a name and the name should be the same across all instances. Other values like the message or the URL list do not need to be the same and may be adapted sensibly. -For the example of Python 2 deprecation, all problems would have `name = "python2-eol"` and then a user may set `config."*".python2-eol = "ignore";` to ignore them. - ### Backwards compatbility, Backporting and stable releases New problems generally should not be added to stable branches if possible, and also not be backported to them, since it may break evaluation. The same rule applies to other changes to a pacakge's `meta` which may generate a problem and thus lead to evaluation failure too. Scenarios where evaluation failure is a desired goal, for example with unfixable security issues, are obviously exempt from this. @@ -143,11 +143,12 @@ The plan is to eventually remove packages with long outstanding problems. The de If a package needs to be removed for some other reason, the problem kind `removal` should be used instead: ```nix -meta.problems = [{ - kind = "removal"; - message = "We don't want this in nixpkgs anymore"; - date = "1970-01-01"; -}]; +meta.problems = { + removal = { + message = "We don't want this in nixpkgs anymore"; + date = "1970-01-01"; + }; +}; ``` ## Drawbacks @@ -255,6 +256,8 @@ On the nixpkgs configuration side, the first iteration used a generic "predicate A second approach used a list type: `list of ("packageName.warningKind" or "packageName.*" or "*.warningKind" or "*.*")`. This was a binary choice (compared to the ternary value today), with an additional boolean `traceIgnoredWarnings` option. One downside is that it does not allow granular control over warnings, only evaluation failures. A bigger issue is that due to how the merge rules on lists work, it would have been difficult to provide good default values for the nixpkgs confinguration while keeping backwards compatibility. +A third approach used a two-level attribute set, similarly to the list above, but with the value setting the handler instead of it being a binary choice. `"*"."*" = "error";`, `myPackage."*" = "ignore";`, `"*".maintainerless = "ignore";`, etc. This provides better merging behavior than a list, while also being more granular. However, people voiced concerns about people accidentally wildcard-ignoring everything. Also, it is ambiguous on matching problem name vs problem kind, which mostly works fine but might lead to weird things happening in the case where new kinds are introduced. + ## Unresolved questions [unresolved]: #unresolved-questions @@ -264,11 +267,12 @@ A second approach used a list type: `list of ("packageName.warningKind" or "pack - Properly implementing this turned out to be non-trivial, so this feature was cut for the sake of simplicity as it was not of high importance anyways. - The ignore mechanism has been refined so that there is less risk of missing future warnings. - ~~Should issues be a list or an attrset?~~· - - We are using a list ~~for now, there is always the possibility to also allow attrsets in the future.~~ + - ~~We are using a list for now, there is always the possibility to also allow attrsets in the future.~~ + - Current design uses an attribute set - Currently, many of the relevant nixpkgs configuration options can also be set impurely via environment variables. The `config.problemHandler` option does however not easily map to some environment variable. - When merging existing features into the problems system, existing environment variable will keep working in the future. - Maybe using less environment variables is all for the better? - - We may always add specific environment variables for specific use cases where needed without having to expose the full expression power of `config.problemHandler`. + - We may always add specific environment variables for specific use cases where needed without having to expose the full expression power of the configuration options. ## Future work [future]: #future-work @@ -282,4 +286,3 @@ A second approach used a list type: `list of ("packageName.warningKind" or "pack - > If the advisories were a list, and we also added them for modules, maybe we could auto-generate most release notes, and move release notes closer to the code they change. [[source]](https://discourse.nixos.org/t/pre-rfc-package-advisories/19509/4) - Issues can certainly be automatically integrated into the release notes somehow. However, this alone would not allow us to move most of our release notes into the packages, because for many release entries breaking eval would be overkill. - There has been discussion about introducing "tracing aliases", aliases that don't `throw` by default. Such an implementation may make use of the problem infrastructure. - From 4f311ded34b215ede107fad52a4c7c2c0bc40522 Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 15 May 2023 22:17:35 +0200 Subject: [PATCH 21/25] Review update (WIP) --- rfcs/0127-issues-warnings.md | 42 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index c4b6a6f03..ed0163e76 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -35,17 +35,15 @@ For some use cases, like for packages without maintainers, we do not want to bre A new attribute is added to the `meta` section of a package: `problems`. If present, it is a an attribute set of attrsets which each have at least the following fields: -- `kind`: Required. The resulting warning will be printed as `kind: message`. Defaults to the attrset key if absent. +- `kind`: The 'kind' of the problem, see [problem kinds](#problem-kinds) for allowed values. Defaults to the attribute set key. - `message`: Required. A string message describing the issue with the package. The value should: - Start with the "This package", "The application" or equivalent, or simply with the package name. - Be capitalized (unless it starts with the package name). - Use a period at the end. -- `name`: Inferred from the attrset key if `kind` is specified. Give the issue a custom name for more easy filtering. - `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. -Other attributes are allowed. Some message kinds may specify additional required attributes. -The keys in the attribute set are used to infer the `name` or `kind` values within, depending on which is present. +Problem kinds may specify additional attributes. Apart from that, other attributes are not allowed. Example values: @@ -58,10 +56,10 @@ meta.problems = { date = "1970-01-01"; urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; }; - # This one will have no name + # This one will have the name "removal" removal = { # kind = "removal"; # Inferred from attribute key - message = "The application has been abandoned upstream, use libfoo instead"; + message = "The application has been abandoned upstream, use libfoo instead."; date = "1970-01-01"; }; }; @@ -73,12 +71,12 @@ At the moment, the following values for the `kind` field of a warning are known: - `removal`: The package is scheduled for removal some time in the future. - `deprecated`: The package has been abandoned upstream or has end of life dependencies. -- `maintainerless`: `meta.maintainers` is empty -- `insecure`: The package has some security vulnerabilities -- `broken`: The package is marked as broken -- `unsupported`: The package is not expected to build on this platform +- `maintainerless`: `meta.maintainers` is empty. Cannot be manually declared in `meta.problems`. +- `insecure`: (Reserved for future use.) The package has some security vulnerabilities. +- `broken`: (Reserved for future use.) The package is broken in some way. +- `unsupported`: (Reserved for future use.) The package is not expected to build on this platform. -Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems, and therefore also no need to give them a name in order to distinguish them. +Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems. ### Nixpkgs configuration @@ -86,7 +84,7 @@ The following new config options are added to nixpkgs: `config.problemHandler` a Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Future values may be added in the future. -`config.problemHandlers` is the simple and most user-facing configuration value. It is a simple doubly-nested attribute set, of style `pkgName.problemName`. +`config.problemHandlers` is the simple and most user-facing configuration value. It is a simple doubly-nested attribute set, of style `pkgName.problemName`. The package name is taken from `lib.getName`, which currently yields its `pname` attribute. ```nix config.problemHandlers = { @@ -115,14 +113,22 @@ config.problemHandlerDefaultMatchers = [ kind = "insecure"; handler = "ignore"; } - { # Disallowed: use problemHandlers instead - pkgName = "hello"; + { # Disallowed: Non-wildcards are better handled by using `problemHandlers` instead + # The equivalent would be: `config.problemHandlers.hello.CVE1234 = "error"; + package = "hello"; name = "CVE1234"; - handler = "ignore"; + handler = "error"; } ] ``` +To make it more explicit, a matches is an attribute set which may contain the following fields: + +- `package`: Match only problems of a specified package. +- `kind`: Match only problems of a specific kind. +- `name`: Match only problems with a specific name. +- `handler`: Required. Sets the level for packages that have been matched by this matcher. + ## Examples and Interactions [examples-and-interactions]: #examples-and-interactions @@ -134,18 +140,18 @@ When the problem requires actions on dependents however, it does not sufficientl ### Backwards compatbility, Backporting and stable releases -New problems generally should not be added to stable branches if possible, and also not be backported to them, since it may break evaluation. The same rule applies to other changes to a pacakge's `meta` which may generate a problem and thus lead to evaluation failure too. Scenarios where evaluation failure is a desired goal, for example with unfixable security issues, are obviously exempt from this. +New problems generally should not be added to stable branches if possible, and also not be backported to them, since it may break evaluation. The same rule applies to other changes to a package or its `meta` which may generate a problem and thus lead to evaluation failure too. Scenarios where evaluation failure is a desired goal, for example with unfixable security issues, are obviously exempt from this. ### Removal of packages -The plan is to eventually remove packages with long outstanding problems. The details will be part of future work, but at the very least a package must have a problem whose kind defaults to "error" for at least one full release cycle (so that stable users have sufficient time to be warned and intervene). +The plan is to eventually remove packages with long outstanding problems of some kinds. The details will be part of future work, but users will be warned sufficiently in advance to give them the chance to intervene. If a package needs to be removed for some other reason, the problem kind `removal` should be used instead: ```nix meta.problems = { removal = { - message = "We don't want this in nixpkgs anymore"; + message = "This package will be removed from Nixpkgs."; date = "1970-01-01"; }; }; From b253df19507b88f4da2c1fcfbae4a45064297440 Mon Sep 17 00:00:00 2001 From: piegames Date: Tue, 30 May 2023 20:07:56 +0200 Subject: [PATCH 22/25] Review update --- rfcs/0127-issues-warnings.md | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index ed0163e76..2fef33bc4 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -13,7 +13,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/177272 ## Summary [summary]: #summary -Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problemHandler` and `problemHandlerDefaultMatchers` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "error" (fail evaluation), "warn" (print a trace message) or "ignore" (do nothing). +Inspired by the various derivation checks like for broken and insecure packages, a new system called "problems" is introduced. It is planned to eventually replace the previously mentioned systems where possible, as well as the current – undocumented – "warnings" (which currently only prints a trace message for unmaintained packages). A `config.problems.handlers` and `config.problems.matchers` option is added to the nixpkgs configuration, with centralized and granular control over how to handle problems that arise: "error" (fail evaluation), "warn" (print a trace message) or "ignore" (do nothing). Additionally, `meta.problems` is added to derivations, which can be used to manually declare that a package has a certain problem. This will then be used to inform users about packages that are in need of maintenance, for example security vulnerabilities or deprecated dependencies. @@ -28,6 +28,8 @@ Let's take the end of life of Python 2 as an example. (This applies to other eco For some use cases, like for packages without maintainers, we do not want to break evaluation of a package and simply warn the user instead. We want the users to configure all these according to their needs and through a single standardized interface. +Nixpkgs already has a couple of mechanisms for doing these kind of things, among others for unfree, insecure or broken packages. However these have been added pretty much ad-hoc over time, and while being similar to each other also may have subtle differences. (For example some allow to provide a predicate for more granular control, others don't.) Usually, adding a new such mechanism (recently done with the source provenance in RFC 89) involves copying one of the existing ones and making adaptations. We need to generalize this concept in a way that is extensible towards new use cases, and to provide a more standardized API for configuration. + ## Detailed design [design]: #detailed-design @@ -76,27 +78,27 @@ At the moment, the following values for the `kind` field of a warning are known: - `broken`: (Reserved for future use.) The package is broken in some way. - `unsupported`: (Reserved for future use.) The package is not expected to build on this platform. -Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems. +Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems. Restrictions like these may be implemented in the metadata check of packages. ### Nixpkgs configuration -The following new config options are added to nixpkgs: `config.problemHandler` and `config.problemHandlerDefaultMatchers`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. +The following new config options are added to nixpkgs: `config.problems.handlers` and `config.problems.matchers`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Future values may be added in the future. -`config.problemHandlers` is the simple and most user-facing configuration value. It is a simple doubly-nested attribute set, of style `pkgName.problemName`. The package name is taken from `lib.getName`, which currently yields its `pname` attribute. +`config.problems.handlers` is the simple and most user-facing configuration value. It is a simple doubly-nested attribute set, of style `pkgName.problemName`. The package name is taken from `lib.getName`, which currently yields its `pname` attribute. ```nix -config.problemHandlers = { +config.problems.handlers = { myPackage.maintainerless = "warn"; otherPackage.CVE1234 = "ignore"; } ``` -Some times, there is the need to set the handler for multiple problems on a package, or for one problem kind across all packages. For this, `config.problemHandlerDefaultMatchers` exists. It is a list of matchers (currently package name, problem name or problem kind), with an associated handler. If multiple matchers match a single package, the one with the highest level (error > warn > ignore) will be used. Since the user configuration is merged with the default configuration, this means that one can not decrease the handler level below the default value. This is to protect users against accidentally disabling entire classes of notifications. Values from `config.problemHandlers` take precedence. +Some times, there is the need to set the handler for multiple problems on a package, or for one problem kind across all packages. For this, `config.problems.matchers` exists. It is a list of matchers (currently package name, problem name or problem kind), with an associated handler. If multiple matchers match a single package, the one with the highest level (error > warn > ignore) will be used. Since the user configuration is merged with the default configuration, this means that one can not decrease the handler level below the default value. This is to protect users against accidentally disabling entire classes of notifications. Values from `config.problems.handlers` take precedence. ```nix -config.problemHandlerDefaultMatchers = [ +config.problems.matchers = [ { # Wildcard matcher for everything handler = "warn"; } @@ -113,8 +115,8 @@ config.problemHandlerDefaultMatchers = [ kind = "insecure"; handler = "ignore"; } - { # Disallowed: Non-wildcards are better handled by using `problemHandlers` instead - # The equivalent would be: `config.problemHandlers.hello.CVE1234 = "error"; + { # Disallowed: Non-wildcards are better handled by using `problems.handlers` instead + # The equivalent would be: `config.problem.handlers.hello.CVE1234 = "error"; package = "hello"; name = "CVE1234"; handler = "error"; @@ -134,7 +136,7 @@ To make it more explicit, a matches is an attribute set which may contain the fo ### Propagation across transitive dependencies -When a package has a problem that `throw`s, all packages that depend on it will fail to evaluate until that problem is ignored or resolved. Most of the time, this is sufficient. +When a package has a problem that `error`s, all packages that depend on it will fail to evaluate until that problem is ignored or resolved. Most of the time, this is sufficient. When the problem requires actions on dependents however, it does not sufficiently inform about all packages that need action. Multiple packages may be annotated with the same problem, in that case it should be given a name and the name should be the same across all instances. Other values like the message or the URL list do not need to be the same and may be adapted sensibly. @@ -144,9 +146,7 @@ New problems generally should not be added to stable branches if possible, and a ### Removal of packages -The plan is to eventually remove packages with long outstanding problems of some kinds. The details will be part of future work, but users will be warned sufficiently in advance to give them the chance to intervene. - -If a package needs to be removed for some other reason, the problem kind `removal` should be used instead: +If a package needs to be removed for some reason (most likely due to outstanding unresolved problems), the problem kind `removal` should be added: ```nix meta.problems = { @@ -154,9 +154,12 @@ meta.problems = { message = "This package will be removed from Nixpkgs."; date = "1970-01-01"; }; + # Probably some more problems here }; ``` +The plan is to eventually remove packages with long outstanding problems of some kinds. The details will be part of future work, but users will be warned sufficiently in advance to give them the chance to intervene: Before removing a package, it should have a `removal` annotation for at least one full release cycle. + ## Drawbacks [drawbacks]: #drawbacks @@ -275,7 +278,7 @@ A third approach used a two-level attribute set, similarly to the list above, bu - ~~Should issues be a list or an attrset?~~· - ~~We are using a list for now, there is always the possibility to also allow attrsets in the future.~~ - Current design uses an attribute set -- Currently, many of the relevant nixpkgs configuration options can also be set impurely via environment variables. The `config.problemHandler` option does however not easily map to some environment variable. +- Currently, many of the relevant nixpkgs configuration options can also be set impurely via environment variables. The `config.problems.handler` option does however not easily map to some environment variable. - When merging existing features into the problems system, existing environment variable will keep working in the future. - Maybe using less environment variables is all for the better? - We may always add specific environment variables for specific use cases where needed without having to expose the full expression power of the configuration options. @@ -283,7 +286,7 @@ A third approach used a two-level attribute set, similarly to the list above, bu ## Future work [future]: #future-work -- The actual process of removing packages is only sketched out here to show how the new infrastructure may improve the situation. It is intentionally left as vague as possible, and details should be figured out in a follow-up discussion. +- The actual process of removing packages is only sketched out here to show how the new infrastructure may improve the situation. It is intentionally left as open as possible, and details should be figured out in a follow-up discussion. - The problems system is designed in a way that it supersedes a lot of our "insecure"/"unfree"/"unsupported" packages infrastructure. There is a lot of code duplication between them. In theory, we could migrate some of these to make use of problems. At the very least, we hope that problems are general enough so that no new similar features will have to be added in the future anymore. - Migrating the existing systems may end up being tricky due to backwards compatibility issues. - Inspired by the automation of aliases, we could build tooling for this as well. This is deemed out of scope of this RFC because only real world usage will tell which actions will be worthwhile automating, but it should definitely be considered in the future. @@ -291,4 +294,4 @@ A third approach used a two-level attribute set, similarly to the list above, bu - Automatically removing packages based on time will likely require providing more information whether it is safe to do so or not. - > If the advisories were a list, and we also added them for modules, maybe we could auto-generate most release notes, and move release notes closer to the code they change. [[source]](https://discourse.nixos.org/t/pre-rfc-package-advisories/19509/4) - Issues can certainly be automatically integrated into the release notes somehow. However, this alone would not allow us to move most of our release notes into the packages, because for many release entries breaking eval would be overkill. -- There has been discussion about introducing "tracing aliases", aliases that don't `throw` by default. Such an implementation may make use of the problem infrastructure. +- There has been discussion about introducing "tracing aliases"; aliases that don't `throw` by default. Such an implementation may make use of the problem infrastructure. From 7939f49d7a9c716cbf80d7e1a9f75eb3deed4cd7 Mon Sep 17 00:00:00 2001 From: piegames Date: Thu, 1 Jun 2023 16:02:15 +0200 Subject: [PATCH 23/25] Update shepherds list --- rfcs/0127-issues-warnings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index 2fef33bc4..ea070f444 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -3,7 +3,7 @@ feature: issues-warnings start-date: 2022-06-11 author: piegames co-authors: — -shepherd-team: @lheckemann, @mweinelt, @fgaz +shepherd-team: @RaitoBezarius, @mweinelt, @infinisil shepherd-leader: @mweinelt related-issues: https://github.com/NixOS/nixpkgs/pull/177272 --- From 33ec9a67298780f7b5addfec18835d79a3c8af9e Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 19 Jun 2023 19:35:53 +0200 Subject: [PATCH 24/25] Meeting update --- rfcs/0127-issues-warnings.md | 41 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index ea070f444..a097ae7d3 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -35,14 +35,13 @@ Nixpkgs already has a couple of mechanisms for doing these kind of things, among ### Package problems -A new attribute is added to the `meta` section of a package: `problems`. If present, it is a an attribute set of attrsets which each have at least the following fields: +A new attribute is added to the `meta` section of a package: `problems`. If present, it is a an attribute set of attrsets where the keys are the problem names and the values have at least the following fields each: - `kind`: The 'kind' of the problem, see [problem kinds](#problem-kinds) for allowed values. Defaults to the attribute set key. - `message`: Required. A string message describing the issue with the package. The value should: - Start with the "This package", "The application" or equivalent, or simply with the package name. - Be capitalized (unless it starts with the package name). - Use a period at the end. -- `date`: Required. An ISO 8601 `yyyy-mm-dd`-formatted date from when the issue was added. - `urls`: Optional, list of strings. Can be used to link issues, pull requests and other related items. Problem kinds may specify additional attributes. Apart from that, other attributes are not allowed. @@ -55,42 +54,50 @@ meta.problems = { python2-eol = { kind = "deprecated"; message = "This package depends on Python 2, which has reached end of life."; - date = "1970-01-01"; urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; }; # This one will have the name "removal" removal = { # kind = "removal"; # Inferred from attribute key message = "The application has been abandoned upstream, use libfoo instead."; - date = "1970-01-01"; }; }; ``` ### Problem kinds -At the moment, the following values for the `kind` field of a warning are known: +At the moment, the following values for the `kind` field of a problem are known: -- `removal`: The package is scheduled for removal some time in the future. +- `removal`: The package is scheduled for removal some time in the future. Only up to one instance per package. + - Packages are already being removed for various reasons on a regular basis. Now we can properly warn users about this in advance. - `deprecated`: The package has been abandoned upstream or has end of life dependencies. -- `maintainerless`: `meta.maintainers` is empty. Cannot be manually declared in `meta.problems`. + - Currently we do mass-pings for all maintainers of affected packages, although many packages are under-maintained. This allows to warn users who depend on that package as last resort stake holders. +- `maintainerless`: `meta.maintainers` is empty. Cannot be manually declared in `meta.problems`. Only up to one instance per package. + - This is currently provided by `config.showDerivationWarnings`, which is only used by Hydra. - `insecure`: (Reserved for future use.) The package has some security vulnerabilities. + - This will hopefully replace the existing insecure warnings (`meta.knownVulnerabilities`, `config.permittedInsecurePackages`) in the future (see Future work). - `broken`: (Reserved for future use.) The package is broken in some way. + - This will hopefully replace or enhance `meta.broken` in the future (see Future work). - `unsupported`: (Reserved for future use.) The package is not expected to build on this platform. + - This will hopefully replace `meta.platforms` and `meta.badPlatforms` in the future (see Future work). -Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). New kinds may be added in the future. Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems. Restrictions like these may be implemented in the metadata check of packages. +New kinds may be added in the future. + +Not all values make sense for declaration in `meta.problems`: Some may be automatically generated from other `meta` attributes (for example `maintainerless`). Furthermore, some kinds are expected to be present only up to once per derivation: for example, we have no use for having multiple `maintainerless` problems. Restrictions like these may be implemented in the metadata check of packages. ### Nixpkgs configuration -The following new config options are added to nixpkgs: `config.problems.handlers` and `config.problems.matchers`. The (currently undocumented) option `config.showDerivationWarnings` will be removed. +The following new config options are added to nixpkgs: `config.problems.handlers` and `config.problems.matchers`. The (currently undocumented) option `config.showDerivationWarnings` will eventually be removed. -Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Future values may be added in the future. +Handler values can be either `"error"`, `"warn"` or `"ignore"`. `"error"` maps to `throw`, `"warn"` maps to `trace`. Further values may be added in the future. `config.problems.handlers` is the simple and most user-facing configuration value. It is a simple doubly-nested attribute set, of style `pkgName.problemName`. The package name is taken from `lib.getName`, which currently yields its `pname` attribute. ```nix config.problems.handlers = { + # If "myPackage" is used (evaluated) somewhere and has a problem named "maintainerless", print a warning myPackage.maintainerless = "warn"; + # This was added because "otherPackage" has a problem "CVE1234" which prevents evaluation, which needs to be ignored to use it nevertheless ("warn" would work too of course) otherPackage.CVE1234 = "ignore"; } ``` @@ -126,7 +133,7 @@ config.problems.matchers = [ To make it more explicit, a matches is an attribute set which may contain the following fields: -- `package`: Match only problems of a specified package. +- `package`: Match only problems of a specified package. (Using `lib.getName` for the name, same as in `config.problems.handlers`) - `kind`: Match only problems of a specific kind. - `name`: Match only problems with a specific name. - `handler`: Required. Sets the level for packages that have been matched by this matcher. @@ -152,7 +159,6 @@ If a package needs to be removed for some reason (most likely due to outstanding meta.problems = { removal = { message = "This package will be removed from Nixpkgs."; - date = "1970-01-01"; }; # Probably some more problems here }; @@ -160,6 +166,12 @@ meta.problems = { The plan is to eventually remove packages with long outstanding problems of some kinds. The details will be part of future work, but users will be warned sufficiently in advance to give them the chance to intervene: Before removing a package, it should have a `removal` annotation for at least one full release cycle. +### Package declarations outside of Nixpkgs + +Since package checks are done via "check-meta" called by `mkDerviation`, these problems can also be declared and checked in third-party packages outside of Nixpkgs. +This is not always desirable though, since third-party packages do not necessarily need to abide by the standards in Nixpkgs (e.g. having maintainer fields). +Because of this, the implementation needs to ensure no warnings or errors get generated without a `meta` declaration to keep the noise for third-parties to a minimum. (The idea being that third-party packages commonly don't specify a `meta` attribute in the first place.) + ## Drawbacks [drawbacks]: #drawbacks @@ -167,6 +179,7 @@ The plan is to eventually remove packages with long outstanding problems of some - One idea I had is to be more verbose on the unstable channels, and then tune down the noise after branch-off. - New lints to packages should be introduced gradually, by making them "silent" by default on start and only going to "warn" after most problems in nixpkgs itself are resolved. - People have voiced strong negative opinions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work. + - We already do remove packages on a regular basis, so now at least we can properly warn people about it in advance. - We do not want to encourage the use of unmaintained software likely to contain security vulnerabilities, and we do not have the bandwidth to maintain packages deprecated by upstream. Nothing is lost though, because we have complete binary cache coverage of old nixpkgs versions, providing a comparatively easy way to pin old very package versions. - This change is a general improvement in the ecosystem even if we do not end up using it to remove any packages. @@ -234,12 +247,10 @@ Some more design options that were considered: meta.problems = { "deprecated/python2-eol" = { message = "This package depends on Python 2, which has reached end of life."; - date = "1970-01-01"; urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; }; removal = { message = "The application has been abandoned upstream, use libfoo instead"; - date = "1970-01-01"; }; }; @@ -247,12 +258,10 @@ meta.problems = { "deprecated" = [{ name = "python2-eol"; message = "This package depends on Python 2, which has reached end of life."; - date = "1970-01-01"; urls = [ "https://github.com/NixOS/nixpkgs/issues/148779" ]; }]; removal = { message = "The application has been abandoned upstream, use libfoo instead"; - date = "1970-01-01"; }; }; ``` From 83319ec43019edc5e1cee45942978c7f0ee75db9 Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 19 Jun 2023 20:47:21 +0200 Subject: [PATCH 25/25] Typos --- rfcs/0127-issues-warnings.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rfcs/0127-issues-warnings.md b/rfcs/0127-issues-warnings.md index a097ae7d3..a577b73c5 100644 --- a/rfcs/0127-issues-warnings.md +++ b/rfcs/0127-issues-warnings.md @@ -131,7 +131,7 @@ config.problems.matchers = [ ] ``` -To make it more explicit, a matches is an attribute set which may contain the following fields: +To make it more explicit, a matcher is an attribute set which may contain the following fields: - `package`: Match only problems of a specified package. (Using `lib.getName` for the name, same as in `config.problems.handlers`) - `kind`: Match only problems of a specific kind. @@ -168,14 +168,14 @@ The plan is to eventually remove packages with long outstanding problems of some ### Package declarations outside of Nixpkgs -Since package checks are done via "check-meta" called by `mkDerviation`, these problems can also be declared and checked in third-party packages outside of Nixpkgs. -This is not always desirable though, since third-party packages do not necessarily need to abide by the standards in Nixpkgs (e.g. having maintainer fields). -Because of this, the implementation needs to ensure no warnings or errors get generated without a `meta` declaration to keep the noise for third-parties to a minimum. (The idea being that third-party packages commonly don't specify a `meta` attribute in the first place.) +Since package checks are done via "check-meta" called by `mkDerivation`, these problems can also be declared and checked in third-party packages outside of Nixpkgs. +This is not always desirable though, since third-party packages do not necessarily need to abide by the standards of Nixpkgs (e.g. having maintainer fields). +Because of this, the implementation needs to ensure no warnings or errors get generated without a `meta` declaration to keep the noise for third-party packages to a minimum. (The idea being that third-party packages commonly don't specify a `meta` attribute in the first place.) ## Drawbacks [drawbacks]: #drawbacks -- Too much warnigns may cause Alarm fatigue +- Too much warnings may cause Alarm fatigue - One idea I had is to be more verbose on the unstable channels, and then tune down the noise after branch-off. - New lints to packages should be introduced gradually, by making them "silent" by default on start and only going to "warn" after most problems in nixpkgs itself are resolved. - People have voiced strong negative opinions about the prospect of removing packages from nixpkgs at all, especially when they still *technically* work.