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

Create uninstallation scripts #14

Open
2 tasks
mwermelinger opened this issue Aug 4, 2023 · 9 comments
Open
2 tasks

Create uninstallation scripts #14

mwermelinger opened this issue Aug 4, 2023 · 9 comments
Labels
effort: high Resolving the issue may take lots of effort priority: low type: feature New feature

Comments

@mwermelinger
Copy link
Member

It would be nice to have uninstallation scripts uninstall.sh and uninstall.ps1. The scripts would do the following:

  • check that the script is running within a folder matching regex [Mm]269-\d\d[Jj] and extract the year from it
  • ask the user to confirm uninstallation, warning them that they'll need a Jupyter environment to open and run their notebooks
  • remove the virtual environment for the matched year
  • remove the allowed.py and m269.json files in the current folder
  • detect the parent shell and remove the aliases from the corresponding shell startup file
    • this won't work if the user has changed the default shell between installing and uninstalling the M269 software
    • the only robust way is to search all 5 possible startup files for the aliases
    • to avoid removing other aliases named nb and allowed, the 'content' of the alias must be checked; maybe the easiest is to check if the alias mentions the current folder
  • remove the M269-related lines from ~/.jupyter/custom/custom.css

Removing lines from the CSS and shell startup files can be done with sed on Unix, but no clue how to do it in Windows.

Once the scripts are done and tested:

  • Shorten Section 5 of the README, just saying how to run the uninstallation script
  • Add the uninstallation script to variable FILES in the corresponding installation script
@mwermelinger mwermelinger added effort: high Resolving the issue may take lots of effort priority: low type: feature New feature labels Aug 4, 2023
@densnow
Copy link
Collaborator

densnow commented Aug 5, 2023

This is just a thought, but could we add some more environment variables during the installation process that would make the uninstall easier. E.g the parent shell, the path to folder, the presentation year etc?

Or could we write some of this information to a file (maybe a hidden file) that lives in the main m269 folder? Of course a file like this could be accidently deleted or changed.

@mwermelinger
Copy link
Member Author

Good ideas! The uninstallation script could be written assuming the existence of variables like SHELL, VENV, RC_FILE, etc. The installation script computes the values of those variables and prepends them to the uninstallation script. In this way, once installation finishes, the uninstallation script is ready to run and contains all info it needs.

@densnow
Copy link
Collaborator

densnow commented Aug 18, 2023

I have a working uninstallation script for Unix (at least on my machine) and will submit the PR. This can be treated like a first draft, as some parts might need a few tweaks and changes etc. A couple of points:

  • I have written variables needed for uninstallation to a .m269rc file that lives in the M269 folder. This file is then sourced in the uninstallation script e.g source .m269rc. There are a couple of reasons for this; firstly I thought the config file might be useful for other projects specific to m269, secondly it seemed a more reliable and readable approach as adding variables directly to the script could involve sed and place holders (can't directly prepend because of shebang)
  • Backups of any config files edited with sed have been created in the script (FILE_NAME.backup) as a precaution. Of course this can be removed easily
  • The README has not been changed

@mwermelinger
Copy link
Member Author

Thanks. This will require some iterations (also for Windows) until completed, so could you please withdraw the PR and instead create an and commit a branch? On this issue's page, on the right sidebar, there's a link to create a branch associated to this issue under the 'Development' heading.

@densnow
Copy link
Collaborator

densnow commented Aug 18, 2023

I have withdrawn the PR, but I don't think I have permissions to create a new branch in this repo. Maybe if you created the new branch I could submit PR to the new branch?

@mwermelinger
Copy link
Member Author

Had a quick look at your PR. Some suggestions:

  • As I mentioned in the first comment of this issue, it may be better to remove aliases and CSS section based on presentation name, ie matching regex [Mm]269-YY[Jj] where YY is the hardwired year (maybe from an env variable). This will prevent removing any other alias also called allowed or having to match exactly the start/end comment of the CSS.
  • It would be good to check if the various folders and files still exist before doing anything, as things might have changed between installation and uninstallation.

@densnow
Copy link
Collaborator

densnow commented Aug 18, 2023

Thanks, I have incorporated those suggestions.

A couple of questions:

  • What do think about having each of the parts of the uninstallation as an option. E.g. "Do you wish to remove styling from ~/.jupyter/custom/custom.css ? [y/N]" and the same for the aliases and other parts? Might be overkill, but the thought just occurred.
  • Would you like a final message printed to the user e.g. "The M269 software has now been uninstalled" or similar?

@densnow
Copy link
Collaborator

densnow commented Aug 20, 2023

I think the Unix uninstallation script is ready for more critique, testing and suggestions.

Some of my thoughts were:

  • I don't like the way that allowed.py and m269.json are "hardcoded" for removal. Maybe a variable containing all the files that will be removed from the M269 folder (created in the installation) would be better?
  • Do we want to keep the .m269rc file to hold the variables needed for uninstallation or should we write to the script directly? if the former, might it be better to keep the file in a known location e.g ~/.m269/m269rc or ~/.config/m269/m269rc? If the latter, do we need to use sed and place holders, or is there a better and easier way to do this?
  • The use of the [Mm]269-YY[Jj] pattern in the style and alias removal might not be exactly as you had intended, it is working, but maybe could be better i.e. more efficient and readable.
  • Is there more that can be done in terms of safely removing files and directories? I have added some tests and checks, but can we do more here?
  • Do we need to remove install.sh, .m269rc (if used), and also have the script delete itself at the end?

@mwermelinger
Copy link
Member Author

I prefer the env vars to be written to uninstall.sh rather than an extra .m269rc file. Makes uninstall.sh simpler and easier to understand. install.sh probably also has to write the shebang to uninstall.sh. Apart from shell_configuration_file var, probably everything else could be hardwired. After all, the two scripts go hand in hand. If next year we e.g. install allowed via pip, then we need to make changes anyway to both scripts.

On further reflection, there's no point in keeping the aliases if the venv and allowed.py are removed. And the whole point of the script is to remove the 1Gb venv. So, to simplify, I suggest the script simply warns the user at the beginning that it will remove the venv in ~/venvs/m269-YYj and the aliases nb, allowed and m269-YYj and that therefore the notebooks will be no longer readable or executable unless another Jupyter environment exists. Then asks the user to confirm to proceed.

The only other thing to ask the user is if they want to keep the M269 style of notebooks, otherwise it will be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: high Resolving the issue may take lots of effort priority: low type: feature New feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants