Skip to content

Commit

Permalink
Remove the YAML tidy code (#30004)
Browse files Browse the repository at this point in the history
This code was used to test buildbox_steps.yml, but Servo no longer uses
buildbot, so this code is essentially unused. In addition, YAML +
Cython 3 is causing issues on the CI.

Fixes #30003

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
  • Loading branch information
mrobinson committed Jul 18, 2023
1 parent da5b861 commit 6628745
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 103 deletions.
4 changes: 0 additions & 4 deletions python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ six == 1.15
# For sending build notifications.
notify-py == 0.3.42

# A few more requirements for tidy.
voluptuous == 0.12.1
PyYAML == 5.4

# For wpt scripts and their tests.
flask
requests
Expand Down
15 changes: 0 additions & 15 deletions python/tidy/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,6 @@ def test_json_with_unordered_keys(self):
self.assertEqual('Unordered key (found b before a)', next(errors)[2])
self.assertNoMoreErrors(errors)

def test_yaml_with_duplicate_key(self):
errors = tidy.collect_errors_for_files(iterFile('duplicate_keys_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
self.assertEqual('Duplicated Key (duplicate_yaml_key)', next(errors)[2])
self.assertNoMoreErrors(errors)

def test_non_list_mapped_buildbot_steps(self):
errors = tidy.collect_errors_for_files(iterFile('non_list_mapping_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
self.assertEqual("expected a list for dictionary value @ data['non-list-key']", next(errors)[2])
self.assertNoMoreErrors(errors)

def test_non_string_list_mapping_buildbot_steps(self):
errors = tidy.collect_errors_for_files(iterFile('non_string_list_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
self.assertEqual("expected str @ data['mapping_key'][0]", next(errors)[2])
self.assertNoMoreErrors(errors)

def test_lock(self):
errors = tidy.collect_errors_for_files(iterFile('duplicated_package.lock'), [tidy.check_lock], [], print_text=False)
msg = """duplicate versions for package `test`
Expand Down
7 changes: 0 additions & 7 deletions python/tidy/tests/duplicate_keys_buildbot_steps.yml

This file was deleted.

2 changes: 0 additions & 2 deletions python/tidy/tests/non_list_mapping_buildbot_steps.yml

This file was deleted.

7 changes: 0 additions & 7 deletions python/tidy/tests/non_string_list_buildbot_steps.yml

This file was deleted.

72 changes: 4 additions & 68 deletions python/tidy/tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

import colorama
import toml
import voluptuous
import yaml

from .licenseck import OLD_MPL, MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml

Expand Down Expand Up @@ -68,8 +66,7 @@ def wpt_path(*args):
# File patterns to include in the non-WPT tidy check.
FILE_PATTERNS_TO_CHECK = ["*.rs", "*.rc", "*.cpp", "*.c",
"*.h", "*.py", "*.sh",
"*.toml", "*.webidl", "*.json", "*.html",
"*.yml"]
"*.toml", "*.webidl", "*.json", "*.html"]

# File patterns that are ignored for all tidy and lint checks.
FILE_PATTERNS_TO_IGNORE = ["*.#*", "*.pyc", "fake-ld.sh", "*.ogv", "*.webm"]
Expand Down Expand Up @@ -217,7 +214,7 @@ def is_apache_licensed(header):


def check_license(file_name, lines):
if any(file_name.endswith(ext) for ext in (".yml", ".toml", ".lock", ".json", ".html")) or \
if any(file_name.endswith(ext) for ext in (".toml", ".lock", ".json", ".html")) or \
config["skip-check-licenses"]:
return

Expand Down Expand Up @@ -255,7 +252,7 @@ def check_modeline(file_name, lines):


def check_length(file_name, idx, line):
if any(file_name.endswith(ext) for ext in (".yml", ".lock", ".json", ".html", ".toml")) or \
if any(file_name.endswith(ext) for ext in (".lock", ".json", ".html", ".toml")) or \
config["skip-check-length"]:
return

Expand Down Expand Up @@ -760,67 +757,6 @@ def check_webidl_spec(file_name, contents):
yield (0, "No specification link found.")


def duplicate_key_yaml_constructor(loader, node, deep=False):
mapping = {}
for key_node, value_node in node.value:
key = loader.construct_object(key_node, deep=deep)
if key in mapping:
raise KeyError(key)
value = loader.construct_object(value_node, deep=deep)
mapping[key] = value
return loader.construct_mapping(node, deep)


def lint_buildbot_steps_yaml(mapping):
from voluptuous import Any, Extra, Required, Schema

# Note: dictionary keys are optional by default in voluptuous
env = Schema({Extra: str})
commands = Schema([str])
schema = Schema({
'env': env,
Extra: Any(
commands,
{
'env': env,
Required('commands'): commands,
},
),
})

# Signals errors via exception throwing
schema(mapping)


class SafeYamlLoader(yaml.SafeLoader):
"""Subclass of yaml.SafeLoader to avoid mutating the global SafeLoader."""
pass


def check_yaml(file_name, contents):
if not file_name.endswith("buildbot_steps.yml"):
return

# YAML specification doesn't explicitly disallow
# duplicate keys, but they shouldn't be allowed in
# buildbot_steps.yml as it could lead to confusion
SafeYamlLoader.add_constructor(
yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG,
duplicate_key_yaml_constructor
)

try:
contents = yaml.load(contents, Loader=SafeYamlLoader)
lint_buildbot_steps_yaml(contents)
except yaml.YAMLError as e:
line = e.problem_mark.line + 1 if hasattr(e, 'problem_mark') else None
yield (line, e)
except KeyError as e:
yield (None, "Duplicated Key ({})".format(e.args[0]))
except voluptuous.MultipleInvalid as e:
yield (None, str(e))


def check_for_possible_duplicate_json_keys(key_value_pairs):
keys = [x[0] for x in key_value_pairs]
seen_keys = set()
Expand Down Expand Up @@ -1115,7 +1051,7 @@ def scan(only_changed_files=False, progress=True, stylo=False, no_wpt=False):
directory_errors = check_directory_files(config['check_ext'])
# standard checks
files_to_check = filter_files('.', only_changed_files and not stylo, progress)
checking_functions = (check_flake8, check_lock, check_webidl_spec, check_json, check_yaml)
checking_functions = (check_flake8, check_lock, check_webidl_spec, check_json)
line_checking_functions = (check_license, check_by_line, check_toml, check_shell,
check_rust, check_spec, check_modeline)
file_errors = collect_errors_for_files(files_to_check, checking_functions, line_checking_functions)
Expand Down

0 comments on commit 6628745

Please sign in to comment.