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

bpo-37481: Deprecate distutils bdist_wininst command #14553

Merged
merged 1 commit into from
Jul 5, 2019
Merged

bpo-37481: Deprecate distutils bdist_wininst command #14553

merged 1 commit into from
Jul 5, 2019

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jul 2, 2019

The distutils bdist_wininst command is now deprecated, use
bdist_wheel (wheel packages) instead.

https://bugs.python.org/issue37481

Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

If we deprecate this here, it should also be deprecated in setuptools (or possibly it should only be deprecated in setuptools - we've tended to leave distutils alone in favor of putting all changes in setuptools).

Also, I'll note that this is actually referenced a number of times in the documentation (though admittedly it is the "legacy" documentation) for distutils: here and more extensively here. If we're actively changing distutils, we may want to update the documentation there.

Finally, if we're deprecating bdist_wininst, should we also be deprecating bdist_msi?

CC @zooba @ncoghlan

@@ -58,6 +60,12 @@ class bdist_wininst(Command):
# bpo-10945: bdist_wininst requires mbcs encoding only available on Windows
_unsupported = (sys.platform != "win32")

def __init__(self, *args, **kw):
super().__init__(*args, **kw)
warnings.warn("bdist_wininst command is deprecated since Python 3.9, "
Copy link
Member

Choose a reason for hiding this comment

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

Should we specify when it will be removed here rather than when it was deprecated?

@vstinner
Copy link
Member Author

vstinner commented Jul 2, 2019

Finally, if we're deprecating bdist_wininst, should we also be deprecating bdist_msi?

Steve Dower recently wrote: "I have no idea. I don't ever use msilib (...)" :-)
https://bugs.python.org/issue37124#msg344472

But if you want to extend https://bugs.python.org/issue37481 to bdist_msi or more, I would prefer to give up and give the task to someone else. I would like to focus on bdist_wininst for reasons explained in https://bugs.python.org/issue37481

Should we specify when it will be removed here rather than when it was deprecated?

"I don't propose to plan bdist_wininst removal yet."
https://bugs.python.org/issue37481#msg347131

If we deprecate this here, it should also be deprecated in setuptools (or possibly it should only be deprecated in setuptools - we've tended to leave distutils alone in favor of putting all changes in setuptools).

setuptools is hosted on a separated GitHub project. Is there just a general remark or a requirement here? I don't plan to touch setuptools.

Also, I'll note that this is actually referenced a number of times in the documentation (though admittedly it is the "legacy" documentation) for distutils: here and more extensively here. If we're actively changing distutils, we may want to update the documentation there.

I updated the doc.

But honestly, it would be better to rewrite this outdated doc, or (better?) remove it.

A doc which starts with "from distutils.core import setup" and then advices to use "python setup.py bdist_wininst" is likely to cause harm than anything else to Python users.

@pganssle
Copy link
Member

pganssle commented Jul 2, 2019

setuptools is hosted on a separated GitHub project. Is there just a general remark or a requirement here? I don't plan to touch setuptools.
...
But honestly, it would be better to rewrite this outdated doc, or (better?) remove it.

A doc which starts with "from distutils.core import setup" and then advices to use "python setup.py bdist_wininst" is likely to cause harm than anything else to Python users.

These comments get to the heart of the problem of modifying distutils, which is that there is a very definite plan for what to do with all of this stuff, but it's a big project, which means it's hard to execute.

The current plan that we mostly agree on is that the public distutils will be removed from the standard library and moved into setuptools, which will become the reference implementation. Everyone already uses setuptools and pip imports setuptools as part of the builds, which triggers setuptools' monkey-patching behavior - meaning that even people who use from distutils import ... are often actually using setuptools, whether they know it or not.

For this reason, @ncoghlan had the same idea as you - remove the distutils docs as harmful - but the problem there is that the setuptools docs assume you have read the distutils docs, since setuptools is intended to be extensions to distutils. Thus the docs were restored as "legacy" docs.

There are two major, related projects that are currently too big for anyone involved to do in a single little weekend project, which are:

  1. Distribute the useful content of the distutils docs into setuptools and packaging.python.org, as appropriate.
  2. Move distutils (along with its test suite) into setuptools, and deprecate or remove the public-facing distutils in the standard library in favor of setuptools.

Until that is done, we have generally left distutils alone except for changes which are needed to build CPython itself (since CPython does not use setuptools) and changes to things where distutils is using some API that has been deprecated or is about to be removed (e.g. keeping the tests working for a given version of Python). All other changes go to setuptools.

The only reason I'm hesitant about saying, "This change should only be in setuptools" is that I suspect that bdist_rpm, bdist_wininst and bdist_msi are all much more likely to be used by people who avoid third party libraries for whatever reason. If we deprecate it only in setuptools, they'll never see the warning, and if it gets removed from setuptools before we manage to migrate distutils, they'll have essentially no warning that their workflow is going to be broken. That said, the overlap between the population of people using Python 3.9 and people using bdist_wininst may also be incredibly small, so maybe the deprecation would never be seen anyway.

@zooba
Copy link
Member

zooba commented Jul 2, 2019

I'm commenting on the bug tracker to keep overall discussion (not specific to the code review) in its proper place. See you there

The distutils bdist_wininst command is now deprecated, use
bdist_wheel (wheel packages) instead.
@vstinner
Copy link
Member Author

vstinner commented Jul 4, 2019

I modified my PR to deprecate the feature in Python 3.8. I'm not sure about scheduling a removal.

@pganssle: Do you require me to rewrite/update the whole distutils documentation to deprecate bdist_wininst command?

I tried to document the deprecation in various places in the doc, and a warning is emitted at runtime.

@pganssle
Copy link
Member

pganssle commented Jul 4, 2019

@vstinner Feel free to do what you feel is best with regards to this. The distutils documentation is horribly out of date already; this makes it worse, but it's already behind a "this is legacy documentation" wall, so it's hardly important to keep it "up to date".

That said, as I may not have made clear in my earlier post - I am not entirely sure this PR is necessary. I don't think it actually helps to ease the maintenance burden of CPython since it doesn't remove any code, and with an open-ended removal period, we'll likely remove all of the user-facing portions of distutils from CPython before we remove this specific command, so it's probably also not shortening the period of time where this command is actively used by end users.

That said, I also don't think it hurts anything - I doubt very many people will even see it.

@vstinner
Copy link
Member Author

vstinner commented Jul 5, 2019

The distutils documentation is horribly out of date already; this makes it worse, but it's already behind a "this is legacy documentation" wall, so it's hardly important to keep it "up to date".

You don't want to remove this doc, so we should at least add deprecation markups when something is deprecated.

Maybe we need to pay someone to work on distutils doc and the project to move it into setuptools.

That said, as I may not have made clear in my earlier post - I am not entirely sure this PR is necessary. I don't think it actually helps to ease the maintenance burden of CPython since it doesn't remove any code, and with an open-ended removal period, we'll likely remove all of the user-facing portions of distutils from CPython before we remove this specific command, so it's probably also not shortening the period of time where this command is actively used by end users.

In Python we never remove anything without any notice: removals must be carefully prepared with at least a mention in the documentation, and maybe also DeprecationWarning emitted at runtime. In some cases, we even start with a PendingDeprecationWarning. Usually, the policy is to require that a feature is deprecated for at least one cycle (ex: Python 3.8) to be removed (ex: Python 3.9).

If you consider that all commands except sdist and bdist_wheel should be removed, I would strongly suggest to first deprecate them.

My long term plan is to remove the feature.

Marking it as deprecated also sends a message to users: we may not fix all bugs anymore. Like "use it at your own risks".


Maybe we should start to mark the whole distutils module as deprecated? Since it seems like more and more people agree that setuptools will be the new standard.

@vstinner vstinner merged commit 1da4462 into python:master Jul 5, 2019
@vstinner vstinner deleted the deprecate_wininst branch July 5, 2019 08:44
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 5, 2019
The distutils bdist_wininst command is now deprecated, use
bdist_wheel (wheel packages) instead.
(cherry picked from commit 1da4462)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
@bedevere-bot
Copy link

GH-14598 is a backport of this pull request to the 3.8 branch.

@vstinner
Copy link
Member Author

vstinner commented Jul 5, 2019

Last time I wanted to touch the distutils module, it was when Tarek Ziade and Eric Araujo were working on distutls2. They denied me to make any change in distutils. I wanted to fix encodings, they were many encoding bugs. After 2 years, their distutils2 project was abandoned. At the end, we lost 2 years and then nobody wanted to touch distutils.

It's great that they are long term plans for setuptools like move distutils into setuptools. But I don't see any concrete plan: there is no schedule, no deprecation period yet, etc.

In the meanwhile, I would prefer to still be able to make small changes to enhance the status quo, rather than waiting for a long plan big plan which may or may not happen ;-)

For example, I don't think that deprecate and/or remove bdist_wininst goes against the plan of moving distutils into setuptools ;-)

miss-islington added a commit that referenced this pull request Jul 5, 2019
The distutils bdist_wininst command is now deprecated, use
bdist_wheel (wheel packages) instead.
(cherry picked from commit 1da4462)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
@vstinner
Copy link
Member Author

vstinner commented Jul 5, 2019

Anyway, let's continue the more general discussion in the Packaging forum https://discuss.python.org/t/deprecate-bdist-wininst/1929 :-)

lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
The distutils bdist_wininst command is now deprecated, use
bdist_wheel (wheel packages) instead.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
The distutils bdist_wininst command is now deprecated, use
bdist_wheel (wheel packages) instead.
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.

6 participants