--- a/servo/python/tidy/servo_tidy/tidy.py
+++ b/servo/python/tidy/servo_tidy/tidy.py
@@ -28,20 +28,20 @@ CONFIG_FILE_PATH = os.path.join(".", "se
# Default configs
config = {
"skip-check-length": False,
"skip-check-licenses": False,
"check-ordered-json-keys": [],
"lint-scripts": [],
"ignore": {
"files": [
- "./.", # ignore hidden files
+ os.path.join(".", "."), # ignore hidden files
],
"directories": [
- "./.", # ignore hidden directories
+ os.path.join(".", "."), # ignore hidden directories
],
"packages": [],
},
"check_ext": {}
}
COMMENTS = ["// ", "# ", " *", "/* "]
@@ -85,16 +85,20 @@ WEBIDL_STANDARDS = [
def is_iter_empty(iterator):
try:
obj = iterator.next()
return True, itertools.chain((obj,), iterator)
except StopIteration:
return False, iterator
+def normilize_paths(paths):
+ return [os.path.join(*path.split('/')) for path in paths]
+
+
# A simple wrapper for iterators to show progress
# (Note that it's inefficient for giant iterators, since it iterates once to get the upper bound)
def progress_wrapper(iterator):
list_of_stuff = list(iterator)
total_files, progress = len(list_of_stuff), 0
for idx, thing in enumerate(list_of_stuff):
progress = int(float(idx + 1) / total_files * 100)
sys.stdout.write('\r Progress: %s%% (%d/%d)' % (progress, idx + 1, total_files))
@@ -118,21 +122,19 @@ class FileList(object):
for root, _, files in os.walk(self.directory):
for f in files:
yield os.path.join(root, f)
def _git_changed_files(self):
args = ["git", "log", "-n1", "--merges", "--format=%H"]
last_merge = subprocess.check_output(args).strip()
args = ["git", "diff", "--name-only", last_merge, self.directory]
- file_list = subprocess.check_output(args)
+ file_list = normilize_paths(subprocess.check_output(args).splitlines())
- for f in file_list.splitlines():
- if sys.platform == 'win32':
- os.path.join(*f.split('/'))
+ for f in file_list:
if not any(os.path.join('.', os.path.dirname(f)).startswith(path) for path in self.excluded):
yield os.path.join('.', f)
def _filter_excluded(self):
for root, dirs, files in os.walk(self.directory, topdown=True):
# modify 'dirs' in-place so that we don't do unnecessary traversals in excluded directories
dirs[:] = [d for d in dirs if not any(os.path.join(root, d).startswith(name) for name in self.excluded)]
for rel_path in files:
@@ -294,71 +296,52 @@ def check_flake8(file_name, contents):
with stdout_redirect(output):
check_code(contents, ignore=ignore)
for error in output.getvalue().splitlines():
_, line_num, _, message = error.split(":", 3)
yield line_num, message.strip()
def check_lock(file_name, contents):
- def find_reverse_dependencies(dependency, version, content):
- dependency_prefix = "{} {}".format(dependency, version)
+ def find_reverse_dependencies(name, content):
for package in itertools.chain([content["root"]], content["package"]):
for dependency in package.get("dependencies", []):
- if dependency.startswith(dependency_prefix):
- yield package["name"]
+ if dependency.startswith("{} ".format(name)):
+ yield package["name"], dependency
if not file_name.endswith(".lock"):
raise StopIteration
- # package names to be neglected (as named by cargo)
+ # Package names to be neglected (as named by cargo)
exceptions = config["ignore"]["packages"]
- # toml.py has a bug(?) that we trip up in [metadata] sections;
- # see https://github.com/uiri/toml/issues/61
- # This should only affect a very few lines (that have embedded ?branch=...),
- # and most of them won't be in the repo
- try:
- content = toml.loads(contents)
- except:
- print "WARNING!"
- print "WARNING! toml parsing failed for Cargo.lock, but ignoring..."
- print "WARNING!"
- raise StopIteration
+ content = toml.loads(contents)
- packages = {}
+ packages_by_name = {}
for package in content.get("package", []):
- packages.setdefault(package["name"], []).append(package["version"])
+ source = package.get("source", "")
+ if source == r"registry+https://github.com/rust-lang/crates.io-index":
+ source = "crates.io"
+ packages_by_name.setdefault(package["name"], []).append((package["version"], source))
- for (name, versions) in packages.iteritems():
- if name in exceptions or len(versions) <= 1:
+ for (name, packages) in packages_by_name.iteritems():
+ if name in exceptions or len(packages) <= 1:
continue
- highest = max(versions)
- for version in versions:
- if version != highest:
- reverse_dependencies = "\n".join(
- "\t\t{}".format(n)
- for n in find_reverse_dependencies(name, version, content)
- )
- substitutions = {
- "package": name,
- "old_version": version,
- "new_version": highest,
- "reverse_dependencies": reverse_dependencies
- }
- message = """
-duplicate versions for package "{package}"
-\t\033[93mfound dependency on version {old_version}\033[0m
-\t\033[91mbut highest version is {new_version}\033[0m
-\t\033[93mtry upgrading with\033[0m \033[96m./mach cargo-update -p {package}:{old_version}\033[0m
-\tThe following packages depend on version {old_version}:
-{reverse_dependencies}
-""".format(**substitutions).strip()
- yield (1, message)
+ message = "duplicate versions for package `{}`".format(name)
+ packages.sort()
+ packages_dependencies = list(find_reverse_dependencies(name, content))
+ for version, source in packages:
+ short_source = source.split("#")[0].replace("git+", "")
+ message += "\n\t\033[93mThe following packages depend on version {} from '{}':\033[0m" \
+ .format(version, short_source)
+ for name, dependency in packages_dependencies:
+ if version in dependency and short_source in dependency:
+ message += "\n\t\t" + name
+ yield (1, message)
def check_toml(file_name, lines):
if not file_name.endswith("Cargo.toml"):
raise StopIteration
ok_licensed = False
for idx, line in enumerate(lines):
if idx == 0 and "[workspace]" in line:
@@ -857,33 +840,27 @@ def check_config_file(config_file, print
# Parse config file
parse_config(conf_file)
def parse_config(content):
config_file = toml.loads(content)
exclude = config_file.get("ignore", {})
# Add list of ignored directories to config
- config["ignore"]["directories"] += exclude.get("directories", [])
+ config["ignore"]["directories"] += normilize_paths(exclude.get("directories", []))
# Add list of ignored files to config
- config["ignore"]["files"] += exclude.get("files", [])
+ config["ignore"]["files"] += normilize_paths(exclude.get("files", []))
# Add list of ignored packages to config
config["ignore"]["packages"] = exclude.get("packages", [])
- # Fix the paths (OS-dependent)
- config['ignore']['files'] = map(lambda path: os.path.join(*path.split('/')),
- config['ignore']['files'])
- config['ignore']['directories'] = map(lambda path: os.path.join(*path.split('/')),
- config['ignore']['directories'])
# Add dict of dir, list of expected ext to config
dirs_to_check = config_file.get("check_ext", {})
# Fix the paths (OS-dependent)
for path, exts in dirs_to_check.items():
- fixed_path = os.path.join(*path.split('/'))
- config['check_ext'][fixed_path] = exts
+ config['check_ext'][normilize_paths([path])[0]] = exts
# Override default configs
user_configs = config_file.get("configs", [])
for pref in user_configs:
if pref in config:
config[pref] = user_configs[pref]
--- a/servo/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/servo/python/tidy/servo_tidy_tests/test_tidy.py
@@ -195,23 +195,27 @@ class CheckTidiness(unittest.TestCase):
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("List mapped to 'mapping_key' contains non-string element", errors.next()[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"
-\t\033[93mfound dependency on version 0.4.9\033[0m
-\t\033[91mbut highest version is 0.5.1\033[0m
-\t\033[93mtry upgrading with\033[0m \033[96m./mach cargo-update -p test:0.4.9\033[0m
-\tThe following packages depend on version 0.4.9:
-\t\ttest2"""
+ msg = """duplicate versions for package `test`
+\t\x1b[93mThe following packages depend on version 0.4.9 from 'crates.io':\x1b[0m
+\t\ttest2
+\t\x1b[93mThe following packages depend on version 0.5.1 from 'crates.io':\x1b[0m"""
self.assertEqual(msg, errors.next()[2])
+ msg2 = """duplicate versions for package `test3`
+\t\x1b[93mThe following packages depend on version 0.5.1 from 'crates.io':\x1b[0m
+\t\ttest4
+\t\x1b[93mThe following packages depend on version 0.5.1 from 'https://github.com/user/test3':\x1b[0m
+\t\ttest5"""
+ self.assertEqual(msg2, errors.next()[2])
self.assertNoMoreErrors(errors)
def test_lint_runner(self):
test_path = base_path + 'lints/'
runner = tidy.LintRunner(only_changed_files=False, progress=False)
runner.path = test_path + 'some-fictional-file'
self.assertEqual([(runner.path, 0, "file does not exist")], list(runner.check()))
runner.path = test_path + 'not_script'