New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to `src` layout breaks documented "use locally from source" installation method #1241

Closed
evansd opened this Issue Nov 2, 2018 · 10 comments

Comments

Projects
None yet
4 participants
@evansd
Copy link

evansd commented Nov 2, 2018

Moving virtualenv.py from the project root into a src subdirectory breaks the documented method for running virtualenv from an extracted tarball without installing it.

From a brief test, simply creating a symlink from the old location to the new fixes the issue. But I suppose this won't work on Windows. I wonder if you could create a small shim Python script at the old location which just invokes the script at the new location.

From what I can tell, #1240 is also caused by this change.

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Nov 2, 2018

We need to update the documentation here indeed, no code change should happen/needed.

@jupe

This comment has been minimized.

Copy link

jupe commented Nov 2, 2018

I understand now why #1240 happens, but if you change documented behaviour its then breaking change and major version should be updated. Please could you bring backward compatibility for current v16.x series because otherwise this causes a lot of headache for us - we have to update several dependant modules and new releases for them....

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Nov 2, 2018

Well then, this is unexpected. This documented, but otherwise untested interface. Sadly bringing back API compatibility would be harder and worth it, so I'm thinking of just making a major version change. Can you pin your tool to at most 16.0? We no longer plan to release and 16.x releases.

@jupe

This comment has been minimized.

Copy link

jupe commented Nov 3, 2018

Can you pin your tool to at most 16.0?

yes we can and did it already here: ARMmbed/node-virtualenv#2 but it doesn’t change that unfortunately fact that existing releases are already broken because of this and we cannot fix them without new releases.. Note that we have been using this tool heavily multiple years without any issues until now so any chance to bring back virtualenv.py to root folder which just import required files from src folder? wouldn’t it do the job? Let me know asap so we know which direction we have go..

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Nov 3, 2018

I would avoid if possible to move it back. And if needed only as a patch release for 16.2. And remove it permanently from root in 17+, which would quickly follow it. Going ahead you should presume it's under source. I don't follow why you need to upgrade existing releases. People wanting to use latest virtualenv should upgrade to your latest version that no longer tries to read from root. I'll add a test to ensure we don't unknowingly break documented installation methods.

@jupe

This comment has been minimized.

Copy link

jupe commented Nov 3, 2018

patch release 16.1.1 is enough for us. 17+ release can break this ”officially” of course..

note that node-virtualenv is just one example which depends on this documented file directly. There is probably more. If that file disappears it is breaking change which might cost a lot when products/tools unexpectedly start failing during deployment phase. Its even more complicated when breaking change happens in dependency dependence like now in our case. Anyway, patch release would be more than welcome but we can survive without it even it makes some work for us..

nixjdm added a commit to lektor/lektor-website that referenced this issue Dec 4, 2018

@Ivoz

This comment has been minimized.

Copy link
Member

Ivoz commented Dec 23, 2018

I think you jumped the gun here @gaborbernat . Using virtualenv.py (with/without needing to install virualenv) has been documented, and used by a small number of project for years, and removing this usage in both 1) a minor bugfix verison 2) with no documentation 3) with no deprecation notice is unwarranted, to say the least.

If nothing else I would suggest adding back a "stub" file to enable the same functionality in a 16.x release

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Dec 23, 2018

We've noted and we'll release a fix after Christmas. Going ahead from next version we'll deprecate this though and encourage people to extract the wheel if they want to use it without installing it. The reason why this did not got noticed before is that although it's documented, there's no test for it to guard against regression.

@gaborbernat gaborbernat self-assigned this Dec 24, 2018

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

revert to a non src layout, fixes pypa#1241
local doc also uses sphinx rtd theme

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

revert to a non src layout, fixes pypa#1241
local doc also uses sphinx rtd theme

@gaborbernat gaborbernat added this to the 16.2 milestone Dec 24, 2018

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

revert to a non src layout, fixes pypa#1241
local doc also uses sphinx rtd theme

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

revert to a non src layout, fixes pypa#1241
local doc also uses sphinx rtd theme

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

revert to a non src layout, fixes pypa#1241
local doc also uses sphinx rtd theme

gaborbernat added a commit to gaborbernat/virtualenv that referenced this issue Dec 24, 2018

revert to a non src layout, fixes pypa#1241
local doc also uses sphinx rtd theme

gaborbernat added a commit that referenced this issue Dec 25, 2018

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Dec 25, 2018

This now is fixed on master and will be released with 16.2 (probably on 31st).

@gaborbernat

This comment has been minimized.

Copy link
Contributor

gaborbernat commented Jan 1, 2019

Released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment