From 1661124b7a39e187b01b6150beaf48a5056c4338 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 7 Feb 2024 10:42:05 -0500 Subject: [PATCH] Enforce that files in the `src` directory are referenced from BUILD.gn (#31960) * Start creating a script * Have much more functionality * Restyle * Add some doc comments ... this starts being usable * Add workflow to validate that gn knows about files * Remove controller from known exceptions: we fixed that one * Fix flake8 * Add more known failures * Better error reporting for gn reachability * Remove the platform specific orphan file listing * Force the "not failures anymore" to be fatal --------- Co-authored-by: Andrei Litvin --- .github/workflows/lint.yml | 160 +++++++++++++++++++++++++++ scripts/tools/not_known_to_gn.py | 182 +++++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+) create mode 100755 scripts/tools/not_known_to_gn.py diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 29a3dc8624e491..2924edff45419d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -42,6 +42,166 @@ jobs: with: platform: linux + - name: Check for orphaned gn files + if: always() + # We should enforce that ALL new files are referenced in our build scripts. + # Several things do not have a clear fix path: + # - various platform implementations (including darwin-specific files as they + # are not using GN) + # - app/clusters (they are fetched dynamically - this should probably be fixed) + # + # All the rest of the exceptions should be driven down to 0: chip should fully + # be defined in build rules. + # + # This check enforces that for any newly added file, it must be part of some + # BUILD.gn file + run: | + ./scripts/run_in_build_env.sh "./scripts/tools/not_known_to_gn.py \ + src \ + --skip-dir app/clusters \ + --skip-dir darwin \ + --skip-dir include \ + --skip-dir platform/Ameba \ + --skip-dir platform/android \ + --skip-dir platform/ASR \ + --skip-dir platform/Beken \ + --skip-dir platform/bouffalolab \ + --skip-dir platform/cc13xx_26xx \ + --skip-dir platform/cc32xx \ + --skip-dir platform/Darwin \ + --skip-dir platform/ESP32 \ + --skip-dir platform/fake \ + --skip-dir platform/FreeRTOS \ + --skip-dir platform/Infineon \ + --skip-dir platform/Linux \ + --skip-dir platform/mbed \ + --skip-dir platform/mt793x \ + --skip-dir platform/nxp \ + --skip-dir platform/OpenThread \ + --skip-dir platform/qpg \ + --skip-dir platform/silabs \ + --skip-dir platform/telink \ + --skip-dir platform/webos \ + --skip-dir platform/Zephyr \ + --skip-dir test_driver \ + --known-failure app/app-platform/ContentApp.cpp \ + --known-failure app/app-platform/ContentApp.h \ + --known-failure app/app-platform/ContentAppPlatform.cpp \ + --known-failure app/app-platform/ContentAppPlatform.h \ + --known-failure controller/ExamplePersistentStorage.cpp \ + --known-failure controller/ExamplePersistentStorage.h \ + --known-failure controller/java/GroupDeviceProxy.h \ + --known-failure controller/java/CHIPEventTLVValueDecoder.h \ + --known-failure controller/python/chip/credentials/cert.h \ + --known-failure controller/python/chip/server/Options.h \ + --known-failure controller/python/chip/crypto/p256keypair.h \ + --known-failure controller/python/chip/commissioning/PlaceholderOperationalCredentialsIssuer.h \ + --known-failure controller/python/chip/native/PyChipError.h \ + --known-failure app/AttributeAccessInterface.h \ + --known-failure app/AttributeAccessToken.h \ + --known-failure app/att-storage.h \ + --known-failure app/BufferedReadCallback.h \ + --known-failure app/CommandHandler.h \ + --known-failure app/CommandHandlerInterface.h \ + --known-failure app/CommandPathParams.h \ + --known-failure app/CommandPathRegistry.h \ + --known-failure app/CommandResponseSender.h \ + --known-failure app/CommandSender.h \ + --known-failure app/CommandSenderLegacyCallback.h \ + --known-failure app/CompatEnumNames.h \ + --known-failure app/ConcreteAttributePath.h \ + --known-failure app/ConcreteCommandPath.h \ + --known-failure app/data-model/ListLargeSystemExtensions.h \ + --known-failure app/EventHeader.h \ + --known-failure app/EventLoggingDelegate.h \ + --known-failure app/EventLogging.h \ + --known-failure app/EventLoggingTypes.h \ + --known-failure app/EventManagement.h \ + --known-failure app/InteractionModelHelper.h \ + --known-failure app/MessageDef/ArrayBuilder.h \ + --known-failure app/MessageDef/ArrayParser.h \ + --known-failure app/MessageDef/CommandDataIB.h \ + --known-failure app/MessageDef/CommandPathIB.h \ + --known-failure app/MessageDef/CommandStatusIB.h \ + --known-failure app/MessageDef/EventFilterIB.h \ + --known-failure app/MessageDef/EventFilterIBs.h \ + --known-failure app/MessageDef/InvokeRequestMessage.h \ + --known-failure app/MessageDef/InvokeRequests.h \ + --known-failure app/MessageDef/InvokeResponseIB.h \ + --known-failure app/MessageDef/InvokeResponseIBs.h \ + --known-failure app/MessageDef/InvokeResponseMessage.h \ + --known-failure app/MessageDef/ListBuilder.h \ + --known-failure app/MessageDef/ListParser.h \ + --known-failure app/MessageDef/StatusResponseMessage.h \ + --known-failure app/MessageDef/StructBuilder.h \ + --known-failure app/MessageDef/StructParser.h \ + --known-failure app/MessageDef/SubscribeRequestMessage.h \ + --known-failure app/MessageDef/SubscribeResponseMessage.h \ + --known-failure app/MessageDef/TimedRequestMessage.h \ + --known-failure app/MessageDef/WriteRequestMessage.h \ + --known-failure app/MessageDef/WriteResponseMessage.h \ + --known-failure app/ObjectList.h \ + --known-failure app/ReadClient.h \ + --known-failure app/ReadHandler.h \ + --known-failure app/ReadPrepareParams.h \ + --known-failure app/reporting/tests/MockReportScheduler.cpp \ + --known-failure app/reporting/tests/MockReportScheduler.h \ + --known-failure app/server/AppDelegate.h \ + --known-failure app/TestEventTriggerDelegate.h \ + --known-failure app/tests/integration/common.h \ + --known-failure app/tests/integration/MockEvents.h \ + --known-failure app/tests/suites/credentials/TestHarnessDACProvider.h \ + --known-failure app/tests/TestOperationalDeviceProxy.cpp \ + --known-failure app/util/af-enums.h \ + --known-failure app/util/af.h \ + --known-failure app/util/af-types.h \ + --known-failure app/util/attribute-metadata.h \ + --known-failure app/util/attribute-storage.cpp \ + --known-failure app/util/attribute-storage.h \ + --known-failure app/util/attribute-storage-null-handling.h \ + --known-failure app/util/attribute-table.cpp \ + --known-failure app/util/attribute-table.h \ + --known-failure app/util/binding-table.cpp \ + --known-failure app/util/binding-table.h \ + --known-failure app/util/common.h \ + --known-failure app/util/config.h \ + --known-failure app/util/DataModelHandler.cpp \ + --known-failure app/util/DataModelHandler.h \ + --known-failure app/util/ember-compatibility-functions.cpp \ + --known-failure app/util/endpoint-config-api.h \ + --known-failure app/util/endpoint-config-defines.h \ + --known-failure app/util/error-mapping.h \ + --known-failure app/util/generic-callbacks.h \ + --known-failure app/util/generic-callback-stubs.cpp \ + --known-failure app/util/im-client-callbacks.h \ + --known-failure app/util/MatterCallbacks.h \ + --known-failure app/util/message.cpp \ + --known-failure app/util/mock/Constants.h \ + --known-failure app/util/mock/Functions.h \ + --known-failure app/util/mock/MockNodeConfig.h \ + --known-failure app/util/odd-sized-integers.h \ + --known-failure app/util/types_stub.h \ + --known-failure app/util/util.cpp \ + --known-failure app/util/util.h \ + --known-failure app/WriteClient.h \ + --known-failure app/WriteHandler.h \ + --known-failure inet/tests/TestInetLayerCommon.hpp \ + --known-failure lib/core/CHIPVendorIdentifiers.hpp \ + --known-failure lib/dnssd/Constants.h \ + --known-failure lib/dnssd/minimal_mdns/core/FlatAllocatedQName.h \ + --known-failure lib/dnssd/minimal_mdns/core/HeapQName.h \ + --known-failure lib/dnssd/minimal_mdns/ListenIterator.h \ + --known-failure lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h \ + --known-failure lib/dnssd/platform/DnssdBrowseDelegate.h \ + --known-failure lib/support/CHIPArgParser.hpp \ + --known-failure messaging/tests/echo/common.h \ + --known-failure platform/DeviceSafeQueue.cpp \ + --known-failure platform/DeviceSafeQueue.h \ + --known-failure platform/GLibTypeDeleter.h \ + --known-failure platform/SingletonConfigurationManager.cpp \ + --known-failure transport/retransmit/tests/TestCacheDriver.cpp \ + " + - name: Check for matter lint errors if: always() run: | diff --git a/scripts/tools/not_known_to_gn.py b/scripts/tools/not_known_to_gn.py new file mode 100755 index 00000000000000..613f8a23ecb707 --- /dev/null +++ b/scripts/tools/not_known_to_gn.py @@ -0,0 +1,182 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Lists files specific files from a source tree and ensures +they are covered by GN in some way. + +'Covered' is very loosely and it just tries to see if the GN text +contains that word without trying to validate if this is a +comment or some actual 'source' element. + +It is intended as a failsafe to not foget adding source files +to gn. +""" +import logging +import os +import sys +from pathlib import Path, PurePath +from typing import Dict, Set + +import click +import coloredlogs + +__LOG_LEVELS__ = { + 'debug': logging.DEBUG, + 'info': logging.INFO, + 'warn': logging.WARN, + 'fatal': logging.FATAL, +} + + +class OrphanChecker: + def __init__(self): + self.gn_data: Dict[str, str] = {} + self.known_failures: Set[str] = set() + self.fatal_failures = 0 + self.failures = 0 + self.found_failures: Set[str] = set() + + def AppendGnData(self, gn: PurePath): + """Adds a GN file to the list of internally known GN data. + + Will read the entire content of the GN file in memory for future reference. + """ + logging.debug(f'Adding GN {gn!s} for {gn.parent!s}') + self.gn_data[str(gn.parent)] = gn.read_text('utf-8') + + def AddKnownFailure(self, k: str): + self.known_failures.add(k) + + def _IsKnownFailure(self, path: str) -> bool: + """check if failing on the given path is a known/acceptable failure""" + for k in self.known_failures: + if path == k or path.endswith(os.path.sep + k): + # mark some found failures to report if something is supposed + # to be known but it is not + self.found_failures.add(k) + return True + return False + + def Check(self, top_dir: str, file: PurePath): + """ + Validates that the given path is somehow referenced in GN files in any + of the parent sub-directories of the file. + + `file` must be relative to `top_dir`. Top_dir is used to resolve relative + paths in error reports and known failure checks. + """ + # Check logic: + # - ensure the file name is included in some GN file inside this or + # upper directory (although upper directory is not ideal) + for p in file.parents: + data = self.gn_data.get(str(p), None) + if not data: + continue + + if file.name in data: + logging.debug("%s found in BUILD.gn for %s", file, p) + return + + path = str(file.relative_to(top_dir)) + if not self._IsKnownFailure(path): + logging.error("UNKNOWN to gn: %s", path) + self.fatal_failures += 1 + else: + logging.warning("UNKNOWN to gn: %s (known error)", path) + + self.failures += 1 + + +@click.command() +@click.option( + '--log-level', + default='INFO', + type=click.Choice(list(__LOG_LEVELS__.keys()), case_sensitive=False), + help='Determines the verbosity of script output', +) +@click.option( + '--extensions', + default=["cpp", "cc", "c", "h", "hpp"], + type=str, multiple=True, + help='What file extensions to consider', +) +@click.option( + '--known-failure', + type=str, multiple=True, + help='What paths are known to fail', +) +@click.option( + '--skip-dir', + type=str, + multiple=True, + help='Skip a specific sub-directory from checks', +) +@click.argument('dirs', + type=click.Path(exists=True, file_okay=False, resolve_path=True), nargs=-1) +def main(log_level, extensions, dirs, known_failure, skip_dir): + coloredlogs.install(level=__LOG_LEVELS__[log_level], + fmt='%(asctime)s %(levelname)-7s %(message)s') + + if not dirs: + logging.error("Please provide at least one directory to scan") + sys.exit(1) + + if not extensions: + logging.error("Need at least one extension") + sys.exit(1) + + checker = OrphanChecker() + for k in known_failure: + checker.AddKnownFailure(k) + + # ensure all GN data is loaded + for directory in dirs: + for name in Path(directory).rglob("BUILD.gn"): + checker.AppendGnData(name) + + skip_dir = set(skip_dir) + + # Go through all files and check for orphaned (if any) + extensions = set(extensions) + for directory in dirs: + for path, dirnames, filenames in os.walk(directory): + if any([s in path for s in skip_dir]): + continue + for f in filenames: + full_path = Path(os.path.join(path, f)) + if not full_path.suffix or full_path.suffix[1:] not in extensions: + continue + checker.Check(directory, full_path) + + if checker.failures: + logging.warning("%d files not known to GN (%d fatal)", checker.failures, checker.fatal_failures) + + if checker.known_failures != checker.found_failures: + not_failing = checker.known_failures - checker.found_failures + logging.warning("NOTE: %d failures are not found anymore:", len(not_failing)) + for name in not_failing: + logging.warning(" - %s", name) + # Assume this is fatal - remove some of the "known-failing" should be easy. + # This forces scripts to always be correct and not accumulate bad input. + sys.exit(1) + + if checker.fatal_failures > 0: + sys.exit(1) + + +if __name__ == '__main__': + main(auto_envvar_prefix='CHIP')