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

Opt to check build dependencies #11117

Merged
merged 1 commit into from May 14, 2022
Merged

Opt to check build dependencies #11117

merged 1 commit into from May 14, 2022

Conversation

q0w
Copy link
Contributor

@q0w q0w commented May 12, 2022

Closes #11116

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with one minor user-facing behaviour change suggestion.

This also needs a bugfix news entry along the lines of Change the build environment depedency checking to be opt-in. and a test that conflicting dependencies don't warn when the flag is not passed.

src/pip/_internal/cli/cmdoptions.py Outdated Show resolved Hide resolved
@uranusjr
Copy link
Member

Does/should this also control environment validation when build isolation is enabled? If not, it should be described in the help string.

@q0w
Copy link
Contributor Author

q0w commented May 13, 2022

What do you mean by "environment validation"?

@uranusjr
Copy link
Member

Validate the requirements are installed correctly in the isolated environment. In other words, since we are making this build dependency check optional without build isolation, should we do the same when there is an isolated build environment?

It makes sense to me to make that check optional as well, because since the isolated build environment was populated by pip in the first place, the requirement should theoratically be valid (at least if we use the modern resolver), and any errors should have been raised when the build requirements are being installed anyway.

@pradyunsg
Copy link
Member

pradyunsg commented May 14, 2022

Well, we can add it in, in a follow up. I think it's useful, but I'd like to stop the "immediate bleeding" AKA you can't work on a project that depends on numpy until this is changed. :)

@pradyunsg pradyunsg merged commit 30af807 into pypa:main May 14, 2022
@rgommers
Copy link

I'd like to stop the "immediate bleeding" AKA you can't work on a project that depends on numpy until this is changed. :)

Thank you, much appreciated!

@jakirkham
Copy link
Contributor

Agreed thank you all! 🙏

@jakirkham
Copy link
Contributor

Would it be possible to get this in a release as well?

@q0w
Copy link
Contributor Author

q0w commented May 15, 2022

@uranusjr but if not raising conflicting/missing reqs errors for isolated env, then it would be raised later. Should it be tested like this or what behavior is expected?

def test_check_build_deps(
    script: PipTestEnvironment, tmpdir: Path, data: TestData
) -> None:
    project_dir = make_project(
        tmpdir, requires=["test_backend", "simplewheel==1.0"], backend="test_backend"
    )
    project_dir.joinpath("backend_reqs.txt").write_text("simplewheel==2.0")
    result = script.pip(
        "install",
        "--no-index",
        "-f",
        data.backends,
        "-f",
        data.packages,
        project_dir,
        expect_error=True,
    )
    assert result.returncode != 0

@neutrinoceros
Copy link

neutrinoceros commented May 18, 2022

@pradyunsg @q0w thank you for working this out. Just to clarify, do you intend to cut a new release soon ? This can easily be mitigated downstream but I’m hesitant to introduce temporary workarounds that may become useless a couple days later.

@pradyunsg
Copy link
Member

Yup, I plan on cutting a release in the next few days.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2022
@q0w q0w deleted the opt-check branch June 23, 2022 19:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New --no-build-isolation check in Pip 22.1 broke oldest-supported-numpy
6 participants