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

PEP 517: The "complete, working build backend" shown doesn't actually work #2536

Closed
wimglenn opened this issue Apr 18, 2022 · 7 comments
Closed
Labels

Comments

@wimglenn
Copy link
Contributor

The minimal example mypackage_custom_build_backend.py shown in the pep doesn't work.

This is what you see using pip 22.0.4 if you try to use it:

$ pip install .
Processing /private/tmp/example
  Getting requirements to build wheel ... done
  Installing backend dependencies ... done
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error
  
  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [17 lines of output]
      Traceback (most recent call last):
        File "/private/tmp/example/.venv/lib/python3.10/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 156, in prepare_metadata_for_build_wheel
          hook = backend.prepare_metadata_for_build_wheel
      AttributeError: module 'mypackage_custom_build_backend' has no attribute 'prepare_metadata_for_build_wheel'
      
      During handling of the above exception, another exception occurred:
      
      Traceback (most recent call last):
        File "/private/tmp/example/.venv/lib/python3.10/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 363, in <module>
          main()
        File "/private/tmp/example/.venv/lib/python3.10/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 345, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "/private/tmp/example/.venv/lib/python3.10/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 160, in prepare_metadata_for_build_wheel
          whl_basename = backend.build_wheel(metadata_directory, config_settings)
        File "/private/tmp/example/mypackage_custom_build_backend.py", line 47, in build_wheel
          from wheel.archive import archive_wheelfile
      ModuleNotFoundError: No module named 'wheel.archive'
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

I didn't bother seeing what happened to wheel.archive because the readme for that project clearly says:

wheel is not intended to be used as a library, and as such there is no stable, public API

So probably it shouldn't be used as a library in this PEP?

@wimglenn wimglenn changed the title The "complete, working build backend" shown in PEP 517 doesn't work PEP 517: The "complete, working build backend" shown doesn't actually work Apr 18, 2022
@CAM-Gerlach
Copy link
Member

@brettcannon is this a valid issue? If so, is it worth updating the PEP text for?

@encukou
Copy link
Member

encukou commented Apr 19, 2022

PEP 517 is a historical document from 2017, so it's expected that something would break.
The current specification is at https://packaging.python.org/en/latest/specifications/source-distribution-format/.
Any changes should be made there. (Where it's still defined by a reference to the PEP, text needs to be moved to the spec first.)

@wimglenn
Copy link
Contributor Author

@encukou That link seems to only talk about the source tree sdist layout, it's not mentioning the python hooks at all. Anyone implementing a build backend would have a hard time without seeing some hook specification .. :)

@CAM-Gerlach
Copy link
Member

PEP 517 (and 518, and 660) haven't yet been migrated to a specification on the PyPA specifications site; see pypa/packaging.python.org#955 . Its on my to-do list to help with, but its a fairly substantial project.

I'm not sure its really that necessary to continue to keep a (self-described "terrible") toy example in an appendix up to date with changes in the internal implementation details of the wheel package. But perhaps it would make sense to simply tweak the text to clarify that it is a complete, working backend at the time of writing, or mention the required version of wheel.

@brettcannon
Copy link
Member

I agree with CAM that I don't think it's worth updating the example. Best to put the effort into moving the spec over to packaging.python.org.

@wimglenn
Copy link
Contributor Author

wimglenn commented May 15, 2022

@CAM-Gerlach Well, I'm not convinced it was ever working, even at the time of writing, because when archive_wheelfile existed (pre Mar 2018 archive.py) it needed to be called with the wheel file basename but the PEP517 example backend calls it with WHEEL_FILENAME = "mypackage-0.1-py2.py3-none-any.whl" (cc @ncoghlan - did this example build backend actually build a working wheel at one stage?)

I still think it would be good to have a real working example in there, especially while there is no better alternative on https://packaging.python.org/en/latest/specifications/ , the PEP is kind of an important one and is the closest thing we have to a spec currently. Maybe it could use an existing lib with an actual API such as distlib or wheelfile?

@zahlman
Copy link

zahlman commented Jul 26, 2024

It's been over two years now and there still doesn't appear to be any proper documentation of this - the PR for documentation on packaging.python.org has been held up for almost exactly two years, and the PEP has not been amended.

Even with fixes to incorporate actual wheel creation (via zipfile.ZipFile), I have found that the code is still buggy, and specifically in a way that misleads about the interface the PEP is trying to describe. Specifically, the build_wheel hook is documented like:

def build_wheel(wheel_directory, config_settings=None, metadata_directory=None):
    ...

However, the example implements:

def build_wheel(wheel_directory,
                metadata_directory=None, config_settings=None):

That is, the order of the optional metadata_directory and config_settings parameters was swapped.

pyproject_hooks passes the values positionally:

def build_wheel(wheel_directory, config_settings, metadata_directory=None):
    """Invoke the mandatory build_wheel hook.

    If a wheel was already built in the
    prepare_metadata_for_build_wheel fallback, this
    will copy it rather than rebuilding the wheel.
    """
    prebuilt_whl = _find_already_built_wheel(metadata_directory)
    if prebuilt_whl:
        shutil.copy2(prebuilt_whl, wheel_directory)
        return os.path.basename(prebuilt_whl)

    return _build_backend().build_wheel(
        wheel_directory, config_settings, metadata_directory
    )

The different order in the example could lead the reader to expect keyword arguments, or could simply be overlooked (as I did). As a result, when I tested my custom backend I found that build_wheel couldn't properly receive config_settings values from the command line for the build program, and this was quite tedious to debug. (If I were using the prepare_metadata_for_build_wheel hook, I presume I would have had more useful debugging information.)

If it was intended that the frontend must pass the arguments by keyword, then they should have been documented as keyword-only arguments (i.e., using a bare * in the signature), and pyproject_hooks would then also have a bug. Otherwise, the example code is wrong, and the documented interface is also not as clear as it should be.

IMO and in general, using keyword-or-positional parameters (especially ones with default values) when describing an interface for others to implement, is a bad idea. It isn't clear to the implementer whether the parameter name is important (because it's allowed to be passed by keyword) and/or whether the order is important (because it's allowed to be passed positionally). The safe assumption, of course, is that both are important; but this is user-unfriendly and defies some informal conventions (when there are multiple parameters that default to None, one doesn't expect the arguments to get passed positionally, because it would be hard to remember the order). It appears in this case that even the author of the PEP expected those informal conventions to be followed, so clearly this is a gotcha for other build backend implementers.

zahlman added a commit to zahlman/bbbb that referenced this issue Jul 28, 2024
Determined that the order of parameters in the Appendix A example code
was wrong, and fixed it. This matters because `pyproject_hooks` passes
them positionally, despite that both default to `None` and it's not at
all obvious what order they "should" be in.

I commented on python/peps#2536, and cited
pypa/packaging.python.org#1111, to draw
attention to the issue in the PEP. The corresponding documentation
issue, pypa/packaging.python.org#955, still
appears not to be resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants