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

Fix mamba init --no-user #2324

Merged
merged 3 commits into from
Feb 26, 2023
Merged

Conversation

xylar
Copy link
Contributor

@xylar xylar commented Feb 26, 2023

This merge makes changes similar to
conda/conda#11949
to fix mamba init so that the --no-user flag actually has the intended effect of not changing a user's .bashrc or equivalent. Without this change, --no-user has no effect as was previously the case in conda, see:
conda/conda#11948

This merge makes changes similar to
conda/conda#11949
to fix `mamba init` so that the `--no-user` flag actually has
the intended effect of not changing a user's `.bashrc` or equivalent.
Without this change, `--no-user` has no effect as was previously
the case in conda, see:
conda/conda#11948
@xylar
Copy link
Contributor Author

xylar commented Feb 26, 2023

I tested this locally by making a fresh install of mambaforge, then doing:

python -m pip install -e mamba
mamba init --no-user
mamba init --user
mamba init

As expected, mamba init and mamba init --user both lead to a modified .bashrc, whereas mamba init --no-user does not modify my .bashrc.

This behavior is needed in situations where we do not wish to have mambaforge activate automatically in a user's shell (e.g. because we have multiple conda or mambaforge installations available on an HPC system and shell scripts that call python scripts may revert back to the wrong conda base environment).

A workaround has been to copy a user's .bashrc (etc.) to a temporary file, then copy it back after calling mamba init but this shouldn't be necessary if --no-user functioned properly.

@jonashaag
Copy link
Collaborator

Will this break backwards compat with older Conda versions?

@xylar
Copy link
Contributor Author

xylar commented Feb 26, 2023

@jonashaag, I can test that but I don't believe so beyond the fact that the --no-user option didn't work for any version prior to the latest release.

@xylar
Copy link
Contributor Author

xylar commented Feb 26, 2023

@jonashaag, yes, unfortunately with conda<23, this appears to have the undesirable effect of making the behavior of mamba init equivalent to mamba init --no-user, whereas with conda>=23, mamba init behaves like mamba init --user (as it is intended to).

I don't see an obvious way to fix this for all conda versions. I will give it some thought but would appreciate suggestions. It is broken as written now so I don't think leaving it as it is would be a very desirable option, especially because I put the work into fixing conda only to discover that mamba is bypassing my fix.

@jonashaag
Copy link
Collaborator

Thanks for checking! I think just adding a Conda version check will be fine:

if conda version is before removal of no_user:
  use old code (before #2249)
else:
  use your new impl

@xylar
Copy link
Contributor Author

xylar commented Feb 26, 2023

@jonashaag, do you have code somewhere for detecting the conda version? I didn't want to add any new dependencies for comparing versions but if you already have such a dependency,

For now, I'm using the presence of args.no_user as the way to detect this.

@jonashaag
Copy link
Collaborator

@jonashaag, do you have code somewhere for detecting the conda version? I didn't want to add any new dependencies for comparing versions but if you already have such a dependency,

For now, I'm using the presence of args.no_user as the way to detect this.

This is fine IMO, actually it’s better than checking for version: checking for availability of a feature. Version numbers are just proxies for that.

This is still broken with the `--no-user`, as noted, but at least
newer versions of conda (>=23.1.0) will work as expected.
Comment on lines +89 to +93
for_user = args.user
if not (args.install and args.user and args.system):
for_user = True
if args.no_user:
for_user = False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, this is the block of code before #2249. So we'll behave as before if args.no_user is present but things will work properly (including the --no-user flag for newer versions where the argument is no longer there.

@xylar
Copy link
Contributor Author

xylar commented Feb 26, 2023

Thanks @jonashaag. I think we're on the same page. Let me know what you think at this point.

@jonashaag jonashaag enabled auto-merge (squash) February 26, 2023 21:27
Copy link
Collaborator

@jonashaag jonashaag left a comment

Choose a reason for hiding this comment

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

Awesome thanks so much!

@jonashaag jonashaag merged commit 4138630 into mamba-org:main Feb 26, 2023
@xylar xylar deleted the fix_mamba_init_no_user branch February 26, 2023 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants