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

Update install.sh #1213

Closed
wants to merge 1 commit into from
Closed

Conversation

PeterDaveHello
Copy link
Collaborator

Clean up origin nvm directory before cloning nvm repository, fix #1212

@@ -75,6 +75,8 @@ install_nvm_from_git() {
exit 1
}
else
echo "=> Cleanup old nvm dir '$INSTALL_DIR'"
command rm -rf $INSTALL_DIR/* $INSTALL_DIR/.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we absoLUTELY do not want to do this - this would delete previously installed node versions, along with all their global modules.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about backup the installed node versions and their modules first and restore after that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a lot of work - it would include all aliases, the "versions" folder, the upcoming cache folder, and all the pre-0.12 versions that are installed in the root.

If the issue is that installation is complaining about an existing directory, let's change that so it's not a problem, rather than removing the directory.

@PeterDaveHello
Copy link
Collaborator Author

@ljharb this should work now.

command git clone "$(nvm_source)" "$INSTALL_DIR" || {
command git init "$INSTALL_DIR"
command git --git-dir="$INSTALL_DIR"/.git remote add origin "$(nvm_source)"
command git --git-dir="$INSTALL_DIR"/.git fetch --tags
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do git fetch origin --tags here, in case someone (like me) has multiple sources set up in their global git config.

Clean up origin nvm directory before cloning nvm repository, fix nvm-sh#1212
@ljharb ljharb closed this in 68bf935 Aug 25, 2016
@ljharb
Copy link
Member

ljharb commented Aug 25, 2016

Thanks! I ended up going with 68bf935 thanks to your inspiration.

@PeterDaveHello
Copy link
Collaborator Author

PeterDaveHello commented Aug 25, 2016

Hey ... why not make this PR work? I don't think it's a good idea to do another fix to close the exist one, I'm not leaving this PR without any response. I meant, no offense, of course you can write better codes, but that's not how open source community work, especially this problem is not in emergency, it's even been a while.

@ljharb
Copy link
Member

ljharb commented Aug 25, 2016

@PeterDaveHello I apologize if I offended - it wasn't about "writing better codes", I just was able to reproduce it, and got a fix in. I'm hoping to get fixes in quickly to avoid churn for the install rewrite PR. I appreciate your contributions!

@PeterDaveHello
Copy link
Collaborator Author

Yeah I can understand your concerns, but as you know, this is not the first day we got the bug, I don't think it's emergency enough to take a working in process PR down with another one, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nvm upgrade failed
2 participants