bypass aliased curl

[ryenus]

Use command curl add nvm_curl so that we can bypass aliased curl.

Context: #2923 (comment)

[ljharb]

With these changes, there's only two places nvm_curl is needed, which suggests that inlining command may be a better choice?

if we keep nvm_curl, we need to remember to unset it in deactivate. also, there may be tests mocking curl that need to now mock nvm_curl.

[ryenus]

With these changes, there's only two places nvm_curl is needed, which suggests that inlining command may be a better choice?

if we keep nvm_curl, we need to remember to unset it in deactivate. also, there may be tests mocking curl that need to now mock nvm_curl.

Indeed, with only 2 places need the -q option I agree it's simpler to avoid the nvm_curl function, which also make testing less complicated. I've updated the PR accordingly.

[ljharb]

Testa are failing on master, but I’d still expect this change to cause more tests to fail, due to their method of mocking curl being defeated by the PR’s change itself.

[ryenus]

Testa are failing on master

Looking at the failed test here: https://app.travis-ci.com/github/nvm-sh/nvm/jobs/586754000

✗ nvm_list_aliases works with LTS aliases
expected lts/gallium, got >lts/hydrogen<

I believe it's because of outdated test data. hopefully it can be fixed by #2933

[ljharb]

Unfortunately it’s much more difficult than that; xenial can’t build node 18 and newer Ubuntus can’t build node 0.6.

[jordyjwilliams]

Checked, all looks as if will work as expected and resolves #2923

[jordyjwilliams]

Running the edited command curl for me does not log the edited one.

[ljharb]

Got tests passing on master and rebased this.

[ljharb]

@ryenus tests are still failing - specifically the ones that mock curl. are you interested in completing this PR? if not, please leave it open so I (or another contributor) can give it a shot :-)

[ryenus]

@ljharb I just realized that the test for nvm_curl_libz_support assumes that curl can be redefined as a shell function, therefore I'm keeping it as-is.

[ljharb]

@ryenus indeed - so i think that with this PR's change, to keep that test passing, we'd have to make the test instead mock out the PATH and provide an actual fake curl binary.

[ryenus]

This causes https://app.travis-ci.com/github/nvm-sh/nvm/jobs/592909538#L547

    nvm_get_latest
    ✗ nvm_get_latest
success path did not return version 'v12.3.456', got 'v0.39.3'
    ✗ nvm_get_latest failed redirect
failed redirect did not report correct error message, got 'v0.39.3'

[ryenus]

so i think that with this PR's change, to keep that test passing, we'd have to make the test instead mock out the PATH and provide an actual fake curl binary.

@ljharb you're right, though IMHO it's a bit too much, as I think in the original issue it's likely a problem with the specific alias definition, not that the curl command cannot be aliased. If that's fine I'd like to close the pr instead.

[ljharb]

Let's keep it open; I'll try to get back to it and finish it.