Tidy: Check Cargo.lock for packages with same version and different sources

[UK992]

r? @wafflespeanut

cc @SimonSapin

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #14695

• There are tests for these changes

---

This change is 

[highfive]

Heads up! This PR modifies the following files:

• @wafflespeanut: python/requirements.txt, python/tidy/servo_tidy_tests/test_tidy.py, python/tidy/servo_tidy_tests/duplicated_package.lock, python/tidy/servo_tidy/tidy.py

[wafflespeanut]

This could just be

return [os.path.join(*path.split('/')) for path in paths]

[wafflespeanut]

Could you also add a similar comment here? That, it's for Packages with same names but different versions?

[wafflespeanut]

Maybe we should have names for version and source, so that we don't have to address them as data["version"] and data["source"] everywhere.

[wafflespeanut]

Enlighten me. Do we have such a case in that list? This looks for an exact match of "github.com/servo/servo in the data["source"] list, no? Don't you think this should be something like,

elif any('github.com/servo/' in source for source in data["source"]):

[wafflespeanut]

Also, we probably need a test for this case.

[wafflespeanut]

Just a few comments. Overall, the PR looks good. Thanks for doing this :)

[SimonSapin]

I’m sorry, the way I filed the issue was misleading. It should have been:

Check that Cargo.lock doesn’t contain duplicate crates (with the same name)

… because the algorithm should be no more complicated than this. Then, the issue could have said:

(By the way, the current algorithm almost works but only misses the case where duplicate packages have the same version number. This can happen for example when one is from crates.io while the other is from a git repository.)

I think the code should not care about what the version numbers or source are, except to include them in the error message.

In other words, I think the check_lock function after content = toml.loads(contents) should be something along the lines of: (code completely untested)

    packages_by_name = {}
    for package in content.get("package", []):
        source = package.get("source", "")
        if source == "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 " + name
        packages.sort()
        for version, source in packages:
            message += "\n\tThe following packages depend on version {} from {}:" \
                       .format(version, source)
            for name in find_reverse_dependencies(name, version, content):
                message += "\n\t\t" + name
        yield (1, message)

[SimonSapin]

Looks good to me! r=me with the fixup commits squashed

[UK992]

done!

[SimonSapin]

@bors-servo r+

[bors-servo]

📌 Commit b760578 has been approved by SimonSapin

[bors-servo]

⌛ Testing commit b760578 with merge 37a5e29...

[bors-servo]

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev