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

editable install after pep660 breaks setup.py that relies on specific exception type to retry setup() after a build error #3572

Closed
CaselIT opened this issue Aug 31, 2022 · 10 comments

Comments

@CaselIT
Copy link

CaselIT commented Aug 31, 2022

setuptools version

65.3, any after 64

Python version

3.7+

OS

any without compilers installed

Additional environment information

No response

Description

Not sure if this more of a bug or more of a discussion, feel free to move

I maintain two libraries, falcon and sqlalchemy, that have a similar install setup, namely they provide some optional extension modules that enable some speed up if compiled, but they have a pure python fallback in case compilation is not possible.

The setup.py take advantage of this and basically tries to compile the extension modules, then in case of a build error retries the setup() call without the extension modules, running a pure python install.
The setup.py files are these

Since version setuptools version 64 that enabled pep660 for editable builds this workflow no longer works, since among other things the exception raised by setup() is a SystemExit one.
Normal, non editable installs (pip install .) continues to work correctly.

I've tried to debug this and found a workaround, but it definitely seems like retrying the setup() call based on some condition is not something that should be done, at least in editable mode.

Will normal installs (pip install .) behave similarly to editable mode when installing from source?
Is there a better way of doing something similar in a supported or at least more robust way?
Should a custom backend be used, similar to what the documentation talks about here?
If that's the case, what's the best way for the backend to check if a compiler is available?

cc @zzzeek @vytas7

Expected behavior

Editable build installs in compiled or pure python version depending on the install machine

How to Reproduce

run in docker run -ti --rm --name py python:3.10-slim bash

apt update && apt install git
git clone --depth=1 https://github.com/sqlalchemy/sqlalchemy.git
cd sqlalchemy
pip install -e .

Output

Obtaining file:///sqlalchemy
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... done
Collecting typing-extensions>=4.1.0
  Downloading typing_extensions-4.3.0-py3-none-any.whl (25 kB)
Collecting greenlet!=0.4.17
  Downloading greenlet-1.1.3-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (155 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 155.9/155.9 kB 6.8 MB/s eta 0:00:00
Building wheels for collected packages: SQLAlchemy
  Building editable for SQLAlchemy (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Building editable for SQLAlchemy (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [105 lines of output]
      running editable_wheel
      creating /tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info
      writing /tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/PKG-INFO
      writing dependency_links to /tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/dependency_links.txt
      writing requirements to /tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/requires.txt
      writing top-level names to /tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/top_level.txt
      writing manifest file '/tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/SOURCES.txt'
      reading manifest file '/tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/SOURCES.txt'
      reading manifest template 'MANIFEST.in'
      warning: no files found matching '*.html' under directory 'doc'
      warning: no files found matching '*.css' under directory 'doc'
      warning: no files found matching '*.js' under directory 'doc'
      warning: no previously-included files found matching 'lib/sqlalchemy/cyextension/*.c'
      warning: no previously-included files found matching 'lib/sqlalchemy/cyextension/*.so'
      no previously-included directories found matching 'doc/build/output'
      adding license file 'LICENSE'
      writing manifest file '/tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy.egg-info/SOURCES.txt'
      creating '/tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy-2.0.0b1.dev0.dist-info'
      adding license file "LICENSE" (matched pattern "LICENSE")
      creating /tmp/pip-wheel-khamx5vf/tmp98_gnvqg/SQLAlchemy-2.0.0b1.dev0.dist-info/WHEEL
      /tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/command/install.py:34: SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip and other standards-based tools.
        warnings.warn(
      running build_py
      running build_ext
      cythoning lib/sqlalchemy/cyextension/collections.pyx to lib/sqlalchemy/cyextension/collections.c
      cythoning lib/sqlalchemy/cyextension/immutabledict.pyx to lib/sqlalchemy/cyextension/immutabledict.c
      cythoning lib/sqlalchemy/cyextension/processors.pyx to lib/sqlalchemy/cyextension/processors.c
      cythoning lib/sqlalchemy/cyextension/resultproxy.pyx to lib/sqlalchemy/cyextension/resultproxy.c
      cythoning lib/sqlalchemy/cyextension/util.pyx to lib/sqlalchemy/cyextension/util.c
      building 'sqlalchemy.cyextension.collections' extension
      creating /tmp/tmp1lvr9mj2.build-temp/lib
      creating /tmp/tmp1lvr9mj2.build-temp/lib/sqlalchemy
      creating /tmp/tmp1lvr9mj2.build-temp/lib/sqlalchemy/cyextension
      gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/usr/local/include/python3.10 -c lib/sqlalchemy/cyextension/collections.c -o /tmp/tmp1lvr9mj2.build-temp/lib/sqlalchemy/cyextension/collections.o
      Traceback (most recent call last):
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/spawn.py", line 57, in spawn
          proc = subprocess.Popen(cmd, env=env)
        File "/usr/local/lib/python3.10/subprocess.py", line 969, in __init__
          self._execute_child(args, executable, preexec_fn, close_fds,
        File "/usr/local/lib/python3.10/subprocess.py", line 1845, in _execute_child
          raise child_exception_type(errno_num, err_msg, err_filename)
      FileNotFoundError: [Errno 2] No such file or directory: 'gcc'

      The above exception was the direct cause of the following exception:

      Traceback (most recent call last):
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/unixccompiler.py", line 186, in _compile
          self.spawn(compiler_so + cc_args + [src, '-o', obj] + extra_postargs)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/ccompiler.py", line 1007, in spawn
          spawn(cmd, dry_run=self.dry_run, **kwargs)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/spawn.py", line 63, in spawn
          raise DistutilsExecError(
      distutils.errors.DistutilsExecError: command 'gcc' failed: No such file or directory

      During handling of the above exception, another exception occurred:

      Traceback (most recent call last):
        File "<string>", line 83, in build_extension
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/command/build_ext.py", line 547, in build_extension
          objects = self.compiler.compile(
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/ccompiler.py", line 599, in compile
          self._compile(obj, src, ext, cc_args, extra_postargs, pp_opts)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/unixccompiler.py", line 188, in _compile
          raise CompileError(msg)
      distutils.errors.CompileError: command 'gcc' failed: No such file or directory

      The above exception was the direct cause of the following exception:

      Traceback (most recent call last):
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/command/editable_wheel.py", line 140, in run
          self._create_wheel_file(bdist_wheel)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/command/editable_wheel.py", line 330, in _create_wheel_file
          files, mapping = self._run_build_commands(dist_name, unpacked, lib, tmp)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/command/editable_wheel.py", line 261, in _run_build_commands
          self._run_build_subcommands()
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/command/editable_wheel.py", line 288, in _run_build_subcommands
          self.run_command(name)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/cmd.py", line 319, in run_command
          self.distribution.run_command(command)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/dist.py", line 1217, in run_command
          super().run_command(command)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/dist.py", line 992, in run_command
          cmd_obj.run()
        File "<string>", line 77, in run
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/Cython/Distutils/old_build_ext.py", line 186, in run
          _build_ext.build_ext.run(self)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/command/build_ext.py", line 346, in run
          self.build_extensions()
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/Cython/Distutils/old_build_ext.py", line 195, in build_extensions
          _build_ext.build_ext.build_extensions(self)
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/command/build_ext.py", line 466, in build_extensions
          self._build_extensions_serial()
        File "/tmp/pip-build-env-k6x4vz1l/overlay/lib/python3.10/site-packages/setuptools/_distutils/command/build_ext.py", line 492, in _build_extensions_serial
          self.build_extension(ext)
        File "<string>", line 85, in build_extension
      BuildFailed
      error: Support for editable installs via PEP 660 was recently introduced
      in `setuptools`. If you are seeing this error, please report to:

      https://github.com/pypa/setuptools/issues

      Meanwhile you can try the legacy behavior by setting an
      environment variable and trying to install again:

      SETUPTOOLS_ENABLE_FEATURES="legacy-editable"
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building editable for SQLAlchemy
Failed to build SQLAlchemy
ERROR: Could not build wheels for SQLAlchemy, which is required to install pyproject.toml-based projects
@CaselIT CaselIT added bug Needs Triage Issues that need to be evaluated for severity and status. labels Aug 31, 2022
@abravalheri
Copy link
Contributor

abravalheri commented Sep 1, 2022

Hi @CaselIT, thank you very much for reporting the problem.

This is a tricky one!

I can see a clash between setuptools and the custom setup script: both parties want to control how the exception is handled. Setuptools wants to capture unknown errors and display instructions for how the user can workaround the situation and report the problem (this is specially important since PEP 660 changes the ways things used to work); the custom setup script wants to use exceptions in some sort of control flow.

I believe setuptools should not stop showing that error message to user, at least not at this point in time. It is very important in other situations (e.g. plugins or local customisations that are not prepared to handle PEP 660).

I can see that you guys already take into account the possibility that some command objects do not raise an exact exception class.
To ensure that happens, the custom script is sub-classing build_ext and using try-except statements to force an specific exception type.
Is there any chance you could do the same for the editable_wheel command?

I can also think about other ways of doing it:

  • Have you guys considered using the optional kwarg in the Extension object?
    (eg.: optional=not os.environ.get("REQUIRE_SQLALCHEMY_CEXT"))
    (Not sure if it works with the deprecated Cython.Distutils.old_build_ext, but it should work with setuptools.command.build_ext which already takes into account Cython). This has the potential to simplify the setup.py script (the drawback are the less informative warning messages).
  • How about handling the exceptions directly inside build_ext instead of expecting it to bubble up unhandled throughout the entire call stack? Have you considered something like the following?
    class ve_build_ext(old_build_ext):
        # This class allows Cython building to fail.
    
        def run(self):  # Not sure if it is needed to wrap `run`
            with OverlookOptionalExtensions():
                super().run()
    
        def build_extension(self, ext):
            with OverlookOptionalExtensions():
                super().build_extension(ext)
    
    class OverlookOptionalExtensions:
        def __enter__(self):
            return None
            
        def __exit__(self, exc_type, exc_value, traceback):
            if exc_value is None:
                return True
            if os.environ.get("REQUIRE_SQLALCHEMY_CEXT"):
                status_msgs(
                    "NOTE: Cython extension build is required because "
                    "REQUIRE_SQLALCHEMY_CEXT is set, and the build has failed; "
                    "will not degrade to non-C extensions"
                )
                return
            if (
                isinstance(exc_value, ValueError) and "'path'" in str(exc_value)
                # ^-- this can happen on Windows 64 bit, see Python issue 7511
                or isinstance(exc_value, ext_errors)
                # ^-- in the case of compiling errors, simply use the Python fallback
            ):
                status_msgs(
                    exc_value,
                    "WARNING: The Cython extension could not be compiled, "
                    "speedups are not enabled.",
                    "Failure information, if any, is above.",
                )
                return True

@abravalheri abravalheri added Waiting User Feedback and removed bug Needs Triage Issues that need to be evaluated for severity and status. labels Sep 1, 2022
@abravalheri abravalheri changed the title [BUG] editable install after pep660 breaks setup.py that retries setup() after a build error editable install after pep660 breaks setup.py that relies on specific exception type to retry setup() after a build error Sep 1, 2022
@abravalheri
Copy link
Contributor

abravalheri commented Sep 1, 2022

Will normal installs (pip install .) behave similarly to editable mode when installing from source?

I would say that editable mode now works more closely to non-editable installs than before (it will never be 1:1)... But that is very subjective. Is there any specific aspect that you are looking for?

Based on the linked setup.py scripts, I can assume you want editable_wheel to bubble up errors instead of trying to handle them... I see the existing error handling mechanism in editable_wheel as valuable for other use cases, so unless we find an alternative method of doing that I think the best is to keep it.

I am more than happy to discuss a different mechanism as long as we preserve he overall functionality (i.e. showing instructions/workarounds that users can do in the case of "mysterious errors" probably associated with plugins/customisations that are not PEP 660-ready). I welcome any PRs that would like to explore that direction.

Is there a better way of doing something similar in a supported or at least more robust way?

I think the closest thing setuptools has is the optional keyword for the Extension constructor, but I discussed a few other things in the previous comment that you might be interested in trying out.

Should a custom backend be used, similar to what the documentation talks about here?

I don't think this is the case for a custom backend. So far the primary use-case I found for a custom backend is to dynamically decide about build dependencies. Another use case would be modifying the config_settings dictionary to customise how setuptools works.

If that's the case, what's the best way for the backend to check if a compiler is available?

Currently there are some ongoing discussions on that regard. I am afraid there is nothing "final" yet, but you might want to have a look on #2806 (specially the insights from Jason).

@CaselIT
Copy link
Author

CaselIT commented Sep 1, 2022

Hi @abravalheri

Thanks for the detailed answer! I'll look into the things you suggested.
It looks like the optional keyword could work for what we are trying to do.
Regarding the other suggestion, without testing it, I'm not sure how that ve_build_ext would retry the build after a build error.

I would say that editable mode now works more closely to non-editable installs than before (it will never be 1:1)... But that is very subjective. Is there any specific aspect that you are looking for?

My question was motivated by the fact that a normal install is still bubbling up the errors, while the editable mode was not, I wanted to know if the idea was to modify the normal install mode to be like the editable one regarding bubbling modes

Based on the linked setup.py scripts, I can assume you want editable_wheel to bubble up errors instead of trying to handle them

Not necessarily, I mean the current setup.py is like it is because exception were bubbled up. The objective of the issue is to find a working solution, if any, with the new system, not to make thing work like before since this is probably a really niche use case, and it's most likely not worth changing the implementation to accommodate for it.

I don't think this is the case for a custom backend. So far the primary use-case I found for a custom backend is to dynamically decide about build dependencies. Another use case would be modifying the config_settings dictionary to customise how setuptools works.

Ok thanks!

@abravalheri
Copy link
Contributor

I'm not sure how that ve_build_ext would retry the build after a build error.

The suggestion using ve_build_ext is not properly a "retry" but rather a "keep going".

Do you need to restart the build and try again? If that is the case, then the suggestion about using a wrapper for the editable_wheel similar to what ve_build_ext might be more appropriate (I haven't tested this option myself, so I don't know if it will work).

I wanted to know if the idea was to modify the normal install mode to be like the editable one regarding bubbling modes

So far, the need for that hasn't presented itself. editable_wheel is a special case because it is a new implementation and there might be customisations/plugins that are not prepared to deal with it, so we need a way to inform the users.

I don't know to which extent we can classify exception handling as part of the explicit API or simply Hyrum's Law in action.

@zzzeek
Copy link

zzzeek commented Sep 1, 2022

I just want to note that on our end (SQLAlchemy), the "retry" thing IMO is not the most important thing. If we could instead have a way to make a very good estimation that compiler tools are not expected to be present, or that they are, we could just choose one build or another. Another way I can see us going is that we go to an entirely non-intelligent system, but when a build fails, we can at least give our own error message that will say "Please use DISABLE_SQLALCHEMY_CEXT if build tools aren't installed".

@abravalheri
Copy link
Contributor

If we could instead have a way to make a very good estimation that compiler tools are not expected to be present, or that they are, we could just choose one build or another

For that maybe the best is to engage in issue #2806. I believe that currently there is no API to check if the compiler is present (but I might be wrong).

@CaselIT
Copy link
Author

CaselIT commented Sep 1, 2022

I just want to note that on our end (SQLAlchemy), the "retry" thing IMO is not the most important thing. If we could instead have a way to make a very good estimation that compiler tools are not expected to be present, or that they are, we could just choose one build or another. Another way I can see us going is that we go to an entirely non-intelligent system, but when a build fails, we can at least give our own error message that will say "Please use DISABLE_SQLALCHEMY_CEXT if build tools aren't installed".

if optional works an option may be to just always declare the extension modules and if it fails it automatically falls back, so there would not even be the need to detect the compilers.

Maybe the only thing we want to keep is a detection of errors to print a log to users regarding installing in pure python mode.

I'll try playing with it and report back!

@abravalheri
Copy link
Contributor

@CaselIT please note that pip will by default hide anything from stdout. Users need to use pip -v install -e . to show the output.

@CaselIT
Copy link
Author

CaselIT commented Sep 1, 2022

@abravalheri thanks a lot for the support!

Have you guys considered using the optional kwarg in the Extension object?

this is working perfectly. We were not aware of that keyword! It's great that's supported by setuptools without any strange shenanigan in the setup file.

The final setup.py for sqlalchemy looks like this https://gerrit.sqlalchemy.org/plugins/gitiles/sqlalchemy/sqlalchemy/+/refs/changes/55/4055/6/setup.py

Thanks again!

@CaselIT CaselIT closed this as completed Sep 1, 2022
@vytas7
Copy link

vytas7 commented Sep 2, 2022

Thanks for shedding light on this @abravalheri!

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

No branches or pull requests

4 participants