typeshed
typeshed copied to clipboard
python
Maybe change METADATA.toml "requires"?
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).
I'd prefer to keep just one requires field. We ship METADATA.toml with 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.
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 https://github.com/typeshed-internal/stub_uploader/pull/59, assuming requires_typeshed becomes a derived property computed from filtering the requires field of METADATA.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 out
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 agree that this is more a theoretical concern at the moment.
Anyway, your opinion should be the default here, so I won't push for this change.
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.
Are you on board with the rest of https://github.com/typeshed-internal/stub_uploader/pull/59
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.
I'd be interested what other people think.
I like @hauntsaninja's idea best.
If we adopted @Akuli's idea in https://github.com/python/typeshed/issues/5768#issuecomment-1251375258, 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 in requires are 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.
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 (unless types-* names are reserved for typeshed somehow?)
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).
@hauntsaninja, are you still interested in pursuing this, or shall we close this for now?
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)