add --dest flag for installer #4
Conversation
|
We need to think about uninstallation process as well, if the package is installed in a location we are not aware, we can not uninstall it. Related task: wasp-lang/wasp#953 (comment) |
| INSTALL_DIR="$HOME" | ||
|
|
||
| while getopts ":d:" opt; do | ||
| case $opt in | ||
| d) INSTALL_DIR="$OPTARG" ;; | ||
| *) echo "Invalid argument: -$OPTARG" >&2; exit 1 ;; | ||
| esac | ||
| done | ||
|
|
||
| # Assign the directories | ||
| HOME_LOCAL_BIN="$INSTALL_DIR/.local/bin" | ||
| HOME_LOCAL_SHARE="$INSTALL_DIR/.local/share" |
There was a problem hiding this comment.
Good idea with the installation destination!
How come you decided to do this, I'm guessing you wanted a global installation option?
Unfortunately, these locations (i.e., .local/something) are not arbitrary - the way we organize our files isn't independent of the top-level installation destination. In other words, we should probably avoid replicating their structure without considering where we're putting them.
Coincidentally, this is exactly what @infomiho and me have been talking about for the last couple of days! Check this comment for the full explanation (it's a whole thing with a long history): wasp-lang/wasp#953 (comment)
Anyway:
- If you just wanted the option to install Wasp globally, the XDG standard tells us exactly what where we should put our files - we can simply add the
-g/--globalflag instead of this one and "hardcode" those paths. - If you wanted to give users full control over where Wasp is installed, we'll discuss how to best approach it.
EDIT: Also, what Miho said here: #4 (comment). It might make sense to postpone doing this in the interest of simplifying the (not-yet-existing) uninstallation process.
There was a problem hiding this comment.
This was intended to make working with online IDEs (Replit, GitPod) a bit easier, as we sometimes need to change the installation destination to get it working correctly on these environements. See: https://github.com/wasp-lang/replit-template/tree/vince-add-replit-template
There was a problem hiding this comment.
Yup, this was per my suggestion, and I didn't think much about uninstalling at that point!
I would suggest the following, to keep it simple: we go with this, but don't tell users about it, so it is mostly just for us and potentially for some power users.
The first version of uninstall, the simple one, will work for everyone that use default paths. It won't work for us and maybe for some very rare power users, but in that case we assume us/them know what they are doing anyway, for now.
Once we do a solution with XDG paths, we can refactor all of this, but then we can do it in one coherent go. I wouldn't let that stop us right now though to get out what we need. But would certainly, in an issue for XDG paths, mention that right now we have an option to install in different than default path, that we need it for replit, and that we should make sure we can still do what we need for replit.
There was a problem hiding this comment.
Nice start @vincanger !
So you ask user for alternative top dir, and then install wasp in it under .local/, and then bin/ or share/ .
That way you construct HOME_LOCAL_BIN and HOME_LOCAL_SHARE, and the rest you didn't touch.
This works fine!
However, as @sodic indicated, you don't really need to preserve this .local structure -> we picked $HOME/.local as nice default location, but if somebody is picking a custom location, no need to have .local be part of it.
What is important here I think is to understand that HOME_LOCAL_SHARE is used in the rest of the script only to define DATA_DST_DIR, while HOME_LOCAL_BIN is used only to define BIN_DST_DIR.
Once that is done, the rest of the script uses BIN_DST_DIR and DATA_DST_DIR to do everything.
So while you focused on customizing HOME_LOCAL_BIN and HOME_LOCAL_SHARE, what we probably want to do instead is focus on customizing DATA_DST_DIR and BIN_DST_DIR -> or at least in that direction.
Notice also that having names HOME_LOCAL_BIN and HOME_LOCAL_SHARE doesn't make much sense if those can also point to something that is not in HOME! Which is now possible, since people can customize it. So that is also smelly.
OK, so let's rethink it.
By default, we like having $HOME/.local/bin and $HOME/.local/share, as locations where wasp stuff is installed. We want to keep that default behaviour.
We also want to let user be able to specify installing wasp somewhere else.
I guess we want them to be able to say where the bin should go, and where the wasp data will go. I see two ways to let them specify that -> either they give us two paths, one for bin and one for data, or they gives us one path, and we create subdirs in it for bin and data.
I think it sounds easier for them to give us one path, and we figure it out from there.
So if they give us one path, let's say $INSTALL_PATH, we could say we will put bin in $INSTALL_PATH/wasp-lang/bin, and data in $INSTALL_PATH/wasp-lang/. Yup, that sounds simple enough. Whatever path they provide, we make sure we have this wasp-lang/ dir to organize wasp stuff in it.
So, this means, by default we want to have:
$HOME/.local/bin
$HOME/.local/share
while if they provide custom path, we might want to go with:
$INSTALL_PATH/wasp-lang/bin
$INSTALL_PATH/wasp-lang
Yup, I think this sounds good.
How would we call these? HOME_LOCAL_BIN and HOME_LOCAL_SHARE are not good names anymore. We could call them WASP_BIN_DIR and WASP_DATA_DIR.
We already have BIN_DST_DIR and DATA_DST_DIR though, so what about those, since names sound very similar? Well, let's forget those for a moment, they probably also need to be fixed.
OK, so we have WASP_BIN_DIR and WASP_DATA_DIR now.
WASP_BIN_DIR we can actually use directly in the rest of the code, as replacement for BIN_DST_DIR -> it is the same thing really.
OK, that leaves only WASP_DATA_DIR vs DATA_DST_DIR. DATA_DST_DIR actually does one more thing -> it scopes by wasp version.
So we could say that DATA_DST_DIR = $WASP_DATA_DIR/$VERSION_TO_INSTALL (since it was "$HOME_LOCAL_SHARE/wasp-lang/$VERSION_TO_INSTALL" so far). If we do that, I think we are all good.
Name is weird though, so let's improve it. It could be $WASP_DATA_DIR_THIS_VERSION. or $WASP_DATA_DIR_CHOSEN_VERSION.
I think that would be it then!
So if we following this, what we would do is:
- Check if user provided install path, if so, save it to $INSTALL_PATH
- Define WASP_BIN_DIR as $HOME/.local/bin by default, or as $INSTALL_PATH/wasp-lang/bin if $INSTALL_PATH is set.
- Define WASP_DATA_DIR as $HOME/.local/share by default, or as $INSTALL_PATH/wasp-lang if $INSTALL_PATH is set.
- Define WASP_DATA_DIR_CURR_VERSION as $WASP_DATA_DIR/$VERSION_TO_INSTALL .
- In the rest of the code, replace DATA_DST_DIR with $WASP_DATA_DIR_CURR_VERSION, and replace BIN_DST_DIR with WASP_BIN_DIR .
That should be it! Then we have nice names and everything makes sense, I think. Notice we wouldn't any more have variables named HOME_LOCAL_BIN, HOME_LOCAL_SHARE, BIN_DST_DIR, and DATA_DST_DIR.
Would you be up for giving this a go? I think it is a good opportunity to practice bash + learning more about installer script!
| # Default install directory | ||
| INSTALL_DIR="$HOME" | ||
|
|
||
| while getopts ":d:" opt; do |
There was a problem hiding this comment.
I believe this part is redundant since you already picked up the argument below, where we also read other arguments?
The worst thing really is -> what if they call installation with installation path set to one location, and then to another location :D? Uff :D! But ok I don't think there is much we can do there :D. Maybe warn them that wasp is already installed or smth. But I think that is too advanced to think about now, we can handle that in the future. |
No description provided.