config: move reading of config from lib crate to CLI crate

The library crate should be usable by e.g. server processes, so it
should not read from the current user's home directory.

Cargo.lock

@@ -436,9 +436,9 @@ dependencies = [
 
 [[package]]
 name = "dirs-sys"
-version = "0.3.6"
+version = "0.3.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "03d86534ed367a67548dc68113a0f5db55432fdfbb6e6f9d77704397d95d5780"
+checksum = "1b1d1d91c932ef41c0f2663aa8b0ca0342d444d842c06914aa0a7e352d0bada6"
 dependencies = [
  "libc",
  "redox_users",
@@ -703,6 +703,7 @@ dependencies = [
  "config",
  "criterion",
  "criterion_bencher_compat",
+ "dirs",
  "git2",
  "hex",
  "indoc",
@@ -732,7 +733,6 @@ dependencies = [
  "bytes",
  "chrono",
  "config",
- "dirs",
  "git2",
  "hex",
  "itertools",

Cargo.toml

@@ -32,6 +32,7 @@ clap_complete = "3.1.1"
 clap_mangen = "0.1"
 config = "0.12.0"
 criterion = "0.3.5"
+dirs = "4.0.0"
 git2 = "0.14.2"
 hex = "0.4.3"
 indoc = "1.0.4"

lib/Cargo.toml

@@ -23,7 +23,6 @@ bytes = "1.1.0"
 byteorder = "1.4.3"
 chrono = "0.4.19"
 config = "0.12.0"
-dirs = "4.0.0"
 git2 = "0.14.2"
 hex = "0.4.3"
 itertools = "0.10.3"

lib/src/settings.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::env;
 use std::path::Path;
 
 use chrono::DateTime;
@@ -30,9 +29,6 @@ pub struct RepoSettings {
     _config: config::Config,
 }
 
-const TOO_MUCH_CONFIG_ERROR: &str =
-    "Both `$HOME/.jjconfig` and `$XDG_CONFIG_HOME/jj/config.toml` were found, please remove one.";
-
 impl UserSettings {
     pub fn from_config(config: config::Config) -> Self {
         let timestamp = match config.get_string("user.timestamp") {
@@ -45,57 +41,6 @@ impl UserSettings {
         UserSettings { config, timestamp }
     }
 
-    pub fn for_user() -> Result<Self, config::ConfigError> {
-        let mut config_builder = config::Config::builder();
-
-        let loaded_from_config_dir = match dirs::config_dir() {
-            None => false,
-            Some(config_dir) => {
-                let p = config_dir.join("jj/config.toml");
-                let exists = p.exists();
-                config_builder = config_builder.add_source(
-                    config::File::from(p)
-                        .required(false)
-                        .format(config::FileFormat::Toml),
-                );
-                exists
-            }
-        };
-
-        if let Some(home_dir) = dirs::home_dir() {
-            let p = home_dir.join(".jjconfig");
-            // we already loaded from the new location, prevent user confusion and make them
-            // remove the old one:
-            if loaded_from_config_dir && p.exists() {
-                return Err(config::ConfigError::Message(
-                    TOO_MUCH_CONFIG_ERROR.to_string(),
-                ));
-            }
-            config_builder = config_builder.add_source(
-                config::File::from(p)
-                    .required(false)
-                    .format(config::FileFormat::Toml),
-            );
-        }
-
-        // TODO: Make the config from environment a separate source instead? Seems
-        // cleaner to separate it like that, especially if the config::Config instance
-        // can keep track of where the config comes from then (it doesn't seem like it
-        // can, however - we don't give a name or anything to the Config object).
-        if let Ok(value) = env::var("JJ_USER") {
-            config_builder = config_builder.set_override("user.name", value)?;
-        }
-        if let Ok(value) = env::var("JJ_EMAIL") {
-            config_builder = config_builder.set_override("user.email", value)?;
-        }
-        if let Ok(value) = env::var("JJ_TIMESTAMP") {
-            config_builder = config_builder.set_override("user.timestamp", value)?;
-        }
-
-        let config = config_builder.build()?;
-        Ok(Self::from_config(config))
-    }
-
     pub fn with_repo(&self, repo_path: &Path) -> Result<RepoSettings, config::ConfigError> {
         let config = config::Config::builder()
             .add_source(self.config.clone())

src/main.rs

@@ -12,15 +12,71 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::env;
+
 use jujutsu::commands::dispatch;
 use jujutsu::ui::Ui;
 use jujutsu_lib::settings::UserSettings;
 
+const TOO_MUCH_CONFIG_ERROR: &str =
+    "Both `$HOME/.jjconfig` and `$XDG_CONFIG_HOME/jj/config.toml` were found, please remove one.";
+
+fn read_config() -> Result<UserSettings, config::ConfigError> {
+    let mut config_builder = config::Config::builder();
+
+    let loaded_from_config_dir = match dirs::config_dir() {
+        None => false,
+        Some(config_dir) => {
+            let p = config_dir.join("jj/config.toml");
+            let exists = p.exists();
+            config_builder = config_builder.add_source(
+                config::File::from(p)
+                    .required(false)
+                    .format(config::FileFormat::Toml),
+            );
+            exists
+        }
+    };
+
+    if let Some(home_dir) = dirs::home_dir() {
+        let p = home_dir.join(".jjconfig");
+        // we already loaded from the new location, prevent user confusion and make them
+        // remove the old one:
+        if loaded_from_config_dir && p.exists() {
+            return Err(config::ConfigError::Message(
+                TOO_MUCH_CONFIG_ERROR.to_string(),
+            ));
+        }
+        config_builder = config_builder.add_source(
+            config::File::from(p)
+                .required(false)
+                .format(config::FileFormat::Toml),
+        );
+    }
+
+    // TODO: Make the config from environment a separate source instead? Seems
+    // cleaner to separate it like that, especially if the config::Config instance
+    // can keep track of where the config comes from then (it doesn't seem like it
+    // can, however - we don't give a name or anything to the Config object).
+    if let Ok(value) = env::var("JJ_USER") {
+        config_builder = config_builder.set_override("user.name", value)?;
+    }
+    if let Ok(value) = env::var("JJ_EMAIL") {
+        config_builder = config_builder.set_override("user.email", value)?;
+    }
+    if let Ok(value) = env::var("JJ_TIMESTAMP") {
+        config_builder = config_builder.set_override("user.timestamp", value)?;
+    }
+
+    let config = config_builder.build()?;
+    Ok(UserSettings::from_config(config))
+}
+
 fn main() {
     // TODO: We need to do some argument parsing here, at least for things like
     // --config,       and for reading user configs from the repo pointed to by
     // -R.
-    match UserSettings::for_user() {
+    match read_config() {
         Ok(user_settings) => {
             let ui = Ui::for_terminal(user_settings);
             let status = dispatch(ui, &mut std::env::args_os());