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

Fix uninstallation of conda-forge-ci-setup #1596

Closed
wants to merge 1 commit into from
Closed

Fix uninstallation of conda-forge-ci-setup #1596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion conda_smithy/templates/appveyor.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ install:
# Tell conda we want an updated version of conda-build and {{ " ".join(remote_ci_setup) }}
- cmd: conda.exe install -n root -c conda-forge --quiet --yes conda-build pip {{ " ".join(remote_ci_setup) }}
{%- if local_ci_setup %}
- cmd: conda.exe uninstall --quiet --yes --force {{ " ".join(remote_ci_setup) }}
# Uninstall the conda-forge-ci-setup package but leave its dependencies
# so that we may bootstrap the installation process
- cmd: conda.exe uninstall --quiet --yes --force conda-forge-ci-setup
Copy link
Member

Choose a reason for hiding this comment

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

This was an option so the package could be renamed. We should preserve that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I think we have two options:

  1. Remove the uninstall step and specify each dependency to conda-forge-ci-setup in the conda-forge.yml file. This seems quite difficult with win/osx/linux specifics.
  2. Create a new option.

To be clear, I figured that "renaming" wasn't so interesting because of the hard coded nature of the package name:

conda-smithy/conda_smithy/configure_feedstock.py

Line 772 in c299172

"conda_forge_ci_setup",

https://github.com/conda-forge/conda-forge-ci-setup-feedstock/blob/main/recipe/meta.yaml#L63

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 3rd option is to leave it as is.

The current "state" works for tensorflow, and works for conda-forge-ci-setup. This is "good enough" for me.

Copy link
Member

Choose a reason for hiding this comment

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

We actually have the version as part of the option:

conda-smithy/conda_smithy/configure_feedstock.py

Line 1759 in c299172

"remote_ci_setup": ["conda-forge-ci-setup=3"],

So smithy has been using a mix of things.

I don't have strong feelings here, but it seems like if folks want to add extra ci setup packages, then we should have added a new optional originally called "extra_ci_setup" or similar.

In any case, most things are half-broken most of the time. I'll defer to the rest of core here.

cc @conda-forge/core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had originally proposed something like extra, it was rejected. I think we hadn't considered the bootstrap procedure. Selfishly speaking, i don't have many reasons to push this PR forward

Copy link
Member

Choose a reason for hiding this comment

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

That is totally fine. Let's close this one and leave things as they are. We can go back and fix later by making the conda-forge specific stuff a separate option since it is special anyways. Thank you for all of the hard work here!

Copy link
Member

Choose a reason for hiding this comment

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

xref: #1597

- cmd: pip install --no-deps .\{{ recipe_dir }}\.
{%- endif %}
- cmd: setup_conda_rc .\ .\{{ recipe_dir }} .\.ci_support\%CONFIG%.yaml
Expand Down
2 changes: 1 addition & 1 deletion conda_smithy/templates/azure-pipelines-win.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ jobs:
- script: |
call activate base
{%- if local_ci_setup %}
conda.exe uninstall --quiet --yes --force {{ " ".join(remote_ci_setup) }}
conda.exe uninstall --quiet --yes --force conda-forge-ci-setup
pip install --no-deps ".\{{ recipe_dir }}\."
{%- endif %}
setup_conda_rc .\ ".\{{ recipe_dir }}" .\.ci_support\%CONFIG%.yaml
Expand Down
4 changes: 3 additions & 1 deletion conda_smithy/templates/build_steps.sh.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ CONDARC
conda-build pip {{ BOA }}{{ " ".join(remote_ci_setup) }}
{%- endif %}
{% if local_ci_setup %}
conda uninstall --quiet --yes --force {{ " ".join(remote_ci_setup) }}
# Uninstall the conda-forge-ci-setup package but leave its dependencies
# so that we may bootstrap the installation process
conda uninstall --quiet --yes --force conda-forge-ci-setup
pip install --no-deps ${RECIPE_ROOT}/.
{%- endif %}
# set up the condarc
Expand Down
2 changes: 1 addition & 1 deletion conda_smithy/templates/github-actions.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ jobs:
mamba.exe install -c conda-forge 'python=3.9' conda-build conda pip {{ BOA }}{{ " ".join(remote_ci_setup) }}
if errorlevel 1 exit 1
{%- if local_ci_setup %}
conda.exe uninstall --quiet --yes --force {{ " ".join(remote_ci_setup) }}"
conda.exe uninstall --quiet --yes --force conda-forge-ci-setup
if errorlevel 1 exit 1
pip install --no-deps ".\{{ recipe_dir }}\."
if errorlevel 1 exit 1
Expand Down
4 changes: 3 additions & 1 deletion conda_smithy/templates/run_osx_build.sh.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ echo -e "\n\nInstalling {{ remote_ci_setup }} and conda-build."
{%- endif %}

{% if local_ci_setup %}
conda uninstall --quiet --yes --force {{ " ".join(remote_ci_setup) }}
# Uninstall the conda-forge-ci-setup package but leave its dependencies
# so that we may bootstrap the installation process
conda uninstall --quiet --yes --force conda-forge-ci-setup
pip install --no-deps {{ recipe_dir }}/.
{%- endif %}

Expand Down
3 changes: 3 additions & 0 deletions news/do_no_uninstall_extra_packages.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Fixed:**

* Do not uninstall extra packages specified in ``remote_ci_setup`` during the bootstrapping phase of the ci-setup of the ci-setup.