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.

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

Associate project dir with new virtual environment #1861

Merged
merged 4 commits into from
May 17, 2018

Conversation

meshy
Copy link
Contributor

@meshy meshy commented Mar 27, 2018

This adds a flag to pew that lets it know which directory to switch to when activating an environment with pew workon foo.

I really miss this feature from virtualenvwrapper, and I'd love still use this workflow, so if I've gone about this the wrong way, please let me know, and I'll see what I can do to make it right.

This fits the workflow I'm used to, but I understand it might impact other people's workflows.

Because of that, this may be blocked waiting for the PR on pew to add an option to prevent the directory changing when running pew workon, as that will offer a way to restore the existing behaviour.
Update: That's been merged now.

I wasn't sure where to add tests for this particular functionality, but I'm not entirely familiar with pytest, so I might have missed something.

If there's a place you think is appropriate for the tests, or a better way to achieve this, please let me know.

@meshy meshy force-pushed the pew-associate-project-dir branch from d4216cd to d7335c2 Compare March 27, 2018 19:46
meshy added a commit to meshy/dotfiles that referenced this pull request Mar 28, 2018
This includes a workaround for pypa/pipenv#1861
that lets `pew workon foo` to change directory when invoked.

Related to 0dd7b51.
@techalchemy
Copy link
Member

Hey @meshy thanks for putting htis together, sorry for the delay! We had some issues with our testing infrastructure recently and we are just getting that sorted. Once that's straightened out I'd say we can merge this as a bolt-on for now. We can sort out the testing bit if we ever actually build out the feature in pipenv.

/cc @pfmoore #1824

@meshy
Copy link
Contributor Author

meshy commented Mar 30, 2018

Hey @techalchemy, no worries on the delay :)

I'm delighted you're happy with the concept of merging this. Thanks very much!

@techalchemy
Copy link
Member

plz update onto master and we can resume work

@meshy meshy force-pushed the pew-associate-project-dir branch from d7335c2 to 9464c3b Compare April 7, 2018 21:22
@meshy
Copy link
Contributor Author

meshy commented Apr 7, 2018

No worries. I've just rebased, and repushed.

@uranusjr
Copy link
Member

uranusjr commented May 3, 2018

Closing in favour of Pipes for the moment. Take a look! #1824 (comment)

@uranusjr uranusjr closed this May 3, 2018
@meshy
Copy link
Contributor Author

meshy commented May 3, 2018

Thanks for pointing me in the direction of Pipes -- it does look very nice, but I'm concerned about moving to a project that's only a little over a day old. I'll keep an eye on it though.

At the moment, my work-around for this is to make the following call immediately after creating the virtualenv:

# Create the venv.
pipenv install --three

# Work-around for https://github.com/pypa/pipenv/pull/1861
echo $(pwd) > $(pipenv --venv)/.project

@meshy meshy deleted the pew-associate-project-dir branch May 3, 2018 07:19
@meshy
Copy link
Contributor Author

meshy commented May 3, 2018

FWIW, this PR solves the issue that the Pipes author mentions in their follow-up tweet.

The --link approach is kind of nasty but I could not think of another way to detect & tie envs to project dirs.

It would be helpful if pipenv stored the project path or provided a way to resolve it
https://twitter.com/gtalarico/status/991908875371675648

@meshy meshy restored the pew-associate-project-dir branch May 3, 2018 07:22
meshy added a commit to meshy/pipenv-pipes that referenced this pull request May 3, 2018
I have a slightly strange case where I'm generating the `.project` file
myself using:

    echo $(pwd) > $(pipenv --venv)/.project

This is to work around pypa/pipenv#1861

Unfortunately, this adds a newline char to the end of the project path,
and ultimately, activating the project with `Pipes` fails, where `pew`
succeeds.

The error I get is:

    FileNotFoundError: [Errno 2] No such file or directory: '/home/charlie/code/my-project\n': '/home/charlie/code/my-project\n'
@erinxocon erinxocon reopened this May 3, 2018
@erinxocon
Copy link
Contributor

@meshy can you pull from master and rebase?

@uranusjr uranusjr added the PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge. label May 3, 2018
This lets pew know which directory to switch to when activating an
environment with `pew workon foo`.
@meshy meshy force-pushed the pew-associate-project-dir branch from 5d28ac3 to a35b157 Compare May 3, 2018 16:15
@meshy
Copy link
Contributor Author

meshy commented May 3, 2018

@erinxocon rebased :)

@uranusjr
Copy link
Member

uranusjr commented May 3, 2018

The tests failures are unrelated, but I think it’s best to fix them first before merging. We can’t have a release until everything’s back working anyway…

@gtalarico
Copy link
Contributor

gtalarico commented May 4, 2018

FWIW, this PR solves the issue that the Pipes author mentions in their follow-up tweet.
The --link approach is kind of nasty but I could not think of another way to detect & tie envs to project dirs.

Hi @meshy - How would this PR solve the issue?
Pipes needs a way to resolve - from a pipenv folder - which project directory the env was created from.
Would these additional pew flags create a .project file in the env folder?

@meshy
Copy link
Contributor Author

meshy commented May 4, 2018

@gtalarico Hey!

Will the additional pew flags create a .project file in the env folder?

Yes, exactly :)

@uranusjr uranusjr merged commit ccf02f5 into pypa:master May 17, 2018
@meshy
Copy link
Contributor Author

meshy commented May 17, 2018

Thanks!

@meshy meshy deleted the pew-associate-project-dir branch May 17, 2018 10:45
techalchemy added a commit that referenced this pull request Jun 24, 2018
Signed-off-by: Dan Ryan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: awaiting-merge The PR related to this issue has been reviewed and is awaiting merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants