Turn off compilation #715
Turn off compilation #715
Conversation
| remove_easy_install_pth(files, preserve_egg_dir=preserve_egg_dir) | ||
| rm_py_along_so() | ||
| if config.CONDA_PY < 30: | ||
| if config.CONDA_PY < 30 and pyc_compilation: |
There was a problem hiding this comment.
Why is compilation only done for Python 2? @ivoflipse you're perhaps not the right one to ask, since you're only maintaining this behavior, but maybe @ilanschnell or @groutr know?
There was a problem hiding this comment.
I don't know, but it probably has to with Python 3 stuff compiled files in a __pycache__ folder
I haven't been able to use Python 3 much until recently, so now a lot of these Python 2/3 issues start creeping up on me :-)
|
+1 from me. The Travis error is just Github complaining about too many requests. Looks OK. I think this is a good addition, in case things should not be compiled for any reason. The danger here is that compilation is a good last check of package validity. People may shoot themselves in the foot with this and distribute broken packages more easily. Still, the issues mentioned above present valid reasons for this change. |
|
Yeah I currently had the compilation lines commented out, but that's obviously not what I want either. Because then it no longer works for package that do require it. One alternative solution would be to specify that I want certain parts of the package's code to be excluded. Either through changing setup.py or by removing them from the package after Another would be to have it try compiling every file, but only raise an exception if this field is set. That way you can chose to ignore errors that you were expecting, but you'd still get compilation of all your other files. |
|
I've been looking into a way to add a list of modules to exclude from compilation, but the code doesn't seem to lend itself to these purposes at all. I had hoped to use the same syntax as imports for testing use, but I didn't manage to find a good way of whitelisting/blacklisting files. The current code basically just passes the entire For now, I think this solution of optionally turning off compilation completely will at least prevent broken builds for misbehaving packages. Personally I don't think the .pyc compilation is that useful, if you want to know if your code works, include a test suite or at least try some import tests (which support platform filters). |
|
@msarahan Any idea on how to proceed with this PR? |
|
Can you come up with a quick test recipe for with/without this option? I don't think it's strictly necessary, but it would be nice to have this feature covered. Otherwise, please pull and merge or rebase to fix the conflict. I think this warrants further discussion about why we're doing compiling, but it seems like now is not the right time - and the gradual Python 3 migration may obviate this issue anyway. |
|
Personally, I think we should be having people set |
|
@jakirkham any hints on where in conda this is used already? I haven't had time to rebase the PR yet, but I hope to do so soon |
|
I don't think this is currently used in conda currently. People set that evironment variable independently of conda. Having a conda command line flag that sets it may help it feel more "native" - but is also sort of against Python's "only one right way to do it" philosophy. I don't have a strong opinion here. Either documenting this environment variable better in the conda docs or creating your config would work, and both require documentation at a minimum. I generally prefer your recipe approach because environment variables are easy to forget and not portable. |
|
Hmm...so I have played around with this a bit more and given this some more thought. Maybe the better question is why are we going through the Python tree and compiling Python files that weren't compiled previously. It seems that is the real problem and maybe the simplest answer is that we should not do this. Unfortunately, the commit that added that |
|
Would be nice to have this. E.g. django 1.9 contains jinja templates with .py extensions, so without this PR you cannot package it (it will try to compile them and crash on the jinja syntax). |
|
Ping @kalefranz. We discussed this in a recent meeting, and people expressed concern about packages without pyc files running slower. @groutr pointed out to me that we could always write pyc files at install time, because at install time by definition have write permissions. |
|
I can understand this, but how does that resolve the issue for the django 1.9 scenario I mentioned in #715 (comment)? |
|
It doesn't. I would prefer to have conda never compile to pyc, and rely on the installed python to compile as necessary at runtime. Others argued that we may not always have write permission at runtime, and need to ship pyc files somehow. If anyone ever tries to compile django's templates, it will fail, no matter when they are compiled, as you point out. |
|
Not having write permissions is a good argument. So having a blacklist as proposed here seems like an approach that is compatible with current use cases. |
|
I am facing this issue with https://github.com/sloria/webargs and have submitted this issue on that repo: marshmallow-code/webargs#105 Here are my thoughts:
|
|
Here is a trace from a concrete example based on this |
|
@ivoflipse I'd like to merge this. Can you merge the conflicts, or would you rather I do it? |
And added a check for this in post.py Also updated the recipe, so I could build a new package to try it out
Also changed it so build.py checks the meta.yaml value
|
There @msarahan I think I cleaned it up. |
|
Errors appear to be ours, due to recent open SSL update. I will close and reopen this PR to redo the CI once we have those errors sorted. |
|
@ijstokes et al., We should consider the inverse case of distributing only |
|
Also agree the conda-build should be passing all environment variables through to both the jinja2 context and the build recipe. That would include |
|
@kalefranz, this might be of interest to you. We considered allowing one to define the environment variables in the Though if are wanting to do something similar with Also, it may be a way (combined with some sort of prep step to set things like |
|
Also, more current work in #1169 |
I added an option to the build section where you can say:
pyc_compilation: Falseand it will skip the pyc compilation step.
This should address #686 and #709. Hope I changed it correctly, though I verified that it works.