Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Set VIRTUAL_ENV enviroment variable in activate_this.py #1057

Merged
merged 6 commits into from
Dec 24, 2018
Merged

Set VIRTUAL_ENV enviroment variable in activate_this.py #1057

merged 6 commits into from
Dec 24, 2018

Conversation

nonylene
Copy link

VIRTUAL_ENV was not set in activate_this.py, while set in other activate scripts.

@nonylene nonylene changed the title Set VIRTUAL_ENV enviroment in activate_this.py Set VIRTUAL_ENV enviroment variable in activate_this.py Jun 13, 2017
@nonylene nonylene mentioned this pull request Oct 5, 2017
Closed
@tgrant59
Copy link

tgrant59 commented Oct 5, 2017

👍

ssbarnea
ssbarnea suggested changes Oct 6, 2017
View reviewed changes
Copy link
Contributor

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please refresh PR?

@nonylene
Copy link
Author

nonylene commented Oct 6, 2017

Rebased against current master and ran bin/rebuild_script.py just now. (No changed)

@nonylene
Copy link
Author

nonylene commented Oct 8, 2017

@pfmoore How about this pr?

@tgrant59
Copy link

@ssbarnea @gst This bug is causing us issues, do you think it could be merged soon?

@jcox-dev
Copy link

This would be nice to have in.

jcox-dev pushed a commit to ONSdigital/eq-survey-runner that referenced this pull request Oct 24, 2017
Remove virtual env warning from app script
8377e4c 
virtualenv does not currently set the VIRTUAL_ENV environment variable when activate_this is used (pypa/virtualenv#1057). pipenv run uses activate_this rather than one of the other activation scripts. We should also not care about reporting a lack of virtual environment now that we use Pipenv.
@jcox-dev jcox-dev mentioned this pull request Oct 24, 2017
Merged
jcox-dev pushed a commit to ONSdigital/eq-survey-runner that referenced this pull request Oct 24, 2017
Remove virtual env warning from app script
2ec6681 
virtualenv does not currently set the VIRTUAL_ENV environment variable when activate_this is used (pypa/virtualenv#1057). pipenv run uses activate_this rather than one of the other activation scripts. We should also not care about reporting a lack of virtual environment now that we use Pipenv.
@hai-ld
Copy link

hai-ld commented Nov 27, 2017

Could this PR be merged soon?

xavfernandez
xavfernandez approved these changes Nov 27, 2017
View reviewed changes
Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A changelog would be nice 👍

@ssbarnea
Copy link
Contributor

@xavfernandez Thanks! I hope I will see more merges performed as current state of virtualenv is "kept in carbonite".

nonylene added a commit to nonylene/virtualenv that referenced this pull request Nov 27, 2017
@nonylene
Update changes.rst
86970de 
pypa#1057
@nonylene
Copy link
Author

@xavfernandez Thank you for response! I added a changelog in 86970de.

@nonylene
Copy link
Author

CI failed, I'm checking now 👀

@nonylene
Copy link
Author

nonylene commented Nov 27, 2017 •
edited
Loading 

setuptools requires Python '>=2.7,!=3.0.*,!=3.1.*,!=3.2.*' but the running Python is 2.6.6
ERROR: could not install deps [mock, pytest]; v = InvocationError('C:\\projects\\virtualenv\\.tox\\py\\Scripts\\pip.EXE install mock pytest (see C:\\projects\\virtualenv\\.tox\\py\\log\\py-1.log)', 1)

Oh, this is because setuptools dropped Python 2.6 support... pypa/setuptools#878

This patch is not relevant to that issue (should be fixed in another PR), so ready to merge!

@pradyunsg
Copy link
Member

You might want to merge master to fix the failing CI.

@nonylene
Copy link
Author

nonylene commented Nov 28, 2017 •
edited
Loading

You might want to merge master to fix the failing CI.

Failed from the same reason. pytest also dropped 2.6 support today.

pytest-dev/pytest#2945

@uranusjr uranusjr mentioned this pull request Feb 22, 2018
Closed
@twig
Copy link

twig commented May 11, 2018

This bug causes newrelic-plugin-agent setup.py to fail because VIRTUAL_ENV isn't set when installing via pipenv

What's preventing this from being released?

@nonylene nonylene force-pushed the set-virtual_env-env-activate_this branch from 8f435b1 to dd5dd56 Compare November 14, 2018 06:49
@nonylene
Copy link
Author

nonylene commented Nov 14, 2018 •
edited
Loading

I resolved conflicts and updated with ./bin/rebuild-script.py.

How about this pr? @gaborbernat

gaborbernat
gaborbernat requested changes Nov 14, 2018
View reviewed changes
Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like some tests for this to prevent regression in the future.

@nonylene nonylene force-pushed the set-virtual_env-env-activate_this branch from dd5dd56 to 6338cb8 Compare November 20, 2018 06:01
@nonylene
Copy link
Author

nonylene commented Nov 20, 2018 •
edited
Loading

Regression test added: 6338cb8
Confirmed in Python 3.5.3 / 2.7.11 from Debian stretch.

(... and I've changed base commit to remove redundant merge commits.)

gaborbernat
gaborbernat requested changes Nov 20, 2018
View reviewed changes
Copy link
Contributor

@gaborbernat gaborbernat left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nm

@nonylene
Copy link
Author

Sorry but what does 'nm' mean?

@gaborbernat
Copy link
Contributor

never mind 😀

@gaborbernat gaborbernat self-assigned this Dec 16, 2018
@gaborbernat gaborbernat force-pushed the set-virtual_env-env-activate_this branch from efb1540 to 6630913 Compare December 23, 2018 20:15
@gaborbernat gaborbernat force-pushed the set-virtual_env-env-activate_this branch 5 times, most recently from 8f9be9d to db22eed Compare December 23, 2018 21:40
@gaborbernat gaborbernat force-pushed the set-virtual_env-env-activate_this branch from db22eed to f94024d Compare December 23, 2018 21:42
@gaborbernat gaborbernat force-pushed the set-virtual_env-env-activate_this branch from 76a3886 to da204fb Compare December 24, 2018 04:46
@gaborbernat
Copy link
Contributor

This now passes the bar of merge-ability. Thanks everyone for the patience, this will be part of the early January release.

@gaborbernat gaborbernat merged commit 971d341 into pypa:master Dec 24, 2018
@nonylene
Copy link
Author

Thank you!

@nonylene nonylene deleted the set-virtual_env-env-activate_this branch December 24, 2018 05:41
@gaborbernat gaborbernat added this to the 16.2 milestone Dec 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaborbernat gaborbernat gaborbernat requested changes

@gst gst gst approved these changes

@ssbarnea ssbarnea ssbarnea approved these changes

@xavfernandez xavfernandez xavfernandez approved these changes

Assignees

@gaborbernat gaborbernat

Labels
None yet
Projects
None yet
Milestone
16.2
Development

Successfully merging this pull request may close these issues.
10 participants
@nonylene @tgrant59 @jcox-dev @hai-ld @ssbarnea @pradyunsg @twig @gaborbernat @gst @xavfernandez