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

CheckMeta does not work #191124

Open
piegamesde opened this issue Sep 13, 2022 · 8 comments
Open

CheckMeta does not work #191124

piegamesde opened this issue Sep 13, 2022 · 8 comments

Comments

@piegamesde
Copy link
Member

I just noticed that meta attributes are actually never checked locally. I noticed that there is an undocumented attribute config.checkMeta which defaults to false, but even when setting it to true and adding obvious errors to a derivation I cannot get it to fail. Ideally, such an option should be enabled by default IMO.

Arbitrarily pinging @ckiee @Artturin @ncfavier

@ncfavier
Copy link
Member

Possibly related to #6794 (comment) ?

@piegamesde
Copy link
Member Author

That issue is 😱, but I'm not sure this is the one I'm running into since even blatant errors like setting description = true; do not fail my evaluation. It really looks like the check is not running at all, but I also can't get it to work somehow.

@ncfavier
Copy link
Member

Can you provide a minimal reproducer? That much seems to work fine for me:

with import <nixpkgs> { config.checkMeta = true; };
runCommand "foo" {
  meta.description = true;
} ""
error: Package ‘foo’ in /home/n/foo.nix:3 has an invalid meta attrset:
       	 - key 'description' has a value 1 of an invalid type bool; expected string, refusing to evaluate.

@piegamesde
Copy link
Member Author

Oh no, I think I found the issue: The meta type check is not started if the package has other issues. This is a bug only surfaced through #177272. See the line else let res = checkMeta (attrs.meta or {}); in if res != [] then in check-meta.nix, it does not apply if any other conditions above matched. At the moment this only works because all entries have valid = "no"; which stops the evaluation anyways.

This should reproduce on master on insecure or unfree packages. The derivation will fail with the usual warning, but actually it should fail with "invalid meta" instead.

piegamesde added a commit to piegamesde/nixpkgs that referenced this issue Sep 25, 2022
- Enable metadata checking by default, see NixOS#25304 (comment)
- Check metadata before any other package issues, see NixOS#191124 (comment)
- Document that type checks only apply to the top level of nested values.

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
@ncfavier
Copy link
Member

Can this be closed now?

@piegamesde
Copy link
Member Author

I don't think we made any significant progress here, since the original fix got reverted.

@ConnorBaker
Copy link
Contributor

Sorry for the bump -- @piegamesde to your knowledge, is checkMeta still effectively broken, or has something changed since?

@piegamesde
Copy link
Member Author

I don't know of any changes on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants