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

Better error for outside references #97

Merged
merged 3 commits into from
Aug 29, 2024
Merged

Conversation

infinisil
Copy link
Member

Motivated by @teto in #64 (comment), improves the error messages when a path outside a package directory is referenced:

- pkgs/by-name/aa/aa: File package.nix at line 1 contains the path expression "../." which may point outside the directory of that package.
  This is undesirable because it creates dependencies between internal paths, making it harder to reorganise Nixpkgs in the future.
  Alternatives include:
  - If you are creating a new version of a package with a common file between versions, consider following the recommendation in https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#recommendation-for-new-packages-with-multiple-versions.
  - If the path being referenced could be considered a stable interface with multiple uses, consider exposing it via a `pkgs` attribute, then taking it as a attribute argument in package.nix.
  - If the path being referenced is internal and has multiple uses, consider passing the file as an explicit `callPackage` argument in `pkgs/top-level/all-packages.nix`.
  - If the path being referenced is internal and will need to be modified independently of the original, consider copying it into the pkgs/by-name/aa/aa directory.

@teto
Copy link
Member

teto commented Aug 24, 2024

The This is undesirable because it creates dependencies between internal paths, making it harder to reorganise Nixpkgs in the future. part is nice to have. As for the rest, it depends how you prefer to manage the project but one solution is to do like shellcheck and reference a wiki page like "https://www.shellcheck.net/wiki/SC2155" which can be edited without needing another release.

@infinisil
Copy link
Member Author

Yeah I'd really like that better, maybe let's keep it simpler for now though

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of hate the - prefix, but I suppose consistency is better.

@infinisil
Copy link
Member Author

infinisil commented Aug 29, 2024

@infinisil infinisil merged commit 480c5a7 into main Aug 29, 2024
3 checks passed
@infinisil infinisil deleted the better-error-outside-reference branch August 29, 2024 14:35
@infinisil infinisil mentioned this pull request Aug 29, 2024
@infinisil
Copy link
Member Author

infinisil commented Aug 29, 2024

The after release workflow didn't fire automatically yet again, just like before

@infinisil
Copy link
Member Author

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

Successfully merging this pull request may close these issues.

3 participants