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

latest setuptool_scm version is not compatible with python 3.7 #379

Closed
wants to merge 1 commit into from
Closed

latest setuptool_scm version is not compatible with python 3.7 #379

wants to merge 1 commit into from

Conversation

JohnnyVM
Copy link

@JohnnyVM JohnnyVM commented Aug 22, 2024

Version 8.0.0 of setuptools_scm onwards imports Protocol from typing but Protocol is not available for versions of python3.7, which is the one used by the odoo:13.0 and odoo:14.0 docker

https://github.com/pypa/setuptools_scm/blob/b5dbba7a154b90f401ac1b9ca37dc920b5e82c33/src/setuptools_scm/_config.py#L11

@JohnnyVM JohnnyVM marked this pull request as ready for review August 22, 2024 05:04
@JohnnyVM
Copy link
Author

the documentation action is failing becouse:
WARNING: Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate (_ssl.c:1007)'))': /oca-simple-and-pypi/setuptools-scm/

I think is not related to my PR, can someone fix it in master? after that i will rebase the PR

@JohnnyVM JohnnyVM marked this pull request as draft August 22, 2024 05:08
@sbidoul
Copy link
Member

sbidoul commented Aug 22, 2024

The certificate issue was due to a DNS error, should be resolved in 5 minutes.

setup.py Show resolved Hide resolved
@JohnnyVM JohnnyVM marked this pull request as ready for review August 22, 2024 05:55
@JohnnyVM JohnnyVM requested a review from sbidoul August 22, 2024 06:03
@JohnnyVM
Copy link
Author

The certificate issue was due to a DNS error, should be resolved in 5 minutes.

i cannot rerun the action, I don't have the necessary permissions, can someone re-launch it for me?

setup.py Outdated Show resolved Hide resolved
@JohnnyVM JohnnyVM requested a review from sbidoul August 26, 2024 04:20
@JohnnyVM
Copy link
Author

JohnnyVM commented Aug 30, 2024

if all is okay can the PR be merged? or i am missing something?

@StefanRijnhart
Copy link
Member

One step towards that would be to squash commits and align the commit message with the guidelines: https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#commit-message

@JohnnyVM
Copy link
Author

Hi, i had time to back
Message fixed :)
Am i missing something more?
Kind regards

Modify setup.py adding because version 8.0.0 of setuptools_scm onwards imports
Protocol from typing but Protocol is not available for versions of python3.7
which is the one used by the odoo:13.0 and odoo:14.0 docker
@pedrobaeza pedrobaeza requested review from sbidoul and removed request for sbidoul September 20, 2024 06:09
@StefanRijnhart
Copy link
Member

@sbidoul Can you update your review?

Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

That said, I still don't understand why this PR is necessary, as I tried installing the master branch in a python 3.7 venv and it works fine:

git clone https://github.com/oca/openupgradelib
python3.7 -m venv venv
venv/bin/pip install -U pip wheel setuptools
venv/bin/pip install ./openupgradelib
venv/bin/pip list

Comment on lines 35 to +36
"setuptools_scm<6.0.0; python_version <= '3.6'",
"setuptools_scm; python_version > '3.6'",
"setuptools_scm<8; python_version < '3.8'",
Copy link
Member

Choose a reason for hiding this comment

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

When using python 3.6, both these lines match. I suppose that works but may not be what is intended.

@JohnnyVM
Copy link
Author

JohnnyVM commented Sep 23, 2024

How to reproduce:

FROM odoo:14.0

USER root

RUN apt update --fix-missing && apt update && apt install -y git
RUN python3  -m pip install wheel setuptools git+https://github.com/OCA/openupgradelib.git@master#egg=openupgradelib

USER odoo

ENTRYPOINT ["/entrypoint.sh"]
CMD ["odoo"]

Error:

WARN[0005] SHELL is not supported for OCI image format, [/bin/bash -xo pipefail -c] will be ignored. Must use `docker` format
--> a8586f9af0a2
STEP 4/7: RUN python3  -m pip install wheel setuptools git+https://github.com/OCA/openupgradelib.git@master#egg=openupgradelib
Collecting wheel
  Downloading https://files.pythonhosted.org/packages/c7/c3/55076fc728723ef927521abaa1955213d094933dc36d4a2008d5101e1af5/wheel-0.42.0-py3-none-any.whl (65kB)
Requirement already satisfied: setuptools in /usr/lib/python3/dist-packages (40.8.0)
Collecting openupgradelib from git+https://github.com/OCA/openupgradelib.git@master#egg=openupgradelib
  Cloning https://github.com/OCA/openupgradelib.git (to revision master) to /tmp/pip-install-we0hvqrr/openupgradelib
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-we0hvqrr/openupgradelib/setup.py", line 62, in <module>
        tests_require=test_requirements,
      File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 145, in setup
        return distutils.core.setup(**attrs)
      File "/usr/lib/python3.7/distutils/core.py", line 108, in setup
        _setup_distribution = dist = klass(attrs)
      File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 447, in __init__
        k: v for k, v in attrs.items()
      File "/usr/lib/python3.7/distutils/dist.py", line 292, in __init__
        self.finalize_options()
      File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 739, in finalize_options
        ep.load()(self, ep.name, value)
      File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 2411, in load
        return self.resolve()
      File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 2417, in resolve
        module = __import__(self.module_name, fromlist=['__name__'], level=0)
      File "/tmp/pip-install-we0hvqrr/openupgradelib/.eggs/setuptools_scm-8.1.0-py3.7.egg/setuptools_scm/__init__.py", line 8, in <module>
        from ._config import DEFAULT_LOCAL_SCHEME
      File "/tmp/pip-install-we0hvqrr/openupgradelib/.eggs/setuptools_scm-8.1.0-py3.7.egg/setuptools_scm/_config.py", line 13, in <module>
        from typing import Protocol
    ImportError: cannot import name 'Protocol' from 'typing' (/usr/lib/python3.7/typing.py)

    ----------------------------------------

current work around? update setupt tools previously to install openupgradelib:

FROM odoo:14.0

USER root

RUN apt update --fix-missing && apt update && apt install -y git
RUN python3  -m pip install -U wheel setuptools
RUN python3  -m pip install git+https://github.com/OCA/openupgradelib.git@master#egg=openupgradelib

USER odoo

ENTRYPOINT ["/entrypoint.sh"]
CMD ["odoo"]

It seem like update setuptools fix the issue because newest versions do some type of fix for the import, but default docker setuptools fails.
I think is more elegant? better? fix the setup.py but give that exist a work around feel free of close the PR, if not i am going to solve the @sbidoul comment and re update in a couple of hours

@sbidoul
Copy link
Member

sbidoul commented Sep 23, 2024

Since the issue seems to be in setuptools and there is a workaround with a simple upgrade I personally find it preferable to recommend that instead of adding complexity in this project.

@StefanRijnhart
Copy link
Member

@sbidoul It is undeniably the case that setuptool_scm 8.0.0 dropped support for python 3.7, introducing the offending import of typing.Protocol that is causing OP's issue: https://github.com/pytest-dev/pytest/pull/11152/files#diff-af0c5977e27eeffde9235704eaccb3f3351575f23ea385f6b1e8f48a87249036
As such I don't think it is unreasonable to distinguish between versions here.

@sbidoul
Copy link
Member

sbidoul commented Sep 23, 2024

@StefanRijnhart Sure but that setuptools_scm commit you mention also adds python_requires = >=3.8 so a sufficiently recent and correct version of setuptools will not select setuptools_scm >= 8 on python 3.7. In that sense, this PR works around a bug in a very old version of setuptools. Or do I misunderstand something?

So the question is do we want to drag along that work around along in openupgradelib, or simply recommand users to upgrade setuptools.

@StefanRijnhart
Copy link
Member

@sbidoul Thanks for the clarification. After having worked for many years without this basic dependency management feature, I wasn't really aware that setuptools caught up at some point 😁

@JohnnyVM do you have a compelling reason not to update setuptools beyond the version that your Python 3.7 distribution came with?

@JohnnyVM
Copy link
Author

No, the work around work for me.
I am going to close the PR then, anyway it was interesting :)
Kind regards

@JohnnyVM JohnnyVM closed this Sep 23, 2024
@StefanRijnhart
Copy link
Member

@JohnnyVM Thanks for confirming!

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.

3 participants