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

Remove monkeypatching of _msvccompiler. #4600

Merged
merged 5 commits into from
Aug 27, 2024
Merged

Remove monkeypatching of _msvccompiler. #4600

merged 5 commits into from
Aug 27, 2024

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Aug 27, 2024

The monkeypatch indicates that the functionality was ported from Python 3.8, so the version in distutils should be adequate now.

I note that there have been some changes to this module since it was ported from Python 3.8, including:

We'll want to have those issues supported by pypa/distutils if not fully dropping support for stdlib distutils before enacting this change.

Merge after #4606.

Summary of changes

Closes

Pull Request Checklist

jaraco added 2 commits August 27, 2024 03:23
The monkeypatch indicates that the functionality was ported from Python 3.8, so the version in distutils should be adequate now.
@abravalheri
Copy link
Contributor

abravalheri commented Aug 27, 2024

There are probably some changes in setuptools/msvc.py that need to be ported to distutils (at least _msvc14_find_vc2017)

@jaraco
Copy link
Member Author

jaraco commented Aug 27, 2024

I updated the description to highlight the relevant changes.

We'll want to have those issues supported by pypa/distutils if not fully dropping support for stdlib distutils before enacting this change.

On further consideration, I think it's safe to force Windows users who need improved VC/VS support not to use distutils from the stdlib, so support in pypa/distutils should be adequate.

jaraco added a commit to pypa/distutils that referenced this pull request Aug 27, 2024
jaraco added a commit to pypa/distutils that referenced this pull request Aug 27, 2024
jaraco added a commit to pypa/distutils that referenced this pull request Aug 27, 2024
@jaraco
Copy link
Member Author

jaraco commented Aug 27, 2024

I've added #1904 to the list of changes to review. There are a lot of changes in there that don't match with what's in distutils, presumably because distutils has evolved since Python 3.8. I'm hoping it's not the case that these additional behaviors (like those to find_vcvarsall) weren't subtly added in #1904.

@jaraco
Copy link
Member Author

jaraco commented Aug 27, 2024

Oh, interesting. So it looks like the contents in msvc were indeed drawn from 3.8.0 but that implementation changed substantially throughout the 3.8 series (especially in python/cpython@0d20364). This means that although bugs were fixed in stdlib distutils, those fixes have been largely unavailable to Setuptools' users.

We do want to adopt the fixes that are applied in distutils (and not go backward on those changes).

That means that there's liable to be more churn when adopting this change than I'd like, as distutils can't be made a drop-in replacement for what setuptools users expect. Depending on the amount of disruption, we may need to yank releases or back out changes until the differences can be assessed.

@jaraco jaraco marked this pull request as ready for review August 27, 2024 18:27
@jaraco jaraco merged commit b4fb917 into main Aug 27, 2024
33 checks passed
@jaraco jaraco deleted the debt/msvc-monkey branch August 27, 2024 19:01
@saschanaz
Copy link
Contributor

saschanaz commented Aug 27, 2024

Arm support #4555 (addressed by pypa/distutils#285)

But those are independent, no? The former is to find vcvarsall and the latter is to get the right tool from that vcvarsall.

@saschanaz
Copy link
Contributor

Oh, folded into 196d44b. Never mind then.

@jaraco
Copy link
Member Author

jaraco commented Aug 27, 2024

Thanks for reviewing. Please let me know how Setuptools 74 works for you. I'm happy to address any unexpected failures promptly.

@saschanaz
Copy link
Contributor

The linked issues are unhappy with setuptools.msvc.EnvironmentInfo being removed, is there an equivalent now?

@jaraco
Copy link
Member Author

jaraco commented Sep 2, 2024

There isn't. I did not realize they were in use. I was assuming and did not verify that those classes were only previously used for internal implementation details for functionality that had been removed.

Since users are relying on it, we should bring them back, and add some tests.

hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 2, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.3.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.3.0-1.spec
    cp ../python-setuptools*75.3.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.3.0/
    rm -rf ../python*-setuptools*75.3.0*.* ../python*-pkg-resources_75.3.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 2, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_73.0.1.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_73.0.1-1.spec
    cp ../python-setuptools*73.0.1*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-73.0.1/
    rm -rf ../python*-setuptools*73.0.1*.* ../python*-pkg-resources_73.0.1*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 2, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_73.0.1.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_73.0.1-1.spec
    cp ../python-setuptools*73.0.1*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-73.0.1/
    rm -rf ../python*-setuptools*73.0.1*.* ../python*-pkg-resources_73.0.1*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
ovesh added a commit to deepgenomics/GenomeKit that referenced this pull request Nov 5, 2024
setuptools broken compat, see pypa/setuptools#4600
ovesh added a commit to deepgenomics/GenomeKit that referenced this pull request Nov 5, 2024
setuptools broken compat, see pypa/setuptools#4600

Release-As: 6.0.2
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 11, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.4.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.4.0-1.spec
    cp ../python-setuptools*75.4.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.4.0/
    rm -rf ../python*-setuptools*75.4.0*.* ../python*-pkg-resources_75.4.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 11, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.4.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.4.0-1.spec
    cp ../python-setuptools*75.4.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.4.0/
    rm -rf ../python*-setuptools*75.4.0*.* ../python*-pkg-resources_75.4.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 11, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.4.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.4.0-1.spec
    cp ../python-setuptools*75.4.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.4.0/
    rm -rf ../python*-setuptools*75.4.0*.* ../python*-pkg-resources_75.4.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 11, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.4.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.4.0-1.spec
    cp ../python-setuptools*75.4.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.4.0/
    rm -rf ../python*-setuptools*75.4.0*.* ../python*-pkg-resources_75.4.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 11, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.4.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.4.0-1.spec
    cp ../python-setuptools*75.4.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.4.0/
    rm -rf ../python*-setuptools*75.4.0*.* ../python*-pkg-resources_75.4.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 11, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.4.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.4.0-1.spec
    cp ../python-setuptools*75.4.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.4.0/
    rm -rf ../python*-setuptools*75.4.0*.* ../python*-pkg-resources_75.4.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 13, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.5.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.5.0-1.spec
    cp ../python-setuptools*75.5.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.5.0/
    rm -rf ../python*-setuptools*75.5.0*.* ../python*-pkg-resources_75.5.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 13, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.5.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.5.0-1.spec
    cp ../python-setuptools*75.5.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.5.0/
    rm -rf ../python*-setuptools*75.5.0*.* ../python*-pkg-resources_75.5.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 21, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.6.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.6.0-1.spec
    cp ../python-setuptools*75.6.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.6.0/
    rm -rf ../python*-setuptools*75.6.0*.* ../python*-pkg-resources_75.6.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 21, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.6.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.6.0-1.spec
    cp ../python-setuptools*75.6.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.6.0/
    rm -rf ../python*-setuptools*75.6.0*.* ../python*-pkg-resources_75.6.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
hswong3i added a commit to alvistack/pypa-setuptools that referenced this pull request Nov 21, 2024
    git clean -xdf
    tar zcvf ../python-setuptools_75.6.0.orig.tar.gz --exclude=.git .
    debuild -uc -us
    cp python-setuptools.spec ../python-setuptools_75.6.0-1.spec
    cp ../python-setuptools*75.6.0*.{gz,xz,spec,dsc} /osc/home\:alvistack/pypa-setuptools-75.6.0/
    rm -rf ../python*-setuptools*75.6.0*.* ../python*-pkg-resources_75.6.0*.*

See pypa#4600

Signed-off-by: Wong Hoi Sing Edison <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants