-
Notifications
You must be signed in to change notification settings - Fork 428
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
main: If user supplies --python, auto-set CONDA_PY environment variable. #662
Conversation
…le. Same for --numpy, --perl, etc.
70b8b36
to
8b6b8b7
Compare
Note: This assumes that conda/conda-build#662 and conda/conda-build#663 will be merged.
To better understand this problem, consider this example:
In this case,
Looks good so far...
Whoops, I expected to see |
# Using --python, --numpy etc. is equivalent to using CONDA_PY, CONDA_NPY, etc. | ||
# Auto-set those env variables | ||
for var in conda_version.values(): | ||
if getattr(config, var): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so I'm clear on this: you are effectively giving priority to any setting passed in with command line stuff, like "--python" over the environment variables, right? I'm ok with that, just want to make sure I understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was already the case. The original behavior was equivalent to the following pseudo-code:
# By default, use os.environ
conda_build.config.Config.CONDA_PY = os.environ['CONDA_PY']
# Override with --python=foo.bar, if available
if args.python:
conda_build.config.Config.CONDA_PY = args.python
I just want os.environ
to also reflect the change. In the pseudo-code, I'm adding a line to the if-block:
# By default, use os.environ
conda_build.config.Config.CONDA_PY = os.environ['CONDA_PY']
# Override with --python=foo.bar, if available
if args.python:
conda_build.config.Config.CONDA_PY = args.python
# Also, update os.environ
os.environ['CONDA_PY'] = args.python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I guess it's going outside the if-block. But it's equivalent.
# By default, use os.environ
conda_build.config.Config.CONDA_PY = os.environ['CONDA_PY']
# Override with --python=foo.bar, if available
if args.python:
conda_build.config.Config.CONDA_PY = args.python
# No matter what, make sure os.environ is in-sync with conda_build.config
os.environ['CONDA_PY'] = conda_build.config.Config.CONDA_PY
when you set |
OK, well +1 from me. Will check in with @groutr |
Edit: That's the original behavior, unchanged in this PR. See here: https://github.com/conda/conda-build/pull/662/files?diff=unified#diff-c79cbf0306ce864b4ea844b108e06d4dL301 |
Ok. I didn't see where they reformatted the string. +1 from me too. |
main: If user supplies --python, auto-set CONDA_PY environment variable.
Excellent. Thanks, guys. |
To build with a specific python version, the user can try two things:
The
CONDA_PY
environment variable, e.g.CONDA_PY=27 conda build my-package
OR
The
--python
option on the command-line, e.g.conda build --python=2.7 my-package
Internally, both mechanisms set
CONDA_PY
withinconda_build.config
. Additionally,os.environ["CONDA_PY"]
can also be used, but only if the user used the first mechanism. It would be nice if that also worked in the second case. (For example, that would make the value consistently available tojinja2
templates withinmeta.yaml
, as well as thebuild.sh
environment.)This little PR just copies
config.CONDA_PY
(andconfig.CONDA_NPY
, etc.) intoos.environ
, so the two modules are in sync for these variables.It allows
meta.yaml
syntax like this, which doesn't always work in the current version ofconda-build
: