deps(git): update git-url-parse

[dkhaye]

What's the problem this PR addresses?
Resolves #4623

...

How did you fix it?
Upgrade to git-url-parse@^12.0.0 within @yarnpkg/plugin-git

...

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[merceyz]

Looks like that version is a bit buggy so I'll put this on hold for now.

[mhassan1]

Based on the result of IonicaBizau/parse-url#46, it seems like git://github.com:user/repo will no longer be supported in git-url-parse@12 and higher. It seems like that could be a breaking change for Yarn users.

It seems like we are only using git-url-parse here and here for:

httpUtils.getNetworkSettings(`https://${GitUrlParse(normalizedRepoUrl).resource}`, {configuration})

Maybe we can remove our dependency on this package completely, since we are already doing URL normalization in normalizeRepoUrl.

[mhassan1]

I added a couple of test cases locally in gitUtils.test.ts:

  it(`should properly normalize ${pattern} ({ git: false })`, () => {
    expect(gitUtils.normalizeRepoUrl(pattern)).toMatchSnapshot();
+   expect(GitUrlParse(gitUtils.normalizeRepoUrl(pattern)).resource).toMatchSnapshot();
  });
  it(`should properly normalize ${pattern} ({ git: true })`, () => {
    expect(gitUtils.normalizeRepoUrl(pattern, {git: true})).toMatchSnapshot();
+   expect(GitUrlParse(gitUtils.normalizeRepoUrl(pattern, {git: true})).resource).toMatchSnapshot();
  });

The snapshots are the same between git-url-parse@11 and git-url-parse@12. So at least none of the test patterns are affected by the upgrade.

[mhassan1]

The reported change in behavior in git-url-parse has been closed as not a problem. It seems like Yarn users should not be affected by it.

[dkhaye]

Looks like that version is a bit buggy so I'll put this on hold for now.

@merceyz Can you look at this again?

As indicated by @mhassan1 here, the issue was closed as not being a problem.

For good measure, I went ahead and updated to the latest, 13.1.0.
There is another possible issue, but as indicated by @mhassan1 here this is not relevant to how this package is used within yarn.

[mhassan1]

@dkhaye It looks like there are conflicts now.

[dkhaye]

@mhassan1 re-merged master
@merceyz PTAL

[mhassan1]

@dkhaye It looks like there are conflicts again.

[dkhaye]

@mhassan1 Thanks again.
For posterity, my method of resolving conflicts on this branch has been:

git pull origin master
git checkout origin/master -- .yarn/cache .pnp.cjs yarn.lock
yarn install
yarn dedupe

@merceyz and @arcanis any chance this can get re-reviewed?

[arcanis]

Sorry for the delay, the tests pass so I think we can merge it.

Note however that as a general policy, I tend to avoid auto-updating dependencies on this repository unless they actually fix something in Yarn (under the premise that blind upgrades are just as risky if not more, as we bring untested and often unneeded new code). The issue reported in #4623 isn't attached to an actual exploitation showcase against Yarn, so upgrading git-url-parse wouldn't seem useful except for the benefit of third-party services like Snyk.

Still, I appreciate you taking time to address an open ticket!