build: add circular dependency checker for build requirements #593
Conversation
jameshilliard
requested review from
FFY00,
gaborbernat,
layday and
henryiii
as code owners
src/build/__init__.py
Outdated
| return metadata | ||
|
|
||
| # fallback to build_wheel hook | ||
| wheel = self.build('wheel', output_directory) | ||
| match = parse_wheel_filename(os.path.basename(wheel)) | ||
| if not match: | ||
| raise ValueError('Invalid wheel') | ||
| self.project_name = match['distribution'] |
There was a problem hiding this comment.
I'm not very comfortable with mutating the builder in a bunch of different places. If we wanna revalidate the build deps I'd rather we reinitialised the builder by passing the dist name as an explicit parameter.
There was a problem hiding this comment.
I'm not very comfortable with mutating the builder in a bunch of different places.
Yeah, this is def a bit messy, unfortunately we don't have a pep517 hook to get our own project name so I'm attempting to pull the name whenever possible.
If we wanna revalidate the build deps I'd rather we reinitialised the builder by passing the dist name as an explicit parameter.
Well re-validation can often be skipped, if we have a name in out pyproject.yaml project table we can grab it from there to do self cycle checks from the start. It's only when we can't determine our own project name before the build that we need to revalidate it AFAIU.
There was a problem hiding this comment.
If we wanna revalidate the build deps I'd rather we reinitialised the builder by passing the dist name as an explicit parameter.
I'm thinking we should probably be validating the normalized project name in the various areas where it can potentially be retrieved are all the same and throw an exception if any are different. Right now there doesn't seem to be anything ensuring that a project name set via say the pyproject.toml project table matches the wheel, sdist and dist-info project names pulled via the regexes.
There was a problem hiding this comment.
I've implemented project name validation which will throw a ProjectNameValidationError exception if at any point a name update is inconsistent with the previous name update. This should ensure that builder project name state changes are done in a way that effectively locks in the normalized project name once one is available.
jameshilliard
force-pushed
the
circular-dep-detection
branch
5 times, most recently
from
9dc2175
to
7c77bf5
Compare
jameshilliard
force-pushed
the
circular-dep-detection
branch
9 times, most recently
from
3e659e0
to
55e242c
Compare
Implement a basic build requirement cycle detector per PEP-517: - Project build requirements will define a directed graph of requirements (project A needs B to build, B needs C and D, etc.) This graph MUST NOT contain cycles. If (due to lack of co-ordination between projects, for example) a cycle is present, front ends MAY refuse to build the project. - Front ends SHOULD check explicitly for requirement cycles, and terminate the build with an informative message if one is found. See: https://www.python.org/dev/peps/pep-0517/#build-requirements
jameshilliard
force-pushed
the
circular-dep-detection
branch
from
55e242c
to
a66fc65
Compare
|
Seems it stalled, @jameshilliard let me know if you want to pick it up again and we can reopen. |
I think this was ready but just waiting on review, or were there unresolved issues? |
|
@jameshilliard can you rebase it against main; seem has merge conflicts now... |
Sure, I'll try and take a look later today. |
Implement a basic build requirement cycle detector per PEP-517:
Project build requirements will define a directed graph of requirements (project A needs B to build, B needs C and D, etc.) This graph MUST NOT contain cycles. If (due to lack of co-ordination between projects, for example) a cycle is present, front ends MAY refuse to build the project.
Front ends SHOULD check explicitly for requirement cycles, and terminate the build with an informative message if one is found.
See:
https://www.python.org/dev/peps/pep-0517/#build-requirements