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

Prevent .whl installation (OOTB) on non Win #394

Closed
wants to merge 8 commits into from

Conversation

CristiFati
Copy link
Contributor

@CristiFati CristiFati commented Dec 5, 2022

COM is Win specific and doesn't work on other OSes, therefore it shouldn't be possible to install the .whl on those OSes.
More: if it is installed on such OSes (although [PyPI]: comtypes mentions the Operating System value as Microsoft :: Windows - but that's easy to be ignored), the error is ambiguous as most people are not aware of the CTypes extension module (_ctypes.pyd / _ctypes*.so). Example: [SO]: cannot import name 'COMError' from '_ctypes'.

This PR addresses the above, using Python's packaging mechanism. Drawbacks:

  • The wheel name is long (comtypes-1.1.14-py3.py2-none-win_amd64.win32.win_arm64.win_arm32.win_ia64.whl*), but that shouldn't be a problem when PIP installing most people (again) pay no attention to this detail
  • The platform list must be maintained (since the entries are now whitelisted, when a new one will be available (new architecture, 128bit?) it will have to be added manually)

Tests:

  • Built the .whl (it works on both Win, Nix). Sample command: "e:\Work\Dev\VEnvs\py_pc064_03.09_test0\Scripts\python.exe" setup.py bdist_wheel --python-tag py3.py2
  • Installed it successfully on a combination of Python versions (3.*, 2.7) and architectures (Intel only (pc064, pc032)) on Win (importing the module afterwards, was successful as well)
    • Failed to install on a Linux

Update #0:

Closed this and replaced by #405.

@junkmd
Copy link
Collaborator

junkmd commented Dec 6, 2022

@CristiFati

Thank you for your suggestions.

Aside from code reviews, I am wondering about something.

The community is currently planning to end support for Python 2.7 and support for Python 3.7+ only.

We are also considering preparing a branch for development based on after drop-Python2.7 support.

In this situation, I am wondering if adding version bridges and workarounds that take Python2 into account would not require extra effort to remove them when Python3-only support is starting.

To the other maintainers; what are your thoughts?

@vasily-v-ryabov
@cfarrow
@jaraco

@jaraco
Copy link
Collaborator

jaraco commented Dec 6, 2022

Prioritize support for non-EOL Pythons and then only support older versions where demand exists and resources allow. In my experience on very popular projects, there’s almost zero demand for Python 2.7.

@CristiFati
Copy link
Contributor Author

CristiFati commented Dec 6, 2022

Of course things move forward, and it's not good to be left behind. So if wanting to benefit from newer language features (fstrings, typing, match, ...) support for older versions should be gradually dropped in future versions.

Now, I'm not familiar with the code so I don't know what it takes keeping code Python 3.5 compatible for example.

If I was to make a decision, I too would aim only for currently supported Python versions, but I'd deprecate the older ones first:

  1. Next version should still support everything it supports now, also letting people know (a (big) release note, announcements, other communication channels) that older Pythons will no longer be supported
  2. Next next version should only support newer Pythons
    • I'd let at least a couple of months between #.1. and #.2.
    • From then on, the process can be repeated (more easily)

Regarding specific demands, I don't know how those are handled, but I'd create a branch starting from a (supported) base state and port any required features into that branch then make a release out of it.

Now regarding this PR: since Python 2.7 is working (installing and importing module) before my changes, I didn't want that this PR to be the one that breaks support, that's why I added some (clumsy) Python2 stuff (which I can easily remove if requested).

@kdschlosser
Copy link
Contributor

Here is a time table for what the OEL for the different Python version are and are going to be as well.

https://endoflife.date/python

It would only make sense to support Python 3.8 and above in any changes that are going to be made. I believe 3.8 is when a lot of type hinting support was added as well.

@junkmd
Copy link
Collaborator

junkmd commented Dec 6, 2022

Thank you for responses.

My other concern is the code changed in this PR could be further changed because distutils that are currently used in setup.py are deprecated and will be removed in Python 3.12.

So unless @CristiFati wants to reflect this PR from the next version of comtypes right now, I think it would be spending less man-hours to PR to the branch to add features that are supposed to support only Python3.

And let's discuss in more appropriate issue #392 about which Python 3.x to start supporting in the future.
In relation to this PR, in setup.py, it does not seem necessary to provide workarounds or bridges for each version of Python 3.6 or later (I just did a quick check, so if not, please let me know.).

@junkmd
Copy link
Collaborator

junkmd commented Dec 11, 2022

NOTE: this is related to #229

@junkmd
Copy link
Collaborator

junkmd commented Dec 11, 2022

@CristiFati

Are you interested in modifying this PR for drop_py2 branch, related to #392?

@CristiFati
Copy link
Contributor Author

@junkmd: Sure.

@junkmd
Copy link
Collaborator

junkmd commented Dec 13, 2022

@CristiFati

OK, please change the base of this PR to drop_py2 and rebase the branch as if it was made from drop_py2 as well.
Then modify your code without worrying about Python2.

@CristiFati CristiFati changed the base branch from master to drop_py2 December 13, 2022 12:40
@CristiFati CristiFati closed this Dec 13, 2022
@CristiFati
Copy link
Contributor Author

Closing as branches have been messed up.

@junkmd
Copy link
Collaborator

junkmd commented Dec 13, 2022

Ok, I am waiting for your next PR.

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

5 participants