Remove svn handling in unpacking.unpack_file

[chrahunt]

There are two code paths that lead to unpacking.unpack_file:

1. download.unpack_url -> {download.unpack_http_url, download.unpack_file_url} -> unpacking.unpack_file
2. wheel.build -> download.unpack_file_url -> unpacking.unpack_file

In 1, we first check whether the provided link in a VCS URL and if so we call vcs_backend.unpack directly.

In 2, we only pass in the wheel file itself - no chance the removed code is called.

[chrahunt]

Actually, I think this would be called when a URL mapped to an SVN repository, but an explicit svn+ was not provided. This is not documented anywhere, so I propose to remove the behavior and direct users to use an explicit prefix.

[pradyunsg]

Yay! Dead code.

[xavfernandez]

Hmm, even if it wasn't documented, given the wide usage of pip, it's quite credible that some user somewhere was actually relying on this ^^
I would have advocated to go through the usual deprecation cycle (even for a short 3 months period).

Since it's already merged and the impacted use case is certainly quite rare and the workaround quite simple (adding the svn+ prefix), a revert seems overkill but we might want to document this workaround in the removal changelog ?

[chrahunt]

Yes that makes sense. Currently the removal wording is:

Remove undocumented support for http:// requirements pointing to SVN repositories.

How does this sound?

Remove undocumented support for un-prefixed URL requirements pointing to SVN repositories. Users relying on this can get the original behavior by prefixing their URL with svn+ (which is backwards-compatible).

[xavfernandez]

How does this sound?

Remove undocumented support for un-prefixed URL requirements pointing to SVN repositories. Users relying on this can get the original behavior by prefixing their URL with svn+ (which is backwards-compatible).

Pretty good 👍

[pradyunsg]

a revert seems overkill

Reverts are cheap. I'd say we shouldn't care whether a feature is in master but rather whether it's been released, when deciding about deprecation stuff.

---

This is super edge-casey but yea, I'm on board to do a single-release deprecation then removal. I don't think it's needed but I'm not gonna crib or anything, if we do have it. :)

[xavfernandez]

Reverts are cheap. I'd say we shouldn't care whether a feature is in master but rather whether it's been released, when deciding about deprecation stuff.

I completely agree but since other PRs have been merged that touch the same code area, it might not be (completely) trivial.

And since we all agree that the use case is pretty niche, is undocumented and has an easy and clean workaround, I don't think it is worth it so let's move forward :)