Introduce copy METHOD for installation #580
Conversation
This method copies the folder containing install.sh (normally also containing the repo) to - unless of course the install.sh being executed is in (where nvm was installed) This is useful for vagrant to test local changes instead of pulling them from git and thus testing what's on the server and not local changes
|
Why can't this just use git and |
|
I didn't notice NVM_SOURCE, but in any case this PR was split (as requested) from the vagrant PR. I used it to install (copy) my working directory into the virtual machine. "git clone" wouldn't have my changes or even the stashed changes. |
|
It would if they were committed to the local repo. I'm not sure I understand the use case here, and I'm hesitant to add new functionality that doesn't seem to offer anything over what's already here. |
|
I'm not sure what the normal workflow is, but before I commit I try to make things work. Without the copy installation one would have to make changes, commit, test in the VM and if it doesn't work, revert and restart the cycle. With the copy method, you make a change and test in the VM. |
| SOURCE="$(readlink "$SOURCE")" | ||
| [[ $SOURCE != /* ]] && SOURCE="$DIR/$SOURCE" # if $SOURCE was a relative symlink, we need to resolve it relative to the path where the symlink file was located | ||
| done | ||
| DIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )" |
There was a problem hiding this comment.
Can you explain again why this above block of code is needed?
If needed, can it be put into a function? Is there nothing built into the shell for this?
There was a problem hiding this comment.
That's just to "cover our bases" when getting the location of install.sh. Without dereferencing, if the script were linked somewhere, it would get the location of the linked script, not the "true" source.
I was thinking about putting it into a function, but I haven't tested that yet. Might be possible.
Didn't find a builtin, that does that. What you see here is from a stackoverflow response. Maybe I should attribute the author?
There was a problem hiding this comment.
Can you elaborate a bit on the problems that would be caused by getting the location of the linked script versus the non-linked path? It seems like the presence of the link shouldn't affect the copy - ie, it would still work with the linked path.
A comment with a link to the SO answer would indeed be helpful. Using a function makes it very clear that the sole purpose is to output a directory path and that you're not intending to rely on other side effects :-)
There was a problem hiding this comment.
Assume we have this scenario. Internal company nvm version with customizations.
root@localhost:~> cd /opt
root@localhost:/opt> git clone git@github.com:LoveIsGrief/nvm.git nvm # or git clone https://github.com/creationix/nvm.git
root@localhost:/opt> ln -s /opt/nvm/install.sh /usr/local/bin/nvm_install
user@localhost:~> nvm_install # Without symlink deref = cp /usr/local/bin $NVM_DIR
user@localhost:~> nvm_install # With symlink deref = cp /opt/nvm/ $NVM_DIRIt seems like the presence of the link shouldn't affect the copy - ie, it would still work with the linked path.
Hmm... do you mean something like this?
> # Folders:
> find -name install.sh -exec ls -l "{}" \;
lrwxrwxrwx 1 ubuntu ubuntu 21 nov. 23 12:02 ./old_nvm/install.sh -> ../new_nvm/install.sh
-rw-rw-r-- 1 ubuntu ubuntu 0 nov. 23 11:59 ./new_nvm/install.sh
> cd old_nvm && ./install.sh # Still installs new nvmIs that your concern?
|
ok, i think i can get behind the development use case here. The symlink stuff concerns me. Splitting this PR even further (ie, "copy" first, "symlink handling" later) might be a good idea too? |
definition on one line initialization on another
|
I believe this PR would be incomplete with the symlink stuff. I'm not even sure how it would be split. |
/bin/sh doesn't like it...
|
Sorry if I'm not getting this - what does this PR achieve that |
|
Your same question could be asked, were there an install.sh that just did the post-install stuff (profiles) and somebody added the git or wget method. They too could be one-liners e.g So, I'm not really sure how to respond to that. This adds exactly that option under the hood - just like the option is given to use git or wget. It was originally in the vagrant PR and I could have written all that in the provisioning script, but figured it could also be useful as an install option. |
|
Sure, I'm just trying to make sure I understand it fully. The need to dereference symlinks worries me. The reason curl/wget and git are options is because URLs and a series of commands are difficult to remember and easy to get wrong - a single copy command and a single install script isn't particularly difficult, and I'm still not sure it warrants being added as an install option. Thanks for being responsive on this; I'll continue thinking about it. |
|
No problem :) Your code review is pretty thorough. Cheers |
ghost
closed this
ghost
deleted the
This method copies the folder containing install.sh (normally also containing the repo) to - unless of course the install.sh being executed is in (where nvm was installed)
This is useful for vagrant to test local changes instead of pulling them from git and thus testing what's on the server and not local changes
As requested, here is a seperate PR for the copy method for installation.