Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check-meta.nix: make non-source consistent with documentation #175495

Merged
merged 2 commits into from Jan 4, 2023
Merged

check-meta.nix: make non-source consistent with documentation #175495

merged 2 commits into from Jan 4, 2023

Conversation

ghost
Copy link

@ghost ghost commented May 30, 2022

Description of changes

The documentation for meta.sourceProvenance in doc/stdenv/meta.chapter.md says: "the meta.sourceProvenance attribute should be a list containing one or more value..."

Let's update check-meta.nix to require that meta.sourceProvenance is a list, as the documentation says, rather than a single element.

Adding two extra keystrokes [ and ] when filling out this field is an insignificant burden for package authors, and being able to assume that the meta.sourceProvenance field is always a list greatly simplifies any code that acts on the value of this field. For example, it simplifies the sourceProvenance example given in check-meta.nix; the simplification is included in this PR.

Since meta.sourceProvenance was just merged a few hours ago now is the easiest time to fix this: nobody is using the feature yet.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ghost ghost requested review from Ericson2314 and matthewbauer as code owners May 30, 2022 17:12
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label May 30, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 30, 2022
@ghost ghost mentioned this pull request May 30, 2022
13 tasks
@ghost
Copy link
Author

ghost commented May 30, 2022

@risicle should be added as a reviewer (am I able to add reviewers to my own PRs?)

@Artturin
Copy link
Member

Artturin commented May 30, 2022

@risicle should be added as a reviewer (am I able to add reviewers to my own PRs?)

Yes it you're a member of the maintainer team which you can join by being a maintainer of a package and accepting the invite

@ghost
Copy link
Author

ghost commented Jun 19, 2022

Yes it you're a member of the maintainer team which you can join by being a maintainer of a package and accepting the invite

Yeah, I really procrastinated a long time before doing this.

@ghost ghost marked this pull request as draft June 19, 2022 08:12
@ghost ghost marked this pull request as ready for review June 19, 2022 08:13
@ghost
Copy link
Author

ghost commented Jun 19, 2022

Resolved the merge conflict and squashed.

@Artturin Artturin requested review from trofi and risicle and removed request for trofi July 3, 2022 04:02
@risicle
Copy link
Contributor

risicle commented Jul 3, 2022

What error does this now give if you assign a single sourceType then?

@ghost
Copy link
Author

ghost commented Jul 3, 2022

Using these instructions:

$ curl -o outpaths.nix https://raw.githubusercontent.com/NixOS/ofborg/released/ofborg/src/outpaths.nix
$ GC_INITIAL_HEAP_SIZE=4g nix-env -f ./outpaths.nix -qaP --no-name --out-path --arg checkMeta true > out-paths
error: value is a set while a list was expected

       at /git/nixpkgs/pkgs/stdenv/generic/check-meta.nix:93:30:

           92|
           93|   isNonSource = sourceTypes: lib.lists.any (t: !t.isSource) sourceTypes;
             |                              ^
           94|
(use '--show-trace' to show detailed location information)

@ghost ghost mentioned this pull request Jul 3, 2022
13 tasks
@ghost
Copy link
Author

ghost commented Jul 3, 2022

If you want to see what OfBorg's complaint would look like, see this, from #180036.

The documentation for `meta.sourceProvenance` in
`doc/stdenv/meta.chapter.md` says: "the `meta.sourceProvenance`
attribute should be a list containing one or more value..."

Let's update check-meta.nix to require that `meta.sourceProvenance` is
a list, as the documentation says, rather than a single element.

Adding two extra keystrokes `[` and `]` when filling out this field is
an insignificant burden for package authors, and being able to assume
that the `meta.sourceProvenance` field is always a list greatly
simplifies any code that acts on the value of this field.

Since `meta.sourceProvenance` was just merged a few hours ago now is
the easiest time to fix this: nobody is using the feature yet.
@ghost
Copy link
Author

ghost commented Jan 2, 2023

Rebased.

@ghost ghost requested a review from piegamesde as a code owner January 2, 2023 02:26
@piegamesde piegamesde merged commit aa8ead3 into NixOS:master Jan 4, 2023
@ghost ghost deleted the pr/sourceProvenance/correct-meta-typecheck branch January 4, 2023 21:10
gador added a commit to gador/nixpkgs that referenced this pull request Jan 10, 2023
due to NixOS#175495

Signed-off-by: Florian Brandes <florian.brandes@posteo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants