-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use .condarc file to configure conda-standalone #99
Conversation
Is I like the ideas presented here and in general I think we should always have |
I don't think it is. When I set
I added a patch to allows for overwriting the search path. I decided to do an opt-in rather than an opt-out scheme to maintain backwards compatibility. We will have to change I still think we should not overwrite the |
@@ -295,6 +298,12 @@ def _conda_main(): | |||
from conda.cli import main | |||
|
|||
_fix_sys_path() | |||
try: | |||
no_rc = sys.argv.index("--no-rc") |
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.
What's the plan for using this flag in constructor
? Older conda-standalone versions will fail if the flag is passed 🤔 Or is it this here just for convenience and we will just set CONDA_RESTRICT_RC_SEARCH_PATH
?
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.
I just submitted a draft PR: conda/constructor#863
Essentially, I will do a version check:
if exe_name == "conda-standalone" and Version(exe_version) >= Version("24.9.0"):
info["_ignore_condarcs_arg"] = "--no-rc"
elif exe_name == "micromamba":
info["_ignore_condarcs_arg"] = "--no-rc"
else:
info["_ignore_condarcs_arg"] = ""
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.
This will be tricky with cross-installers because we might not be able to run the to-be-bundled conda_exe, but I'll add that in the constructor PR.
If I run an installer based on conda-standalone: will a random "broken" .condarc in the SEARCH_PATH of the target system have an influence on the outcome of the installation? is this something we want / should prevent / warn / should be optional? |
Yes, this has been the case til this PR, where there's now an opt-in way of ignoring existing condarcs in the search path. |
I think this is something we want to prevent. If there is a bad .condarc file, |
src/conda.exe.spec
Outdated
# Add .condarc file to bundle to configure channels | ||
# during the package building stage | ||
if "PYINSTALLER_CONDARC_DIR" in os.environ: | ||
condarc = os.path.join(os.environ["RECIPE_DIR"], ".condarc") | ||
if os.path.exists(condarc): |
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.
Wouldn't it be more explicit to simply pass the path to the condarc and then copy it? Or is it to save the rename logic?
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.
As far as I know, pyinstaller does not support renaming, or at least not all versions do.
|
||
tmp_root = None | ||
if recipe_dir: | ||
# Quick way to get the location conda-standalone is extracted into |
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.
Huh, is this stable across runs? I thought the name had some random characters everytime.
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.
It seems to be because I am using the {{ RECIPE_DIR }}
environment variable. However, I decided to generalize the variable names.
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.
LGTM. Just had a couple questions, non-blocking.
Description
conda
by default points to channels owned by Anaconda, which will soon be deprecated to makeconda
vendor agnostic (see conda/conda#14217).conda-standalone
contains the same fallback. This means thatconda-standalone
packages will point to Anaconda's channel even if they are built from another source (e.g.,conda-forge
). Additionally, once the fallback is removed,conda-standalone
would not have a channel to point to by default.Whenconda-standalone
extracts itself into a temporary directory, it treats that directoryCONDA_ROOT
and can look for.condarc
files there. This PR uses that behavior to add a.condarc
file, which can be used to point to a channel and define other types of behavior of the package.There is one unique situation that causes this to fail: whenCONDA_ROOT
is set externally. This happens during the test phase ofconda build
. In that case, the.condarc
file is not picked up anymore. I created a workaround by setting theCONDARC
environment variable, but that changes the priority of the.condarc
file. Under normal usage circumstances though, that workaround should not be needed though.Instead of the above,
CONDARC
is always set unless externally managed. To ensure that only the conda-standalone configuration is loaded, an environment variable has been added. It can additionally be set by running theconda
subcommand with the--no-rc
command, similar to what micromamba is doing.Closes #97. While this doesn't make
conda-standalone
channel agnostic right away, it will followconda
's deprecation cycle.Checklist - did you ...
news
directory (using the template) for the next release's release notes?