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

Support use as pdm-backend plugin #37

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

dairiki
Copy link

@dairiki dairiki commented Nov 11, 2023

This PR adds support for using hatch-fancy-pypi-readme as a PDM-backend plugin.

I was looking for a plugin exactly like this one, except to use with pdm-backend instead of hatchling. As it turns out, adding a pdm-backend plugin was pretty straight-forward.

I also decided to dynamically switch the pyproject.toml key where the plugin finds its configuration table depending on the build backend. Now when build-system.build-backend is set to pdm.backend, the configuration now gets pulled from tool.pdm.build.hooks.hatch-fancy-pypi-readme (rather than tool.hatch.metadata.hooks.fancy-pypi-readme).
This switch markedly increases the diff count of the PR, however it seems to me that the potential for confusion if a pdm-backend plugin pulls its configuration from tool.hatch.* is large, so the extra mess is worth it.

@Tinche
Copy link

Tinche commented Nov 11, 2023

Why don't you use PDM with the hatchling backend? Feels like a more composable solution.

@dairiki dairiki marked this pull request as draft November 11, 2023 17:02
@dairiki
Copy link
Author

dairiki commented Nov 11, 2023

Why don't you use PDM with the hatchling backend? Feels like a more composable solution.

In simple cases, either pdm-backend or hatchling can be used, but there are (ever-evolving) differences between the two which may lead one to choose one over the other. For example, suppose one wants to be able to use a fancy-pypi-readme along with another pdm-backend plugin?

Get configuration from
"tool.pdm.build.hooks.fancy-pypi-readme" (rather than
"tool.hatch.metadata.hooks.fancy-pypi-readme") when running as a
pdm-backend plugin.
Fix CLI to change config source ("tools.pdm.build..." vs
"tools.hatch.metadata...") when the build backend is set to
pdm.backend.
@dairiki dairiki marked this pull request as ready for review November 12, 2023 02:42
@hynek
Copy link
Owner

hynek commented Nov 17, 2023

This feature has been percolating in my brain for months, but I've been hardcore procrastinating on it (as I've been on your PR – sorry).

But I find it a bit ugly to have a package named hatch-fancy-pypi-readme with PDM support.

I wonder if we could use your changes (sorry couldn't look closer yet, because I'm positively overwhelmed rn) to somehow extract a fancy-pypi-readme library and refactor hatch-fancy-pypi-readme and a pdm-fancy-pypi-readme on top of it? 🤔

@dairiki
Copy link
Author

dairiki commented Nov 18, 2023

But I find it a bit ugly to have a package named hatch-fancy-pypi-readme with PDM support.

Yeah, that occurred to me, too...

I wonder if we could use your changes (sorry couldn't look closer yet, because I'm positively overwhelmed rn) to somehow extract a fancy-pypi-readme library and refactor hatch-fancy-pypi-readme and a pdm-fancy-pypi-readme on top of it? 🤔

That would definitely be cleaner. I didn't suggest it initially because I wasn't sure how receptive you'd be to splitting off a new distribution. If it weren't for the CLI, it would, I think, be pretty straight-forward.

Here's an outline of what's in this PR and how involved each bit is.

The Build Hook

Implementing the pdm build hook required minimal tweaks in the rest of the hatch-fancy-pypi-readme code.

The only complication stems from the fact that (from what I can tell) hatchling prescribes the config key under which your hooks config table will be found (tool.hatch.metadata.hooks.fancy-pypi-readme). Using a tool.hatch... prefix for a pdm hook is just too daring, so I decided to pull the configuration from tool.pdm.build.hook.fancy-pypi-readme for the pdm build hook.

This was simple and required zero change to any existing code. The pdm hook code fetches its config from the pyproject.toml dict that pdm passes it, passes it to hatch-fancy-pypi-readme._config.load_and_validate_config. The result from that then goes to ._builder.build_text to generate the fancy README.

The only issue is that any error message generated by load_and_validate_config list the wrong config keys. So I added a base parameter to load_and_validate_config just to tweak the error messages.

The CLI

So now with two different (three if you count hatch.toml) for the readme configuration, it makes life for the CLI more complicated.

I added a --backend {auto,hatchling,pdm.backend} switch to allow for specifying where the readme configuration should be pulled from. The default, auto, causes the CLI to guess based on the setting of build-system.build-backend in the pyproject.toml. (That's probably all that's necessary in most use cases.)

The changes are nothing complicated, but the diff line count is not insignificant.


Other than just the mechanics of creating two new distributions, splitting this into three distribution would not be hard.

The only real question is what to do about the CLI. Given that the CLI is pretty simple, and that the current one is already named hatch-fancy-pypi-readme, it's probably easiest for each backend-specific distribution to implement its own CLI. (Somehow that feels a little wrong, since both CLIs would do the same thing: print out the assembled README. But most of the cli-specific code is dealing with fetch the config which is backend-specific. After looking at it, there's not a lot of duplicated code if one just write two separate CLIs — one for hatchling, one for pdm.builder.)

If we do that, then __main__.py, _cli.py, and hooks.py stay in the hatch-fancy-pypi-readme dist, while _builder.py, _config.py, _fragments.py, _substitutions.py, and exceptions.py get moved to the new fancy-pypi-readme. The split is pretty clean.

I think the only code that needs any tweaking is in _config.py, and again, that's only to make the error messages correct when a different config key prefix is in use.

I can try, if you'd like. (Though it may be a little while before I get to it.)

@dairiki
Copy link
Author

dairiki commented Nov 19, 2023

I did a quick job of splitting things into three separate dists.

This is not done. In particular more thought needs to go into what API gets exported from fancy-pypi-readme, and whether it could use any more tweaks to make general usage easier. Perhaps a test fixture?

Also, I haven't touched the READMEs, etc at all, so they obviously need work. This is more a proof of concept at this point.

Anyhow, the three dists are in three branches of my forked repo:


It all splits out fairly logically. However, there is now more packaging than code!

@dairiki
Copy link
Author

dairiki commented Nov 19, 2023

Having slept on it, here's another idea:

  1. Move/rename everything from hatch-fancy-pypi-readme to fancy-pypi-readme. (Rename the CLI, as well, from hatch_fancy_pypi_readme to fancy_pypi_readme.)
  2. For backward-compatibility, turn this package (hatch-fancy-pypi-readme), into a stub that:
    • depends on fancy-pypi-readme
    • creates a hatch_fancy_pypi_readme CLI which is just an alias for fancy_pypi_readme
    • (maybe?) issues a deprecation warning when used as a hatchling build hook, and/or when the aliased CLI is run

If we take this route, I propose adding a new function to the API which would be used to extract the fancy-readme config data from the (parsed) pyproject.toml (and possibly hatch.toml) data. This new function would be used by the CLI, and would replace most of the logic that's currently in cli_run. The signature of the new function would be something like:

@dataclass
ConfigData:
    data: dict[str, Any]

    # Information on where the config data was extracted from
    source_key: str # e.g. "tool.pdm.build.hooks.fancy-pypi-readme"
    source_filename: str | None  # e.g. "pyproject.toml"



def find_config_data(
    pyproject_toml: dict[str, Any], # parsed pyproject.toml data
    build_backend: str | None = None, 
    *,
    hatch_toml: dict[str, Any] | None = None,  # parsed hatch.toml data (when appropriate)
) -> ConfigData:
    ...

The build_backend argument is used to specify the build backend in use. Choices would include "hatchling", "pdm.backend" (and probably the aliases "hatch", "pdm"). The default value None means autodetect based on the value of pyproject_data["build-system"]["build-backend"].

This function would perform various sanity checks, then return the data from the appropriate fancy-pypi-readme config subtable. E.g. for build_backend="hatchling" the first found of hatch_toml["metadata"]["metadata"]["fancy-pypi-readme"] or pyproject_toml["tool"]["hatch"]["metadata"]["metadata"]["fancy-pypi-readme"] would be returned.

The return value would also contain extra attribute(s) carrying metadata regarding where the config data was extracted from. This extra metadata would be used by load_and_validate_config when constructing error messages.

try:
    config_data = find_config_data(pyproject_toml, hatch_toml=hatch_toml, build_backend=build_backend)
    config = load_and_validate_config(config_data)
except ConfigurationError as e:
   # report error
print(build_text(config.fragments, config.substitutions), file=out)

The new function would also be used by the PDM build hook to extract the readme config from the parsed pyproject.toml data.

@hynek
Copy link
Owner

hynek commented Nov 21, 2023

Ha you've been busy – sorry I can't quite keep up ATM! I'm currently mostly stuck in pondering how to separate them mechanically. I think I don't want to separate out more repositories… so I guess some kind of monorepo would be apt? I wonder what the most elegant way to achieve this would be. I almost suspect the most straight-forward way would be to make one fancy-pypi-readme and use conditional imports within it to add the backend-specific features?

@dairiki
Copy link
Author

dairiki commented Nov 22, 2023

I think I don't want to separate out more repositories… so I guess some kind of monorepo would be apt?

If the plan is to (add pdm-backend compatibility and) rename the current hatch-fancy-pypi-readme distribution to fancy-pypi-readme, I think keeping everything in the current repo is fine.

  • The name of the distribution in the top-level pyproject.toml of this repo would be renamed to fancy-pypi-readme.
  • src/hatch-fancy-pypi-readme would be renamed to src/fancy-pypi-readme, etc...
  • The code for the replacement hatch-fancy-pypi-readme stub distribution could go in a subdirectory. I don't foresee that that stub would need updating very often at all (it just declares a dependency fancy-pypi-readme and a console-script entry point which points to fancy-pypi-readme.__main__:main) so perhaps it could just be manually published to PyPI (no github workflow necessary for that).

I almost suspect the most straight-forward way would be to make one fancy-pypi-readme and use conditional imports within it to add the backend-specific features?

As far as the build backend hooks, that's pretty much how it works, automatically. The build backends find the hooks via declared entry-points. As long as those entry points point to different modules for the different build backends that's exactly what happens.

@henryiii
Copy link

henryiii commented Dec 31, 2023

FYI, you should look at my dynamic-metadata proposal and system. It's not finalized yet, but it would provide a way to make general plugins for any backend. This is already supported (via a bit of fragile usage of private APIs) in scikit-build-core.

Making a plugin for each backend quickly becomes unmanageable!

@hynek
Copy link
Owner

hynek commented Jan 8, 2024

OK happy new year and sorry, I've come to the conclusion that I don't have the time and energy to do this properly. So, if you're willing to rebase and make it work again, I'm willing to take a closer look. If not, I understand.

(Henry: I'm only entertaining this because I use PDM myself :))

@hynek hynek mentioned this pull request Jul 22, 2024
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.

4 participants