pypa / hatch

Modern, extensible Python project management
https://hatch.pypa.io/latest/
MIT License
5.66k stars 277 forks source link

Context formatting of project.dependencies is not standard compliant? #1331

Open sbidoul opened 3 months ago

sbidoul commented 3 months ago

It seems that hatchling supports context formatting in project.dependencies or project.optional_dependencies, so resulting metadata in the built artifacts differs from what is specified in pyproject.toml.

However, the pyproject.toml spec says: A build back-end MUST honour statically-specified metadata (which means the metadata did not list the key in dynamic)..

So it seems that hatchling is deviating from the spec in this respect?

This is potentially problematic as some tools may rely on metadata in pyproject.toml being static as an optimization to bypass a potentially costly PEP 517 metadata preparation step. For instance this kind optimization is considered for implementation of pip install --only-deps.

What do you think?

ofek commented 3 months ago

My instinct is to close this because it's up to the user whether or not they require such features and therefore may not be compliant with the spec i.e. users who do not absolutely require this functionality will necessarily not use it. UV just added support for this use case: https://github.com/astral-sh/uv/issues/2475#issuecomment-2002003895

If you want, I could add a warning in the documentation?

sbidoul commented 3 months ago

I personally think deviating from standards is dangerous and has the potential to create a lot of confusion.

uv worked around it in one case, but for instance uv pip compile pyproject.toml still fails in with such a project as it reads project.dependencies without bulding.

So I guess, yes, calling that out the documentation as a warning would be a good step.

ofek commented 3 months ago

Okay, how does this sound: when Hatch introduces workspaces (which would be what users actually want) then I will change the functionality in Hatchling to be such that an option would be required to enable context formatting on static fields. There are other options like that: https://hatch.pypa.io/latest/config/metadata/#allowing-direct-references

ofek commented 3 months ago

Actually wait, that is the current behavior because Hatchling will by default error for direct references such as local paths. So back to the beginning I suppose all I can do is add a warning because this is already doubly opt-in: once for usage and then enabling the option to make usage work.

charliermarsh commented 3 months ago

I suspect uv pip compile pyproject.toml will continue to fail here even once we add support for PEP 517 hooks there, because we'll still want to read metadata directly from the pyproject.toml when we can.

ofek commented 3 months ago

I don't know how to make the situation better than a doubly opt-in option. I could of course remove the feature but then there are users that require it.

hauntsaninja commented 3 months ago

This seems like a pretty straightforward violation of the standards to me. Hatch should have enforced use of dynamic and put the extra information elsewhere, or not used PEP 621. (Or Hatch should swallow the world and provide its own frontend / locking / etc)

Note there is one standards violating thing Hatch could do that would help. Which is, Hatch could still enforce that you have to declare dynamic = ["dependencies"], but not enforce the following part of PEP 621:

A build back-end MUST raise an error if the metadata specifies the name in dynamic.

This means that anything checking dynamic to determine whether it can skip build backend invocation would still work. For instance, https://github.com/jazzband/pip-tools/pull/1964