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

Workaround for bdist_wheel.dist_info_dir problems #4684

Merged
merged 4 commits into from
Oct 16, 2024

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Oct 15, 2024

Summary of changes

This is a workaround identified in #4647 (comment)

We should be able to see the integration tests failing or passing for PyYAML in https://github.com/pypa/setuptools/actions/runs/11353165220

Closes #4683

Pull Request Checklist

@abravalheri
Copy link
Contributor Author

@abravalheri abravalheri reopened this Oct 15, 2024
@abravalheri abravalheri marked this pull request as ready for review October 15, 2024 19:53
@abravalheri
Copy link
Contributor Author

@pelson could you please have a look?

The objective is to avoid breaking old packages in the ecosystem that have not updated yet.

@pelson
Copy link
Contributor

pelson commented Oct 16, 2024

First impression: complex but reasonable. Thanks for tidying this up 👍.

I have to say that in general perhaps a more defensive infrastructure is needed around the command concept, otherwise we can never extend the capabilities of a particular command without breaking something for somebody who implemented an alternative command of the same name.

@abravalheri
Copy link
Contributor Author

abravalheri commented Oct 16, 2024

First impression: complex but reasonable. Thanks for tidying this up 👍.

Yeah, I tried adding the exception handling in at least 3 other places before landing on this approach, which I thing was the best between all the ones I tried... Not sure I can do better than that.

I have to say that in general perhaps a more defensive infrastructure is needed around the command concept, otherwise we can never extend the capabilities of a particular command without breaking something for somebody who implemented an alternative command of the same name.

Agreed. Do you have any suggestion for that?

@abravalheri
Copy link
Contributor Author

I will go ahead and merge this now so that we can cut a release.

We can revisit the implementation and make it simpler in a follow up if anyone has a better idea.

@abravalheri abravalheri merged commit 8ad3ea7 into main Oct 16, 2024
83 of 86 checks passed
@abravalheri abravalheri deleted the fix-integration-tests branch October 16, 2024 09:47
@bebound
Copy link
Contributor

bebound commented Oct 22, 2024

Our pipeline starts fail after Oct 16, when 75.2.0 is released. I'm not sure if it is related to this PR.

I can only repro the error on macOS.

Please ignore TypeError: print() got an unexpected keyword argument 'color'. getopt.GetoptError: option --dist-info-dir not recognized seems to be related?

Updating to a newer version can fix it, but I'm curious why this error occurs.

pip install psutil==5.9.5 --no-binary :all:                                                                                                     

Collecting psutil==5.9.5
  Using cached psutil-5.9.5.tar.gz (493 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: psutil
  Building wheel for psutil (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Building wheel for psutil (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [81 lines of output]
      Traceback (most recent call last):
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/_distutils/fancy_getopt.py", line 245, in getopt
          opts, args = getopt.getopt(args, short_opts, self.long_opts)
                       ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/getopt.py", line 93, in getopt
          opts, args = do_longs(opts, args[0][2:], longopts, args[1:])
                       ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/getopt.py", line 157, in do_longs
          has_arg, opt = long_has_args(opt, longopts)
                         ~~~~~~~~~~~~~^^^^^^^^^^^^^^^
        File "/opt/homebrew/Cellar/[email protected]/3.13.0_1/Frameworks/Python.framework/Versions/3.13/lib/python3.13/getopt.py", line 174, in long_has_args
          raise GetoptError(_('option --%s not recognized') % opt, opt)
      getopt.GetoptError: option --dist-info-dir not recognized

      During handling of the above exception, another exception occurred:

      Traceback (most recent call last):
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/_distutils/core.py", line 170, in setup
          ok = dist.parse_command_line()
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/_distutils/dist.py", line 472, in parse_command_line
          args = self._parse_command_opts(parser, args)
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/dist.py", line 893, in _parse_command_opts
          nargs = _Distribution._parse_command_opts(self, parser, args)
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/_distutils/dist.py", line 576, in _parse_command_opts
          (args, opts) = parser.getopt(args[1:])
                         ~~~~~~~~~~~~~^^^^^^^^^^
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/_distutils/fancy_getopt.py", line 247, in getopt
          raise DistutilsArgError(msg)
      distutils.errors.DistutilsArgError: option --dist-info-dir not recognized

      During handling of the above exception, another exception occurred:

      Traceback (most recent call last):
        File "<string>", line 462, in main
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/__init__.py", line 117, in setup
          return distutils.core.setup(**attrs)
                 ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/_distutils/core.py", line 172, in setup
          raise SystemExit(gen_usage(dist.script_name) + f"\nerror: {msg}")
      SystemExit: usage: _in_process.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
         or: _in_process.py --help [cmd1 cmd2 ...]
         or: _in_process.py --help-commands
         or: _in_process.py cmd --help

      error: option --dist-info-dir not recognized

      During handling of the above exception, another exception occurred:

      Traceback (most recent call last):
        File "/Users/kk/venv/lib/python3.13/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
          ~~~~^^
        File "/Users/kk/venv/lib/python3.13/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
                                   ~~~~^^^^^^^^^^^^^^^^^^^^^^^^
        File "/Users/kk/venv/lib/python3.13/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 251, in build_wheel
          return _build_backend().build_wheel(wheel_directory, config_settings,
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                              metadata_directory)
                                              ^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 434, in build_wheel
          return _build(['bdist_wheel', '--dist-info-dir', metadata_directory])
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 422, in _build
          return self._build_with_temp_dir(
                 ~~~~~~~~~~~~~~~~~~~~~~~~~^
              cmd,
              ^^^^
          ...<3 lines>...
              self._arbitrary_args(config_settings),
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          )
          ^
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 403, in _build_with_temp_dir
          self.run_setup()
          ~~~~~~~~~~~~~~^^
        File "/private/var/folders/mg/sgkthmt12jn_gq4ymlyb_pcw0000gn/T/pip-build-env-uq16auuo/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 318, in run_setup
          exec(code, locals())
          ~~~~^^^^^^^^^^^^^^^^
        File "<string>", line 497, in <module>
        File "<string>", line 480, in main
      TypeError: print() got an unexpected keyword argument 'color'
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for psutil
Failed to build psutil
ERROR: ERROR: Failed to build installable wheels for some pyproject.toml based projects (psutil)

@abravalheri
Copy link
Contributor Author

abravalheri commented Oct 22, 2024

Hi @bebound, this seems to be a different problem.

The getopt.GetoptError: option --dist-info-dir not recognized is transformed by distutils to distutils.errors.DistutilsArgError and in turn into a SystemExit, which as per this PR, setuptools is well equipped to handle.

Now the other error that seems to be the root case of the problem is TypeError: print() got an unexpected keyword argument 'color', and that seems to come from the setup.py script of the package in question. Could you please have a look on that? If it persists please open an issue with a minimal reproducer.

@pelson
Copy link
Contributor

pelson commented Oct 22, 2024

setuptools is well equipped to handle

👍 - thanks again. The backwards compatibility of projects who used to import wheel in their setup.py was not something I had foreseen in my PR.

Now the other error that seems to be the root case of the problem is TypeError: print() got an unexpected keyword argument 'color', and that seems to come from the setup.py script of the package in question. Could you please have a look on that? If it persists please open an issue with a minimal reproducer.

The psutil 5.x versions imported wheel in their setup.py.
https://github.com/giampaolo/psutil/blob/27a1432daeacd46b287517c11bfea2af4fd95a88/setup.py#L37

The error for release-5.9.5 was seemingly fixed in giampaolo/psutil@6e23a51#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7R505 (5.9.6), and isn't an issue with setuptools at all.

@bebound
Copy link
Contributor

bebound commented Oct 23, 2024

Thanks for the response.

TypeError: print() got an unexpected keyword argument 'color' is caused by a wrong misplaced parenthesis in setup.py. (color="red" should be part of hilite) It's not related to setuptools.
https://github.com/giampaolo/psutil/blob/0d4900b073f8697ab21c47d823621bea61f39a3b/setup.py#L480-L481

            elif MACOS:
                print(hilite("XCode (https://developer.apple.com/xcode/) "
                             "is not installed"), color="red", file=sys.stderr)

I'm still confused.

In 75.1.0, if I have from wheel.bdist_wheel import bdist_wheel in setup.py, then getopt.GetoptError: option --dist-info-dir not recognized will be raised.

In 75.2.0, this PR tries to suppress the SystemExit converted from getopt.GetoptError.

However, for psutil==5.9.5, it works in 75.1.0, but fails in 75.2.0, which is completely reversed?

@abravalheri
Copy link
Contributor Author

abravalheri commented Oct 23, 2024

As codebases naturally change, this kind of stuff is normal. It is very common as dependencies evolve that previously unravelled code paths start to be expanded. If these code paths are untested, there is potential for error.

This PR in setuptools does not only "supress" the error. It identifies an anomally (usage of a deprecated functionality from wheel by the package being built) and adds the appropriate workaround.

The package tries to wrap the call to setup.py in a try... finally block. In principle, this seems to be a very robust idea, but unfortunately, the implementation introduces an argument error. That is what made it prone to error. When using a version of the package that fixed this coding error, support is restored.

The assumption motivating the try...finally block also ended up being partially incorrect: there are many ways a failure is possible at that stage (including due to the usage of deprecated functionality), not necessarily only the one the devs intended to capture.

The general advice1 for package developers that want long-term support from setuptools is:

  1. Keep an eye on deprecations and try to address them as soon as possible.
  2. Avoid creating brittle setup.py scripts and don't assume that the setup.py is going to be the outer most Python frame executed or that it is always going to be executed in the context of a CLI tool (that view is incompatible with PEP 517).

Footnotes

  1. Not necessarily related to this specific instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bdist_wheel.dist_info_dir seems to have broken building some projects (e.g. pyyaml).
3 participants