diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2016-12-26 08:57:04 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-12-26 08:57:04 -0800 |
commit | 37a5e29147f0dc489888377d6f7bb53282dc04f9 (patch) | |
tree | e9fa47a3697f5bfc9ddfc42e6c5676ffcacc378b | |
parent | c2448d3963508777f1a7485492ec5e9677c875c8 (diff) | |
parent | b760578f0fd9fb33dd83815ce490410ba9a59f88 (diff) | |
download | servo-37a5e29147f0dc489888377d6f7bb53282dc04f9.tar.gz servo-37a5e29147f0dc489888377d6f7bb53282dc04f9.zip |
Auto merge of #14715 - UK992:tidy-check-lock, r=SimonSapin
Tidy: Check Cargo.lock for packages with same version and different sources
<!-- Please describe your changes on the following line: -->
r? @Wafflespeanut
cc @SimonSapin
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #14695
<!-- Either: -->
- [X] There are tests for these changes
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14715)
<!-- Reviewable:end -->
-rw-r--r-- | python/requirements.txt | 2 | ||||
-rw-r--r-- | python/tidy/servo_tidy/tidy.py | 91 | ||||
-rw-r--r-- | python/tidy/servo_tidy_tests/duplicated_package.lock | 28 | ||||
-rw-r--r-- | python/tidy/servo_tidy_tests/test_tidy.py | 16 | ||||
-rw-r--r-- | servo-tidy.toml | 2 |
5 files changed, 73 insertions, 66 deletions
diff --git a/python/requirements.txt b/python/requirements.txt index 34794fd5031..8248a21e930 100644 --- a/python/requirements.txt +++ b/python/requirements.txt @@ -4,7 +4,7 @@ mozdebug == 0.1 mozinfo == 0.8 mozlog == 3.3 setuptools == 18.5 -toml == 0.9.1 +toml == 0.9.2 Mako == 1.0.4 # For Python linting diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 7855d4c7a50..cae431205f8 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -33,10 +33,10 @@ config = { "lint-scripts": [], "ignore": { "files": [ - "./.", # ignore hidden files + os.path.join(".", "."), # ignore hidden files ], "directories": [ - "./.", # ignore hidden directories + os.path.join(".", "."), # ignore hidden directories ], "packages": [], }, @@ -90,6 +90,10 @@ def is_iter_empty(iterator): 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): @@ -123,11 +127,9 @@ class FileList(object): 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) @@ -299,61 +301,42 @@ def check_flake8(file_name, contents): 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): @@ -862,23 +845,17 @@ 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", []) diff --git a/python/tidy/servo_tidy_tests/duplicated_package.lock b/python/tidy/servo_tidy_tests/duplicated_package.lock index 77777fdd82c..6075f99164c 100644 --- a/python/tidy/servo_tidy_tests/duplicated_package.lock +++ b/python/tidy/servo_tidy_tests/duplicated_package.lock @@ -15,7 +15,33 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "test2" version = "0.1.0" -source = "git+https://github.com/" +source = "git+https://github.com/user/test2#c54edsf" dependencies = [ "test 0.4.9 (registry+https://github.com/rust-lang/crates.io-index)", ] + +[[package]] +name = "test3" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" + +[[package]] +name = "test3" +version = "0.5.1" +source = "git+https://github.com/user/test3#c54edsf" + +[[package]] +name = "test4" +version = "0.1.0" +source = "git+https://github.com/user/test4#c54edsf" +dependencies = [ + "test3 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "test5" +version = "0.1.0" +source = "git+https://github.com/" +dependencies = [ + "test3 0.5.1 (git+https://github.com/user/test3)", +] diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index 265c485a106..7adcbda2dfe 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -200,13 +200,17 @@ class CheckTidiness(unittest.TestCase): 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): diff --git a/servo-tidy.toml b/servo-tidy.toml index 6ca99cf57b6..c5d64c6f30b 100644 --- a/servo-tidy.toml +++ b/servo-tidy.toml @@ -59,4 +59,4 @@ directories = [ # Directories that are checked for correct file extension [check_ext] # directory, list of expected file extensions -"components/script/dom/webidls" = [".webidl"] +"./components/script/dom/webidls" = [".webidl"] |