PR: Update to `napari/label/bundle_tools_3` (Installers) #21125

mrclary · 2023-07-12T22:06:24Z

This PR updates the cond-based installer tools to the latest napari/label/bundle_tools_3, which include several updates to constructor and menuinst.

Additionally:

Added app_user_model_id to shortcut definition for Windows
Improved logging of the installation test on CI
Fixed issues with sh type installer for macOS
Accommodate use of .nonadmin with updated menuinst
All-user installation on Linux now adds aliases to the shell startup file for each real user.
Uninstall script alias is only added to shell startup file if installing for single user.

ccordoba12

A couple of comments for you @mrclary, the rest looks good to me.

.github/workflows/installers-conda.yml

installers-conda/resources/post-install.sh

mrclary · 2023-07-15T19:59:18Z

Actually, I'm going to verify some other issues with all-user installation for Linux; converting back to draft.

mrclary · 2023-07-16T23:47:23Z

@ccordoba12, thanks for your previous review; I've made the suggested correction. Additionally, when installing for all users, each user's shell startup will have the Spyder launch alias. The uninstall alias is only included for user-only install.

ccordoba12 · 2023-07-17T16:29:59Z

@mrclary, looks good to me now, but I'd like to ask you if you have tested the admin install in a VM. I'd like to help you with that but I don't want to mess with my system.

mrclary · 2023-07-17T18:00:07Z

@mrclary, looks good to me now, but I'd like to ask you if you have tested the admin install in a VM. I'd like to help you with that but I don't want to mess with my system.

Yes, I've tested the admin install on my Ubuntu VM. I added a test user as well and confirmed that the Spyder alias is properly created and Spyder launches for all accounts.

Note that constructor does not provide a default install location for all users on admin installs; /usr/spyder-6 (or wherever) must be specified either with -p flag or at the interactive prompt. Otherwise the environment prefix will fall back to /root/.local/spyder-6 which will causes a permission denied error for any user other than root.

Ping @jaimergp.

mrclary · 2023-07-17T18:03:39Z

@ccordoba12, I've removed the uninstall Spyder alias from shell startup for admin installs because it requires sudo privileges and when invoking sudo the alias isn't recognized anyway. Should the uninstall Spyder alias be added to /root/.bashrc in this case?

installers-conda/resources/spyder-menu.json

ccordoba12 · 2023-07-17T21:35:54Z

Yes, I've tested the admin install on my Ubuntu VM. I added a test user as well and confirmed that the Spyder alias is properly created and Spyder launches for all accounts.

Ok, great!

Note that constructor does not provide a default install location for all users on admin installs; /usr/spyder-6 (or wherever) must be specified either with -p flag or at the interactive prompt. Otherwise the environment prefix will fall back to /root/.local/spyder-6 which will causes a permission denied error for any user other than root.

I think that the default directory should be /opt/spyder-6, but we can add it once constructor has support for it.

I've removed the uninstall Spyder alias from shell startup for admin installs because it requires sudo privileges and when invoking sudo the alias isn't recognized anyway.

Ok, no problem. Then I recommend to show a message saying that system admins need to remove the directory where Spyder was installed plus the desktop file to uninstall it.

Should the uninstall Spyder alias be added to /root/.bashrc in this case?

That probably won't work and the alternatives don't seem promising.

I think adding a proper script in /path/to/install_dir/uninstall-spyder.sh should be enough. The thing is in that case we don't want to provide a global command to uninstall Spyder because regular users won't be able to run it and it doesn't make sense for them.

ccordoba12 · 2023-07-18T03:05:38Z

Perhaps the instruction to source .bashrc in the current shell should be omitted in the case of admin installs?

Agreed. That part should be replaced by telling users to open a new terminal and start Spyder there.

mrclary · 2023-07-18T21:22:27Z

@ccordoba12 here is the new message for admin install on Linux.

###############################################################################
#                             !!! IMPORTANT !!!
###############################################################################
Spyder can be launched by standard methods in Gnome and KDE desktop
environments. It can also be launched from the command line on all Linux
distros with the command:

$ spyder

This command will only be available in new shell sessions.

To uninstall Spyder, run the following from the command line:

$ sudo /opt/spyder-6/uninstall-spyder.sh

###############################################################################

ccordoba12 · 2023-07-18T22:50:32Z

@ccordoba12 here is the new message for admin install on Linux.

Looks good to me now, thanks!

ccordoba12

A couple of additional comments/suggestions, then this should be ready.

installers-conda/resources/post-install.sh

ccordoba12

Last suggestions to prevent overwriting the Spyder desktop file installed by the Linux distro package manager.

.github/workflows/installers-conda.yml

installers-conda/resources/post-install.sh

jitseniesen · 2023-07-19T09:21:18Z

here is the new message for admin install on Linux.

@mrclary Sorry to butt in, but I think it looks more professional to say "distributions" in full instead of "distros".

…_3 channel. For some reason, conda is taken from conda-forge instead of bundle_tools_3 and setup-micromamba does not provide option to set strict channel priority, so must explicitly use channel for each package.

…nstaller

…-user install on Linux. Do not alias uninstall script for all-user installation. Use same sed command options for macOS and Linux

Note that pkill won't close Spyder on macOS; it will terminate Spyder.app/Contents/MacOS/spyder process but not the application. osascript must be used. Also note that if the IPython interpreter is "same as spyder" then osascript will close the kernel (it thinks it is Spyder.app) and not the application. This may be fixed by resolving sys.executable in KernelSpec.

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com> Co-authored-by: Jitse Niesen <jitseniesen@yahoo.com>

ccordoba12

Looks good to me now, thanks @mrclary!

mrclary · 2023-07-20T14:10:16Z

@ccordoba12 @jitseniesen @jaimergp, thanks for all your feedback!

mrclary self-assigned this Jul 12, 2023

mrclary force-pushed the bundle_tools_3 branch 7 times, most recently from 4b2fcb3 to b79daf9 Compare July 14, 2023 19:19

mrclary marked this pull request as ready for review July 14, 2023 19:20

mrclary force-pushed the bundle_tools_3 branch from b79daf9 to ff45434 Compare July 15, 2023 05:33

ccordoba12 changed the title ~~PR: Update to napari/label/bundle_tools_3~~ PR: Update to napari/label/bundle_tools_3 (Installers) Jul 15, 2023

ccordoba12 added type:Enhancement Installers labels Jul 15, 2023

ccordoba12 added this to the v6.0alpha2 milestone Jul 15, 2023

ccordoba12 reviewed Jul 15, 2023

View reviewed changes

.github/workflows/installers-conda.yml Outdated Show resolved Hide resolved

installers-conda/resources/post-install.sh Outdated Show resolved Hide resolved

mrclary marked this pull request as draft July 15, 2023 19:58

mrclary force-pushed the bundle_tools_3 branch from 80d9a7a to 3969998 Compare July 16, 2023 07:17

mrclary marked this pull request as ready for review July 16, 2023 23:13

mrclary marked this pull request as draft July 16, 2023 23:17

mrclary force-pushed the bundle_tools_3 branch from 3969998 to 020f1fc Compare July 16, 2023 23:46

mrclary marked this pull request as ready for review July 16, 2023 23:47

mrclary force-pushed the bundle_tools_3 branch from 020f1fc to 3d8d100 Compare July 17, 2023 17:45

jaimergp reviewed Jul 17, 2023

View reviewed changes

installers-conda/resources/spyder-menu.json Outdated Show resolved Hide resolved

mrclary force-pushed the bundle_tools_3 branch from 3d8d100 to 020f1fc Compare July 17, 2023 20:22

mrclary mentioned this pull request Jul 18, 2023

Set app ID on Windows conda/menuinst#127

Closed

2 tasks

mrclary force-pushed the bundle_tools_3 branch 2 times, most recently from f767729 to ba9fe93 Compare July 18, 2023 21:54

ccordoba12 reviewed Jul 18, 2023

View reviewed changes

installers-conda/resources/post-install.sh Outdated Show resolved Hide resolved

installers-conda/resources/post-install.sh Show resolved Hide resolved

mrclary commented Jul 19, 2023

View reviewed changes

installers-conda/resources/post-install.sh Outdated Show resolved Hide resolved

ccordoba12 reviewed Jul 19, 2023

View reviewed changes

.github/workflows/installers-conda.yml Show resolved Hide resolved

installers-conda/resources/post-install.sh Show resolved Hide resolved

mrclary and others added 15 commits July 19, 2023 07:11

Update to napari bundle_tools_3

5512cd8

Add app_user_model_id to Windows menu definition.

ee17684

Add option for installer type

7b04274

Revisions for macOS sh installer

84ea3e5

Use TMPDIR for sh installer on macOS, SHARED_INSTALLER_TEMP for pkg i…

be9a393

…nstaller

Use .nonadmin to determine determine shortcut location

0a961ab

Additional installer info for CI

baf164b

Check .nonadmin to set shortcut path

531d270

Fix incorrect shortcut location on Linux for all-user install

b906d4c

Ensure that aliases are created in shell startup for all users if all…

d0e91bb

…-user install on Linux. Do not alias uninstall script for all-user installation. Use same sed command options for macOS and Linux

app_user_model_id is strictly set by Qt and that value must be used.

6a55f97

Adapt message for Linux admin install

767e73c

Apply suggestions from code review

92bb1c6

Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com> Co-authored-by: Jitse Niesen <jitseniesen@yahoo.com>

mrclary force-pushed the bundle_tools_3 branch from 4e6804d to 92bb1c6 Compare July 20, 2023 00:15

ccordoba12 approved these changes Jul 20, 2023

View reviewed changes

ccordoba12 merged commit 4904d7f into spyder-ide:master Jul 20, 2023
6 checks passed

mrclary deleted the bundle_tools_3 branch July 21, 2023 02:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Update to `napari/label/bundle_tools_3` (Installers) #21125

PR: Update to `napari/label/bundle_tools_3` (Installers) #21125

mrclary commented Jul 12, 2023 •

edited

Loading

ccordoba12 left a comment

mrclary commented Jul 15, 2023

mrclary commented Jul 16, 2023

ccordoba12 commented Jul 17, 2023

mrclary commented Jul 17, 2023

mrclary commented Jul 17, 2023

ccordoba12 commented Jul 17, 2023

ccordoba12 commented Jul 18, 2023

mrclary commented Jul 18, 2023

ccordoba12 commented Jul 18, 2023

ccordoba12 left a comment

ccordoba12 left a comment

jitseniesen commented Jul 19, 2023

ccordoba12 left a comment

mrclary commented Jul 20, 2023

PR: Update to napari/label/bundle_tools_3 (Installers) #21125

PR: Update to napari/label/bundle_tools_3 (Installers) #21125

Conversation

mrclary commented Jul 12, 2023 • edited Loading

ccordoba12 left a comment

Choose a reason for hiding this comment

mrclary commented Jul 15, 2023

mrclary commented Jul 16, 2023

ccordoba12 commented Jul 17, 2023

mrclary commented Jul 17, 2023

mrclary commented Jul 17, 2023

ccordoba12 commented Jul 17, 2023

ccordoba12 commented Jul 18, 2023

mrclary commented Jul 18, 2023

ccordoba12 commented Jul 18, 2023

ccordoba12 left a comment

Choose a reason for hiding this comment

ccordoba12 left a comment

Choose a reason for hiding this comment

jitseniesen commented Jul 19, 2023

ccordoba12 left a comment

Choose a reason for hiding this comment

mrclary commented Jul 20, 2023

PR: Update to `napari/label/bundle_tools_3` (Installers) #21125

PR: Update to `napari/label/bundle_tools_3` (Installers) #21125

mrclary commented Jul 12, 2023 •

edited

Loading