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

🏡 Home Assistant Refresh #340251

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented Sep 7, 2024

Description of changes

Resolve a bunch of maintenance tasks:

  • Replace top-level with lib
  • Allow passing extraArgs to the command line
  • Make various issues around the NixOS test
    • restart/reload expectation around exit code 100
    • replace backup component in dependency change test, since it is a default integration by now
    • [ ]
  • Update various lingo bits
    • Remove the custom option prefixes (e.g. from customLovelaceModules) and make options names more concrete
    • Talk about integrations instead of components. Apparently components is a legacy naming.
    • Use PEP517 attribute names in custom components
  • Use runTest instead of handleTest
  • Autocalling for component and lovelace module package sets

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.

When sending SIGHUP to hass it will exit with code 100, which is the
codified exit code to trigger a restart. This is useful, because it can
allow triggering a restart from within the frontend.

It was previously assumed that it would result in a reload, which would
keep the same interpreter process intact. That is not the case and so the
assumption that the PID would stay the same was flawed and only succeeded
due to race conditions.
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 7, 2024
@mweinelt mweinelt force-pushed the home-assistant-refresh branch from b59fe97 to 7593efc Compare September 7, 2024 04:12
The backup module is part of the default integrations and as such it will
always be loaded. Replace it with the prometheus module, for which this
is probably unlikely to ever become the case.
In particular replace propagatedBuildInputs with dependencies.
…alling

Kill the tedious work of setting up attributes manually.
@mweinelt mweinelt force-pushed the home-assistant-refresh branch from 7593efc to 02cc60e Compare September 7, 2024 04:21
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Sep 7, 2024
@kira-bruneau
Copy link
Contributor

kira-bruneau commented Sep 7, 2024

I was going to open a new PR for this: kira-bruneau@c0ed7c1, but I noticed that this refresh PR was just opened.

Would it be ok to include those changes here?

@kira-bruneau
Copy link
Contributor

kira-bruneau commented Sep 7, 2024

Oh wait nvm, I thought this touched the package, I'll just open a new PR: #340324

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 27, 2024
Comment on lines +711 to +714
ExecReload = (escapeSystemdExecArgs [
(lib.getExe' pkgs.coreutils "kill")
"-HUP"
]) + " $MAINPID";
Copy link
Member

Choose a reason for hiding this comment

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

The escapeSystemdExecArgs is superfluous here as the input is not user controlled and will never escape something.

@@ -398,7 +406,7 @@ in {
themes = "!include_dir_merge_named themes";
};
http = {};
feedreader.urls = [ "https://nixos.org/blogs.xml" ];
Copy link
Member

Choose a reason for hiding this comment

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

FYI the new URL would be https://nixos.org/blog/feed.xml

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an example, and I think pointing to the nix tag at lobsters is much more interesting than the announcements on the homepage.

@@ -11,7 +44,7 @@ let
# options shown in settings.
# We post-process the result to add support for YAML functions, like secrets or includes, see e.g.
# https://www.home-assistant.io/docs/configuration/secrets/
filteredConfig = lib.converge (lib.filterAttrsRecursive (_: v: ! elem v [ null ])) (lib.recursiveUpdate customLovelaceModulesResources (cfg.config or {}));
filteredConfig = converge (filterAttrsRecursive (_: v: ! elem v [ null ])) (recursiveUpdate lovelaceResourceConfig (cfg.config or {}));
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
filteredConfig = converge (filterAttrsRecursive (_: v: ! elem v [ null ])) (recursiveUpdate lovelaceResourceConfig (cfg.config or {}));
filteredConfig = converge (filterAttrsRecursive (_: v: v != null)) (recursiveUpdate lovelaceResourceConfig (cfg.config or {}));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants