From 8ab0f9138b8c062e2c1a1d118d6ba57ff9ba700c Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Tue, 2 Oct 2018 19:11:31 +0200 Subject: [PATCH 1/7] Initial draft for Default name From pname --- rfcs/0000-default-name-from-pname.md | 131 +++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 rfcs/0000-default-name-from-pname.md diff --git a/rfcs/0000-default-name-from-pname.md b/rfcs/0000-default-name-from-pname.md new file mode 100644 index 000000000..0e4df1a49 --- /dev/null +++ b/rfcs/0000-default-name-from-pname.md @@ -0,0 +1,131 @@ +--- +feature: default_name_from_pname +start-date: 2018-10-02 +author: Patrick Hilhorst (@Synthetica9) +co-authors: (find a buddy later to help our with the RFC) +related-issues: + - https://github.com/NixOS/nixpkgs/pull/41627 +--- + +# Summary +[summary]: #summary + +Whenever the `version` and `pname` attribute are both present, use +`"${pname}-${version}"` as the default value for `name` for `mkDerivation`. + +# Motivation +[motivation]: #motivation + +The string `"${pname}-${version}"` appears verbatim in the nixpkgs repo [627 +times](appendixA), at the time of this writing. Variants of this string appear +[746 times](appendixA), with the most common variant being `baseName`. This RFC +would reduce repetition in nixpkgs, and would allow for the `rec` keyword to be +removed from an unknown number of places. + +This RFC was originally submitted as [nixpkgs PR #41627](originalPR). This PR +received some positive attention: + +![Positive attention on the original PR](Upvotes) + +However, it was [suggested](useRFC) by @edolstra that this should go through the +RFC process instead of directly implementing it in nixpkgs. + +# Detailed design +[design]: #detailed-design + +The basic design is simple: change the default value for the `name` attribute of +`mkDerivation` to `"${pname}-${version}"` if both attributes are present +(implemented in [`c313e07`](basicChange)) + +Care should be taken to assure that position information is transferred +correctly (implemented in [`0c1d5d1`](positionInfo)) and to add an assertion +that the generated name is consistent with the actual name if all three +attributes are present (implemented in [`e0d2348`](checkConsistent)). + +`git cherry-pick`-ing these three commits should be sufficient to get this RFC +implemented. It is discouraged to continue on the original PR, since a lot of it +has gone out of date since. + +# Drawbacks +[drawbacks]: #drawbacks + + * It could confuse users unfamiliar with this RFC where a package gets its + name from. + * It makes a currently unofficial (but not discouraged) practice official, + and enshrines the `pname` attribute name into "law". Once this RFC is + implemented, there is no easy way of changing this attribute name again. + +# Alternatives +[alternatives]: #alternatives + +Other than doing nothing, there doesn't seem to be an alternative design. + +# Unresolved questions +[unresolved]: #unresolved-questions + +* Is `pname` the correct attribute name to use for this? Or should we go with + the (~7.5×) less common, but more descriptive `baseName`? + +# Future work +[future]: #future-work + +Future PR's that use the old pattern of `"${pname}-{version}"` (or a variant +thereof) should be requested to change to use the new pattern suggested here. + +There could be a gradual cleanup of old code, to make sure all derivations use +this new pattern. + +# Appendix A: Occurances of Variants +[appendixA]: #appendix-A + +```sh +$ ag --nix '"\$\{\w+\}-\$\{version\}"' --no-filename --only-matching | sort | uniq --count | sort --numeric-sort + 1 "${appname}-${version}" + 1 "${appName}-${version}" + 1 "${attr}-${version}" + 1 "${cmd}-${version}" + 1 "${drvName}-${version}" + 1 "${flavor}-${version}" + 1 "${fullName}-${version}" + 1 "${nameMajor}-${version}" + 1 "${name_}-${version}" + 1 "${package_name}-${version}" + 1 "${packageName}-${version}" + 1 "${pkgName}-${version}" + 1 "${plainName}-${version}" + 1 "${pName}-${version}" + 1 "${prefix}-${version}" + 1 "${shortName}-${version}" + 1 "${simpleName}-${version}" + 1 "${srcName}-${version}" + 1 "${stname}-${version}" + 1 "${toolName}-${version}" + 2 "${artifactId}-${version}" + 2 "${pkg}-${version}" + 2 "${p_name}-${version}" + 2 "${shortname}-${version}" + 3 "${basename}-${version}" + 3 "${libname}-${version}" + 3 "${_name}-${version}" + 3 "${package}-${version}" + 3 "${program}-${version}" + 4 "${project}-${version}" + 5 "${pkgname}-${version}" + 5 "${product}-${version}" + 7 "${gemName}-${version}" + 9 "${repo}-${version}" + 24 "${name}-${version}" + 85 "${baseName}-${version}" + 627 "${pname}-${version}" + 746 +``` + + + +[originalPR]: https://github.com/NixOS/nixpkgs/pull/41627 +[upvotes]: https://i.imgur.com/vosd6YG.png +[useRFC]: https://github.com/NixOS/nixpkgs/pull/41627#issuecomment-395750781 + +[basicChange]: https://github.com/NixOS/nixpkgs/pull/41627/commits/c313e07 +[positionInfo]: https://github.com/NixOS/nixpkgs/pull/41627/commits/0c1d5d1 +[checkConsistent]: https://github.com/NixOS/nixpkgs/pull/41627/commits/e0d2348 From d7d403fb1d1f03d672ea1a577f0d8c55cd337be0 Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Tue, 2 Oct 2018 19:14:50 +0200 Subject: [PATCH 2/7] Use assigned number --- ...default-name-from-pname.md => 0035-default-name-from-pname.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rfcs/{0000-default-name-from-pname.md => 0035-default-name-from-pname.md} (100%) diff --git a/rfcs/0000-default-name-from-pname.md b/rfcs/0035-default-name-from-pname.md similarity index 100% rename from rfcs/0000-default-name-from-pname.md rename to rfcs/0035-default-name-from-pname.md From 264325a9c1d07342ab1da78d48cb6bb642604ba3 Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Tue, 2 Oct 2018 19:21:57 +0200 Subject: [PATCH 3/7] Fix links --- rfcs/0035-default-name-from-pname.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rfcs/0035-default-name-from-pname.md b/rfcs/0035-default-name-from-pname.md index 0e4df1a49..d62aabdf3 100644 --- a/rfcs/0035-default-name-from-pname.md +++ b/rfcs/0035-default-name-from-pname.md @@ -18,16 +18,16 @@ Whenever the `version` and `pname` attribute are both present, use The string `"${pname}-${version}"` appears verbatim in the nixpkgs repo [627 times](appendixA), at the time of this writing. Variants of this string appear -[746 times](appendixA), with the most common variant being `baseName`. This RFC +[746 times][appendixA], with the most common variant being `baseName`. This RFC would reduce repetition in nixpkgs, and would allow for the `rec` keyword to be removed from an unknown number of places. -This RFC was originally submitted as [nixpkgs PR #41627](originalPR). This PR +This RFC was originally submitted as [nixpkgs PR #41627][originalPR]. This PR received some positive attention: -![Positive attention on the original PR](Upvotes) +![Positive attention on the original PR][Upvotes] -However, it was [suggested](useRFC) by @edolstra that this should go through the +However, it was [suggested][useRFC] by @edolstra that this should go through the RFC process instead of directly implementing it in nixpkgs. # Detailed design @@ -35,12 +35,12 @@ RFC process instead of directly implementing it in nixpkgs. The basic design is simple: change the default value for the `name` attribute of `mkDerivation` to `"${pname}-${version}"` if both attributes are present -(implemented in [`c313e07`](basicChange)) +(implemented in [`c313e07`][basicChange]) Care should be taken to assure that position information is transferred -correctly (implemented in [`0c1d5d1`](positionInfo)) and to add an assertion +correctly (implemented in [`0c1d5d1`][positionInfo]) and to add an assertion that the generated name is consistent with the actual name if all three -attributes are present (implemented in [`e0d2348`](checkConsistent)). +attributes are present (implemented in [`e0d2348`][checkConsistent]). `git cherry-pick`-ing these three commits should be sufficient to get this RFC implemented. It is discouraged to continue on the original PR, since a lot of it From 4672e5e6345327b96c8676eba92d5d9622bf8259 Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Wed, 3 Oct 2018 10:53:48 +0200 Subject: [PATCH 4/7] Mention Python in drawbacks --- rfcs/0035-default-name-from-pname.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0035-default-name-from-pname.md b/rfcs/0035-default-name-from-pname.md index d62aabdf3..532c623b0 100644 --- a/rfcs/0035-default-name-from-pname.md +++ b/rfcs/0035-default-name-from-pname.md @@ -54,6 +54,8 @@ has gone out of date since. * It makes a currently unofficial (but not discouraged) practice official, and enshrines the `pname` attribute name into "law". Once this RFC is implemented, there is no easy way of changing this attribute name again. + _Note: `pname` is already used for Python packages, but in this context, + `name = "${python.libPrefix}-${pname}-${version}"`. # Alternatives [alternatives]: #alternatives From d1a48e29b59a42227c955197eb479502fe7b894b Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Wed, 3 Oct 2018 10:56:27 +0200 Subject: [PATCH 5/7] Mention hasSuffix --- rfcs/0035-default-name-from-pname.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/rfcs/0035-default-name-from-pname.md b/rfcs/0035-default-name-from-pname.md index 532c623b0..f2a8b730c 100644 --- a/rfcs/0035-default-name-from-pname.md +++ b/rfcs/0035-default-name-from-pname.md @@ -40,11 +40,14 @@ The basic design is simple: change the default value for the `name` attribute of Care should be taken to assure that position information is transferred correctly (implemented in [`0c1d5d1`][positionInfo]) and to add an assertion that the generated name is consistent with the actual name if all three -attributes are present (implemented in [`e0d2348`][checkConsistent]). - -`git cherry-pick`-ing these three commits should be sufficient to get this RFC -implemented. It is discouraged to continue on the original PR, since a lot of it -has gone out of date since. +attributes are present (implemented in [`e0d2348`][checkConsistent]). Because +some packages already define a prefix to their `pname`-`version` pair (for +example: `python2.7-setuptools-40.2.0`), it might be better to use +`lib.strings.hasSuffix` here instead of `(==)`. + +`git cherry-pick`-ing these three commits (keeping the mentioned caviats in +mind) should be sufficient to get this RFC implemented. It is discouraged to +continue on the original PR, since a lot of it has gone out of date since. # Drawbacks [drawbacks]: #drawbacks From a605d37e44b8d9d5e17a1c796bd213fea20beacb Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Wed, 3 Oct 2018 11:20:59 +0200 Subject: [PATCH 6/7] s/caviats/caveats/ --- rfcs/0035-default-name-from-pname.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0035-default-name-from-pname.md b/rfcs/0035-default-name-from-pname.md index f2a8b730c..621d9f8d4 100644 --- a/rfcs/0035-default-name-from-pname.md +++ b/rfcs/0035-default-name-from-pname.md @@ -45,7 +45,7 @@ some packages already define a prefix to their `pname`-`version` pair (for example: `python2.7-setuptools-40.2.0`), it might be better to use `lib.strings.hasSuffix` here instead of `(==)`. -`git cherry-pick`-ing these three commits (keeping the mentioned caviats in +`git cherry-pick`-ing these three commits (keeping the mentioned caveats in mind) should be sufficient to get this RFC implemented. It is discouraged to continue on the original PR, since a lot of it has gone out of date since. From 565700f94ddc712ad92d6ac3a42fd5f861adce0e Mon Sep 17 00:00:00 2001 From: Patrick Hilhorst Date: Mon, 29 Oct 2018 15:24:46 +0100 Subject: [PATCH 7/7] Add new PR --- rfcs/0035-default-name-from-pname.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0035-default-name-from-pname.md b/rfcs/0035-default-name-from-pname.md index 621d9f8d4..3b96f2773 100644 --- a/rfcs/0035-default-name-from-pname.md +++ b/rfcs/0035-default-name-from-pname.md @@ -5,6 +5,7 @@ author: Patrick Hilhorst (@Synthetica9) co-authors: (find a buddy later to help our with the RFC) related-issues: - https://github.com/NixOS/nixpkgs/pull/41627 + - https://github.com/NixOS/nixpkgs/pull/49398 --- # Summary