-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[Fix] specify 'origin' remote name with git clone ...
installs
#3341
base: master
Are you sure you want to change the base?
Conversation
Thanks, this is a great catch! Unfortunately, i believe git didn't add this option until v2.29, and we currently only require git v1.7.10. Would there be an alternative approach where instead of hardcoding "origin", we derive the default remote name, and just use that throughout the install script? |
Hmm. To my knowledge, there is no real default remote after the repo is created. I don't believe A couple of ways to deal with the issue that come to mind are just using the Then using Alternatively, the script could I think the first option is likely the better choice. It should work for any version, be localized to a single code location, and should always result in the expected 'origin' setup. |
96cdb38
to
5d2a10d
Compare
I pushed a modified version. |
git docs say |
Yep, but that's only really good at the time of initial cloning. |
ah, so you're saying that that would work for the initial clone, but wouldn't help with future git operations? wouldn't we be able to assume that the default, if configured, is unlikely to change? |
I added references and corrected the mentioned versions to my second post. And, in looking at them again, it looks like I could just simply change it to use |
If some user changed it, I think it's actually more likely to change in the future. And the change will be external and unknown as a change to the repo. I think that setting it up with the expected 'origin' name from the start is going to be much more robust. Much less likely that a user would go into the repo and rename it than rethinking the config choice. Personally, I had 'main' for a while, then went with 'local' on my home PC because I have a lot of customized soft-forks. But, I'm likely a 0.1%-er here (or even 0.001%-er 😄). |
`git` may be configured locally to use a non-'origin' default remote name. So, specify 'origin' as the remote name when cloning to get the expected setup.
alrighty, let's go with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can there be any install tests that cover this?
@@ -163,7 +163,8 @@ install_nvm_from_git() { | |||
} | |||
else | |||
# Cloning repo | |||
command git clone "$(nvm_source)" --depth=1 "${INSTALL_DIR}" || { | |||
command git clone "$(nvm_source)" --depth=1 -o origin "${INSTALL_DIR}" 2> /dev/null \ | |||
|| command git clone "$(nvm_source)" --depth=1 "${INSTALL_DIR}" || { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|| command git clone "$(nvm_source)" --depth=1 "${INSTALL_DIR}" || { | |
|| command git clone "$(nvm_source)" --depth=1 -o origin "${INSTALL_DIR}" || { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just shrink it to the single command/line since at least -o origin
looks like it'll work with the version restriction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is that the first one suppresses stderr; it's intentional to have the command twice so that when it fails, it tries again and prints out stderr for debugging purposes.
Sure. I'll take a look at your tests and try to work up a something that fits. I think I should be able to add something to install_nvm_from_git. Are you testing with the minimum git version? I don't see that anywhere. |
No, git version is just an implicit requirement. |
c6cfc3a
to
c20db2a
Compare
Since
git
may be configured locally to use a non-'origin' default remote name, specify 'origin' as the remote name when cloning to get the expected local repo setup.