Fallback to pep517 if setup.py is present and setuptools cannot be imported

[hrnciar]

This is WIP PR without any documentation or tests. I'd like to first know your opinion whether the idea is acceptable. Thank you.

[pfmoore]

I'm inclined to agree with the suggestion @uranusjr made on Discourse - if there's no setup.py and setuptools cannot be imported, the only possible thing that will work is to use PEP 517, so why not do that? Which means this PR can simply be "use PEP 517 if setuptools cannot be imported".

[hroncok]

I'd argue that when the user explicitly used --no-pep517 and setuptools is not installed, that deserves an error.

[hroncok]

That combined basically means something like:

    ... elif not has_setuptools:
        if use_pep517 is not None and not use_pep517:
            raise InstallationError(
                "Disabling PEP 517 processing is invalid: "
                "setuptools not installed"
            )
        use_pep517 = True

[uranusjr]

The “can’t do --no-use-pep517 because setuptools is not found” error can even be raised as early as command argument parsing, instead of half-way into package installation.

[hroncok]

Note that the setuptools check can be technically made much sooner in this function, but this way, it is avoided when not necessary. Not sure which approach is better.

[pradyunsg]

FWIW, I was hoping to go down a different avenue here: dc217bb

That presents a clearer error message instead of opting these projects into PEP 517.

[hroncok]

At least to me, opting to PEP 517 when setuptools is not installed sounds like a more user-friendly thing to do than erroring out (and gradually downloading a lesser and lesser version of the package only to error again and again).

[pradyunsg]

Yea... #10655 is annoying.

It seems to me that there's reasonable agreement that we should stop backtracking on build failures, which combined with better presentation of build failures (I'll file this as a PR once #10703 merges) could be a more "complete" solution here; rather than needing to change this one special case?

[hroncok]

Even without backtracking, I still think that using PEP 517 when setuptools is not present is the nicer thing to do, but I won't fight you on that. A better error report and no backtracking makes this good enough for me.

[pradyunsg]

Thinking about this a bit more, I think we should do all three things: this PR, #10722 and #10723. They're all better behaviours, and I think it's totally reasonable for us to do all of them.

[hrnciar]

I was trying to solve the CI issues, but I ran into some test failures. I don't think they are caused by this PR, because I am getting the same failures when I run the tests on the main branch locally. If that's the case, I think PR is ready for review.

[uranusjr]

Those are caused by #10742, don’t mind them.

[hroncok]

Is this ready to be merged, or do we need to change something?

[hroncok]

What si the next step here?

[uranusjr]

Anyone else wants to take a look? If not I’m inclined to include this in the 22.1 release.

[pfmoore]

One minor point, but otherwise LGTM

[hroncok]

I would still prefer this not be wrapped.

So, use a long line or split the string literal?

[pfmoore]

use a long line or split the string literal?

Ultimately whatever stops black whining about the layout, I don't really care 🙂 If I were writing this without regard for black, I'd use a long line.

[uranusjr]

I added a commit to split the literal.

[sbidoul]

Ok with the concept and implementation LGTM. Does our test suite lend itself to adding a test case for this ?

[uranusjr]

This needs a rebase to fix CI.

[pradyunsg]

Thanks @hrnciar! Hopefully me short-circuiting the CI because of my lack of patience won't bite me (eg: because the linters were updated). :)