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

Use xdg schemas for config saving/reading (minified) #2714

Merged
merged 6 commits into from
Jul 31, 2023

Conversation

danpf
Copy link
Contributor

@danpf danpf commented Jul 26, 2023

This is a 'mini' version of #1836

I tried not to rock the boat too much with this one.

This PR added some helper functions in order to support the xdg directory schema, as well as pollute the users home directory less.

Fixes: #1787

Maybe I shouldn't have, but it felt weird intercalating the mamba and conda configs, so i separated them and just made all mamba configs superseed the conda configs.

as an aside, (sorry for the un-requested opinion), I totally understand what this org is doing, but I think trying to act as a drop in replacement for conda while at the same time building out its own configs (mambarc etc) is very confusing. I think it should just be one or the other. Or at least only condarc etc until the project is mature enough, and then switch to mambarc after a major version bump when it's ready.

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.

Thanks a bunch, perfect work :)

}

std::vector<fs::u8path> mamba_user = {
env::user_config_dir() / ".mambarc", env::user_config_dir() / "mambarc",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pavelzw do you think we should be adding these? I don't have a strong opinion. Nobody will use them anyways.

Copy link
Member

Choose a reason for hiding this comment

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

I think actually @0xbe7a uses them 😄 so yes

if (maybe_user_config_dir.empty())
{
#ifdef _WIN32
maybe_user_config_dir = ::mamba::win::get_folder("roamingappdata");
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pavelzw see discussion in the other PR. I'm fine with the decision to use this as the Windows default folder; people can always set the XDG vars.

Copy link
Collaborator

Choose a reason for hiding this comment

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

On Cygwin I'd be surprised if ~/.config wasn't read by default though...

Copy link
Member

Choose a reason for hiding this comment

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

I'm also fine with this. If people want to have XDG like stuff on cygwin, they should just set it in their bashrc/bash_profile.

@jonashaag
Copy link
Collaborator

@pavelzw

@pavelzw
Copy link
Member

pavelzw commented Jul 31, 2023

Sorry, I forgot to look at this. I'll do it today 😅

Copy link
Member

@pavelzw pavelzw left a comment

Choose a reason for hiding this comment

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

Thanks! 💜 LGTM

}

std::vector<fs::u8path> mamba_user = {
env::user_config_dir() / ".mambarc", env::user_config_dir() / "mambarc",
Copy link
Member

Choose a reason for hiding this comment

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

I think actually @0xbe7a uses them 😄 so yes

if (maybe_user_config_dir.empty())
{
#ifdef _WIN32
maybe_user_config_dir = ::mamba::win::get_folder("roamingappdata");
Copy link
Member

Choose a reason for hiding this comment

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

I'm also fine with this. If people want to have XDG like stuff on cygwin, they should just set it in their bashrc/bash_profile.

@pavelzw
Copy link
Member

pavelzw commented Jul 31, 2023

trying to act as a drop in replacement for conda while at the same time building out its own configs (mambarc etc) is very confusing

I personally agree!
I also think that the main reason for existence for mamba is now gone since conda also implemented the libmamba parser. (i.e. deprecate mamba and only continue working on micromamba 😄 or rebrand mamba to be the same as micromamba in a v2 release)

@jonashaag jonashaag merged commit 00fb86c into mamba-org:main Jul 31, 2023
20 checks passed
@jonashaag
Copy link
Collaborator

🎉

cvanelteren pushed a commit to cvanelteren/mamba that referenced this pull request Aug 29, 2023
Co-authored-by: Danny Farrell <dan@cyrusbio.com>
@jonashaag
Copy link
Collaborator

@danpf Interestingly for me this doesn't work if I don't create an empty ~/.config/mamba folder:

$ pwd
/Users/j/.config

$ micromamba info | grep populated
 populated config files : /Users/j/.config/mamba/../conda/.condarc

$ rmdir mamba/
$ micromamba info | grep populated
 populated config files :

@danpf
Copy link
Contributor Author

danpf commented Jan 9, 2024

@jonashaag It looks like the code has been significantly refactored since this was merged. Although my long comment is still there, (seems like it should be deleted now) it looks like someone expanded the config/environment. So I don't think you should still see the ../conda filepaths at all.

@jonashaag
Copy link
Collaborator

Not sure what you are referring to. Your tests still seem to be there. So if the tests used to worked we should have realized if something broke. I'll try to investigate.

@jonashaag
Copy link
Collaborator

Sorry, now I understand where the changes are coming from: #2983

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.

Support XDG directories
3 participants