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

Vendor setuptools 67.7.2 #11997

Merged
merged 3 commits into from
Apr 25, 2023
Merged

Vendor setuptools 67.7.2 #11997

merged 3 commits into from
Apr 25, 2023

Conversation

pfmoore
Copy link
Member

@pfmoore pfmoore commented Apr 25, 2023

This PR vendors setuptools 67.7.2, and suppresses the deprecation warning for pkg_resources.

@pfmoore
Copy link
Member Author

pfmoore commented Apr 25, 2023

I've only tested this manually, using python -Werror -m pip list, which produces an error without the warning suppression code, but works with it. We don't test that pip works with -Werror, and I don't honestly think this is the time to start doing so - if we want to do that (I don't personally think we need to), let's cover it in a follow-up PR for 23.2.

@pfmoore pfmoore requested a review from pradyunsg April 25, 2023 15:33
@hroncok
Copy link
Contributor

hroncok commented Apr 25, 2023

How does this suppression work when pip builds a project that imports from pkg_resources (e.g. in the setup.py script)? I suppose this goes trough subprocess, so the warning will still be visible in that case, correct?

@pradyunsg
Copy link
Member

pradyunsg commented Apr 25, 2023

I suppose this goes trough subprocess, so the warning will still be visible in that case, correct?

If the build succeeds, then no. If it fails, whatever the default behaviour with warnings is.

@hroncok
Copy link
Contributor

hroncok commented Apr 25, 2023

I was just checking whether the behavior stays the same as now. I suppose your comment answers that.

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... the major version bump is due to a behaviour change in handling of parsed_version relying on PEP 440, which is #11715 basically.

I think it's worth doing a manual test that having a non-PEP 440 version installed in the environment doesn't blow up pip unconditionally. If it does, we should protect against that.

The rest LGTM!

@pradyunsg
Copy link
Member

I'll be walking to PyCon US's sprints now; and will dig into this if I don't get pulled into a conversation. :)

@pfmoore
Copy link
Member Author

pfmoore commented Apr 25, 2023

I think it's worth doing a manual test that having a non-PEP 440 version installed in the environment doesn't blow up pip unconditionally. If it does, we should protect against that.

I'm not sure I follow what your concern is, so I'd appreciate it if you could manually test this.

If it does blow up, then we can consider how best to proceed, but I'm inclined to say that in that case we would need to stop and consider the implications carefully, and not let ourselves be pushed into rushing a solution just because of the time pressure around the imp deprecation work for Python 3.12. Ultimately, the problem here is that setuptools hasn't left their users (i.e., us!) with a clean upgrade path, and that's something we should be pushing back on them, rather than firefighting multiple breakages as part of our release process.

@pradyunsg
Copy link
Member

Trying to reproduce this now.

@pradyunsg
Copy link
Member

pradyunsg commented Apr 25, 2023

pip list | grep pip
pip                23.whatever /Users/pradyunsg/Developer/github/pip/srcpip install packaging
Requirement already satisfied: packaging in ./.venv/lib/python3.11/site-packages (23.0)pip install requests
Collecting requests
  Using cached requests-2.28.2-py3-none-any.whl (62 kB)
Requirement already satisfied: charset-normalizer<4,>=2 in ./.venv/lib/python3.11/site-packages (from requests) (3.1.0)
Requirement already satisfied: idna<4,>=2.5 in ./.venv/lib/python3.11/site-packages (from requests) (3.4)
Collecting urllib3<1.27,>=1.21.1 (from requests)
  Using cached urllib3-1.26.15-py2.py3-none-any.whl (140 kB)
Requirement already satisfied: certifi>=2017.4.17 in ./.venv/lib/python3.11/site-packages (from requests) (2022.12.7)
Installing collected packages: urllib3, requests
Successfully installed requests-2.28.2 urllib3-1.26.15pip uninstall requests urllib3
Found existing installation: requests 2.28.2
Uninstalling requests-2.28.2:
  Would remove:
    /Users/pradyunsg/Developer/github/pip/.venv/lib/python3.11/site-packages/requests-2.28.2.dist-info/*
    /Users/pradyunsg/Developer/github/pip/.venv/lib/python3.11/site-packages/requests/*
Proceed (Y/n)? y
  Successfully uninstalled requests-2.28.2
Found existing installation: urllib3 1.26.15
Uninstalling urllib3-1.26.15:
  Would remove:
    /Users/pradyunsg/Developer/github/pip/.venv/lib/python3.11/site-packages/urllib3-1.26.15.dist-info/*
    /Users/pradyunsg/Developer/github/pip/.venv/lib/python3.11/site-packages/urllib3/*
Proceed (Y/n)? y
  Successfully uninstalled urllib3-1.26.15

Looks like that's not an issue! :)

Also, this needed me to do a hacky-ish install (to achieve a 23.whatever version being installed). With the latest setuptools, you have:

pip install -e .
Obtaining file:///Users/pradyunsg/Developer/github/pip
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... error
  error: subprocess-exited-with-error
  
  × Getting requirements to build editable did not run successfully.
  │ exit code: 1
  ╰─> [77 lines of output]
      /private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/dist.py:520: SetuptoolsDeprecationWarning: Invalid version: '23.whatever'.
      !!
      
              ********************************************************************************
              The version specified is not a valid version according to PEP 440.
              This may not work as expected with newer versions of
              setuptools, pip, and PyPI.
      
              By 2023-Sep-26, you need to update your project and remove deprecated calls
              or your builds will no longer be supported.
      
              See https://peps.python.org/pep-0440/ for details.
              ********************************************************************************
      
      !!
        self._validate_version(self.metadata.version)
      running egg_info
      /private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/command/egg_info.py:131: SetuptoolsDeprecationWarning: Invalid version: '23.whatever'.
      !!
      
              ********************************************************************************
              Version '23.whatever' is not valid according to PEP 440.
      
              Please make sure to specify a valid version for your package.
              Also note that future releases of setuptools may halt the build process
              if an invalid version is given.
      
              By 2023-Sep-26, you need to update your project and remove deprecated calls
              or your builds will no longer be supported.
      
              See https://peps.python.org/pep-0440/ for details.
              ********************************************************************************
      
      !!
        return _normalization.best_effort_version(tagged)
      Traceback (most recent call last):
        File "/Users/pradyunsg/Developer/github/pip/src/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
        File "/Users/pradyunsg/Developer/github/pip/src/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/Users/pradyunsg/Developer/github/pip/src/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 132, in get_requires_for_build_editable
          return hook(config_settings)
                 ^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 450, in get_requires_for_build_editable
          return self.get_requires_for_build_wheel(config_settings)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 341, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=['wheel'])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 323, in _get_build_requires
          self.run_setup()
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 338, in run_setup
          exec(code, locals())
        File "<string>", line 26, in <module>
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/__init__.py", line 107, in setup
          return distutils.core.setup(**attrs)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 185, in setup
          return run_commands(dist)
                 ^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 201, in run_commands
          dist.run_commands()
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 969, in run_commands
          self.run_command(cmd)
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/dist.py", line 1244, in run_command
          super().run_command(command)
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 987, in run_command
          cmd_obj.ensure_finalized()
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/_distutils/cmd.py", line 111, in ensure_finalized
          self.finalize_options()
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/command/egg_info.py", line 219, in finalize_options
          parsed_version = packaging.version.Version(self.egg_version)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/y1/j465wvf92vs938kmgqh63bj80000gn/T/pip-build-env-rh1ektmt/overlay/lib/python3.11/site-packages/setuptools/_vendor/packaging/version.py", line 197, in __init__
          raise InvalidVersion(f"Invalid version: '{version}'")
      setuptools.extern.packaging.version.InvalidVersion: Invalid version: '23.whatever'
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build editable did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

@pfmoore
Copy link
Member Author

pfmoore commented Apr 25, 2023

So we don't have a problem, correct? As another data point, we didn't have any reports of issues with non-PEP440 versions in the period when 23.1 was released before 23.1.1, so that indicates we don't have a problem, as well.

If you're OK with this, then just approve and I'll merge it and cut a 23.1.2. And then hopefully we're done...

@pradyunsg
Copy link
Member

So we don't have a problem, correct?

We do not, and this is good to go!

@pfmoore pfmoore merged commit bc7621a into pypa:main Apr 25, 2023
@pfmoore
Copy link
Member Author

pfmoore commented Apr 25, 2023

Thanks, I'll do 23.3.2 tomorrow (UK time).

@pfmoore pfmoore deleted the vendoring-setuptools branch April 25, 2023 21:49
Comment on lines +50 to +56
# Suppress the pkg_resources deprecation warning
# Note - we use a module of .*pkg_resources to cover
# the normal case (pip._vendor.pkg_resources) and the
# devendored case (a bare pkg_resources)
warnings.filterwarnings(
action="ignore", category=DeprecationWarning, module=".*pkg_resources"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this also filter out pip._internal.metadata.pkg_resources? It’s probably fine since we probably won’t deprecate anything in that module anyway (any new development happens in the importlib.metadata backend instead), but I just want to clarify this in case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would. Ideally, I'd have made the filter just on pip._vendor.pkg_resources, but that wouldn't have handled the devendoring case. I could have had 2 separate filters, or even left it for whoever does the devendoring to sort out, but that seems unnecessarily unfriendly.

I'd be fine with someone submitting a PR to tighten this up (I didn't even think of pip._internal.metadata.pkg_resources), but like you I think it's probably fine as it is.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants