Merge branch 'bugfix/codeowners_patterns' into 'master'

fix CODEOWNERS file patterns

See merge request espressif/esp-idf!10455
This commit is contained in:
Ivan Grokhotkov 2020-09-16 02:56:28 +08:00
commit deb7ca8bac
2 changed files with 251 additions and 119 deletions

View File

@ -1,107 +1,167 @@
# For the syntax of this file, see:
#
# https://docs.gitlab.com/ee/user/project/code_owners.html#the-syntax-of-code-owners-files
#
# If more than one rule matches a given file, the latest rule is used.
# The file should be generally kept sorted, except when it is necessary
# to use a different order due to the fact above. In that case, use
# '# sort-order-reset' comment line to reset the sort order.
#
# Recipes for a few common cases:
#
# 1. Specific directory with all its contents:
#
# /components/app_trace/
#
# Note the trailing slash!
#
# 2. File with certain extension in any subdirectory of a certain directory:
#
# /examples/**/*.py
#
# This includes an *.py files in /examples/ directory as well.
#
# 3. Contents of a directory with a certain name, anywhere in the tree:
#
# test_*_host/
#
# Will match everything under components/efuse/test_efuse_host/,
# components/heap/test_multi_heap_host/, components/lwip/test_afl_host/, etc.
#
# 4. Same as above, except limited to a specific place in the tree:
#
# /components/esp32*/
#
# Matches everything under /components/esp32, /components/esp32s2, etc.
# Doesn't match /tools/some-test/components/esp32s5.
#
# 5. Specific file:
#
# /tools/tools.json
#
# 6. File with a certain name anywhere in the tree
#
# .gitignore
#
* @esp-idf-codeowners/other * @esp-idf-codeowners/other
components/app_trace @esp-idf-codeowners/tools /.* @esp-idf-codeowners/tools
components/app_update @esp-idf-codeowners/system @esp-idf-codeowners/app-utilities /.gitlab-ci.yml @esp-idf-codeowners/ci
components/asio @esp-idf-codeowners/network /.readthedocs.yml @esp-idf-codeowners/docs
components/bootloader*/** @esp-idf-codeowners/system @esp-idf-codeowners/security /CMakeLists.txt @esp-idf-codeowners/build-config
components/bt @esp-idf-codeowners/bluetooth /Kconfig @esp-idf-codeowners/build-config
components/cbor @esp-idf-codeowners/app-utilities /add_path.sh @esp-idf-codeowners/tools
components/coap @esp-idf-codeowners/app-utilities /export.* @esp-idf-codeowners/tools
components/console @esp-idf-codeowners/system @esp-idf-codeowners/app-utilities /install.* @esp-idf-codeowners/tools
components/cxx @esp-idf-codeowners/system /sdkconfig.rename @esp-idf-codeowners/build-config
components/driver @esp-idf-codeowners/peripherals
components/efuse @esp-idf-codeowners/system
components/esp32* @esp-idf-codeowners/system
components/esp_adc_cal @esp-idf-codeowners/peripherals
components/esp_common @esp-idf-codeowners/system
components/esp_eth @esp-idf-codeowners/network
components/esp_event @esp-idf-codeowners/system
components/esp_gdbstub @esp-idf-codeowners/tools
components/esp_hid @esp-idf-codeowners/bluetooth
components/esp_http_client @esp-idf-codeowners/app-utilities
components/esp_http_server @esp-idf-codeowners/app-utilities
components/esp_https_ota @esp-idf-codeowners/app-utilities
components/esp_https_server @esp-idf-codeowners/app-utilities
components/esp_ipc @esp-idf-codeowners/system
components/esp_local_ctrl @esp-idf-codeowners/app-utilities
components/esp_netif @esp-idf-codeowners/network
components/esp_ringbuf @esp-idf-codeowners/system
components/esp_rom @esp-idf-codeowners/system
components/esp_serial_slave_link @esp-idf-codeowners/peripherals
components/esp_system @esp-idf-codeowners/system
components/esp_timer @esp-idf-codeowners/system
components/esp-tls @esp-idf-codeowners/app-utilities
components/esp_websocket_client @esp-idf-codeowners/network
components/esp_wifi @esp-idf-codeowners/wifi
components/espcoredump @esp-idf-codeowners/tools
components/esptool_py @esp-idf-codeowners/tools
components/expat @esp-idf-codeowners/app-utilities
components/fatfs @esp-idf-codeowners/storage
components/freemodbus @esp-idf-codeowners/peripherals
components/freertos @esp-idf-codeowners/system
components/heap @esp-idf-codeowners/system
components/idf_test @esp-idf-codeowners/ci
components/jsmn @esp-idf-codeowners/app-utilities
components/json @esp-idf-codeowners/app-utilities
components/libsodium @esp-idf-codeowners/security
components/log @esp-idf-codeowners/system
components/lwip @esp-idf-codeowners/lwip
components/mbedtls @esp-idf-codeowners/app-utilities @esp-idf-codeowners/security
components/mdns @esp-idf-codeowners/network
components/mqtt @esp-idf-codeowners/network
components/newlib @esp-idf-codeowners/system @esp-idf-codeowners/tools
components/nghttp @esp-idf-codeowners/app-utilities
components/nvs_flash @esp-idf-codeowners/storage
components/openssl @esp-idf-codeowners/network
components/partition_table @esp-idf-codeowners/system
components/perfmon @esp-idf-codeowners/tools
components/protobuf-c @esp-idf-codeowners/app-utilities
components/protocomm @esp-idf-codeowners/app-utilities
components/pthread @esp-idf-codeowners/system
components/sdmmc @esp-idf-codeowners/storage
components/soc @esp-idf-codeowners/peripherals
components/spi_flash @esp-idf-codeowners/peripherals
components/spiffs @esp-idf-codeowners/storage
components/tcp_transport @esp-idf-codeowners/network
components/tcpip_adapter @esp-idf-codeowners/network
components/tinyusb @esp-idf-codeowners/peripherals
components/ulp @esp-idf-codeowners/system
components/unity @esp-idf-codeowners/ci
components/vfs @esp-idf-codeowners/storage
components/wear_levelling @esp-idf-codeowners/storage
components/wifi_provisioning @esp-idf-codeowners/app-utilities
components/wpa_supplicant @esp-idf-codeowners/wifi
components/xtensa @esp-idf-codeowners/system
docs @esp-idf-codeowners/docs # sort-order-reset
examples/**/*.py @esp-idf-codeowners/ci @esp-idf-codeowners/tools /components/app_trace/ @esp-idf-codeowners/tools
/components/app_update/ @esp-idf-codeowners/system @esp-idf-codeowners/app-utilities
/components/asio/ @esp-idf-codeowners/network
/components/bootloader*/ @esp-idf-codeowners/system @esp-idf-codeowners/security
/components/bt/ @esp-idf-codeowners/bluetooth
/components/cbor/ @esp-idf-codeowners/app-utilities
/components/coap/ @esp-idf-codeowners/app-utilities
/components/console/ @esp-idf-codeowners/system @esp-idf-codeowners/app-utilities
/components/cxx/ @esp-idf-codeowners/system
/components/driver/ @esp-idf-codeowners/peripherals
/components/efuse/ @esp-idf-codeowners/system
/components/esp32*/ @esp-idf-codeowners/system
/components/esp_adc_cal/ @esp-idf-codeowners/peripherals
/components/esp_common/ @esp-idf-codeowners/system
/components/esp_eth/ @esp-idf-codeowners/network
/components/esp_event/ @esp-idf-codeowners/system
/components/esp_gdbstub/ @esp-idf-codeowners/tools
/components/esp_hid/ @esp-idf-codeowners/bluetooth
/components/esp_http_client/ @esp-idf-codeowners/app-utilities
/components/esp_http_server/ @esp-idf-codeowners/app-utilities
/components/esp_https_ota/ @esp-idf-codeowners/app-utilities
/components/esp_https_server/ @esp-idf-codeowners/app-utilities
/components/esp_ipc/ @esp-idf-codeowners/system
/components/esp_local_ctrl/ @esp-idf-codeowners/app-utilities
/components/esp_netif/ @esp-idf-codeowners/network
/components/esp_ringbuf/ @esp-idf-codeowners/system
/components/esp_rom/ @esp-idf-codeowners/system
/components/esp_serial_slave_link/ @esp-idf-codeowners/peripherals
/components/esp_system/ @esp-idf-codeowners/system
/components/esp_timer/ @esp-idf-codeowners/system
/components/esp-tls/ @esp-idf-codeowners/app-utilities
/components/esp_websocket_client/ @esp-idf-codeowners/network
/components/esp_wifi/ @esp-idf-codeowners/wifi
/components/espcoredump/ @esp-idf-codeowners/tools
/components/esptool_py/ @esp-idf-codeowners/tools
/components/expat/ @esp-idf-codeowners/app-utilities
/components/fatfs/ @esp-idf-codeowners/storage
/components/freemodbus/ @esp-idf-codeowners/peripherals
/components/freertos/ @esp-idf-codeowners/system
/components/heap/ @esp-idf-codeowners/system
/components/idf_test/ @esp-idf-codeowners/ci
/components/jsmn/ @esp-idf-codeowners/app-utilities
/components/json/ @esp-idf-codeowners/app-utilities
/components/libsodium/ @esp-idf-codeowners/security
/components/log/ @esp-idf-codeowners/system
/components/lwip/ @esp-idf-codeowners/lwip
/components/mbedtls/ @esp-idf-codeowners/app-utilities @esp-idf-codeowners/security
/components/mdns/ @esp-idf-codeowners/network
/components/mqtt/ @esp-idf-codeowners/network
/components/newlib/ @esp-idf-codeowners/system @esp-idf-codeowners/tools
/components/nghttp/ @esp-idf-codeowners/app-utilities
/components/nvs_flash/ @esp-idf-codeowners/storage
/components/openssl/ @esp-idf-codeowners/network
/components/partition_table/ @esp-idf-codeowners/system
/components/perfmon/ @esp-idf-codeowners/tools
/components/protobuf-c/ @esp-idf-codeowners/app-utilities
/components/protocomm/ @esp-idf-codeowners/app-utilities
/components/pthread/ @esp-idf-codeowners/system
/components/sdmmc/ @esp-idf-codeowners/storage
/components/soc/ @esp-idf-codeowners/peripherals
/components/spi_flash/ @esp-idf-codeowners/peripherals
/components/spiffs/ @esp-idf-codeowners/storage
/components/tcp_transport/ @esp-idf-codeowners/network
/components/tcpip_adapter/ @esp-idf-codeowners/network
/components/tinyusb/ @esp-idf-codeowners/peripherals
/components/ulp/ @esp-idf-codeowners/system
/components/unity/ @esp-idf-codeowners/ci
/components/vfs/ @esp-idf-codeowners/storage
/components/wear_levelling/ @esp-idf-codeowners/storage
/components/wifi_provisioning/ @esp-idf-codeowners/app-utilities
/components/wpa_supplicant/ @esp-idf-codeowners/wifi
/components/xtensa/ @esp-idf-codeowners/system
examples/bluetooth @esp-idf-codeowners/bluetooth /docs/ @esp-idf-codeowners/docs
examples/build_system @esp-idf-codeowners/build-config
examples/common_components @esp-idf-codeowners/system
examples/cxx @esp-idf-codeowners/system
examples/ethernet @esp-idf-codeowners/network
examples/get-started @esp-idf-codeowners/system
examples/mesh @esp-idf-codeowners/wifi
examples/peripherals @esp-idf-codeowners/peripherals
examples/protocols @esp-idf-codeowners/network @esp-idf-codeowners/app-utilities
examples/provisioning @esp-idf-codeowners/app-utilities
examples/security @esp-idf-codeowners/security
examples/storage @esp-idf-codeowners/storage
examples/system @esp-idf-codeowners/system
examples/wifi @esp-idf-codeowners/wifi
make @esp-idf-codeowners/build-config /examples/**/*.py @esp-idf-codeowners/ci @esp-idf-codeowners/tools
tools @esp-idf-codeowners/tools /examples/bluetooth/ @esp-idf-codeowners/bluetooth
tools/*_apps.py @esp-idf-codeowners/ci /examples/build_system/ @esp-idf-codeowners/build-config
tools/ble @esp-idf-codeowners/app-utilities /examples/common_components/ @esp-idf-codeowners/system
tools/catch @esp-idf-codeowners/ci /examples/cxx/ @esp-idf-codeowners/system
tools/ci @esp-idf-codeowners/ci /examples/ethernet/ @esp-idf-codeowners/network
tools/cmake @esp-idf-codeowners/build-config /examples/get-started/ @esp-idf-codeowners/system
tools/esp_prov @esp-idf-codeowners/app-utilities /examples/mesh/ @esp-idf-codeowners/wifi
tools/find_build_apps @esp-idf-codeowners/ci /examples/peripherals/ @esp-idf-codeowners/peripherals
tools/kconfig* @esp-idf-codeowners/build-config /examples/protocols/ @esp-idf-codeowners/network @esp-idf-codeowners/app-utilities
tools/ldgen @esp-idf-codeowners/build-config /examples/provisioning/ @esp-idf-codeowners/app-utilities
tools/mass_mfg @esp-idf-codeowners/app-utilities /examples/security/ @esp-idf-codeowners/security
/examples/storage/ @esp-idf-codeowners/storage
/examples/system/ @esp-idf-codeowners/system
/examples/wifi/ @esp-idf-codeowners/wifi
/make/ @esp-idf-codeowners/build-config
/tools/ @esp-idf-codeowners/tools
/tools/*_apps.py @esp-idf-codeowners/ci
/tools/ble/ @esp-idf-codeowners/app-utilities
/tools/catch/ @esp-idf-codeowners/ci
/tools/ci/ @esp-idf-codeowners/ci
/tools/cmake/ @esp-idf-codeowners/build-config
/tools/esp_prov/ @esp-idf-codeowners/app-utilities
/tools/find_build_apps/ @esp-idf-codeowners/ci
/tools/kconfig*/ @esp-idf-codeowners/build-config
/tools/ldgen/ @esp-idf-codeowners/build-config
/tools/mass_mfg/ @esp-idf-codeowners/app-utilities
requirements.txt @esp-idf-codeowners/tools

View File

@ -17,17 +17,79 @@
# limitations under the License. # limitations under the License.
import argparse import argparse
from collections import defaultdict
import os import os
import re
import subprocess import subprocess
import sys import sys
CODEOWNERS_PATH = os.path.join(os.path.dirname(__file__), "..", ".gitlab", "CODEOWNERS") CODEOWNERS_PATH = os.path.join(os.path.dirname(__file__), "..", ".gitlab", "CODEOWNERS")
CODEOWNER_GROUP_PREFIX = "@esp-idf-codeowners/" CODEOWNER_GROUP_PREFIX = "@esp-idf-codeowners/"
def get_all_files():
"""
Get list of all file paths in the repository.
"""
idf_root = os.path.join(os.path.dirname(__file__), "..")
# only split on newlines, since file names may contain spaces
return subprocess.check_output(["git", "ls-files"], cwd=idf_root).decode("utf-8").strip().split('\n')
def pattern_to_regex(pattern):
"""
Convert the CODEOWNERS path pattern into a regular expression string.
"""
orig_pattern = pattern # for printing errors later
# Replicates the logic from normalize_pattern function in Gitlab ee/lib/gitlab/code_owners/file.rb:
if not pattern.startswith('/'):
pattern = '/**/' + pattern
if pattern.endswith('/'):
pattern = pattern + '**/*'
# Convert the glob pattern into a regular expression:
# first into intermediate tokens
pattern = (pattern.replace('**/', ':REGLOB:')
.replace('**', ':INVALID:')
.replace('*', ':GLOB:')
.replace('.', ':DOT:')
.replace('?', ':ANY:'))
if pattern.find(':INVALID:') >= 0:
raise ValueError("Likely invalid pattern '{}': '**' should be followed by '/'".format(orig_pattern))
# then into the final regex pattern:
re_pattern = (pattern.replace(':REGLOB:', '(?:.*/)?')
.replace(':GLOB:', '[^/]*')
.replace(':DOT:', '[.]')
.replace(':ANY:', '.') + '$')
if re_pattern.startswith('/'):
re_pattern = '^' + re_pattern
return re_pattern
def files_by_regex(all_files, regex):
"""
Return all files in the repository matching the given regular expresion.
"""
return [file for file in all_files if regex.search('/' + file)]
def files_by_pattern(all_files, pattern=None):
"""
Return all the files in the repository matching the given CODEOWNERS pattern.
"""
if not pattern:
return all_files
return files_by_regex(all_files, re.compile(pattern_to_regex(pattern)))
def action_identify(args): def action_identify(args):
best_match = [] best_match = []
all_files = get_all_files()
with open(CODEOWNERS_PATH) as f: with open(CODEOWNERS_PATH) as f:
for line in f: for line in f:
line = line.strip() line = line.strip()
@ -36,19 +98,23 @@ def action_identify(args):
tokens = line.split() tokens = line.split()
path_pattern = tokens[0] path_pattern = tokens[0]
owners = tokens[1:] owners = tokens[1:]
files = files_by_pattern(path_pattern) files = files_by_pattern(all_files, path_pattern)
if args.path in files: if args.path in files:
best_match = owners best_match = owners
for owner in best_match: for owner in best_match:
print(owner) print(owner)
def files_by_pattern(pattern=None): def action_test_pattern(args):
args = ["git", "ls-files"] re_pattern = pattern_to_regex(args.pattern)
if pattern:
args.append(pattern) if args.regex:
idf_root = os.path.join(os.path.dirname(__file__), "..") print(re_pattern)
return subprocess.check_output(args, cwd=idf_root).decode("utf-8").split() return
files = files_by_regex(get_all_files(), re.compile(re_pattern))
for f in files:
print(f)
def action_ci_check(args): def action_ci_check(args):
@ -57,12 +123,15 @@ def action_ci_check(args):
def add_error(msg): def add_error(msg):
errors.append("Error at CODEOWNERS:{}: {}".format(line_no, msg)) errors.append("Error at CODEOWNERS:{}: {}".format(line_no, msg))
files_by_owner = defaultdict(int) all_files = get_all_files()
prev_path_pattern = "" prev_path_pattern = ""
with open(CODEOWNERS_PATH) as f: with open(CODEOWNERS_PATH) as f:
for line_no, line in enumerate(f, start=1): for line_no, line in enumerate(f, start=1):
# Skip empty lines and comments # Skip empty lines and comments
line = line.strip() line = line.strip()
if line.startswith("# sort-order-reset"):
prev_path_pattern = ""
if not line or line.startswith("#"): if not line or line.startswith("#"):
continue continue
@ -75,26 +144,19 @@ def action_ci_check(args):
# Check that the file is sorted by path patterns # Check that the file is sorted by path patterns
path_pattern_for_cmp = path_pattern.replace("-", "_") # ignore difference between _ and - for ordering path_pattern_for_cmp = path_pattern.replace("-", "_") # ignore difference between _ and - for ordering
if path_pattern_for_cmp < prev_path_pattern: if prev_path_pattern and path_pattern_for_cmp < prev_path_pattern:
add_error("file is not sorted: {} < {}".format(path_pattern_for_cmp, prev_path_pattern)) add_error("file is not sorted: {} < {}".format(path_pattern_for_cmp, prev_path_pattern))
prev_path_pattern = path_pattern_for_cmp prev_path_pattern = path_pattern_for_cmp
# Check that the pattern matches at least one file # Check that the pattern matches at least one file
files = files_by_pattern(path_pattern) files = files_by_pattern(all_files, path_pattern)
if not files: if not files:
add_error("no files matched by pattern {}".format(path_pattern)) add_error("no files matched by pattern {}".format(path_pattern))
# Count the number of files per owner
for o in owners: for o in owners:
# Sanity-check the owner group name # Sanity-check the owner group name
if not o.startswith(CODEOWNER_GROUP_PREFIX): if not o.startswith(CODEOWNER_GROUP_PREFIX):
add_error("owner {} doesn't start with {}".format(o, CODEOWNER_GROUP_PREFIX)) add_error("owner {} doesn't start with {}".format(o, CODEOWNER_GROUP_PREFIX))
files_by_owner[o] += len(files)
owners_sorted = sorted([(owner, cnt) for owner, cnt in files_by_owner.items()], key=lambda p: p[0])
print("File count per owner (not including submodules):")
for owner, cnt in owners_sorted:
print("{}: {} files".format(owner, cnt))
if not errors: if not errors:
print("No errors found.") print("No errors found.")
@ -110,17 +172,27 @@ def main():
sys.argv[0], description="Internal helper script for working with the CODEOWNERS file." sys.argv[0], description="Internal helper script for working with the CODEOWNERS file."
) )
subparsers = parser.add_subparsers(dest="action") subparsers = parser.add_subparsers(dest="action")
identify = subparsers.add_parser( identify = subparsers.add_parser(
"identify", "identify",
help="Lists the owners of the specified path within IDF." help="List the owners of the specified path within IDF."
"This command doesn't support files inside submodules, or files not added to git repository.", "This command doesn't support files inside submodules, or files not added to git repository.",
) )
identify.add_argument("path", help="Path of the file relative to the root of the repository") identify.add_argument("path", help="Path of the file relative to the root of the repository")
subparsers.add_parser( subparsers.add_parser(
"ci-check", "ci-check",
help="Check CODEOWNERS file: every line should match at least one file, sanity-check group names, " help="Check CODEOWNERS file: every line should match at least one file, sanity-check group names, "
"check that the file is sorted by paths", "check that the file is sorted by paths",
) )
test_pattern = subparsers.add_parser(
"test-pattern",
help="Print files in the repository for a given CODEOWNERS pattern. Useful when adding new rules."
)
test_pattern.add_argument("--regex", action="store_true", help="Print the equivalent regular expression instead of the file list.")
test_pattern.add_argument("pattern", help="Path pattern to get the list of files for")
args = parser.parse_args() args = parser.parse_args()
if args.action is None: if args.action is None: