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

doc/python: add section about replaced/stubbed/avoided packages #336775

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Aug 23, 2024

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Comment on lines 2035 to 2036
| cmake | cmake (stub) | The package normally downloads cmake from the internet. The stub provides our cmake with the usual setup hook. |
| ninja | ninja (stub) | The package normally downloads ninja from the internet. The stub provides our cmake with the usual setup hook. |
Copy link
Member

Choose a reason for hiding this comment

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

This seems more like an implementation detail than actionable advice for package authors, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, jain. It is probably a good hint to kinda know the difference between pkgs.ninja and pkgs.python3Packages.ninja and what to do when you encounter it.

Copy link
Member Author

Choose a reason for hiding this comment

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

For example with this information people know that they don't need to remove the dependency which would otherwise be necessary because downloading things in the sandbox is well known to not work.

@natsukium
Copy link
Member

I'm currently extending nixpkgs-lint to detect these packages.
https://github.com/natsukium/nixpkgs-lint

And I think including this information in the document is also necessary.
Let's list which packages to exclude.


This list is useful for reviewers as well as for self-checking when submitting packages.

## Dependencies to avoid or replace {#python-dependencies-avoid-replace}
Copy link
Member

Choose a reason for hiding this comment

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

To scope it within nixpkgs contributions, rather than for example downstream usage like ad-hoc shells

Suggested change
## Dependencies to avoid or replace {#python-dependencies-avoid-replace}
### Dependencies to avoid or replace {#python-dependencies-avoid-replace}

Comment on lines +2035 to +2038
| cmake | cmake (stub) | The package normally downloads cmake from the internet. The stub provides our cmake with the usual setup hook. |
| ninja | ninja (stub) | The package normally downloads ninja from the internet. The stub provides our ninja with the usual setup hook. |
| pytest-cov | pytest-cov-stub | coverage is only useful for developers. |
| pytest-runner | | Dependencies cannot be downloaded and should be provided through nativeCheckInputs. |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| cmake | cmake (stub) | The package normally downloads cmake from the internet. The stub provides our cmake with the usual setup hook. |
| ninja | ninja (stub) | The package normally downloads ninja from the internet. The stub provides our ninja with the usual setup hook. |
| pytest-cov | pytest-cov-stub | coverage is only useful for developers. |
| pytest-runner | | Dependencies cannot be downloaded and should be provided through nativeCheckInputs. |
| cmake | `python3Packages.cmake` (stub) | The package normally downloads cmake from the internet. The stub provides our cmake with the usual setup hook. |
| ninja | `python3Packages.ninja` (stub) | The package normally downloads ninja from the internet. The stub provides our ninja with the usual setup hook. |
| pytest-cov | `python3Packages.pytest-cov-stub` | coverage is only useful for developers. |
| pytest-runner | - | Dependencies cannot be downloaded and should be provided through nativeCheckInputs. |

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

Successfully merging this pull request may close these issues.

4 participants