Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report an error if a package has duplicates allowed but there are no duplicates #19306

Closed
jdm opened this issue Nov 20, 2017 · 3 comments
Closed

Report an error if a package has duplicates allowed but there are no duplicates #19306

jdm opened this issue Nov 20, 2017 · 3 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Nov 20, 2017

In servo-tidy.toml there is a [ignore] section that lists package names that are allowed to have duplicate versions in Cargo.lock. However, when we remove the duplicate versions, sometimes we forget to remove the packages from the list of allowed exceptions. We should make ./mach test-tidy report an error if there is a package listed under [ignore] which only has a single version present.

Code:

def check_lock(file_name, contents):
def find_reverse_dependencies(name, content):
for package in itertools.chain([content.get("root", {})], content["package"]):
for dependency in package.get("dependencies", []):
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)
exceptions = config["ignore"]["packages"]
content = toml.loads(contents)
packages_by_name = {}
for package in content.get("package", []):
if "replace" in package:
continue
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, packages) in packages_by_name.iteritems():
if name in exceptions or len(packages) <= 1:
continue
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)
# Check to see if we are transitively using any blocked packages
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)
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)

Test: add a new test similar to this one and run them with ./mach test-tidy --self

@highfive
Copy link

@highfive highfive commented Nov 20, 2017

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. 😄

@m-novikov
Copy link
Contributor

@m-novikov m-novikov commented Nov 21, 2017

I will do this. @highfive : assign me

@highfive
Copy link

@highfive highfive commented Nov 21, 2017

Hey @m-novikov! Thanks for your interest in working on this issue. It's now assigned to you!

@highfive highfive added the C-assigned label Nov 21, 2017
m-novikov added a commit to m-novikov/servo that referenced this issue Nov 21, 2017
m-novikov added a commit to m-novikov/servo that referenced this issue Nov 21, 2017
m-novikov added a commit to m-novikov/servo that referenced this issue Nov 21, 2017
m-novikov added a commit to m-novikov/servo that referenced this issue Nov 21, 2017
bors-servo added a commit that referenced this issue Nov 22, 2017
Report an errror if a package has duplicates allowed but there are no duplicates

Resolves: #19306

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #19306 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/19325)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.