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.

Sign up for GitHub

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

Jump to bottom

--force-reinstall old conda to ensure it's working before we try to install conda packages #920

Merged
merged 5 commits into from
Jun 9, 2023
Merged

--force-reinstall old conda to ensure it's working before we try to install conda packages #920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f0179a
bump minimum conda/mamba versions
minrk Jun 9, 2023
4cc55df
force upgrade of conda itself
minrk Jun 9, 2023
29b354b
use force-reinstall
minrk Jun 9, 2023
ee23e04
reinstall conda/mamba in a separate, unconditional step
minrk Jun 9, 2023
39b1e1a
preserve version pin when reinstalling conda
minrk Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 10 additions & 5 deletions tljh/conda.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def install_miniconda(installer_path, prefix):
fix_permissions(prefix)


def ensure_conda_packages(prefix, packages):
def ensure_conda_packages(prefix, packages, force_reinstall=False):
"""
Ensure packages (from conda-forge) are installed in the conda prefix.

Expand All @@ -110,13 +110,18 @@ def ensure_conda_packages(prefix, packages):
# fallback on conda if mamba is not present (e.g. for mamba to install itself)
conda_executable = os.path.join(prefix, "bin", "conda")

cmd = [conda_executable, "install", "--yes"]

if force_reinstall:
# use force-reinstall, e.g. for conda/mamba to ensure everything is okay
# avoids problems with RemoveError upgrading conda from old versions
cmd += ["--force-reinstall"]

abspath = os.path.abspath(prefix)

utils.run_subprocess(
[
conda_executable,
"install",
"-y",
cmd
+ [
"-c",
"conda-forge", # Make customizable if we ever need to
"--prefix",
Expand Down
51 changes: 36 additions & 15 deletions tljh/installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,21 +242,42 @@ def ensure_user_environment(user_requirements_txt_file):
)
to_upgrade.append(pkg)

cf_pkgs_to_upgrade = list(set(to_upgrade) & {"conda", "mamba"})
if cf_pkgs_to_upgrade:
conda.ensure_conda_packages(
USER_ENV_PREFIX,
# we _could_ explicitly pin Python here,
# but conda already does this by default
cf_pkgs_to_upgrade,
)
pypi_pkgs_to_upgrade = list(set(to_upgrade) & {"pip"})
if pypi_pkgs_to_upgrade:
conda.ensure_pip_packages(
USER_ENV_PREFIX,
pypi_pkgs_to_upgrade,
upgrade=True,
)
# force reinstall conda/mamba to ensure a basically consistent env
# avoids issues with RemoveError: 'requests' is a dependency of conda
# only do this for 'old' conda versions known to have a problem
# we don't know how old, but we know 4.10 is affected and 23.1 is not
if not is_fresh_install and V(package_versions.get("conda", "0")) < V("23.1"):
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really like what I ended up doing here, but I bracketed the force reinstall on a version older than what's going to be in 1.0, with the hope that we actually remove this step when we drop support for upgrading from earlier than 1.0.

I think you need pretty old conda to get here, but that's what we have in 0.2. It really is a special-case to handle:

  • quite old conda
  • inconsistent environment in conda's own dependencies (caused by our own pip install --upgrade commands)

but this is what you'll get if you start with tljh 0.2 and haven't upgraded conda, so we should handle it. I think it is appropriate to treat it specially, as if users have broken their own envs such that conda itself doesn't work, I think it's appropriate for us to not handle that and require users to fix their conda (or delete their user env to start fresh) before upgrading. But since we are creating a broken environment, we need to handle it, at least for the very specific case of upgrading from a previous tljh known to have this issue.

# force-reinstall doesn't upgrade packages
# it reinstalls them in-place
# only reinstall packages already present
to_reinstall = []
for pkg in ["conda", "mamba"]:
if pkg in package_versions:
# add version pin to avoid upgrades
to_reinstall.append(f"{pkg}=={package_versions[pkg]}")
logger.info(
f"Reinstalling {', '.join(to_reinstall)} to ensure a consistent environment"
)
conda.ensure_conda_packages(
USER_ENV_PREFIX, list(to_reinstall), force_reinstall=True
)

cf_pkgs_to_upgrade = list(set(to_upgrade) & {"conda", "mamba"})
Copy link
Member Author

Choose a reason for hiding this comment

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

These lines are unchanged, but they are dedented because they should have always been run after the for loop, not on each iteration. Results should be unchanged, it was just a few unnecessary calls to conda install:

# first 'conda' iteration
conda install conda
# second 'mamba' iteration
conda install conda mamba
# third 'pip' iteration
conda install conda mamba
pip install --upgrade pip

when it should have just been the last two calls.

if cf_pkgs_to_upgrade:
conda.ensure_conda_packages(
USER_ENV_PREFIX,
# we _could_ explicitly pin Python here,
# but conda already does this by default
cf_pkgs_to_upgrade,
)

pypi_pkgs_to_upgrade = list(set(to_upgrade) & {"pip"})
if pypi_pkgs_to_upgrade:
conda.ensure_pip_packages(
USER_ENV_PREFIX,
pypi_pkgs_to_upgrade,
upgrade=True,
)

# Install/upgrade the jupyterhub version in the user env based on the
# version specification used for the hub env.
Expand Down