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

BUILD: End bdist_wininst support in setup.py #214

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

wkschwartz
Copy link
Contributor

@wkschwartz wkschwartz commented Jul 2, 2020

Fixes #191
Fixes #196
Fixes #199
Ref indygreg/PyOxidizer#202

I'll start with the post hoc justification for this change: bdist_wininst has been deprecated starting in Python 3.8. See

The real reason for this change is that I can't seem to get around #199 when building a PyOxidizer package using PyOxidizer's dist.pip_install Skylark function. Upgrading Wheel to the latest version solves #199 for me in a regular virtual environment, but requiring the most recent Wheel version in my pyproject.toml's build-system.requires list doesn't solve #199. (Verbose output from Pip confirms the latest Wheel version is indeed installed.)

So removing bdist_wininst support solves my very niche problem, but because bdist_wininst is so rarely used and is deprecated anyway, I figure there's little harm to anyone else in removing it from comtypes.

I'll start with the post hoc justification for this change:
`bdist_wininst` has been deprecated starting in Python 3.8. See
notice:      https://docs.python.org/3/whatsnew/3.8.html#deprecated
discussion:  https://discuss.python.org/t/deprecate-bdist-wininst/1929
bug tracker: https://bugs.python.org/issue37481

The real reason for this change is that I can't seem to get around enthought#199
when building a PyOxidizer package using PyOxidizer's `dist.pip_install`
Skylark function. Upgrading Wheel to the latest version solves enthought#199 for
me in a regular virtual environment, but requiring the most recent Wheel
version in my `pyproject.toml`'s `build-system.requires` list doesn't
solve enthought#199. (Verbose output from Pip confirms the latest Wheel version
is indeed installed.) (Also ref enthought#196 enthought#191 indygreg/PyOxidizer#202.)

So removing `bdist_wininst` support solves my very niche problem, but
because `bdist_wininst` is so rarely used and is deprecated anyway, I
figure there's little harm to anyone else in removing it from comtypes.
wkschwartz added a commit to wkschwartz/pywin32 that referenced this pull request Jul 8, 2020
Ref enthought/comtypes#191
Ref enthought/comtypes#196
Ref enthought/comtypes#199
Ref enthought/comtypes#214 (basically the same as this commit)
Ref indygreg/PyOxidizer#202

I'll start with the post hoc justification for this change:
`bdist_wininst` has been deprecated starting in Python 3.8. See

- notice:      https://docs.python.org/3/whatsnew/3.8.html#deprecated
- discussion:  https://discuss.python.org/t/deprecate-bdist-wininst/1929
- bug tracker: https://bugs.python.org/issue37481

The real reason for this change is that Pip fails to build pywin32 when
run from PyOxidizer's packaging system using PyOxidizer's `dist.pip_install`
Skylark function. Pip issues the following error (after 8 minutes of
compiling...):

> error in setup script: command 'bdist_wininst' has no such option 'install_script'

Upgrading Wheel to the latest version solves this problem for me in a
regular virtual environment, but requiring the most recent Wheel
version in my `pyproject.toml`'s `build-system.requires` list doesn't
solve it when building with PyOxidizer. (Verbose output from Pip confirms
the latest Wheel version is indeed installed.)

So removing `bdist_wininst` support solves my very niche problem, but
because `bdist_wininst` is so rarely used and is deprecated anyway, I
figure there's little harm to anyone else in removing it from pywin32.
@vasily-v-ryabov
Copy link
Collaborator

Hmm... Clearing comtypes cache is important "post-install" step. Can you think about getting it back another way?

@wkschwartz
Copy link
Contributor Author

Looks like clear_comtypes_cache.py already/still runs during installation:

comtypes/setup.py

Lines 112 to 124 in 47517d6

def run(self):
install.run(self)
# Custom script we run at the end of installing
if not self.dry_run and not self.root:
filename = os.path.join(self.install_scripts, "clear_comtypes_cache.py")
if not os.path.isfile(filename):
raise RuntimeError("Can't find '%s'" % (filename,))
print("Executing post install script...")
print('"' + sys.executable + '" "' + filename + '" -y')
try:
subprocess.check_call([sys.executable, filename, '-y'])
except subprocess.CalledProcessError:
print("Failed to run post install script!")

I believe that the code I removed only affects when someone runs python setup.py bdist_wininst, not any other of setup.py's commands.

@vasily-v-ryabov
Copy link
Collaborator

OK, that makes sense. Thanks for the diving into the problem!

@vasily-v-ryabov vasily-v-ryabov merged commit ae2f519 into enthought:master Jul 28, 2020
@wkschwartz wkschwartz deleted the pyox-wininst-err branch July 30, 2020 21:08
@wkschwartz
Copy link
Contributor Author

wkschwartz commented Nov 25, 2020

UPDATE: comtypes 1.1.8 has been released and incorporates this PR.

I need this PR, but since the next release won't come until early 2021 and I'm using Pip's hash-checking mode (which doesn't support installing from Git), I need an archive somewhere accessible. I'm putting it here since its main value is that it contains the patch in this PR. This way if anyone else needs it, they can find it. It's 8d67890 so it also contains #215, #172, and #220.

For the avoidance of doubt: this is not an official release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants