handle git+ssh with semver#6239
Merged
Merged
Conversation
|
💖 Thanks for opening this pull request! 💖 |
andreineculau
commented
Mar 18, 2023
Contributor
Author
|
offtopic: it is great to see pnpm can install git dependencies without a package.json 🎉 🚀 pnpm is a generic package manager 😱 Is that true? If so, I L O V E I T 😍 🤩 LATER EDIT: not quite apparently |
Member
|
Could you add some tests? |
Contributor
Author
|
I added two tests (SCP-like and regular URL) for semver range. Both are failing on the current version highlighting the two bugs:
|
Member
|
ok, also add a changeset for your changes. |
zkochan
approved these changes
Mar 20, 2023
|
Congrats on merging your first pull request! 🎉🎉🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
5 years later I'm still pestering you with git+ssh+semver @zkochan :)
Currently pnpm fails to install
git+ssh://git@example.com:org/repo.git#semver:~1.2.3(where example.com is an internal git server, not github.com) because it will end up looking for refspec/~1.2.3which doesn't match any git tag. We end up there both because of escaping the colon insemver, but also because we assume git+ssh URL cannot accept semver range spec.Trying to install
https://example.com/org/repo.git#semver:~1.2.3works as expected.Looking through the code, we had quite a few patches to handle SCP-like urls, because we wanted to use
new URL, and not the deprecatedurl.parse.Yarn has this patch https://github.com/yarnpkg/yarn/blob/master/src/util/git.js#L103
but npm https://github.com/npm/git/blob/f60bb15da404679160c4bb6ad1c31a66d5f6ae72/lib/clone.js#L20 was mentioning the shim in https://github.com/npm/hosted-git-info/blob/f03bfbd3022c8f6283a991ff879ed97704ac35fa/lib/parse-url.js#L42
The shim didn't work out, so a similar take as yarn's landed in this PR.