Cleanup and abstract setup.py command construction in req_install.py

[gvalkov]

While working on #2537 I noticed that there was some code duplication in req_install.py. I hope this change makes things a little clearer.

[xavfernandez]

Looks similar to #2579 :)

[gvalkov]

We're both aiming at the same thing - I think I'm being slightly more minimal in my approach. @msabramo, what do you think? Are you looking into rebasing #2579?

This change was part of PR #790 (link to code) but was dropped in #2537 (i.e. the re-implementation) to limit the scope of the change. That was ~2 years ago :)

[pfmoore]

You've restructured the code here, and introduced a number of local variables, which will be visible within the exec. I can't imagine how those extra variables could alter the behaviour of the setup.py invocation, but it is a change in behaviour. I'm inclined to say to leave the injection code as it was - the readability benefit seems minimal.

[gvalkov]

You're right. I didn't think about all the wild cases in which a user's setup.py might break. I could mangle the names, but that would hardly be an improvement over the current:

import setuptools, tokenize;__file__=%r; exec(compile(getattr(tokenize, 'open', open)(__file__).read().replace('\\r\\n', '\\n'), __file__, 'exec'))

I'll revert to that. Thanks.

[msabramo]

I rebased #2579.

[xavfernandez]

Related to 57db684 which was merged

[Ivoz]

So this no longer applies / could be closed, right?

[xavfernandez]

Well, I thought there might be more to it than the setuptools shim but I'm not sure it is worth the rebase hassle, so indeed, closing this :)

[Ivoz]

Thanks for the good thought anyway @gvalkov , your refactoring is there in spirit.

[gvalkov]

Hi @Ivoz. No problem at all - I'm happy to see pip keep getting better and better!