From 3ced8d4ddcc7328e27231f9db9ce2f441b01d37e Mon Sep 17 00:00:00 2001 From: Siddhartha Mishra Date: Sat, 21 Sep 2019 13:05:05 +0530 Subject: [PATCH 1/2] have tidy always check Cargo.lock --- python/tidy/servo_tidy/tidy.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 46da89d3907..1477fe188ab 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -65,7 +65,7 @@ COMMENTS = ["// ", "# ", " *", "/* "] # File patterns to include in the non-WPT tidy check. FILE_PATTERNS_TO_CHECK = ["*.rs", "*.rc", "*.cpp", "*.c", - "*.h", "Cargo.lock", "*.py", "*.sh", + "*.h", "*.py", "*.sh", "*.toml", "*.webidl", "*.json", "*.html", "*.yml"] @@ -193,6 +193,9 @@ def filter_file(file_name): def filter_files(start_dir, only_changed_files, progress): file_iter = FileList(start_dir, only_changed_files=only_changed_files, exclude_dirs=config["ignore"]["directories"], progress=progress) + # always yield Cargo.lock so that the correctness of transitive dependacies is checked + yield "./Cargo.lock" + for file_name in file_iter: base_name = os.path.basename(file_name) if not any(fnmatch.fnmatch(base_name, pattern) for pattern in FILE_PATTERNS_TO_CHECK): From aa7ba1a850543a147074ec0c693d7bfcdfda9797 Mon Sep 17 00:00:00 2001 From: Siddhartha Mishra Date: Sat, 21 Sep 2019 18:12:41 +0530 Subject: [PATCH 2/2] report when blocked package exception is not needed --- python/tidy/servo_tidy/tidy.py | 16 +++++++++++- .../servo_tidy_tests/blocked_package.lock | 26 +++++++++++++++++++ python/tidy/servo_tidy_tests/test_tidy.py | 19 ++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 python/tidy/servo_tidy_tests/blocked_package.lock diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 1477fe188ab..0069f344fa7 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -402,18 +402,32 @@ def check_lock(file_name, contents): yield (1, message) # Check to see if we are transitively using any blocked packages + blocked_packages = config["blocked-packages"] + # Create map to keep track of visited exception packages + visited_whitelisted_packages = {package: {} for package in blocked_packages.keys()} + for package in content.get("package", []): package_name = package.get("name") package_version = package.get("version") for dependency in package.get("dependencies", []): dependency = dependency.split() dependency_name = dependency[0] - whitelist = config['blocked-packages'].get(dependency_name) + whitelist = blocked_packages.get(dependency_name) if whitelist is not None: if package_name not in whitelist: fmt = "Package {} {} depends on blocked package {}." message = fmt.format(package_name, package_version, dependency_name) yield (1, message) + else: + visited_whitelisted_packages[dependency_name][package_name] = True + + # Check if all the exceptions to blocked packages actually depend on the blocked package + for dependency_name, package_names in blocked_packages.iteritems(): + for package_name in package_names: + if not visited_whitelisted_packages[dependency_name].get(package_name): + fmt = "Package {} is not required to be an exception of blocked package {}." + message = fmt.format(package_name, dependency_name) + yield (1, message) def check_toml(file_name, lines): diff --git a/python/tidy/servo_tidy_tests/blocked_package.lock b/python/tidy/servo_tidy_tests/blocked_package.lock new file mode 100644 index 00000000000..521fdfcfb74 --- /dev/null +++ b/python/tidy/servo_tidy_tests/blocked_package.lock @@ -0,0 +1,26 @@ +[root] +name = "servo" +version = "0.0.1" + +[[package]] +name = "test_blocked" +version = "0.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "test_exception" +version = "0.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "test_unneeded_exception" +version = "0.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ +] diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index 280190931c2..b7ec67cf9c9 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -229,6 +229,25 @@ class CheckTidiness(unittest.TestCase): self.assertNoMoreErrors(errors) + def test_lock_exceptions(self): + tidy.config["blocked-packages"]["rand"] = ["test_exception", "test_unneeded_exception"] + errors = tidy.collect_errors_for_files(iterFile('blocked_package.lock'), [tidy.check_lock], [], print_text=False) + + msg = ( + "Package test_blocked 0.0.2 depends on blocked package rand." + ) + + msg2 = ( + "Package test_unneeded_exception is not required to be an exception of blocked package rand." + ) + + self.assertEqual(msg, errors.next()[2]) + self.assertEqual(msg2, errors.next()[2]) + self.assertNoMoreErrors(errors) + + # needed to not raise errors in other test cases + tidy.config["blocked-packages"]["rand"] = [] + def test_lint_runner(self): test_path = base_path + 'lints/' runner = tidy.LintRunner(only_changed_files=False, progress=False)