From 35174e5413f6acbf316d8d1d6b466ad02595a6cb Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 11 Apr 2019 23:38:07 -0700 Subject: [PATCH 01/13] Update manifest validator --- script/gen_requirements_all.py | 4 +- script/manifest/validate.py | 100 ------------------ script/manifest_validate/__main__.py | 46 ++++++++ .../codeowners.py | 0 script/manifest_validate/dependencies.py | 59 +++++++++++ script/manifest_validate/manifest.py | 38 +++++++ .../manifest_helper.py | 0 script/manifest_validate/model.py | 43 ++++++++ .../requirements.py | 0 9 files changed, 189 insertions(+), 101 deletions(-) delete mode 100755 script/manifest/validate.py create mode 100644 script/manifest_validate/__main__.py rename script/{manifest => manifest_validate}/codeowners.py (100%) create mode 100644 script/manifest_validate/dependencies.py create mode 100644 script/manifest_validate/manifest.py rename script/{manifest => manifest_validate}/manifest_helper.py (100%) create mode 100644 script/manifest_validate/model.py rename script/{manifest => manifest_validate}/requirements.py (100%) diff --git a/script/gen_requirements_all.py b/script/gen_requirements_all.py index c8622837cf5dd..fef56bcc1583a 100755 --- a/script/gen_requirements_all.py +++ b/script/gen_requirements_all.py @@ -7,7 +7,9 @@ import re import sys -from script.manifest.requirements import gather_requirements_from_manifests +from script.manifest_validate.requirements import ( + gather_requirements_from_manifests +) COMMENT_REQUIREMENTS = ( 'Adafruit-DHT', diff --git a/script/manifest/validate.py b/script/manifest/validate.py deleted file mode 100755 index e5db1c9368c53..0000000000000 --- a/script/manifest/validate.py +++ /dev/null @@ -1,100 +0,0 @@ -#!/usr/bin/env python3 -"""Validate all integrations have manifests and that they are valid.""" -import json -import pathlib -import sys - -import voluptuous as vol -from voluptuous.humanize import humanize_error - - -MANIFEST_SCHEMA = vol.Schema({ - vol.Required('domain'): str, - vol.Required('name'): str, - vol.Required('documentation'): str, - vol.Required('requirements'): [str], - vol.Required('dependencies'): [str], - vol.Required('codeowners'): [str], -}) - - -COMPONENTS_PATH = pathlib.Path('homeassistant/components') - - -def validate_dependency(path, dependency, loaded, loading): - """Validate dependency is exist and no circular dependency.""" - dep_path = path.parent / dependency - return validate_integration(dep_path, loaded, loading) - - -def validate_integration(path, loaded, loading): - """Validate that an integrations has a valid manifest.""" - errors = [] - path = pathlib.Path(path) - - manifest_path = path / 'manifest.json' - - if not manifest_path.is_file(): - errors.append('Manifest file {} not found'.format(manifest_path)) - return errors # Fatal error - - try: - manifest = json.loads(manifest_path.read_text()) - except ValueError as err: - errors.append("Manifest contains invalid JSON: {}".format(err)) - return errors # Fatal error - - try: - MANIFEST_SCHEMA(manifest) - except vol.Invalid as err: - errors.append(humanize_error(manifest, err)) - - if manifest['domain'] != path.name: - errors.append('Domain does not match dir name') - - for dep in manifest['dependencies']: - if dep in loaded: - continue - if dep in loading: - errors.append("Found circular dependency {} in {}".format( - dep, path - )) - continue - loading.add(dep) - - errors.extend(validate_dependency(path, dep, loaded, loading)) - - loaded.add(path.name) - return errors - - -def validate_all(): - """Validate all integrations.""" - invalid = [] - - for fil in COMPONENTS_PATH.iterdir(): - if fil.is_file() or fil.name == '__pycache__': - continue - - errors = validate_integration(fil, set(), set()) - - if errors: - invalid.append((fil, errors)) - - if not invalid: - return 0 - - print("Found invalid manifests") - print() - - for integration, errors in invalid: - print(integration) - for error in errors: - print("*", error) - print() - - return 1 - - -if __name__ == '__main__': - sys.exit(validate_all()) diff --git a/script/manifest_validate/__main__.py b/script/manifest_validate/__main__.py new file mode 100644 index 0000000000000..65201e02fa0d4 --- /dev/null +++ b/script/manifest_validate/__main__.py @@ -0,0 +1,46 @@ +"""Validate manifests.""" +import pathlib +import sys + +from .model import Integration +from . import dependencies, manifest + + +COMPONENTS_PATH = pathlib.Path('homeassistant/components') + + +def main(): + """Validate manifests.""" + integrations = {} + invalid = [] + + for fil in COMPONENTS_PATH.iterdir(): + if fil.is_file() or fil.name == '__pycache__': + continue + + integration = Integration(fil) + integration.load_manifest() + integrations[integration.domain] = integration + + manifest.validate_all(integrations) + dependencies.validate_all(integrations) + + invalid = [itg for itg in integrations.values() if itg.errors] + + if not invalid: + return 0 + + print("Found invalid manifests") + print() + + for integration in sorted(invalid, key=lambda itg: itg.domain): + print(integration.domain) + for error in integration.errors: + print("*", error) + print() + + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/script/manifest/codeowners.py b/script/manifest_validate/codeowners.py similarity index 100% rename from script/manifest/codeowners.py rename to script/manifest_validate/codeowners.py diff --git a/script/manifest_validate/dependencies.py b/script/manifest_validate/dependencies.py new file mode 100644 index 0000000000000..832ac7744b7a4 --- /dev/null +++ b/script/manifest_validate/dependencies.py @@ -0,0 +1,59 @@ +"""Validate dependencies.""" +import pathlib +import re +from typing import Set, Dict + +from .model import Integration + + +def grep_dir(path: pathlib.Path, glob_pattern: str, search_pattern: str) \ + -> Set[str]: + """Recursively go through a dir and it's children and find the regex.""" + pattern = re.compile(search_pattern) + found = set() + + for fil in path.glob(glob_pattern): + if not fil.is_file(): + continue + + for match in pattern.finditer(fil.read_text()): + found.add(match.groups()[0]) + + return found + + +# Allowed components to be referend without being a dependency +ALLOWED_USED_COMPONENTS = { + 'persistent_notification', +} + + +def validate_dependencies(integration: Integration): + """Validate all dependencies.""" + # Find usage of hass.components + referenced = grep_dir(integration.path, "**/*.py", + r"hass\.components\.(\w+)") + referenced -= ALLOWED_USED_COMPONENTS + referenced -= set(integration.dependencies) + + if referenced: + for domain in sorted(referenced): + integration.errors.append( + "Using component {} but it's not a dependency".format(domain)) + + +def validate_all(integrations: Dict[str, Integration]): + """Validate all dependencies.""" + # check for non-existing dependencies + for integration in integrations.values(): + if not integration.manifest: + continue + + validate_dependencies(integration) + + # check that all referenced dependencies exist + for dep in integration.dependencies: + if dep not in integrations: + integration.errors.append( + "Dependency {} does not exist" + ) diff --git a/script/manifest_validate/manifest.py b/script/manifest_validate/manifest.py new file mode 100644 index 0000000000000..1f95a48040ae9 --- /dev/null +++ b/script/manifest_validate/manifest.py @@ -0,0 +1,38 @@ +"""Manifest validation.""" +from typing import Dict + +import voluptuous as vol +from voluptuous.humanize import humanize_error + +from .model import Integration + + +MANIFEST_SCHEMA = vol.Schema({ + vol.Required('domain'): str, + vol.Required('name'): str, + vol.Required('documentation'): str, + vol.Required('requirements'): [str], + vol.Required('dependencies'): [str], + vol.Required('codeowners'): [str], +}) + + +def validate_manifest(integration: Integration): + """Validate manifest.""" + try: + MANIFEST_SCHEMA(integration.manifest) + except vol.Invalid as err: + integration.errors.append( + "Invalid manifest: {}".format( + humanize_error(integration.manifest, err))) + return + + if integration.manifest['domain'] != integration.path.name: + integration.errors.append('Domain does not match dir name') + + +def validate_all(integrations: Dict[str, Integration]): + """Validate all integrations manifests.""" + for integration in integrations.values(): + if integration.manifest: + validate_manifest(integration) diff --git a/script/manifest/manifest_helper.py b/script/manifest_validate/manifest_helper.py similarity index 100% rename from script/manifest/manifest_helper.py rename to script/manifest_validate/manifest_helper.py diff --git a/script/manifest_validate/model.py b/script/manifest_validate/model.py new file mode 100644 index 0000000000000..2197f52c0170e --- /dev/null +++ b/script/manifest_validate/model.py @@ -0,0 +1,43 @@ +"""Models for manifest validator.""" +import json +import pathlib + +import attr + + +@attr.s +class Integration: + path = attr.ib(type=pathlib.Path) + manifest = attr.ib(type=dict, default=None) + errors = attr.ib(type=list, factory=list) + + @property + def domain(self): + """Integration domain.""" + return self.path.name + + @property + def manifest_path(self): + """Integration manifest path.""" + return self.path / 'manifest.json' + + @property + def dependencies(self): + """Get dependencies.""" + return [] if self.manifest is None else self.manifest['dependencies'] + + def load_manifest(self): + """Load manifest.""" + if not self.manifest_path.is_file(): + self.errors.append( + "Manifest file {} not found".format(self.manifest_path)) + return + + try: + manifest = json.loads(self.manifest_path.read_text()) + except ValueError as err: + self.errors.append( + "Manifest contains invalid JSON: {}".format(err)) + return + + self.manifest = manifest diff --git a/script/manifest/requirements.py b/script/manifest_validate/requirements.py similarity index 100% rename from script/manifest/requirements.py rename to script/manifest_validate/requirements.py From 5658699f48e369302a12dca6bdc0676145e4b2d4 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 11 Apr 2019 23:40:25 -0700 Subject: [PATCH 02/13] Update circle --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 70d2f7af3a32f..e3007e9c42b3a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -110,7 +110,7 @@ jobs: name: validate manifests command: | . venv/bin/activate - python script/manifest/validate.py + python -m script.manifest_validate - run: name: run gen_requirements_all From d7576320b78dba74237c8214ea86086811ec7704 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 11 Apr 2019 23:42:58 -0700 Subject: [PATCH 03/13] Update text --- script/manifest_validate/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/manifest_validate/__main__.py b/script/manifest_validate/__main__.py index 65201e02fa0d4..6e9d56d35f9c6 100644 --- a/script/manifest_validate/__main__.py +++ b/script/manifest_validate/__main__.py @@ -30,7 +30,7 @@ def main(): if not invalid: return 0 - print("Found invalid manifests") + print("Found invalid integrations") print() for integration in sorted(invalid, key=lambda itg: itg.domain): From 390410032212fe3b3c802556e085a5cb4d2d9710 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 07:28:30 -0700 Subject: [PATCH 04/13] Typo --- script/manifest_validate/dependencies.py | 2 +- script/manifest_validate/model.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/script/manifest_validate/dependencies.py b/script/manifest_validate/dependencies.py index 832ac7744b7a4..9f4f0fdc4732a 100644 --- a/script/manifest_validate/dependencies.py +++ b/script/manifest_validate/dependencies.py @@ -22,7 +22,7 @@ def grep_dir(path: pathlib.Path, glob_pattern: str, search_pattern: str) \ return found -# Allowed components to be referend without being a dependency +# Allowed components to be referenced without being a dependency ALLOWED_USED_COMPONENTS = { 'persistent_notification', } diff --git a/script/manifest_validate/model.py b/script/manifest_validate/model.py index 2197f52c0170e..0781de3741e19 100644 --- a/script/manifest_validate/model.py +++ b/script/manifest_validate/model.py @@ -7,6 +7,8 @@ @attr.s class Integration: + """Represent an integration in our validator.""" + path = attr.ib(type=pathlib.Path) manifest = attr.ib(type=dict, default=None) errors = attr.ib(type=list, factory=list) From 8724cbe6a206179468897e0cfe976cb082403cf3 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 08:53:11 -0700 Subject: [PATCH 05/13] fix link to codeowners --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e3007e9c42b3a..e9b0b441725e9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -95,7 +95,7 @@ jobs: name: validate CODEOWNERS command: | . venv/bin/activate - python script/manifest/codeowners.py validate + python script/manifest_validate/codeowners.py validate - run: name: run static type check From 51eee6ef71da1a43eae784ade3c67c56f3268e2e Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 10:09:00 -0700 Subject: [PATCH 06/13] Merge CODEOWNERS into hassfest --- .circleci/config.yml | 8 +- script/gen_requirements_all.py | 29 +++++- script/hassfest/__init__.py | 1 + script/hassfest/__main__.py | 68 ++++++++++++++ script/hassfest/codeowners.py | 84 +++++++++++++++++ .../dependencies.py | 17 ++-- .../manifest.py | 10 ++- .../manifest_helper.py | 0 script/hassfest/model.py | 90 +++++++++++++++++++ script/manifest_validate/__main__.py | 46 ---------- script/manifest_validate/codeowners.py | 74 --------------- script/manifest_validate/model.py | 45 ---------- script/manifest_validate/requirements.py | 22 ----- 13 files changed, 285 insertions(+), 209 deletions(-) create mode 100644 script/hassfest/__init__.py create mode 100644 script/hassfest/__main__.py create mode 100755 script/hassfest/codeowners.py rename script/{manifest_validate => hassfest}/dependencies.py (78%) rename script/{manifest_validate => hassfest}/manifest.py (76%) rename script/{manifest_validate => hassfest}/manifest_helper.py (100%) create mode 100644 script/hassfest/model.py delete mode 100644 script/manifest_validate/__main__.py delete mode 100755 script/manifest_validate/codeowners.py delete mode 100644 script/manifest_validate/model.py delete mode 100644 script/manifest_validate/requirements.py diff --git a/.circleci/config.yml b/.circleci/config.yml index e9b0b441725e9..19542b05aee74 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -91,12 +91,6 @@ jobs: . venv/bin/activate flake8 - - run: - name: validate CODEOWNERS - command: | - . venv/bin/activate - python script/manifest_validate/codeowners.py validate - - run: name: run static type check command: | @@ -110,7 +104,7 @@ jobs: name: validate manifests command: | . venv/bin/activate - python -m script.manifest_validate + python -m script.hassfest validate - run: name: run gen_requirements_all diff --git a/script/gen_requirements_all.py b/script/gen_requirements_all.py index fef56bcc1583a..f71b8944d7cb2 100755 --- a/script/gen_requirements_all.py +++ b/script/gen_requirements_all.py @@ -3,13 +3,12 @@ import fnmatch import importlib import os +import pathlib import pkgutil import re import sys -from script.manifest_validate.requirements import ( - gather_requirements_from_manifests -) +from script.hassfest.model import Integration COMMENT_REQUIREMENTS = ( 'Adafruit-DHT', @@ -221,7 +220,7 @@ def gather_modules(): errors = [] - gather_requirements_from_manifests(process_requirements, errors, reqs) + gather_requirements_from_manifests(errors, reqs) gather_requirements_from_modules(errors, reqs) for key in reqs: @@ -237,6 +236,28 @@ def gather_modules(): return reqs +def gather_requirements_from_manifests(errors, reqs): + """Gather all of the requirements from manifests.""" + integrations = Integration.load_dir(pathlib.Path( + 'homeassistant/components' + )) + for domain in sorted(integrations): + integration = integrations[domain] + + if not integration.manifest: + errors.append( + 'The manifest for component {} is invalid.'.format(domain) + ) + continue + + process_requirements( + errors, + integration.manifest['requirements'], + 'homeassistant.components.{}'.format(domain), + reqs + ) + + def gather_requirements_from_modules(errors, reqs): """Collect the requirements from the modules directly.""" for package in sorted( diff --git a/script/hassfest/__init__.py b/script/hassfest/__init__.py new file mode 100644 index 0000000000000..2fa7997162f2f --- /dev/null +++ b/script/hassfest/__init__.py @@ -0,0 +1 @@ +"""Manifest validator.""" diff --git a/script/hassfest/__main__.py b/script/hassfest/__main__.py new file mode 100644 index 0000000000000..fa6feb2c2698e --- /dev/null +++ b/script/hassfest/__main__.py @@ -0,0 +1,68 @@ +"""Validate manifests.""" +import pathlib +import sys + +from .model import Integration, Config +from . import dependencies, manifest, codeowners + +PLUGINS = [ + manifest, + dependencies, + codeowners, +] + + +def get_config() -> Config: + """Return config.""" + if not pathlib.Path('requirements_all.txt').is_file(): + raise RuntimeError("Run from project root") + + return Config( + root=pathlib.Path('.'), + action='validate' if sys.argv[-1] == 'validate' else 'generate', + ) + + +def main(): + """Validate manifests.""" + try: + config = get_config() + except RuntimeError as err: + print(err) + return 1 + + integrations = Integration.load_dir( + pathlib.Path('homeassistant/components') + ) + manifest.validate(integrations, config) + dependencies.validate(integrations, config) + codeowners.validate(integrations, config) + + invalid = [itg for itg in integrations.values() if itg.errors] + + print("Integrations:", len(integrations)) + print("Invalid integrations:", len(invalid)) + + if not invalid and not config.errors: + codeowners.generate(integrations, config) + return 0 + + print() + + if config.errors: + print("Generic errors:") + for error in config.errors: + print("*", error) + print() + + for integration in sorted(invalid, key=lambda itg: itg.domain): + print(integration.domain) + for error in integration.errors: + print("*", error) + print() + + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/script/hassfest/codeowners.py b/script/hassfest/codeowners.py new file mode 100755 index 0000000000000..ab61448281f25 --- /dev/null +++ b/script/hassfest/codeowners.py @@ -0,0 +1,84 @@ +"""Generate CODEOWNERS.""" +from typing import Dict + +from .model import Integration, Config + +BASE = """ +# This file is generated by script/manifest/codeowners.py +# People marked here will be automatically requested for a review +# when the code that they own is touched. +# https://github.com/blog/2392-introducing-code-owners + +# Home Assistant Core +setup.py @home-assistant/core +homeassistant/*.py @home-assistant/core +homeassistant/helpers/* @home-assistant/core +homeassistant/util/* @home-assistant/core + +# Virtualization +Dockerfile @home-assistant/docker +virtualization/Docker/* @home-assistant/docker + +# Other code +homeassistant/scripts/check_config.py @kellerza + +# Integrations +""".strip() + +INDIVIDUAL_FILES = """ +# Individual files +homeassistant/components/group/cover @cdce8p +homeassistant/components/demo/weather @fabaff +""" + + +def generate_and_validate(integrations: Dict[str, Integration]): + """Generate CODEOWNERS.""" + parts = [BASE] + + for domain in sorted(integrations): + integration = integrations[domain] + + if not integration.manifest: + continue + + codeowners = integration.manifest['codeowners'] + + if not codeowners: + continue + + for owner in codeowners: + if not owner.startswith('@'): + integration.add_error( + 'codeowners', + 'Code owners need to be valid GitHub handles.' + ) + + parts.append("homeassistant/components/{}/* {}".format( + domain, ' '.join(codeowners))) + + parts.append('\n' + INDIVIDUAL_FILES.strip()) + + return '\n'.join(parts) + + +def validate(integrations: Dict[str, Integration], config: Config): + """Validate CODEOWNERS.""" + codeowners_path = config.root / 'CODEOWNERS' + config.cache['codeowners'] = content = generate_and_validate(integrations) + + with open(codeowners_path, 'r') as fp: + if fp.read().strip() != content: + config.add_error( + "codeowners", + "File CODEOWNERS is not up to date. " + "Run python3 -m script.hassfest" + ) + return + + +def generate(integrations: Dict[str, Integration], config: Config): + """Generate CODEOWNERS.""" + codeowners_path = config.root / 'CODEOWNERS' + with open(codeowners_path, 'w') as fp: + fp.write(config.cache['codeowners'] + '\n') diff --git a/script/manifest_validate/dependencies.py b/script/hassfest/dependencies.py similarity index 78% rename from script/manifest_validate/dependencies.py rename to script/hassfest/dependencies.py index 9f4f0fdc4732a..14fd3373b2fd2 100644 --- a/script/manifest_validate/dependencies.py +++ b/script/hassfest/dependencies.py @@ -34,16 +34,18 @@ def validate_dependencies(integration: Integration): referenced = grep_dir(integration.path, "**/*.py", r"hass\.components\.(\w+)") referenced -= ALLOWED_USED_COMPONENTS - referenced -= set(integration.dependencies) + referenced -= set(integration.manifest['dependencies']) if referenced: for domain in sorted(referenced): - integration.errors.append( - "Using component {} but it's not a dependency".format(domain)) + integration.add_error( + 'dependencies', + "Using component {} but it's not a dependency".format(domain) + ) -def validate_all(integrations: Dict[str, Integration]): - """Validate all dependencies.""" +def validate(integrations: Dict[str, Integration], config): + """Handle dependencies for integrations.""" # check for non-existing dependencies for integration in integrations.values(): if not integration.manifest: @@ -52,8 +54,9 @@ def validate_all(integrations: Dict[str, Integration]): validate_dependencies(integration) # check that all referenced dependencies exist - for dep in integration.dependencies: + for dep in integration.manifest['dependencies']: if dep not in integrations: - integration.errors.append( + integration.add_error( + 'dependencies', "Dependency {} does not exist" ) diff --git a/script/manifest_validate/manifest.py b/script/hassfest/manifest.py similarity index 76% rename from script/manifest_validate/manifest.py rename to script/hassfest/manifest.py index 1f95a48040ae9..b644ec7d055f1 100644 --- a/script/manifest_validate/manifest.py +++ b/script/hassfest/manifest.py @@ -22,17 +22,19 @@ def validate_manifest(integration: Integration): try: MANIFEST_SCHEMA(integration.manifest) except vol.Invalid as err: - integration.errors.append( + integration.add_error( + 'manifest', "Invalid manifest: {}".format( humanize_error(integration.manifest, err))) + integration.manifest = None return if integration.manifest['domain'] != integration.path.name: - integration.errors.append('Domain does not match dir name') + integration.add_error('manifest', 'Domain does not match dir name') -def validate_all(integrations: Dict[str, Integration]): - """Validate all integrations manifests.""" +def validate(integrations: Dict[str, Integration], config): + """Handle all integrations manifests.""" for integration in integrations.values(): if integration.manifest: validate_manifest(integration) diff --git a/script/manifest_validate/manifest_helper.py b/script/hassfest/manifest_helper.py similarity index 100% rename from script/manifest_validate/manifest_helper.py rename to script/hassfest/manifest_helper.py diff --git a/script/hassfest/model.py b/script/hassfest/model.py new file mode 100644 index 0000000000000..f8f6ab00b529d --- /dev/null +++ b/script/hassfest/model.py @@ -0,0 +1,90 @@ +"""Models for manifest validator.""" +import json +from typing import List, Dict, Any +import pathlib + +import attr + + +@attr.s +class Error: + """Error validating an integration.""" + + plugin = attr.ib(type=str) + error = attr.ib(type=str) + + def __str__(self): + """String reprepsentation of the error.""" + return "[{}] {}".format(self.plugin.upper(), self.error) + + +@attr.s +class Config: + """Config for the run.""" + + root = attr.ib(type=pathlib.Path) + action = attr.ib(type=str) + errors = attr.ib(type=List[Error], factory=list) + cache = attr.ib(type=Dict[str, Any], factory=dict) + + def add_error(self, *args, **kwargs): + """Add an error.""" + self.errors.append(Error(*args, **kwargs)) + + +@attr.s +class Integration: + """Represent an integration in our validator.""" + + @classmethod + def load_dir(cls, path: pathlib.Path): + """Load all integrations in a directory.""" + assert path.is_dir() + integrations = {} + for fil in path.iterdir(): + if fil.is_file() or fil.name == '__pycache__': + continue + + integration = cls(fil) + integration.load_manifest() + integrations[integration.domain] = integration + + return integrations + + path = attr.ib(type=pathlib.Path) + manifest = attr.ib(type=dict, default=None) + errors = attr.ib(type=List[Error], factory=list) + + @property + def domain(self) -> str: + """Integration domain.""" + return self.path.name + + @property + def manifest_path(self) -> pathlib.Path: + """Integration manifest path.""" + return self.path / 'manifest.json' + + def add_error(self, *args, **kwargs): + """Add an error.""" + self.errors.append(Error(*args, **kwargs)) + + def load_manifest(self) -> None: + """Load manifest.""" + if not self.manifest_path.is_file(): + self.add_error( + 'model', + "Manifest file {} not found".format(self.manifest_path) + ) + return + + try: + manifest = json.loads(self.manifest_path.read_text()) + except ValueError as err: + self.add_error( + 'model', + "Manifest contains invalid JSON: {}".format(err) + ) + return + + self.manifest = manifest diff --git a/script/manifest_validate/__main__.py b/script/manifest_validate/__main__.py deleted file mode 100644 index 6e9d56d35f9c6..0000000000000 --- a/script/manifest_validate/__main__.py +++ /dev/null @@ -1,46 +0,0 @@ -"""Validate manifests.""" -import pathlib -import sys - -from .model import Integration -from . import dependencies, manifest - - -COMPONENTS_PATH = pathlib.Path('homeassistant/components') - - -def main(): - """Validate manifests.""" - integrations = {} - invalid = [] - - for fil in COMPONENTS_PATH.iterdir(): - if fil.is_file() or fil.name == '__pycache__': - continue - - integration = Integration(fil) - integration.load_manifest() - integrations[integration.domain] = integration - - manifest.validate_all(integrations) - dependencies.validate_all(integrations) - - invalid = [itg for itg in integrations.values() if itg.errors] - - if not invalid: - return 0 - - print("Found invalid integrations") - print() - - for integration in sorted(invalid, key=lambda itg: itg.domain): - print(integration.domain) - for error in integration.errors: - print("*", error) - print() - - return 1 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/script/manifest_validate/codeowners.py b/script/manifest_validate/codeowners.py deleted file mode 100755 index 96b2b252e3d4e..0000000000000 --- a/script/manifest_validate/codeowners.py +++ /dev/null @@ -1,74 +0,0 @@ -#!/usr/bin/env python3 -"""Generate CODEOWNERS.""" -import os -import sys - -from manifest_helper import iter_manifests - -BASE = """ -# This file is generated by script/manifest/codeowners.py -# People marked here will be automatically requested for a review -# when the code that they own is touched. -# https://github.com/blog/2392-introducing-code-owners - -# Home Assistant Core -setup.py @home-assistant/core -homeassistant/*.py @home-assistant/core -homeassistant/helpers/* @home-assistant/core -homeassistant/util/* @home-assistant/core - -# Virtualization -Dockerfile @home-assistant/docker -virtualization/Docker/* @home-assistant/docker - -# Other code -homeassistant/scripts/check_config.py @kellerza - -# Integrations -""" - -INDIVIDUAL_FILES = """ -# Individual files -homeassistant/components/group/cover @cdce8p -homeassistant/components/demo/weather @fabaff -""" - - -def generate(): - """Generate CODEOWNERS.""" - parts = [BASE.strip()] - - for manifest in iter_manifests(): - if not manifest['codeowners']: - continue - - parts.append("homeassistant/components/{}/* {}".format( - manifest['domain'], ' '.join(manifest['codeowners']))) - - parts.append('\n' + INDIVIDUAL_FILES.strip()) - - return '\n'.join(parts) - - -def main(validate): - """Runner for CODEOWNERS gen.""" - if not os.path.isfile('requirements_all.txt'): - print('Run this from HA root dir') - return 1 - - content = generate() - - if validate: - with open('CODEOWNERS', 'r') as fp: - if fp.read().strip() != content: - print("CODEOWNERS is not up to date. " - "Run python script/manifest/codeowners.py") - return 1 - return 0 - - with open('CODEOWNERS', 'w') as fp: - fp.write(content + '\n') - - -if __name__ == '__main__': - sys.exit(main(sys.argv[-1] == 'validate')) diff --git a/script/manifest_validate/model.py b/script/manifest_validate/model.py deleted file mode 100644 index 0781de3741e19..0000000000000 --- a/script/manifest_validate/model.py +++ /dev/null @@ -1,45 +0,0 @@ -"""Models for manifest validator.""" -import json -import pathlib - -import attr - - -@attr.s -class Integration: - """Represent an integration in our validator.""" - - path = attr.ib(type=pathlib.Path) - manifest = attr.ib(type=dict, default=None) - errors = attr.ib(type=list, factory=list) - - @property - def domain(self): - """Integration domain.""" - return self.path.name - - @property - def manifest_path(self): - """Integration manifest path.""" - return self.path / 'manifest.json' - - @property - def dependencies(self): - """Get dependencies.""" - return [] if self.manifest is None else self.manifest['dependencies'] - - def load_manifest(self): - """Load manifest.""" - if not self.manifest_path.is_file(): - self.errors.append( - "Manifest file {} not found".format(self.manifest_path)) - return - - try: - manifest = json.loads(self.manifest_path.read_text()) - except ValueError as err: - self.errors.append( - "Manifest contains invalid JSON: {}".format(err)) - return - - self.manifest = manifest diff --git a/script/manifest_validate/requirements.py b/script/manifest_validate/requirements.py deleted file mode 100644 index 5a370510484ab..0000000000000 --- a/script/manifest_validate/requirements.py +++ /dev/null @@ -1,22 +0,0 @@ -"""Helpers to gather requirements from manifests.""" -from .manifest_helper import iter_manifests - - -def gather_requirements_from_manifests(process_requirements, errors, reqs): - """Gather all of the requirements from manifests.""" - for manifest in iter_manifests(): - assert manifest['domain'] - - if manifest.get('requirements') is None: - errors.append( - 'The manifest for component {} is invalid. Please run' - 'script/manifest/validate.py'.format(manifest['domain']) - ) - continue - - process_requirements( - errors, - manifest['requirements'], - 'homeassistant.components.{}'.format(manifest['domain']), - reqs - ) From 77761f1318d0d6749f6af64aade53d9f9881ec9d Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 10:32:20 -0700 Subject: [PATCH 07/13] Annotate errors with fixable --- CODEOWNERS | 7 +++++++ script/hassfest/__main__.py | 32 ++++++++++++++++++++++++-------- script/hassfest/codeowners.py | 5 +++-- script/hassfest/model.py | 1 + 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 2b45acea2e1b4..c40359bf6c3af 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -9,6 +9,13 @@ homeassistant/*.py @home-assistant/core homeassistant/helpers/* @home-assistant/core homeassistant/util/* @home-assistant/core + + + + + + + # Virtualization Dockerfile @home-assistant/docker virtualization/Docker/* @home-assistant/docker diff --git a/script/hassfest/__main__.py b/script/hassfest/__main__.py index fa6feb2c2698e..5be95788ca17e 100644 --- a/script/hassfest/__main__.py +++ b/script/hassfest/__main__.py @@ -38,25 +38,41 @@ def main(): dependencies.validate(integrations, config) codeowners.validate(integrations, config) - invalid = [itg for itg in integrations.values() if itg.errors] + # When we generate, all errors that are fixable will be ignored, + # as generating them will be fixed. + if config.action == 'generate': + general_errors = [err for err in config.errors if not err.fixable] + invalid_itg = [ + itg for itg in integrations.values() + if any( + not error.fixable for error in itg.errors + ) + ] + else: + # action == validate + general_errors = config.errors + invalid_itg = [itg for itg in integrations.values() if itg.errors] print("Integrations:", len(integrations)) - print("Invalid integrations:", len(invalid)) + print("Invalid integrations:", len(invalid_itg)) - if not invalid and not config.errors: + if not invalid_itg and not general_errors: codeowners.generate(integrations, config) return 0 print() + if config.action == 'generate': + print("Found errors. Generating files canceled.") + print() - if config.errors: - print("Generic errors:") - for error in config.errors: + if general_errors: + print("General errors:") + for error in general_errors: print("*", error) print() - for integration in sorted(invalid, key=lambda itg: itg.domain): - print(integration.domain) + for integration in sorted(invalid_itg, key=lambda itg: itg.domain): + print("Integration {}:".format(integration.domain)) for error in integration.errors: print("*", error) print() diff --git a/script/hassfest/codeowners.py b/script/hassfest/codeowners.py index ab61448281f25..2785ff7b24bc2 100755 --- a/script/hassfest/codeowners.py +++ b/script/hassfest/codeowners.py @@ -51,7 +51,7 @@ def generate_and_validate(integrations: Dict[str, Integration]): if not owner.startswith('@'): integration.add_error( 'codeowners', - 'Code owners need to be valid GitHub handles.' + 'Code owners need to be valid GitHub handles.', ) parts.append("homeassistant/components/{}/* {}".format( @@ -72,7 +72,8 @@ def validate(integrations: Dict[str, Integration], config: Config): config.add_error( "codeowners", "File CODEOWNERS is not up to date. " - "Run python3 -m script.hassfest" + "Run python3 -m script.hassfest", + fixable=True ) return diff --git a/script/hassfest/model.py b/script/hassfest/model.py index f8f6ab00b529d..18fafe2b75f2c 100644 --- a/script/hassfest/model.py +++ b/script/hassfest/model.py @@ -12,6 +12,7 @@ class Error: plugin = attr.ib(type=str) error = attr.ib(type=str) + fixable = attr.ib(type=bool, default=False) def __str__(self): """String reprepsentation of the error.""" From e30a4993eb4dfdcbc4d69e54fb5a4210178f34a3 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 17:42:03 -0700 Subject: [PATCH 08/13] Convert error to warning --- CODEOWNERS | 7 ------- homeassistant/components/cloud/manifest.json | 3 ++- homeassistant/components/demo/manifest.json | 4 +++- homeassistant/components/hassio/manifest.json | 3 ++- homeassistant/components/map/manifest.json | 4 +++- homeassistant/components/panel_custom/__init__.py | 5 ++++- homeassistant/components/websocket_api/__init__.py | 2 +- homeassistant/components/websocket_api/commands.py | 3 +-- script/hassfest/dependencies.py | 13 ++++++++----- 9 files changed, 24 insertions(+), 20 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index c40359bf6c3af..2b45acea2e1b4 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -9,13 +9,6 @@ homeassistant/*.py @home-assistant/core homeassistant/helpers/* @home-assistant/core homeassistant/util/* @home-assistant/core - - - - - - - # Virtualization Dockerfile @home-assistant/docker virtualization/Docker/* @home-assistant/docker diff --git a/homeassistant/components/cloud/manifest.json b/homeassistant/components/cloud/manifest.json index b7822fcd90382..61e9600302f05 100644 --- a/homeassistant/components/cloud/manifest.json +++ b/homeassistant/components/cloud/manifest.json @@ -6,7 +6,8 @@ "hass-nabucasa==0.11" ], "dependencies": [ - "http" + "http", + "webhook" ], "codeowners": [ "@home-assistant/core" diff --git a/homeassistant/components/demo/manifest.json b/homeassistant/components/demo/manifest.json index ab079a4c2eed1..4f167ecae25af 100644 --- a/homeassistant/components/demo/manifest.json +++ b/homeassistant/components/demo/manifest.json @@ -5,7 +5,9 @@ "requirements": [], "dependencies": [ "conversation", - "zone" + "zone", + "group", + "configurator" ], "codeowners": [ "@home-assistant/core" diff --git a/homeassistant/components/hassio/manifest.json b/homeassistant/components/hassio/manifest.json index be345fb5adbc1..23095064d558a 100644 --- a/homeassistant/components/hassio/manifest.json +++ b/homeassistant/components/hassio/manifest.json @@ -4,7 +4,8 @@ "documentation": "https://www.home-assistant.io/hassio", "requirements": [], "dependencies": [ - "http" + "http", + "panel_custom" ], "codeowners": [ "@home-assistant/hass-io" diff --git a/homeassistant/components/map/manifest.json b/homeassistant/components/map/manifest.json index 993dfc6577eab..d26d7d9530fdd 100644 --- a/homeassistant/components/map/manifest.json +++ b/homeassistant/components/map/manifest.json @@ -3,6 +3,8 @@ "name": "Map", "documentation": "https://www.home-assistant.io/components/map", "requirements": [], - "dependencies": [], + "dependencies": [ + "frontend" + ], "codeowners": [] } diff --git a/homeassistant/components/panel_custom/__init__.py b/homeassistant/components/panel_custom/__init__.py index 9367f10244180..f6a4fcdb73393 100644 --- a/homeassistant/components/panel_custom/__init__.py +++ b/homeassistant/components/panel_custom/__init__.py @@ -124,9 +124,12 @@ async def async_register_panel( async def async_setup(hass, config): """Initialize custom panel.""" + if DOMAIN not in config: + return True + success = False - for panel in config.get(DOMAIN): + for panel in config[DOMAIN]: name = panel[CONF_COMPONENT_NAME] kwargs = { diff --git a/homeassistant/components/websocket_api/__init__.py b/homeassistant/components/websocket_api/__init__.py index 6c4935b9d950a..6bb4ea9c1c4bd 100644 --- a/homeassistant/components/websocket_api/__init__.py +++ b/homeassistant/components/websocket_api/__init__.py @@ -43,5 +43,5 @@ def async_register_command(hass, command_or_handler, handler=None, async def async_setup(hass, config): """Initialize the websocket API.""" hass.http.register_view(http.WebsocketAPIView) - commands.async_register_commands(hass) + commands.async_register_commands(hass, async_register_command) return True diff --git a/homeassistant/components/websocket_api/commands.py b/homeassistant/components/websocket_api/commands.py index 32bbd90aad1eb..a076ffc1386c6 100644 --- a/homeassistant/components/websocket_api/commands.py +++ b/homeassistant/components/websocket_api/commands.py @@ -14,9 +14,8 @@ @callback -def async_register_commands(hass): +def async_register_commands(hass, async_reg): """Register commands.""" - async_reg = hass.components.websocket_api.async_register_command async_reg(handle_subscribe_events) async_reg(handle_unsubscribe_events) async_reg(handle_call_service) diff --git a/script/hassfest/dependencies.py b/script/hassfest/dependencies.py index 14fd3373b2fd2..af8a782906bbb 100644 --- a/script/hassfest/dependencies.py +++ b/script/hassfest/dependencies.py @@ -22,7 +22,7 @@ def grep_dir(path: pathlib.Path, glob_pattern: str, search_pattern: str) \ return found -# Allowed components to be referenced without being a dependency +# These components will always be set up ALLOWED_USED_COMPONENTS = { 'persistent_notification', } @@ -38,10 +38,13 @@ def validate_dependencies(integration: Integration): if referenced: for domain in sorted(referenced): - integration.add_error( - 'dependencies', - "Using component {} but it's not a dependency".format(domain) - ) + print("Warning: {} references integration {} but it's not a " + "dependency".format(integration.domain, domain)) + # Not enforced yet. + # integration.add_error( + # 'dependencies', + # "Using component {} but it's not a dependency".format(domain) + # ) def validate(integrations: Dict[str, Integration], config): From 2e9fdcce589bea1ad2cd50bdbbd60a93c69586a3 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 19:26:13 -0700 Subject: [PATCH 09/13] Lint --- script/hassfest/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/hassfest/model.py b/script/hassfest/model.py index 18fafe2b75f2c..15e3bdf6284dd 100644 --- a/script/hassfest/model.py +++ b/script/hassfest/model.py @@ -14,8 +14,8 @@ class Error: error = attr.ib(type=str) fixable = attr.ib(type=bool, default=False) - def __str__(self): - """String reprepsentation of the error.""" + def __str__(self) -> str: + """Reprepsentat error as string.""" return "[{}] {}".format(self.plugin.upper(), self.error) From aeb97d73aae13d929119a9fe0bdae8afef7569af Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 19:32:01 -0700 Subject: [PATCH 10/13] Make abs path --- script/hassfest/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/hassfest/__main__.py b/script/hassfest/__main__.py index 5be95788ca17e..2514db6314d81 100644 --- a/script/hassfest/__main__.py +++ b/script/hassfest/__main__.py @@ -18,7 +18,7 @@ def get_config() -> Config: raise RuntimeError("Run from project root") return Config( - root=pathlib.Path('.'), + root=pathlib.Path('.').absolute(), action='validate' if sys.argv[-1] == 'validate' else 'generate', ) From da3db0604b5f4f6e2e71ad747a2026d5bd8357ce Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 12 Apr 2019 19:36:04 -0700 Subject: [PATCH 11/13] Python 3.5... --- script/hassfest/codeowners.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/hassfest/codeowners.py b/script/hassfest/codeowners.py index 2785ff7b24bc2..8ba2008f1cdbb 100755 --- a/script/hassfest/codeowners.py +++ b/script/hassfest/codeowners.py @@ -67,7 +67,7 @@ def validate(integrations: Dict[str, Integration], config: Config): codeowners_path = config.root / 'CODEOWNERS' config.cache['codeowners'] = content = generate_and_validate(integrations) - with open(codeowners_path, 'r') as fp: + with open(str(codeowners_path), 'r') as fp: if fp.read().strip() != content: config.add_error( "codeowners", @@ -81,5 +81,5 @@ def validate(integrations: Dict[str, Integration], config: Config): def generate(integrations: Dict[str, Integration], config: Config): """Generate CODEOWNERS.""" codeowners_path = config.root / 'CODEOWNERS' - with open(codeowners_path, 'w') as fp: + with open(str(codeowners_path), 'w') as fp: fp.write(config.cache['codeowners'] + '\n') From 38382251f1795b84caccce820a7d8ffad70d946d Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 13 Apr 2019 07:36:41 -0700 Subject: [PATCH 12/13] Typo --- script/hassfest/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/hassfest/model.py b/script/hassfest/model.py index 15e3bdf6284dd..c2a72ebd5090e 100644 --- a/script/hassfest/model.py +++ b/script/hassfest/model.py @@ -15,7 +15,7 @@ class Error: fixable = attr.ib(type=bool, default=False) def __str__(self) -> str: - """Reprepsentat error as string.""" + """Represent error as string.""" return "[{}] {}".format(self.plugin.upper(), self.error) From 81455058acbbf22669350fa387fec3160786b60f Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sat, 13 Apr 2019 11:04:09 -0700 Subject: [PATCH 13/13] Fix tests --- homeassistant/components/websocket_api/commands.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/websocket_api/commands.py b/homeassistant/components/websocket_api/commands.py index a076ffc1386c6..d9834758c8021 100644 --- a/homeassistant/components/websocket_api/commands.py +++ b/homeassistant/components/websocket_api/commands.py @@ -16,13 +16,13 @@ @callback def async_register_commands(hass, async_reg): """Register commands.""" - async_reg(handle_subscribe_events) - async_reg(handle_unsubscribe_events) - async_reg(handle_call_service) - async_reg(handle_get_states) - async_reg(handle_get_services) - async_reg(handle_get_config) - async_reg(handle_ping) + async_reg(hass, handle_subscribe_events) + async_reg(hass, handle_unsubscribe_events) + async_reg(hass, handle_call_service) + async_reg(hass, handle_get_states) + async_reg(hass, handle_get_services) + async_reg(hass, handle_get_config) + async_reg(hass, handle_ping) def pong_message(iden):