Fix creation of 32-bit environment with dual-architecture python on macOS#1438
Fix creation of 32-bit environment with dual-architecture python on macOS#1438mayeut wants to merge 1 commit intopypa:masterfrom
Conversation
|
CI tests failing because of #1439 |
|
Rebased on #1439 to get CI passing. |
1954902 to
ac66490
Compare
gaborbernat
left a comment
There was a problem hiding this comment.
Thank you for your contribution, however I feel this PR solves the problem at the wrong place and adds significant overhead, so I'll need to ask you to amend it.
| reason="macOS specific test on python 2.7, 3.5, 3.6 & 3.7", | ||
| ) | ||
| def test_dual_arch_macosx(tmpdir): | ||
| lookup = { |
There was a problem hiding this comment.
this test is very heavyweight, I mean downloading the installer from upstream 🤔
There was a problem hiding this comment.
There's no CI with dual-architecture python install to my knowledge. If you know of one, please do tell.
That means for complete integration tests as the one implemented here, I don't know better.
Maybe lighter tests could be done by mocking things a bit.
Anyway, I think your next comment needs to be addressed first.
| logger.debug("EPD framework detected") | ||
| original_python = os.path.join(prefix, "bin/python") | ||
| shutil.copy(original_python, py_executable) | ||
| original_maxsize = int( |
There was a problem hiding this comment.
this part to feels a very heavyweight; the way things work feels to me that the copy is that original_python is bad during reinvocation at the start; so that should be fixed not this
There was a problem hiding this comment.
This is more of an issue. I understand that the current master branch of virtualenv feels bad at the start. But unfortunately, I don't have time to commit on working on this task which is much more that a small fix and needs understanding why this was done in the first place in order to ensure not breaking anything.
|
I understand your comments but unfortunately, I don't have any time to rework the PR the way you'd like to. I just can't rework the whole macOS framework If master gets reworked the way you'd like, I'd be happy to do some checks regarding this specific issue and open a PR. |
|
Yes 👍let's go with trying to validate if this is still an issue with the new rewrite of virtualenv, to be released sometimes next months |
|
@gaborbernat, |
|
It's not ready for a RC, but hopefully we'll be there in a month 🤞 |
Fixes #1437
When using official
python.orgmacOS 64-bit/32-bit installer, creating a virtual environment from a 32-bit interpreter (pythonX.Y-32 -m virtualenv .venv) results in a 64-bit virtual environment when host supports 64-bit. This PR aims to check wether we're running 32/64 bit interpreter and pull the right interpreter if running 32-bit by thinning the original interpreter fori386.Thanks for contributing a pull request, see checklist all is good!
docs/changelogfolderI think there's no need for a documentation update for this bug fix. News fragment should be enough.