Skip to content

install.sh: respect git config --global clone.defaultRemoteName#3548

Draft
ryan-williams wants to merge 3 commits into
nvm-sh:masterfrom
ryan-williams:install
Draft

install.sh: respect git config --global clone.defaultRemoteName#3548
ryan-williams wants to merge 3 commits into
nvm-sh:masterfrom
ryan-williams:install

Conversation

@ryan-williams

Copy link
Copy Markdown

Fixes #3547

Uses git config --global clone.defaultRemoteName, if it is set, otherwise falls back to origin.

@ljharb ljharb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add some tests?

Comment thread install.sh Outdated
Comment thread install.sh Outdated
ryan-williams and others added 2 commits March 13, 2025 08:39
Co-authored-by: Jordan Harband <ljharb@gmail.com>

@ryan-williams ryan-williams left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks. I looked at install_nvm_from_git but am not sure how I should test this there. Would a GHA be a better place to test it?

Also it looks like the 2 GHAs that failed are also failing on master, so I'm assuming I can ignore those here.

Comment thread install.sh Outdated
@ryan-williams ryan-williams requested a review from ljharb March 13, 2025 12:53
@ljharb

ljharb commented Mar 13, 2025

Copy link
Copy Markdown
Member

Yes, GHA is the better place to add new tests, and yes, those v0.40.0 tests will fail forever and that's ok :-)

@Snapp949 Snapp949 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@ljharb ljharb marked this pull request as draft April 4, 2025 17:53
@ryan-williams

Copy link
Copy Markdown
Author

I can make a standalone GHA that tests this, is that what you have in mind @ljharb?

I looked at the existing tests a bit last month, but couldn't get oriented.

@ljharb

ljharb commented Apr 7, 2025

Copy link
Copy Markdown
Member

It'd be best to add it inside the existing test suite

XProudfootX

This comment was marked as spam.

XProudfootX

This comment was marked as spam.

@Amirrznjd Amirrznjd mentioned this pull request May 10, 2025
z-jpg

This comment was marked as spam.

@LemmingAvalanche

Copy link
Copy Markdown

I just encountered this problem. Thanks for fixing.

My idea was to force the clone to use origin. Then nothing else needs to change I guess?

command git clone --origin=origin "$(nvm_source)" --depth=1 "${INSTALL_DIR}"

@ljharb

ljharb commented Jul 3, 2025

Copy link
Copy Markdown
Member

@LemmingAvalanche what will that do on a git version that doesn't support --origin?

@LemmingAvalanche

Copy link
Copy Markdown

@LemmingAvalanche what will that do on a git version that doesn't support --origin?

--origin has been part of git(1) since 1.4.4. --depth has been part of git(1) since 1.5.0.

But you can override the config variable instead of using --origin.

command git -c clone.defaultRemoteName=origin clone "$(nvm_source)" --depth=1 "${INSTALL_DIR}"

But clone.defaultRemoteName is newer: since 2.30.0.

@ljharb

ljharb commented Jul 4, 2025

Copy link
Copy Markdown
Member

Thanks! We can use --origin then, that seems like a great change for this PR. If you'd like to do that, please don't open a new PR, just comment here with a branch link and i'll pull in the changes.

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.

install.sh fails with non-default git config clone.defaultRemoteName

6 participants