homeassistant: [WIP] init at 0.30.2#19508
Conversation
|
@peterhoeg, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @joachifm and @offlinehacker to be potential reviewers. |
FRidh
left a comment
There was a problem hiding this comment.
Some small changes should be made to the Python packages. I didn't check the module.
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
this package is already in this file but unfortunately without the hyphen: pytestcov.
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
and you've checked whether the tests run?
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
An application that is build using Python shouldn't be here. This file is only for Python libraries.
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
why do you specify outputs when you only have the default output? This can be removed.
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
does that mean the package should only be available for Python >= 3.5? disabled = pythonOlder "3.5";
pkgs/top-level/python-packages.nix
Outdated
pkgs/top-level/python-packages.nix
Outdated
pkgs/top-level/python-packages.nix
Outdated
pkgs/top-level/python-packages.nix
Outdated
There was a problem hiding this comment.
I saw that zeroconf uses this. I've reported upstream that they should use environment markers instead in zeroconf
python-zeroconf/python-zeroconf#87
pkgs/top-level/all-packages.nix
Outdated
There was a problem hiding this comment.
if its an application and not a library then it shouldn't be in python-packages.nix. See below as well.
|
I have worked with home-assistant before.
requirements-all.txt includes all the packages you would possibly need. The dynamic |
Lots of dependencies -> lots of work. That's just how it is right now. Outside of Nixpkgs you could use |
|
first of all, thank you @FRidh for all the feedback, much appreciated! This is a WIP as I wasn't sure if my approach was correct, so I will get all the items cleaned up you mentioned. All tests will be enabled of course, I just wanted to see if I could get this thing up and running first. But it offhand seems like there should be a better way. Coming from ruby where the combination of |
|
@peterhoeg you're welcome. There should indeed be a better way, but unfortunately upstream doesn't provide any. Python packaging is quite a mess. People can put whatever code they want in As soon as Hydra allows network access during evaluation time, I will further work on #16005, which at least reduces the amount of work the packaging requires. |
|
(hmm, but that should actually work now since NixOS/nix@06bbfb6) |
|
So for the time being, what would you recommend for packaging home assistant? The way I see it, we can do one of the following:
2 seems a bit pointless as there will be modules that require PIP anyway and there is not much difference between having 70% and 97% of the packages dynamically installed. 3 requires additional work on the part of the person using home assistant which doesn't seem like a nice solution. That really only leaves us with option 1. |
Also includes a basic nixos module. This is ridiculously painful due to how dependencies are handled (or I'm missing something completely). The basic dependencies for the application itself are declared up front, but each sub-module has a ```REQUIREMENTS``` array where other python packages are listed. homeassistant will then try to load those dependencies if the module is activated and if failing to do so, will fall back to using pip to install them. I honestly don't know what the best way to deal with this is. It's a rabbit hole of dependencies that reminds me of .rpm hell in the old RH days. Additionally, these sub modules will sometimes require specific versions which means multiple versions now need to be maintained and the maintenance cost just shoots up.
This won't work because it cannot write in the store. You would have to specify another directory to install the modules, however, the Nix Python won't be able to find those modules. I suggest for now that you create a new repo in which you define the module, and where you create the expressions with |
|
@FRidh assuming the home-assistant binary installs using In any case, thanks to @garbas, Would you (the collective nix(os) maintainers be open to moving that repository under the main NixOS github organization if we handle changes similarly to nixpkgs (ie, only trusted project members can merge)? That should enable us to ship modules that cannot make it into the main repository but still be of the same high standard. |
If you check
I think at some point it might make sense to put such repo's together under one umbrella. |
|
For anyone else stumbling over this issue, @garbas has created a dedicated repos for python packages generated using pypi2nix which is where I'll be submitting the home-assistant stuff. https://github.com/garbas/nixpkgs-python @FRidh, once again thank you for taking the time to explain things. |
Motivation for this change
Also includes a basic nixos module.
This is ridiculously painful due to how dependencies are handled (or I'm missing something completely).
The basic dependencies for the application itself are declared up front, but each sub-module has a
REQUIREMENTSarray where other python packages are listed. homeassistant will then try to load those dependencies if the module is activated and if failing to do so, will fall back to using pip to install them.I honestly don't know what the best way to deal with this is. It's a rabbit hole of dependencies that reminds me of .rpm hell in the old RH days. Additionally, these sub modules will sometimes require specific versions which means multiple versions now need to be maintained and the maintenance cost just shoots up.
Any thoughts @FRidh?
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)