Repository navigation
Change METADATA.toml "dependencies" into internal and external #8312
Description
The context is I'm looking into #5768.
There's a lot of logic we have that either assumes or checks that entries in "requires" are typeshed stub packages. It seems potentially useful to split "requires" into "requires_typeshed" (a list of required packages that originate from typeshed) and "requires_external" (a list of external dependencies). Doing so could e.g. help avoid issues where we have typos in some requires_typeshed deps or potentially implicitly trust packages that start with "types-". Could also make it easier to build tooling (e.g. knowing what we could put on MYPYPATH to test against latest typeshed).
Activity
Allow stubs to have external dependencies
srittau commentedon Sep 18, 2022
I'd prefer to keep just one
requiresfield. We shipMETADATA.tomlwith the types package in question and for external users the source of the dependency should not matter and seems rather artificial. For internal purposes it seems rather trivial to check whether a dependency is internal.hauntsaninja commentedon Sep 19, 2022
I checked on grep.app and I think we're the only people who use
requires, so not sure we should worry much about external users. I guess how much tooling and security benefit this would bring is debatable, but at the minimum this would be nice typo protection.Anyway, your opinion should be the default here, so I won't push for this change. Are you on board with the rest of typeshed-internal/stub_uploader#59, assuming
requires_typeshedbecomes a derived property computed from filtering therequiresfield ofMETADATA.toml? I think adding something to the Metadata class is the sanest way to make sure all the old call sites that assume all deps come from typeshed only see typeshed deps. If not, I'll just close it outsrittau commentedon Sep 19, 2022
I agree that this is more a theoretical concern at the moment.
Well, your suggestion got a lot of upvotes, so it's quite possible I'm in the minority here, and it's also not an issue I'll fight tooth and nail over. So, I'd be interested what other people think.
While I haven't reviewed the PR in detail yet, because of the issues under discussion, the general idea seems sound to me. And it definitely makes sense to distinguish between "internal" and "external" dependencies.
AlexWaygood commentedon Sep 20, 2022
I like @hauntsaninja's idea best.
If we adopted @Akuli's idea in #5768 (comment), it would reduce the concern regarding typos, since we could fairly easily assert that all dependencies either have names starting with
types-, or are explicitly listed as dependencies of the runtime package in question. However, I still prefer @hauntsaninja's proposal. There's a tonne of places scattered across our test suite where we assume for various reasons that all packages listed inrequiresare internal typeshed packages. I like the simplicity of having the typeshed dependencies listed separately, so we don't have to separate out the typeshed dependencies from the non-typeshed dependencies in every test script.AlexWaygood commentedon Sep 20, 2022
It also feels slightly more "principled", to me, to list the typeshed-internal dependencies separately, rather than assuming that all
types-*packages originate with typeshed (unlesstypes-*names are reserved for typeshed somehow?)hauntsaninja commentedon Sep 20, 2022
To be clear, we should never make assumptions based on the
types-prefix. I assume srittau's suggestion was to partition requires based off of listing the stubs directory in typeshed or checking https://github.com/typeshed-internal/stub_uploader/blob/main/data/uploaded_packages.txt in stub_uploader.Fwiw, the two differ in two cases: stubs permanently removed from typeshed and stubs newly added to typeshed. In stub_uploader we should probably only use uploaded_packages.txt to avoid sketchy things like race conditions where someone adds something to typeshed then squats the distribution before stub_uploader uploads it or bugs caused by things happening out of order when stubs are deleted (e.g. if we continue to have types-cryptography deps after deleting cryptography stubs).
AlexWaygood commentedon Jan 5, 2023
@hauntsaninja, are you still interested in pursuing this, or shall we close this for now?
hauntsaninja commentedon Jan 6, 2023
I think this is still worth doing, especially so now that we're starting to have typeshed packages that aren't just
types-.(Of course, stub_uploader will still need to validate, like it does today, and if we have any typeshed CI workflows with write permissions we'd also need to validate)
[-]Maybe change METADATA.toml "requires"?[/-][+]Change METADATA.toml "dependencies" into internal and external[/+]