Remove clone and download dir and lock file config

I don't see a good reason why these should be configurable and it makes it more difficult for sheldon to ever change the way it stores the data. In particular, it would be nice to use a separate lock file per profile.
rossmacarthur · Oct 7, 2022 · 4bbbff4 · 4bbbff4
1 parent ee575f3
commit 4bbbff4
Show file tree

Hide file tree

Showing 28 changed files with 56 additions and 197 deletions.
diff --git a/README.md b/README.md
@@ -51,9 +51,6 @@
       - [`--config-dir <path>`](#--config-dir-path)
       - [`--data-dir <path>`](#--data-dir-path)
       - [`--config-file <path>`](#--config-file-path)
-      - [`--lock-file <path>`](#--lock-file-path)
-      - [`--clone-dir <path>`](#--clone-dir-path)
-      - [`--download-dir <path>`](#--download-dir-path)
       - [`--profile <profile>`](#--profile-profile)
     - [XDG directory structure](#xdg-directory-structure)
   - [Completions](#completions)
@@ -350,27 +347,6 @@ this will default to `XDG_DATA_HOME/sheldon` otherwise it will default to
 Set the path to the config file. This defaults to `<config-dir>/plugins.toml`
 where `<config-dir>` is the config directory.
 
-##### `--lock-file <path>`
-
-*Environment variable:* `SHELDON_LOCK_FILE`
-
-Set the path to the lock file. This defaults to `<data-dir>/plugins.lock` where
-`<data-dir>` is the data directory.
-
-##### `--clone-dir <path>`
-
-*Environment variable:* `SHELDON_CLONE_DIR`
-
-Set the path to the directory where Git plugins will be cloned to. This defaults
-to `<data-dir>/repos` where `<data-dir>` is the data directory.
-
-##### `--download-dir <path>`
-
-*Environment variable:* `SHELDON_DOWNLOAD_DIR`
-
-Set the path to the directory where non-Git plugins will be downloaded to. This
-defaults to `<data-dir>/repos` where `<data-dir>` is the data directory.
-
 ##### `--profile <profile>`
 
 *Environment variable:* `SHELDON_PROFILE`

diff --git a/completions/sheldon.bash b/completions/sheldon.bash
@@ -43,7 +43,7 @@ _sheldon() {
 
     case "${cmd}" in
         sheldon)
-            opts="-h -V -q -v --help --version --quiet --verbose --color --home --config-dir --data-dir --config-file --lock-file --clone-dir --download-dir --profile init add edit remove lock source completions version"
+            opts="-h -V -q -v --help --version --quiet --verbose --color --home --config-dir --data-dir --config-file --profile init add edit remove lock source completions version"
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0
@@ -69,18 +69,6 @@ _sheldon() {
                     COMPREPLY=($(compgen -f "${cur}"))
                     return 0
                     ;;
-                --lock-file)
-                    COMPREPLY=($(compgen -f "${cur}"))
-                    return 0
-                    ;;
-                --clone-dir)
-                    COMPREPLY=($(compgen -f "${cur}"))
-                    return 0
-                    ;;
-                --download-dir)
-                    COMPREPLY=($(compgen -f "${cur}"))
-                    return 0
-                    ;;
                 --profile)
                     COMPREPLY=($(compgen -f "${cur}"))
                     return 0

diff --git a/completions/sheldon.zsh b/completions/sheldon.zsh
@@ -20,9 +20,6 @@ _sheldon() {
 '--config-dir=[The configuration directory]:PATH: ' \
 '--data-dir=[The data directory]:PATH: ' \
 '--config-file=[The config file]:PATH: ' \
-'--lock-file=[The lock file]:PATH: ' \
-'--clone-dir=[The directory where git sources are cloned to]:PATH: ' \
-'--download-dir=[The directory where remote sources are downloaded to]:PATH: ' \
 '--profile=[The profile used for conditional plugins]:PROFILE: ' \
 '-h[Print help information]' \
 '--help[Print help information]' \

diff --git a/docs/src/Command-line-interface.md b/docs/src/Command-line-interface.md
@@ -162,27 +162,6 @@ this will default to `XDG_DATA_HOME/sheldon` otherwise it will default to
 Set the path to the config file. This defaults to `<config-dir>/plugins.toml`
 where `<config-dir>` is the config directory.
 
-#### `--lock-file <path>`
-
-*Environment variable:* `SHELDON_LOCK_FILE`
-
-Set the path to the lock file. This defaults to `<data-dir>/plugins.lock` where
-`<data-dir>` is the data directory.
-
-#### `--clone-dir <path>`
-
-*Environment variable:* `SHELDON_CLONE_DIR`
-
-Set the path to the directory where Git plugins will be cloned to. This defaults
-to `<data-dir>/repos` where `<data-dir>` is the data directory.
-
-#### `--download-dir <path>`
-
-*Environment variable:* `SHELDON_DOWNLOAD_DIR`
-
-Set the path to the directory where non-Git plugins will be downloaded to. This
-defaults to `<data-dir>/repos` where `<data-dir>` is the data directory.
-
 #### `--profile <profile>`
 
 *Environment variable:* `SHELDON_PROFILE`

diff --git a/src/cli/mod.rs b/src/cli/mod.rs
@@ -68,9 +68,6 @@ impl Opt {
             data_dir,
             config_dir,
             config_file,
-            lock_file,
-            clone_dir,
-            download_dir,
             profile,
             command,
         } = raw_opt;
@@ -168,9 +165,9 @@ impl Opt {
         let config_dir = config_dir.unwrap_or(config_pre);
         let data_dir = data_dir.unwrap_or(data_pre);
         let config_file = config_file.unwrap_or_else(|| config_dir.join("plugins.toml"));
-        let lock_file = lock_file.unwrap_or_else(|| data_dir.join("plugins.lock"));
-        let clone_dir = clone_dir.unwrap_or_else(|| data_dir.join("repos"));
-        let download_dir = download_dir.unwrap_or_else(|| data_dir.join("downloads"));
+        let lock_file = data_dir.join("plugins.lock");
+        let clone_dir = data_dir.join("repos");
+        let download_dir = data_dir.join("downloads");
 
         let ctx = Context {
             version: build::CRATE_RELEASE.to_string(),

diff --git a/src/cli/raw.rs b/src/cli/raw.rs
@@ -49,18 +49,6 @@ pub struct RawOpt {
     #[clap(long, value_name = "PATH", env = "SHELDON_CONFIG_FILE")]
     pub config_file: Option<PathBuf>,
 
-    /// The lock file.
-    #[clap(long, value_name = "PATH", env = "SHELDON_LOCK_FILE")]
-    pub lock_file: Option<PathBuf>,
-
-    /// The directory where git sources are cloned to.
-    #[clap(long, value_name = "PATH", env = "SHELDON_CLONE_DIR")]
-    pub clone_dir: Option<PathBuf>,
-
-    /// The directory where remote sources are downloaded to.
-    #[clap(long, value_name = "PATH", env = "SHELDON_DOWNLOAD_DIR")]
-    pub download_dir: Option<PathBuf>,
-
     /// The profile used for conditional plugins.
     #[clap(long, value_name = "PROFILE", env = "SHELDON_PROFILE")]
     pub profile: Option<String>,

diff --git a/src/cli/testdata/raw_opt_help.golden b/src/cli/testdata/raw_opt_help.golden
@@ -6,18 +6,15 @@ USAGE:
     sheldon [OPTIONS] <SUBCOMMAND>
 
 OPTIONS:
-    -q, --quiet                  Suppress any informational output
-    -v, --verbose                Use verbose output
-        --color <WHEN>           Output coloring: always, auto, or never [default: auto]
-        --config-dir <PATH>      The configuration directory [env: SHELDON_CONFIG_DIR=]
-        --data-dir <PATH>        The data directory [env: SHELDON_DATA_DIR=]
-        --config-file <PATH>     The config file [env: SHELDON_CONFIG_FILE=]
-        --lock-file <PATH>       The lock file [env: SHELDON_LOCK_FILE=]
-        --clone-dir <PATH>       The directory where git sources are cloned to [env: SHELDON_CLONE_DIR=]
-        --download-dir <PATH>    The directory where remote sources are downloaded to [env: SHELDON_DOWNLOAD_DIR=]
-        --profile <PROFILE>      The profile used for conditional plugins [env: SHELDON_PROFILE=]
-    -h, --help                   Print help information
-    -V, --version                Print version information
+    -q, --quiet                 Suppress any informational output
+    -v, --verbose               Use verbose output
+        --color <WHEN>          Output coloring: always, auto, or never [default: auto]
+        --config-dir <PATH>     The configuration directory [env: SHELDON_CONFIG_DIR=]
+        --data-dir <PATH>       The data directory [env: SHELDON_DATA_DIR=]
+        --config-file <PATH>    The config file [env: SHELDON_CONFIG_FILE=]
+        --profile <PROFILE>     The profile used for conditional plugins [env: SHELDON_PROFILE=]
+    -h, --help                  Print help information
+    -V, --version               Print version information
 
 SUBCOMMANDS:
     init           Initialize a new config file

diff --git a/src/cli/tests.rs b/src/cli/tests.rs
@@ -79,9 +79,6 @@ fn raw_opt_no_options() {
             config_dir: None,
             data_dir: None,
             config_file: None,
-            lock_file: None,
-            clone_dir: None,
-            download_dir: None,
             profile: None,
             command: RawCommand::Lock {
                 update: false,
@@ -108,12 +105,6 @@ fn raw_opt_options() {
             "/test",
             "--config-file",
             "/plugins.toml",
-            "--lock-file",
-            "/test/plugins.lock",
-            "--clone-dir",
-            "/repos",
-            "--download-dir",
-            "/downloads",
             "--profile",
             "profile",
             "lock",
@@ -126,9 +117,6 @@ fn raw_opt_options() {
             config_dir: Some("/test".into()),
             data_dir: Some("/test".into()),
             config_file: Some("/plugins.toml".into()),
-            lock_file: Some("/test/plugins.lock".into()),
-            clone_dir: Some("/repos".into()),
-            download_dir: Some("/downloads".into()),
             profile: Some("profile".into()),
             command: RawCommand::Lock {
                 update: false,

diff --git a/src/context.rs b/src/context.rs
@@ -17,10 +17,14 @@ pub struct Context {
     pub config_dir: PathBuf,
     pub data_dir: PathBuf,
     pub config_file: PathBuf,
+    pub profile: Option<String>,
+
+    #[serde(skip)]
     pub lock_file: PathBuf,
+    #[serde(skip)]
     pub clone_dir: PathBuf,
+    #[serde(skip)]
     pub download_dir: PathBuf,
-    pub profile: Option<String>,
     #[serde(skip)]
     pub output: Output,
     #[serde(skip)]

diff --git a/src/lock/clean.rs b/src/lock/clean.rs
@@ -12,13 +12,6 @@ use crate::lock::LockedConfig;
 impl LockedConfig {
     /// Clean the clone and download directories.
     pub fn clean(&self, ctx: &Context, warnings: &mut Vec<Error>) {
-        let clean_clone_dir = self.ctx.clone_dir().starts_with(self.ctx.data_dir());
-        let clean_download_dir = self.ctx.download_dir().starts_with(self.ctx.data_dir());
-
-        if !clean_clone_dir && !clean_download_dir {
-            return;
-        }
-
         // Track the source directories, all the plugin directory parents, and all the
         // plugin files.
         let mut source_dirs = HashSet::new();
@@ -41,31 +34,27 @@ impl LockedConfig {
         parent_dirs.insert(self.ctx.clone_dir());
         parent_dirs.insert(self.ctx.download_dir());
 
-        if clean_clone_dir {
-            for entry in WalkDir::new(self.ctx.clone_dir())
-                .into_iter()
-                .filter_entry(|e| !source_dirs.contains(e.path()))
-                .filter_map(result::Result::ok)
-                .filter(|e| !parent_dirs.contains(e.path()))
-            {
-                if let Err(err) = remove_path(ctx, entry.path()) {
-                    warnings.push(err);
-                }
+        for entry in WalkDir::new(self.ctx.clone_dir())
+            .into_iter()
+            .filter_entry(|e| !source_dirs.contains(e.path()))
+            .filter_map(result::Result::ok)
+            .filter(|e| !parent_dirs.contains(e.path()))
+        {
+            if let Err(err) = remove_path(ctx, entry.path()) {
+                warnings.push(err);
             }
         }
 
-        if clean_download_dir {
-            for entry in WalkDir::new(self.ctx.download_dir())
-                .into_iter()
-                .filter_map(result::Result::ok)
-                .filter(|e| {
-                    let p = e.path();
-                    !files.contains(p) && !parent_dirs.contains(p)
-                })
-            {
-                if let Err(err) = remove_path(ctx, entry.path()) {
-                    warnings.push(err);
-                }
+        for entry in WalkDir::new(self.ctx.download_dir())
+            .into_iter()
+            .filter_map(result::Result::ok)
+            .filter(|e| {
+                let p = e.path();
+                !files.contains(p) && !parent_dirs.contains(p)
+            })
+        {
+            if let Err(err) = remove_path(ctx, entry.path()) {
+                warnings.push(err);
             }
         }
     }

diff --git a/src/lock/file.rs b/src/lock/file.rs
@@ -58,6 +58,12 @@ impl LockedConfig {
         P: AsRef<Path>,
     {
         let path = path.as_ref();
+        if let Some(parent) = path.parent() {
+            fs::create_dir_all(parent).with_context(s!(
+                "failed to create parent directory `{}`",
+                parent.display()
+            ))?;
+        }
         fs::write(
             path,
             &toml::to_string(&self).context("failed to serialize locked config")?,

diff --git a/src/lock/mod.rs b/src/lock/mod.rs
@@ -271,9 +271,6 @@ fn is_context_equal(left: &Context, right: &Context) -> bool {
         && left.config_dir == right.config_dir
         && left.data_dir == right.data_dir
         && left.config_file == right.config_file
-        && left.lock_file == right.lock_file
-        && left.clone_dir == right.clone_dir
-        && left.download_dir == right.download_dir
         && left.profile == right.profile
 }
 
@@ -402,9 +399,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 plugins = []
 
 [templates]

diff --git a/tests/cases/clean b/tests/cases/clean
@@ -6,9 +6,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 plugins = []
 [templates.PATH]
 value = "export PATH=\"{{ dir }}:$PATH\""

diff --git a/tests/cases/clean_permission_denied b/tests/cases/clean_permission_denied
@@ -6,9 +6,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 plugins = []
 [templates.PATH]
 value = "export PATH=\"{{ dir }}:$PATH\""

diff --git a/tests/cases/directories b/tests/cases/directories
@@ -8,9 +8,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 
 [[plugins]]
 name = "test_downloads"

diff --git a/tests/cases/directories_incremental b/tests/cases/directories_incremental
@@ -11,9 +11,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 
 [[plugins]]
 name = "test_downloads"

diff --git a/tests/cases/empty b/tests/cases/empty
@@ -6,9 +6,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 plugins = []
 [templates.PATH]
 value = "export PATH=\"{{ dir }}:$PATH\""

diff --git a/tests/cases/github_bad_reinstall b/tests/cases/github_bad_reinstall
@@ -9,9 +9,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 
 [[plugins]]
 name = "test"

diff --git a/tests/cases/github_branch b/tests/cases/github_branch
@@ -9,9 +9,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 
 [[plugins]]
 name = "test"

diff --git a/tests/cases/github_git b/tests/cases/github_git
@@ -9,9 +9,6 @@ home = "<home>"
 config_dir = "<config>"
 data_dir = "<data>"
 config_file = "<config>/plugins.toml"
-lock_file = "<data>/plugins.lock"
-clone_dir = "<data>/repos"
-download_dir = "<data>/downloads"
 
 [[plugins]]
 name = "test"