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

Resolve build problems in 3.3.1.0 #1026

Merged
merged 4 commits into from Feb 15, 2024

Conversation

cderici
Copy link
Member

@cderici cderici commented Feb 14, 2024

Description

This solves 2 problems with the latest release (also described in this comment):

  1. We depend on the packaging module, but didn't include it in the setup.py (because our testing and dev environments have tox installed and tox satisfies that requirement for itself, so we didn't notice).
  2. We accidentally depend on the static VERSION file where we keep track of the version of pylibjuju.

This change fixes #1025, solving these issues by:

  1. Adding the packaging to the required modules.
  2. Removing the file VERSION, and moving the static version information into the version.py module (just like in Juju) and use that information from there. The places that dynamically use that information (outside of the juju modules) are i) setup.py to get the version info into the build, ii) Makefile to use it in the release target iii) docs/config.py for building the docs. All of these spots are updated to use the info within the version.py.

QA Steps

We'll use the steps described in #1025. After pulling the branch in this PR;

  • Create a new virtual env:
python -m venv venv
  • Activate it:
source venv/bin/activate
  • Build pylibjuju at this spot:
python setup.py sdist
  • The sdist build will create a dist/juju-3.3.1.0.tar.gz, so install that:
pip install dist/juju-3.3.1.0.tar.gz
  • Now try to import a Controller.
python3 -c "from juju.controller import Controller"

Without the (1)st change in this PR, this command would result in:

ModuleNotFoundError: No module named 'packaging'

And without the (2)nd change, this command would error with

FileNotFoundError: [Errno 2] No such file or directory: '......./venv/lib/python3.10/site-packages/VERSION'

But with this change, we shouldn't see either (or any other) error. I.e., the command should succeed with no problems.

Notes & Discussion

Note that this changes the release process. In particular where we need to manually change the version information is moved into the version.py module (the release process document will be updated right before this PR is merged).

Moreover, we need to make a patch release (3.3.1.1) after this for these changes to take effect (i.e. for pylibjuju to be usable in the environments where it's installed as a dependency).

JUJU-5484

file

This fixes juju#1025 by removing the dependency to the external static
VERSION file from within the project, allowing it to work in
environments where pylibjuju is installed as a dependency library.

Note that this changes the release process. In particular where we
need to manually change the version information is moved into
version.py (the release process document will be updated) This also
removes the VERSION file.
@cderici cderici added bug fix A PR that fixes a bug 3.x labels Feb 14, 2024

# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
# documentation root, use os.path.abspath to make it absolute, like shown here.
sys.path.insert(0, os.path.abspath('..'))

from juju.version import CLIENT_VERSION
Copy link
Member

Choose a reason for hiding this comment

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

Imports should be at the top no?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one should stay there because it won't work before the sys.path.insert(0, os.path.abspath('..')) line above


setup(
name='juju',
version=version.read_text().strip(),
version=CLIENT_VERSION.strip(),
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to strip?

Copy link
Member Author

Choose a reason for hiding this comment

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

We currently don't need to, but the line that's coming from will be manually edited by people who're making releases (replaces the file VERSION) in the future, so I thought this would cleanup any silly mistakes like "4.2.0 " (whitespace in the end).

@cderici
Copy link
Member Author

cderici commented Feb 15, 2024

/merge

@jujubot jujubot merged commit 0b210a1 into juju:master Feb 15, 2024
6 of 8 checks passed
@cderici cderici mentioned this pull request Feb 15, 2024
jujubot added a commit that referenced this pull request Feb 15, 2024
#1027

## What's Changed

This is a patch release for fixing some build problems in the 3.3.1.0 release that rendered it unusable in some scenarios.

* Resolve build problems in 3.3.1.0 by @cderici in #1026


#### Notes & Discussion

JUJU-5483
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x bug fix A PR that fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Libjuju 3.3.1.0 is unusable
3 participants