-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat(env): Make rustic more portable #1059
base: main
Are you sure you want to change the base?
Conversation
To make rustic more portable, we can use `RUSTIC_HOME` to locate the users wanted config directories without relying on conventional/standard directories varying on the operating system. On Windows we add another layer to get compatibility paths (e.g. in the USERPROFILE). Signed-off-by: simonsan <[email protected]>
Signed-off-by: simonsan <[email protected]>
… locations Signed-off-by: simonsan <[email protected]>
Signed-off-by: simonsan <[email protected]>
Signed-off-by: simonsan <[email protected]>
Signed-off-by: simonsan <[email protected]>
…n to easily extend vec up the call stack Signed-off-by: simonsan <[email protected]>
Signed-off-by: simonsan <[email protected]>
Signed-off-by: simonsan <[email protected]>
// us mutating that vector | ||
#[allow(unused_mut)] | ||
let mut paths = vec![ | ||
get_home_config_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.
Why not simply add two entries
std::env::var_os("USERPROFILE").map(|path| {
PathBuf::from(path).join(r".config\rustic")),
std::env::var_os("USERPROFILE").map(|path| {
PathBuf::from(path).join(r".rustic")),
here? If this seems to heavy, create 2 functions which return these results.
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.
Sorry, I don't understand. Instead of what should we do that, you marked get_home_config_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.
Do you mean like that:
let mut paths = vec![
get_home_config_path(),
ProjectDirs::from("", "", "rustic")
.map(|project_dirs| project_dirs.config_dir().to_path_buf()),
get_global_config_path(),
Some(PathBuf::from(".")),
#[cfg(target_os = "windows")]
std::env::var_os("USERPROFILE").map(|path| PathBuf::from(path).join(r".config\rustic")),
#[cfg(target_os = "windows")]
std::env::var_os("USERPROFILE").map(|path| PathBuf::from(path).join(".rustic")),
];
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.
If that was the idea, I found it a bit cleaner, although we need to have all that lint deactivated and mutable variable, to have that in its own function. Just because, we can have access to that, to just get the config paths singly, when we need them.
For example, this:
fn get_config_paths(filename: &str) -> Vec<PathBuf> {
vec![
std::env::var_os("RUSTIC_HOME").map(|home_dir| PathBuf::from(home_dir).join("config")),
ProjectDirs::from("", "", "rustic")
.map(|project_dirs| project_dirs.config_dir().to_path_buf()),
get_global_config_path(),
Some(PathBuf::from(".")),
#[cfg(target_os = "windows")]
std::env::var_os("USERPROFILE").map(|path| PathBuf::from(path).join(r".config\rustic")),
#[cfg(target_os = "windows")]
std::env::var_os("USERPROFILE").map(|path| PathBuf::from(path).join(".rustic")),
]
.into_iter()
.filter_map(|path| path.map(|p| p.join(filename)))
.collect::<Vec<_>>()
}
would work, but would not let us have RUSTIC_HOME
access in a function, if we just need it for something else, e.g. opening a pdf in that folder, when we would want to open documentation or anything else in the home folder. Also I feel it makes it a bit 'chaotic' somehow, at least from reading it, but this may be personal preference.
Hence I think extracting these things into their own functions is better long-term.
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.
Yes, then simply use get_userprofile_config_rustic()
and get_userprofile_rustic()
or something like this.
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.
Thinking about it, all those directories are a bit different:
RUSTIC_HOME
: we use RUSTIC_HOME/config
here for the configs, RUSTIC_HOME/logs
I reserved on scoop
for log files
USERPROFILE\.config\rustic
: we use a root directory here for the configs
USERPROFILE\.rustic
: same here, we use a root directory
I think consistency is important in that regard, so people can for example use their rustic-package
unpack it, change some configs and start. Which would mean, that we would need config/
as a subdirectory in each of these, although it doesn't make a lot of sense in
USERPROFILE\.config\rustic
to have another subdir for config
. But then, we also can't use this directory as a kind of HOME directory, where for example also log
files would be found and other resources. 🤔
To make rustic more portable, we can use
RUSTIC_HOME
to locate the user's wanted config directories without relying on conventional/standard directories varying on the operating system. On Windows, we add another layer to get compatibility paths (e.g. in theUSERPROFILE
).Fixes #859