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

PythonFull improvements #4495

Merged
merged 3 commits into from
Oct 16, 2014
Merged

PythonFull improvements #4495

merged 3 commits into from
Oct 16, 2014

Conversation

domenkozar
Copy link
Member

No description provided.

@domenkozar
Copy link
Member Author

Ready for review.

@peti
Copy link
Member

peti commented Oct 12, 2014

Wow, there are a lot of changes ...

@@ -108,18 +115,16 @@ let
, internalName ? "_" + moduleName
, deps
}:
stdenv.mkDerivation rec {
if (includeModules) then null else stdenv.mkDerivation rec {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe those parens are not required.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@aristidb
Copy link
Contributor

What exactly is pythonFullWithPkgs?

@domenkozar
Copy link
Member Author

pythonFullWithPkgs could be called also something like pythonFullBundle, you can specify what packages environment would include and get a python interpreter with all those packages. It uses buildEnv internally. Previously it was called pythonFull, which got unfortunate results when used with nix-shell

@garbas
Copy link
Member

garbas commented Oct 12, 2014

i would rename pythonFullWithPkgs to createEnv and place it on python attr path itself eg. python27.createEnv, python26.createEnv. as you said it about creating python environments and lets call it then something similar.

those of you who work with python+nixpkgs you might notice that how we handle python is fairly broken, since what can happen is that in python environment we create multiple versions of the same package can be placed in PYTHONPATH. what is even more scary is that this many times happens silently and while you think your project is using version you specified you are actually using some other version which got pulled in as dependency of some other package.

anyway +1 for the change i'll test it this week on our code and report back if there are any errors, but please lets use some more saner naming.

@domenkozar
Copy link
Member Author

I fully agree with python.createEnv, I'll rename it tomorrow.

For "fairly broken part", this is the issue: #2412

@domenkozar
Copy link
Member Author

@garbas can't actually do that. pythonFullWithPkgs has a dependency on pythonPackages.recursivePthLoader and putting that inside python interpreter creates inf. recursion. We can name it pythonFullCreateEnv if you'd like.

@garbas
Copy link
Member

garbas commented Oct 16, 2014

+1

@domenkozar domenkozar force-pushed the pythonFull_improvements branch from 4bf72fd to 68b183e Compare October 16, 2014 11:13
domenkozar added a commit that referenced this pull request Oct 16, 2014
@domenkozar domenkozar merged commit 74fde7c into master Oct 16, 2014
@domenkozar domenkozar deleted the pythonFull_improvements branch October 16, 2014 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants