mhammond / pywin32

Python for Windows (pywin32) Extensions
5.01k stars 792 forks source link

Re-implement `newer` and `newer_group` to avoid deprecated `distutils.util` #2141

Closed Avasam closed 11 months ago

Avasam commented 11 months ago

Work towards #2119

setuptools.util does not currently re-expose methods from distutils.util. And may not want to do so (see discussion in https://github.com/pypa/setuptools/pull/4069 ). This PR explores re-implementing newer and newer_group methods to avoid depending on deprecated distutils.util. There might be a better location to move these methods.

The two usages of newer_group in setup.py also come with a comment suggesting that this method may not be necessary and possibly improved/removed in the future. To me it looks like a build optimization to not regenerate certain files. I would like your thoughts on this @mhammond

mhammond commented 11 months ago

Why the TestLoader as TestLoader?

But in general, I guess this is fine if there's an actual problem, but I'm not sure this is worthwhile until the functions no longer work. What is the actual impact of not taking this at the current time?

Avasam commented 11 months ago

Why the TestLoader as TestLoader?

Explicit re-export since it is used elsewhere and not in this module. Probably better left for a future static checking PR.

What is the actual impact of not taking this at the current time?

Honestly, minor to none in the short-term:

I was looking at what is possible to migrate now to help static-checking efforts and deprecated/obsolete code update. And if you had any more insight or input on why these functions are used and if they're there to stay. But this may be deferred to after bdist_wininst removal (see first point)

Avasam commented 11 months ago

Also wondering if there's a common module I could move these to that can be used both by setup.py and by tests.

mhammond commented 11 months ago

both by setup.py and by tests.

One doesn't exist already - setup.py would need to do sys.path mangling to make that work as it can't assume such a module already was installed (and if it does, it could not assume it's the "correct" version)

Avasam commented 11 months ago

setuptools will re-export distutils.dep_util as setuptools.modified. I'd like to keep this here exploratory PR as-is for posterity. I'll re-open a new one after the next setuptools release.