Allow non-types dependencies

[srittau]

This was previously discussed in #5618 and came up in #5765. I think the consensus is that we should allow this. This issue is mainly to track this.

[intgr]

Copying relevant discussion from #5618 for context:

srittau commented on Jun 18

Not speaking about stubs packages in general, but I think it's reasonable if typeshed packages depend on dependencies of the corresponding upstream package. I.e. if package foo depends on cryptography, it's fine that our foo-types depends on cryptography as well, since it doesn't introduce "new" dependencies. Apart from that we should vet dependencies carefully.

JukkaL commented on Jun 18

I agree with srittau.

I think that it would be sufficient to maintain an allowlist of vetted dependencies. I'd expect that the number of such dependencies will be fairly small, so it shouldn't be much effort to maintain.

[hmc-cs-mdrissi]

I'm running into this issue for tensorflow stubs. Most tensorflow type annotations depend on a couple basic numpy types. So at moment my options look like don't include numpy types and have most annotations be too strict or have Any almost everywhere.

What's needed to work on this task? I can see one issue is pyright check produces errors from dependency not being installed. Is the prerequisite being able to run checks for only changed stubs as mentioned here, #5952

[Akuli]

If I understand correctly, the next steps are:

• Add a check to tests/check_consistent.py that fails if stubs/foo depends on anything else than:
  • A types-bar package where bar is so that stubs/bar exists in typeshed
  • Or a dependency of foo on pypi that has py.typed
    • We can use pypi's API for this, although checking the presence of py.typed with just the API is tricky. Can we just assume that py.typed exists, or do we need to potentially download the sdist to check? How about packages that don't provide sdist?
• Change one dependency of one stubs-package to use upstream types
• Make a pull request
• Fix pull request until CI is green
• Before merging, run the third-party stubs check in daily.yml passes (this can be ran manually)

A potential problem is if packages foo and bar have dependencies that cannot be installed simultaneously. Then daily.yml will not work at all. I think we should fix this later, in a separate issue and a separate PR, because users are already complaining about not being able to use upstream type hints, and this issue won't come up until we have many non-stubs dependencies in typeshed.

If we have a package foo with dependency bar and stubs in stubs/foo, we probably want types-foo to depend on bar and not a specific version like bar==1.2.3, so that you can upgrade to bar==1.2.4 without breaking your pip install -r requirements.txt -r requirements-dev.txt (assuming bar==1.2.4 is in requirements.txt and types-foo==x.y.z is in requirements-dev.txt).

Is this all there's to it, or am I missing something? I haven't followed this discussion in detail before, because it seemed to be complicated and also spread up across many places.

[hauntsaninja]

We need changes to mypy_test and the others, see e.g. #5952 (comment)

We need some changes in stub_uploader as well, which has validation around this. And I think it's a good opportunity to do #8312

I have the stub_uploader patch locally, but haven't PR-ed it because of some conflicts with typeshed-internal/stub_uploader#57 which I was hoping would get merged

[Akuli]

I'm mostly confused about "changes to mypy_test and the others" part.

• Why exactly is Testing third-party stubs in isolated environments #5952 (comment) relevant now? It says "I personally wouldn't hate getting started on [this issue] with a global venv" which is what I'm suggesting.
• Is all this included in my "Fix pull request until CI is green" part, or is there something subtle that our CI wouldn't catch if forgotten?

[hauntsaninja]

Our CI wouldn't catch changes needed for stub_uploader. But yes, everything else is probably covered under "get CI green".

Here's code that checks if a package contains a py.typed:

typeshed/scripts/stubsabot.py

Line 141 in cc7d256

async def package_contains_py_typed(release_to_download: dict[str, Any], session: aiohttp.ClientSession) -> bool:

But we probably don't need it; we should get errors in type checking tests if something doesn't have py.typed. People have previously suggested using an allowlist of packages (I believe for security reasons), which would also take care of that.

[Akuli]

I personally don't see how duplicating the dependencies into a separate allowlist in the same repository improves security.

[hauntsaninja]

It'd be in stub_uploader. I don't have opinions on it, but mentioned it as a thing that has been previously discussed, since e.g. an allowlist would make validation like checking for py.typed not very important.

[ilevkivskyi]

As I mentioned on the PR typeshed-internal/stub_uploader#59 this introduces a security hole. Having an allowlist in stub_uploader (to which intentionally only few maintainers have access) is a possible mitigation, but I want to first hear what other people (in particular @JukkaL and @JelleZijlstra) think about this.

[AlexWaygood]

this introduces a security hole

Could you expand on what the security risk is, exactly? Like @Akuli, I'm not sure I've ever seen it fully explained (probably because I'm somewhat new as a maintainer :-)

I agree with the consensus on this issue that this is a feature we really need at this point. The situation with types-cryptography is becoming something of a farce. We've also got an increasing number of packages that would benefit from having numpy as a dependency.

[ilevkivskyi]

Arbitrary code can be executed during a Python package installation, so if someone adds a malicious package as a dependency, it can cause really big damage, as many of types-... packages have very high install rates (like 100K+ per day).

[AlexWaygood]

Arbitrary code can be executed during a Python package installation

That's surely a security risk for any open-source package depending on another open-source package -- is the security hole here that users might not be expecting the installation of a stubs package to install, e.g., numpy or whatever as a side effect?

[ilevkivskyi]

The risk is that someone my add e.g. nmupy as a dependency (whether intentionally or not), so typosquatting, that can't usually harm many people, will now get magnified to some cosmic scales, as you just need one typo to break hundreds thousands users.

[Avasam]

@AlexWaygood I'd love to see this completed. Is there anything I could help with to free up some of your (and others') time?

[AlexWaygood]

I'd love to see it completed as well, but just haven't had the time to work on it lately, with all the other typeshed-related things going on :(

If you fancy taking a look at getting pytype_test.py and/or pyright to work with non-types dependencies, that would be a huge help! I don't think all the scripts/workflows need to be updated in one PR -- we can make changes on the basis of "this would make this script compatible with non-types dependencies, if we had any" for now. PRs updating mypy_test.py and/or regr_test.py would also be very welcome -- I haven't had the time to seriously look at them yet :(

[AlexWaygood]

Okay, so in terms of what's left to do here on typeshed's tests:

• check_consistent.py: shouldn't require any updates
• stubtest_stdlib.py: shouldn't require any updates
• stubtest_third_party.py: shouldn't require any updates
• typecheck_typeshed.py: shouldn't require any updates
• regr_test.py: Allow non-types dependencies #9382 makes regr_test.py work with non-types dependencies
• mypy_test.py will also need to be updated: mypy_test.py: Allow non-types dependencies #9408
• pyright_test.py and the way we run pyright in CI will need updates. Support external-dependencies in pyright #9434 by @Avasam.
• pytype_test.py:
  • Support third-party stub external dependencies in pytype #9449 by @Avasam gets us some way there, but there's still some more to do
  • pytype_test: Mark typeshed-external dependencies as missing. #9486 by @rchen152 finishes pytype support!

[Avasam]

For visibility / tracking. There is still one issue left with pytype: #9459 (comment)

There's possible workarounds (silence the error or ignore the file in pytype), but I think we're currently waiting to see if pytype can provide a non-hacky solution with an update.

[AlexWaygood]

Non-types dependencies are now finally, officially, Allowed 🥳🥳.

(But let's maybe wait for the stub-uploader to upload the new versions of types-paramiko and types-pyOpenSSL before we add non-types dependencies to any more packages, just to be on the safe side.)

[AlexWaygood]

Thanks @hauntsaninja for the PRs to the stub-uploader repo, thanks @Avasam for the PR fixing our pyright tests, and thanks @Avasam and @rchen152 for the PRs fixing our pytype tests!

[AlexWaygood]

Okay, the new versions of types-paramiko and types-cryptography have been uploaded, and everything seems to be going great!

• https://pypi.org/project/types-paramiko/
• https://pypi.org/project/types-pyOpenSSL/