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

Store version number in both setup.py & __init__.py #789

Merged
merged 4 commits into from
Jan 25, 2022
Merged

Conversation

michaeljones
Copy link
Collaborator

@michaeljones michaeljones commented Jan 24, 2022

The PR has changed a bit. Title updated to reflect that but not the text immediately below.

So that we can import it cleanly without importing the whole package and
all of sphinx which some environments don't have for some of the stages
that they run.

This is a naive and uninformed attempt to solve some of our current issues. I'm very out of touch with python packaging and the best way to handle this but for the moment our readthedocs builds are failing (https://readthedocs.org/projects/breathe/builds/15850101/) because they don't have sphinx available when the setup.py code is trying to access breathe.__version__ and it would be good to get them passing again.

Failure stacktrace:

    File "/tmp/pip-build-env-oli4rdc4/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 158, in run_setup
      exec(compile(code, __file__, 'exec'), locals())
    File "setup.py", line 11, in <module>
      from breathe import __version__
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/checkouts/latest/breathe/__init__.py", line 1, in <module>
      from breathe.directives.setup import setup as directive_setup
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/checkouts/latest/breathe/directives/__init__.py", line 1, in <module>
      from breathe.finder.factory import FinderFactory
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/checkouts/latest/breathe/finder/__init__.py", line 1, in <module>
      from breathe.project import ProjectInfo
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/checkouts/latest/breathe/project.py", line 3, in <module>
      from sphinx.application import Sphinx
  ModuleNotFoundError: No module named 'sphinx'

@michaeljones
Copy link
Collaborator Author

I see now that it has been noted that this is likely flawed as Python will import breathe.__init__.py on the way to getting breathe.version and so we have the same problem. I have forgotten that was an aspect of Python importing.

Explained here: #714 (comment)

@michaeljones
Copy link
Collaborator Author

We can reopen #746 or just have two copies of the version number (one in setup.py and one in breathe.__init__.py) and maybe some kind of CI check to make sure they are in sync.

As it is hard to import the version number from within the 'breathe'
package hierarchy without triggering an import of 'sphinx' which isn't
necessarily available at all times that we want a version number.
To make sure that the two version numbers continue to match.
@michaeljones michaeljones changed the title Move version number in version.py sub module Store version number in both setup.py & __init__.py Jan 24, 2022
@michaeljones
Copy link
Collaborator Author

I've updated the PR to just have the version number twice and have a CI script to check that they stay in sync. I'm sufficiently out of the loop that there may be plenty of reasons why this is not a good idea but for the moment it seems vaguely feasible and stops us having to worry about the annoyances of the Python import system.

@michaeljones michaeljones merged commit 6aede12 into master Jan 25, 2022
@michaeljones
Copy link
Collaborator Author

Merging as it is a small change and hopefully gets us out of the current jam. We can reconsider in the future if desirable.

@michaeljones michaeljones deleted the version branch January 25, 2022 21:06
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.

1 participant