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

pythonPackages.pytest-flake8: init at 0.8.1 #25549

Merged
merged 1 commit into from
May 6, 2017

Conversation

jluttine
Copy link
Member

@jluttine jluttine commented May 6, 2017

Motivation for this change

Add pytest-flake8 Python package. This is a build-time dependency for another package that I'm going to pull request soon.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@jluttine, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

pname = "pytest-flake8";
version = "0.8.1";

propagatedBuildInputs = [ pytest flake8 ];
Copy link
Member

Choose a reason for hiding this comment

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

do not add pytest as propagatedBuildInput but as buildInput. This package is an exception.

Copy link
Member

Choose a reason for hiding this comment

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

other than that this is good

Copy link
Member Author

Choose a reason for hiding this comment

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

Why pytest shouldn't be added as propagatedBuildInput? It is a runtime dependency (see https://github.com/tholo/pytest-flake8/blob/master/pytest_flake8.py#L10) and also listed in install_requires (see https://github.com/tholo/pytest-flake8/blob/master/setup.py#L38).

Copy link
Member

Choose a reason for hiding this comment

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

see d317e83

Copy link
Member

Choose a reason for hiding this comment

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

and be020a2

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, thanks. I did that fix now. Is it ok?

In general, I would love to see that nix wouldn't replace Python packages with their new versions but would contain all versions that have ever been added to nix. One of them would a default and built by Hydra. Perhaps a few others versions too in some cases (like pytest). But then users could install different versions of packages similarly as with pip. Each Python package could have a list of sources and adding a new version would just mean adding a new version option, url and sha256. Also, using a different version would make the package point to that requested version, so there wouldn't be any problem defining pytest as a propagatedBuildInputs because pytest points to the version the user wants to install. Anyway, that's a different topic and a more general comment, not sure it would work with nix though.

Copy link
Member

Choose a reason for hiding this comment

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

That is never going to happen. Packages don't only have version-dependent dependencies but also things like patches, tests and non-Python dependencies which can change within the package set. There's no way we can maintain that, and especially not while Python packages are not declarative.

If you want to change version, how would Nix deal with that? Would it be dumb and just pick the version (override within the fixed-point) or should it try to resolve dependencies like conda does with a SAT-solver. That brings further issues with determinism where the eventual package set depends on the exact implementation and weights given to the SAT-solver. And, it would require the declarative requirements of all packages (although we could have those then in Nixpkgs). They're looking at implementing a SAT-solver for pip but some now also start to realize that to actually make it useful they need to have the requirements of all (possible) dependencies. PyPI doesn't have that, the set would be huge, and would likely have to be kept locally.

Perhaps most important is actually why you would want to work with multiple versions in the first place? As user or developer you're interested in features and security fixes. Why should you bother explicitly with versions of each of your packages? Wouldn't it make far more sense if you would get a package set that provides packages that are absolutely known to work together, and as developer would only support your package within certain specific package sets. Thereby, reducing the huge amount of combinations that could exist.

I've brought this issue up on distutils-sig once, where I suggested creating curated package sets like Haskell LTS. With Haskell LTS you have periodic releases of the package sets along with maintenance releases. This seriously reduces the amount of maintenance for developers, packagers, and makes it easier for users as they're more likely to get a set of packages that actually works. This is also the reason why we can relatively easily support Haskell LTS in Nixpkgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the comprehensive explanation, that was enlightening! 👍

@FRidh FRidh merged commit 37a48c9 into NixOS:master May 6, 2017
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.

None yet

3 participants