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

default profile directory #10338

Closed
paulrouget opened this issue Apr 1, 2016 · 13 comments
Closed

default profile directory #10338

paulrouget opened this issue Apr 1, 2016 · 13 comments
Labels

Comments

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Apr 1, 2016

#10114 introduces a preference file.

The profiler directory is set via the --profile-dir option.

We should have a default directory.

This directory should not be created if it doesn't exist, but just if --profile-dir is not specified and default_profile_dir/prefs.json exists, then we should read this file.

I guess these are good-enough values:

  • windows: %APPDATA%\servo\
  • mac: ~/Library/Application Support/Servo/
  • linux: use XDG Base Directory spec (or just ~/.config/servo/)

I would not be surprised if there was already a crate that resolves the default configuration directory for each platform.

@creativcoder
Copy link
Contributor

@creativcoder creativcoder commented Apr 1, 2016

I would like to work on it.

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Apr 2, 2016

@creativcoder Sure! Let us know if you have any question.

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Apr 2, 2016

One of my long-standing irritations about Firefox is its lack of XDG Base Directory support. In particular, Firefox uses a single profile folder for all of its files, but the XDG spec has separate paths for configuration/preferences, caches, and data. I find this separation useful because it lets me use a git repo to keep my preferences synced, ignore the cache directory, and use regular backups to keep track of data. I would really appreciate it if we could support this separation in Servo and use separate folders; placing the entire profile folder under ~/.config/servo breaks this contract.

Because we're starting fresh here, I'd appreciate getting this right from the start; it's much harder to change the default directory once something has been hardcoded and there is a significant user base already using the old directories. For example, Atom released their 1.0 without addressing this and is now locked into their current defaults.

I'm not sure what similar norms exist for Windows and OS X, but we should make sure to follow them.

@creativcoder
Copy link
Contributor

@creativcoder creativcoder commented Apr 3, 2016

@aneeshusa Ok. Looks like we have a library already that follows XDG base directory spec https://github.com/whitequark/rust-xdg . We can use that. Will need @paulrouget suggestions on this.

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Apr 3, 2016

@SimonSapin Wanna chime in? I've seen you commented there: whitequark/rust-xdg#8

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 3, 2016

I agree that we should separate non-essential / cache files from configuration / preferences and user data. The conventional way to do that on Linux is per XDG, but I don’t know about other systems. I’d be in favor of putting that separation in place even if we only have preferences so far.

Unfortunately that makes a nice single --profile-dir command-line option not enough. I don’t have a good answer there.

The rust-xdg crate looks good. We may want to make another crate that abstracts different platform’s conventions (using rust-xdg on #[cfg(all(unix, not(target_os = "macos"), not(target_os = "ios")))])

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Apr 3, 2016

Thanks for listening, this is a small thing that makes a big usability difference for me. :)

XDG specifies a set of environment variables that allow overriding the folders used for config/cache/data separately; it looks like the rust-xdg crate already uses these.

As far as command line options, I think a --config-dir option may be useful; I don't think I've ever used a --cache-dir or --data-dir option before. (Some applications also 'bootstrap' by making cache and data directory configuration settings, so you can control everything via one option.)

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 3, 2016

(Some applications also 'bootstrap' by making cache and data directory configuration settings, so you can control everything via one option.)

Sounds good to me.

@creativcoder
Copy link
Contributor

@creativcoder creativcoder commented Apr 3, 2016

@SimonSapin
So i went through the relevant code, and am thinking of something like this.
Tell me if this is the way to go.

First we'll add an extra field config-dir to Opts struct, then
within default_opts() method on initializing the config-dir field, we will call something like : xdg::get_default_config_dirs()

We can create a mod xdg.rs under util, which will contain #[cfg(target_os="")]
conditional method namd get_default_config_dirs(), and when platform is linux, that method, will
initialize directories according to XDG spec, but for other platforms it will return directory strings
what @paulrouget suggests.

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 4, 2016

Sounds good, except we should have a thing (module, crate, …) to abstract platform differences called something like "basedirs" but not "xdg", since XDG is only relevant and xdg-rs is only used on some platforms.

@creativcoder
Copy link
Contributor

@creativcoder creativcoder commented Apr 5, 2016

@SimonSapin @paulrouget Hi have a look at basedirs module; tell me if this looks good, or suggest if this can be done better. https://gist.github.com/creativcoder/7f4a3b57c2c3dc08da47f26a15db7298

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 7, 2016

@creativcoder These functions should return Option<PathBuf> rather than Option<String>. And to leave room for multiple profiles they should add a servo/default suffix rather than just servo. Looks good other than these details, thanks for pushing on this front!

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Apr 7, 2016

rust-xdg actually has profiles built in; just replace calls to with_prefix with with_profile and use default for the profile parameter.

bors-servo added a commit that referenced this issue May 24, 2016
Adding default config directories.

Fixes #10338
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10498)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 25, 2016
Adding default config directories.

Fixes #10338
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10498)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.