Unix: add uninstall.sh and update install.sh to modify it during install #24
Conversation
densnow
force-pushed
the
14-create-uninstallation-scripts
branch
from
b9aedc3
to
841898e
Compare
densnow
force-pushed
the
14-create-uninstallation-scripts
branch
from
841898e
to
3a98edb
Compare
…UNINSTALL to FILES
There was a problem hiding this comment.
As noted in #14, automatic uninstallation is high effort and low priority because it's just a 'nice to have': manual uninstallation is neither difficult nor time consuming. I don't have now the hours to put into this, so I'm sending this PR back to you but I don't expect further work from you as I won't be able to review.
install.sh
Outdated
| # Set variables in uninstall.sh | ||
| echo "Setting variables into uninstall.sh ..." | ||
| sed -i "14iFOLDER=$FOLDER" "$FOLDER/$UNINSTALL" |
There was a problem hiding this comment.
I've only tried running install.sh in macOS Big Sur and didn't work. Gives and error on this and the next line and doesn't modify uninstall.sh.
The installed sed doesn't seem to be GNU sed. The -i flag expects (after a space) an extension for the backup. Can be '' to not have a backup. Probably you need to check what POSIX sed allows to be compatible across platforms.
However, both the macOS version and GNU sed expect a backslash after 14i with the text to be inserted in the next line. Maybe you can insert the two lines in one single sed insert command?
There was a problem hiding this comment.
Yes looks like macOS uses BSD version of sed whereas Linux (mostly) uses gnu version. It seems like the -i flag in particular works differently on each as you have noted.
I have submitted a possible fix using selection and the $OSTYPE environment variable to see if the script is running on macOS or Linux. It then (hopefully, as I don't have access to mac) uses the appropriate syntax for each version of sed.
I know this is very low priority , so I will just leave it for now until we have more time to test and possibly do more fixes.
|
I am not sure why I did not realise this before, but reading in between (or directly) the lines of Michels comments and picking up the subtle hints. We do not need to introduce code into the main branch at this time (so close to the 23j release) that could potentially break; it is just so unnecessary. Nobody has the time to test this thoroughly at present and the gains are minimal at best. We can still have the 14-create-uninstallation-scripts branch to work on and hopefully we can get some adventurous 23j students to help test and contribute towards this script. Thanks for you patience Michel 😄 |
Note: The URL on line 91 has been changed to download from 14-create-uninstallation-scripts instead of main for testing purposes. This will need to be changed back before merging.
Another couple of points:
FILESso will be downloaded if no arg is given. If arg is giveninstall.shwill copyuninstall.shto the M269 folder. So whichever way the script is rununinstall.shwill end up in the M269 folder; not sure if this is correct based on what the second mode is supposed to be used for.