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

Re-export distutils.dep_util through setuptools.modified #4069

Merged
merged 9 commits into from Nov 19, 2023

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Oct 1, 2023

Summary of changes

Re-export distutils.dep_util through setuptools.modified

I'm looking into progressively migrating distutil usage in pywin32 (mhammond/pywin32#2119), where distutils.dep_util.newer and distutils.dep_util.newer_group are used.

Migration guides (https://setuptools.pypa.io/en/latest/deprecated/distutils-legacy.html and https://peps.python.org/pep-0632/#migration-advice) don't mention distutils.dep_util.

setuptools.dep_util already exists. It seems sensible to re-export the rest of distutils.dep_util as part of setuptool's API.
Edit: see conversations below, re-exporting thought setuptools.modified.

Pull Request Checklist

setuptools/dep_util.py Outdated Show resolved Hide resolved
@jaraco
Copy link
Member

jaraco commented Oct 27, 2023

Before we naively expose this behavior, let's ask ourselves what these functions are used for (in pywin32) and determine where is the best place to host them. Setuptools is aiming to be primarily a build backend and not have dependencies on the library (except in setup.py). I see that pywin32 uses two functions, one in setup.py and the other in a test. Moreover, the one use in setup.py has a comment "unclear why we need to check this".

I'd like to first determine what newer_group does, if it's needed to build pywin32, and if so, what purpose it serves. I'd still be tempted to provide it outside setuptools, or limit calls that are exposed here.

@Avasam
Copy link
Contributor Author

Avasam commented Oct 27, 2023

let's ask ourselves what these functions are used for (in pywin32) and determine where is the best place to host them. Setuptools is aiming to be primarily a build backend and not have dependencies on the library (except in setup.py).

Sounds sensible to me. I'll do some investigation on pywin32's side and see how much value these utility methods bring it (or dont).

@Avasam
Copy link
Contributor Author

Avasam commented Nov 3, 2023

In pywin32 it's used to check if a file should be rebuilt/re-generated. All 3 instances of newer and newer_group seem to simply be a build optimisation.
The one newer in tests could be moved to test utils.
newer_group in setup.py I might have to re-implement to avoid depending on distutils directly.
Exploratory PR here: mhammond/pywin32#2141

[...] determine where is the best place to host them. [...] I'd still be tempted to provide it outside setuptools

I think I get what you're going for here. Either these 4 file-edit-time comparison methods could be moved to their own library, or included as part of an existing file-manipulation one. Where it would belong better than in setuptools. I think it does make sense, I'm not sure where it would belong best though and where to start requesting.

@jaraco
Copy link
Member

jaraco commented Nov 5, 2023

Thanks for the exploration. I do think we want to expose these functions from setuptools.

I did some work in distutils to modernize and clean up the implementations. That should make implementing newer_pairwise_groups much easier.

Before we expose these new public names, I do think we should seriously consider re-naming this module. dep_util doesn't convey very well the purpose. In fact "util" in any module name is usually superfluous. And "dep" doesn't tell me much. I'm thinking "fresh" or "freshness" or "modified". Pick your favorite and let's expose these functions there (under setuptools.fresh (or similar).

I can deal with the migration to the new name.

@Avasam
Copy link
Contributor Author

Avasam commented Nov 5, 2023

Thank you for the consideration

Before we expose these new public names, I do think we should seriously consider re-naming this module. dep_util doesn't convey very well the purpose. In fact "util" in any module name is usually superfluous. And "dep" doesn't tell me much.

I completely agree.

I'm thinking "fresh" or "freshness" or "modified". Pick your favorite and let's expose these functions there (under setuptools.fresh (or similar).

I don't have a strong opinion on this.

Some thoughts: Functions in the module currently only compare modification date, and the name "modified" is quite telling of what the module does, but maybe that's to close to implementation?
"fresh/ness" sounds off but maybe that's just me.
Something like "file_compare" may allow more types on comparisons in the future, but maybe that's too vague and not wanted (like creation date, file size).
setuptools.freshness.newer_group
setuptools.modified.newer_group
setuptools.file_compare.newer_group

I can deal with the migration to the new name.

Thanks! Feel free to close this PR as superseded by the new one.

@jaraco jaraco self-assigned this Nov 8, 2023
Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After merging #4103, this will be ready to go.

@jaraco jaraco changed the base branch from main to feature/distutils-sync November 8, 2023 14:24
@Avasam Avasam changed the title Re-export distutils.dep_util through setuptools.dep_util Re-export distutils.dep_util through setuptools.modified Nov 8, 2023
@jaraco jaraco deleted the branch pypa:main November 18, 2023 19:48
@jaraco jaraco closed this Nov 18, 2023
@jaraco
Copy link
Member

jaraco commented Nov 18, 2023

Well that's weird. It used to be the case that if I pointed a PR to target a branch and then merged that branch into main, Github would repoint the original PR to main, but in this case, it closed this PR and I'm unable to reopen (the re-open is greyed out).

@jaraco jaraco reopened this Nov 18, 2023
@jaraco
Copy link
Member

jaraco commented Nov 18, 2023

Aha! By re-pushing the feature/distutils-sync branch, I was able to re-open this PR.

@jaraco jaraco changed the base branch from feature/distutils-sync to main November 18, 2023 20:04
@jaraco jaraco merged commit a2d4a4e into pypa:main Nov 19, 2023
20 checks passed
@Avasam Avasam deleted the distutils.dep_util branch November 19, 2023 18:02
petscbot pushed a commit to petsc/petsc that referenced this pull request Dec 19, 2023
Use functions from setuptools.modified instead.
!!

        ********************************************************************************
        Please use `setuptools.modified` instead of `setuptools.dep_util`.

        By 2024-May-21, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.

        See pypa/setuptools#4069 for details.
        ********************************************************************************

!!
joseeroman pushed a commit to slepc/slepc that referenced this pull request Dec 19, 2023
Use functions from setuptools.modified instead.
!!

        ********************************************************************************
        Please use `setuptools.modified` instead of `setuptools.dep_util`.

        By 2024-May-21, you need to update your project and remove deprecated calls
        or your builds will no longer be supported.

        See pypa/setuptools#4069 for details.
        ********************************************************************************

!!
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.

None yet

2 participants