PR: Change logic to detect conda-based installers and micromamba on them #20827

mrclary · 2023-04-15T16:53:13Z

Description of Changes

Replaced pcregrep with pcre2grep in Github workflow since pcre2grep is already included in macOS runner image
Distinction between conda-based installation and traditional installation of Spyder, for all platforms, is determined by the existence of spyder-menu.json in the environment.
Renamed running_in_mac_app -> is_conda_based_app, which now determines whether the Spyder instance is a conda-based installation for any platform. is_pyinst is obsolete and removed.
Spyder's conda executable is no longer micromamba since the conda-based installer includes a standalone conda executable as well as mamba; accommodations are made accordingly.
For conda-based installations, Spyder's runtime environment and its root environment are excluded from the list of conda environments.
spyder-kernels is now run using conda's run command rather than external shell scripts.

ccordoba12 · 2023-04-17T14:58:53Z

@mrclary, please rebase with master to get the fixes to our tests. Also, quick review from your message above:

The standalone conda executable is included in the conda-based installer and is no longer micromamba and accommodations are made accordingly.

I think we should keep micromamba in the installers because we could use it for Spyder-env-manager, right @dalthviz?

mrclary · 2023-04-17T15:21:45Z

The standalone conda executable is included in the conda-based installer and is no longer micromamba and accommodations are made accordingly.

I think we should keep micromamba in the installers because we could use it for Spyder-env-manager, right @dalthviz?

It seems to me that micromamba would be redundant. The standalone executable also uses the libmamba library (@jaimergp, correct me if I'm wrong) and performs the same role in the conda-based implementation as micromamba does for our existing standalone applications.

jaimergp · 2023-04-17T15:43:00Z

Not right now, since the installation code doesn't require a solver at all. Also conda-standalone has a really bad startup time (needs to self-extract), so not really useful for anything else than bootstrapping an install.

jaimergp · 2023-04-17T15:43:44Z

You can add 'conda' to the base environment though, since Spyder needs a Python interpreter anyway.

mrclary · 2023-04-17T16:49:56Z

Not right now, since the installation code doesn't require a solver at all.

Just to be clear, conda.exe uses the standard conda solver, but does not use libmamba, correct? Because I haven't seen any issues creating environments with conda.exe.

Also conda-standalone has a really bad startup time (needs to self-extract), so not really useful for anything else than bootstrapping an install.

Interesting...
Is there a reason, then, that menuinst uses <base>/conda.exe instead of <base>/bin/activate when specifying "activate": True? or is <base>/bin/activate only introduced by conda/mamba?

Also, I haven't noticed any performance issues using conda.exe. If there is any "unpacking" time, I don't notice it in the following:

`$ time command info -e`

>> time mamba info -e
# conda environments:
#
                         /Users/rclary/Library/spyder-5.4.4.dev1473
                         /Users/rclary/Library/spyder-5.4.4.dev1473/envs/spyder-5.4.4.dev1473
base                     /Users/rclary/opt/mambaforge
bokeh                    /Users/rclary/opt/mambaforge/envs/bokeh
bokeh_apps               /Users/rclary/opt/mambaforge/envs/bokeh_apps
bokeh_py310              /Users/rclary/opt/mambaforge/envs/bokeh_py310
c2w                      /Users/rclary/opt/mambaforge/envs/c2w
c2w_310                  /Users/rclary/opt/mambaforge/envs/c2w_310
spy-dev                  /Users/rclary/opt/mambaforge/envs/spy-dev
spy-inst                 /Users/rclary/opt/mambaforge/envs/spy-inst
test                     /Users/rclary/opt/mambaforge/envs/test


real	0m0.966s
user	0m0.434s
sys	0m0.209s
>> time ~/Library/spyder-5.4.4.dev1473/conda.exe info -e
# conda environments:
#
                         /Users/rclary/Library/spyder-5.4.4.dev1473
                         /Users/rclary/Library/spyder-5.4.4.dev1473/envs/spyder-5.4.4.dev1473
                         /Users/rclary/opt/mambaforge
                         /Users/rclary/opt/mambaforge/envs/bokeh
                         /Users/rclary/opt/mambaforge/envs/bokeh_apps
                         /Users/rclary/opt/mambaforge/envs/bokeh_py310
                         /Users/rclary/opt/mambaforge/envs/c2w
                         /Users/rclary/opt/mambaforge/envs/c2w_310
                         /Users/rclary/opt/mambaforge/envs/spy-dev
                         /Users/rclary/opt/mambaforge/envs/spy-inst
                         /Users/rclary/opt/mambaforge/envs/test
base                     /var/folders/5v/28jqvwxs2cd5fj93gvwnykdrqc926z/T/_MEIsjVFap


real	0m0.766s
user	0m0.465s
sys	0m0.192s
>> time ~/Library/spyder-5.4.4.dev1473/bin/mamba info -e
# conda environments:
#
base                     /Users/rclary/Library/spyder-5.4.4.dev1473
spyder-5.4.4.dev1473     /Users/rclary/Library/spyder-5.4.4.dev1473/envs/spyder-5.4.4.dev1473
                         /Users/rclary/opt/mambaforge
                         /Users/rclary/opt/mambaforge/envs/bokeh
                         /Users/rclary/opt/mambaforge/envs/bokeh_apps
                         /Users/rclary/opt/mambaforge/envs/bokeh_py310
                         /Users/rclary/opt/mambaforge/envs/c2w
                         /Users/rclary/opt/mambaforge/envs/c2w_310
                         /Users/rclary/opt/mambaforge/envs/spy-dev
                         /Users/rclary/opt/mambaforge/envs/spy-inst
                         /Users/rclary/opt/mambaforge/envs/test


real	0m0.772s
user	0m0.326s
sys	0m0.140s

You can add 'conda' to the base environment though, since Spyder needs a Python interpreter anyway.

We currently add python, conda, mamba, and pip to the base environment of the installation. Perhaps this is why we have <base>/bin/activate...

Nevertheless, if there are any concerns about conda.exe, we can just change our find_conda command in this PR to grab <base>/bin/mamba instead of <base>/conda.exe.

dalthviz · 2023-04-17T20:58:11Z

I think we should keep micromamba in the installers because we could use it for Spyder-env-manager, right @dalthviz?

Well, to give some context, the plugin is using some of the spyder functions to define what 'conda-like' executable should be used. That is done at https://github.com/spyder-ide/spyder-env-manager/blob/37f2837a077bd5dabd2b123c015c5f071826e8b9/spyder_env_manager/spyder/config.py#L24-L36

After a quick check to the changes done here, seems like that will need an update, but we are using already there the find_conda function so I guess it should not be much of a problem. The plugin at the end can work with micromamba or conda without issue 👍 that's why we test the envs-manager with conda and micromamba: https://github.com/spyder-ide/envs-manager/actions/runs/4189392934

mrclary · 2023-04-18T00:15:00Z

Well, to give some context, the plugin is using some of the spyder functions to define what 'conda-like' executable should be used. That is done at https://github.com/spyder-ide/spyder-env-manager/blob/37f2837a077bd5dabd2b123c015c5f071826e8b9/spyder_env_manager/spyder/config.py#L24-L36

After a quick check to the changes done here, seems like that will need an update, but we are using already there the find_conda function so I guess it should not be much of a problem. The plugin at the end can work with micromamba or conda without issue 👍 that's why we test the envs-manager with conda and micromamba: https://github.com/spyder-ide/envs-manager/actions/runs/4189392934

Yes, get_spyder_umamba_path is removed and find_conda preferentially returns Spyder's runtime conda executable, falling back to CONDA_EXE or MAMBA_EXE environment variables, and finally resorting to find_program.

jaimergp · 2023-04-18T10:06:36Z

Interesting...
Is there a reason, then, that menuinst uses /conda.exe instead of /bin/activate when specifying "activate": True? or is /bin/activate only introduced by conda/mamba?

conda.exe is guaranteed to exist, while bin/activate requires conda on your base environment, and that's an assumption we didn't want to make or force on users (like having python... this is being worked on). We can probably add some logic to use other (faster) activation providers and fallback to conda.exe if nothing else was found, but well... menuinst 2 is still an alpha project.

Also, I haven't noticed any performance issues using conda.exe. If there is any "unpacking" time, I don't notice it in the following.

Usually it takes a few seconds to return, but maybe you have it cached already. I think pyinstaller extracts to a random, but predictable location? Not sure.

mrclary · 2023-04-18T16:30:41Z

conda.exe is guaranteed to exist, while bin/activate requires conda on your base environment, and that's an assumption we didn't want to make or force on users (like having python... this is being worked on). We can probably add some logic to use other (faster) activation providers and fallback to conda.exe if nothing else was found, but well... menuinst 2 is still an alpha project.

Ahh, okay, understood.

Usually it takes a few seconds to return, but maybe you have it cached already. I think pyinstaller extracts to a random, but predictable location? Not sure.

I did notice that conda.exe required writing to a temp directory; now I know why.

spyder/__init__.py

spyder/config/base.py

spyder/dependencies.py

spyder/plugins/variableexplorer/widgets/collectionsdelegate.py

spyder/utils/conda.py

spyder/plugins/application/confpage.py

mrclary · 2023-04-19T04:50:16Z

@ccordoba12, Thank you for all your feedback and taking the time to review the PR. I've marked it as draft until we investigate further some the review comments. Perhaps we can discuss some of the points at the next developer meeting.

mrclary · 2023-04-25T01:42:59Z

I changed where conda-based environments were filtered: instead of in spyder.utils.conda.get_list_conda_env, now it is in spyder.plugins.maininterpreter.confpage.
This means that get_list_conda_env will still list all environments on the system without prejudice, but the custom interpreters still do not reveal conda-based runtime environment or its base.

dalthviz

Thanks @mrclary ! Left a couple of comments, but other than that, this LGTM 👍

spyder/plugins/variableexplorer/widgets/collectionsdelegate.py

spyder/utils/conda.py

ccordoba12

Thanks @mrclary for addressing our reviews! One last suggestion for you, then this should be ready.

spyder/plugins/variableexplorer/widgets/collectionsdelegate.py

… effect of reporting these values when running from conda-based installers, otherwise no effect.

…now conda-based

is_conda_based_app checks if Spyder is from a conda-based installer. Conda-based installer is determined by existence of spyder-menu.json file in environment instead of SPYDER_APP environment variable.

…ow bundled with conda-based installers

… Spyder's mamba executable (from conda-based installers). When testing for standalone conda executable, use endswith in order to capture '_conda.exe' as well as 'conda.exe' Non-standalone conda executables are in the same directory as activate; no need to back out two directory levels and go back in. Enforce correct CONDA_EXE environment variable for conda-based installers.

… thereof, from custom interpreter list. Note that sys.executable may be a symlink for conda-based runtime environments. Windows requires USERPROFILE environment variable to find all conda environments.

…s conda-activate.[sh|bat] and get_conda_activation_script.

Co-authored-by: Carlos Cordoba <[email protected]>

Flow control in projects plugin does not need to distinguish conda-based Spyder from other installations. CLI works for macOS, Linux, and Windows. Corrected syntax to the Windows command allows for commandline arguments passed to the batch file.

…es not occur.

ccordoba12

Thanks @mrclary!

mrclary self-assigned this Apr 15, 2023

mrclary requested review from dalthviz and ccordoba12 April 15, 2023 16:53

mrclary force-pushed the cbi-updates branch from fa6e977 to b7114d4 Compare April 16, 2023 21:13

mrclary mentioned this pull request Apr 16, 2023

Conda-based Installer Parent Issue #20001

Closed

16 tasks

mrclary force-pushed the cbi-updates branch from 07c0d0b to b7114d4 Compare April 16, 2023 22:58

mrclary marked this pull request as draft April 17, 2023 04:46

mrclary force-pushed the cbi-updates branch from b7114d4 to da508c5 Compare April 17, 2023 15:03

mrclary force-pushed the cbi-updates branch from da508c5 to e4a662b Compare April 17, 2023 15:31

mrclary force-pushed the cbi-updates branch 4 times, most recently from e487732 to e57da03 Compare April 17, 2023 20:36

mrclary marked this pull request as ready for review April 18, 2023 00:15

mrclary force-pushed the cbi-updates branch from 49d304b to 07774c8 Compare April 18, 2023 00:47

ccordoba12 reviewed Apr 19, 2023

View reviewed changes

ccordoba12 changed the title ~~PR: Updated logic for running_in_mac_app and micromamba for conda-based installer~~ PR: Change logic to detect conda-based installers and micromamba on them Apr 19, 2023

ccordoba12 added this to the v6.0alpha1 milestone Apr 19, 2023

mrclary commented Apr 19, 2023

View reviewed changes

spyder/plugins/application/confpage.py Outdated Show resolved Hide resolved

mrclary marked this pull request as draft April 19, 2023 04:47

mrclary force-pushed the cbi-updates branch from 65c2b2e to 4a43ab0 Compare April 25, 2023 01:45

dalthviz mentioned this pull request Apr 25, 2023

PR: Update conda like executable spyder-ide/spyder-env-manager#16

Merged

dalthviz reviewed Apr 25, 2023

View reviewed changes

spyder/plugins/variableexplorer/widgets/collectionsdelegate.py Outdated Show resolved Hide resolved

spyder/utils/conda.py Outdated Show resolved Hide resolved

mrclary force-pushed the cbi-updates branch 2 times, most recently from df9ea36 to 2d0589b Compare April 26, 2023 21:54

ccordoba12 reviewed Apr 27, 2023

View reviewed changes

spyder/plugins/variableexplorer/widgets/collectionsdelegate.py Outdated Show resolved Hide resolved

mrclary force-pushed the cbi-updates branch from 2d0589b to 81e58c4 Compare April 27, 2023 02:22

mrclary and others added 17 commits April 27, 2023 14:18

Use pcre2grep, which is already installed by git

edaeef1

Rename post-install script for linux. Clarify uninstall instruction

8cd728a

Fall back to SPY_COMMIT and SPY_BRANCH for git revision. This has the…

5672c26

… effect of reporting these values when running from conda-based installers, otherwise no effect.

running_in_mac_app is no longer applicable here since the mac app is …

3bb088b

…now conda-based

Rename running_in_mac_app -> is_conda_based_app.

ddb8cfc

is_conda_based_app checks if Spyder is from a conda-based installer. Conda-based installer is determined by existence of spyder-menu.json file in environment instead of SPYDER_APP environment variable.

Remove is_pynsist, obsolete

c727e4d

Remove reference to micromamba since standalone conda executable is n…

a83bebb

…ow bundled with conda-based installers

Remove conda-based Spyder runtime environments, and base environments…

eddeb6e

… thereof, from custom interpreter list. Note that sys.executable may be a symlink for conda-based runtime environments. Windows requires USERPROFILE environment variable to find all conda environments.

Simplify spyder-kernels startup by using conda's run command. Obviate…

57432a0

…s conda-activate.[sh|bat] and get_conda_activation_script.

Windows CI now sees test environment

7710c1a

Apply suggestions from code review

86511fc

Co-authored-by: Carlos Cordoba <[email protected]>

Apply suggestions from code review

89965ac

Apply suggestions from code review.

1924941

Flow control in projects plugin does not need to distinguish conda-based Spyder from other installations. CLI works for macOS, Linux, and Windows. Corrected syntax to the Windows command allows for commandline arguments passed to the batch file.

Apply suggestions from code review. Regression of spyder-ide#20390 do…

7639ea7

…es not occur.

Apply review suggestions.

a13aeb2

Apply review suggestions.

c524c92

mrclary force-pushed the cbi-updates branch from 81e58c4 to c524c92 Compare April 27, 2023 21:18

ccordoba12 approved these changes Apr 28, 2023

View reviewed changes

ccordoba12 merged commit 90ce9ee into spyder-ide:master Apr 28, 2023

mrclary deleted the cbi-updates branch April 29, 2023 02:25

jitseniesen mentioned this pull request Dec 19, 2024

Jupyter Notebook: "Interrupt kernel" restarts kernel, all variables lost spyder-ide/spyder-notebook#478

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Change logic to detect conda-based installers and micromamba on them #20827

PR: Change logic to detect conda-based installers and micromamba on them #20827

mrclary commented Apr 15, 2023 •

edited

Loading

ccordoba12 commented Apr 17, 2023

mrclary commented Apr 17, 2023

jaimergp commented Apr 17, 2023

jaimergp commented Apr 17, 2023

mrclary commented Apr 17, 2023

dalthviz commented Apr 17, 2023

mrclary commented Apr 18, 2023

jaimergp commented Apr 18, 2023

mrclary commented Apr 18, 2023

mrclary commented Apr 19, 2023

mrclary commented Apr 25, 2023

dalthviz left a comment

ccordoba12 left a comment

ccordoba12 left a comment

PR: Change logic to detect conda-based installers and micromamba on them #20827

PR: Change logic to detect conda-based installers and micromamba on them #20827

Conversation

mrclary commented Apr 15, 2023 • edited Loading

Description of Changes

ccordoba12 commented Apr 17, 2023

mrclary commented Apr 17, 2023

jaimergp commented Apr 17, 2023

jaimergp commented Apr 17, 2023

mrclary commented Apr 17, 2023

dalthviz commented Apr 17, 2023

mrclary commented Apr 18, 2023

jaimergp commented Apr 18, 2023

mrclary commented Apr 18, 2023

mrclary commented Apr 19, 2023

mrclary commented Apr 25, 2023

dalthviz left a comment

Choose a reason for hiding this comment

ccordoba12 left a comment

Choose a reason for hiding this comment

ccordoba12 left a comment

Choose a reason for hiding this comment

mrclary commented Apr 15, 2023 •

edited

Loading