Fix a bug when using multiple configuration files (#57)

* Fix a bug when using multiple configuration files

* Cargo fmt

.gitignore

@@ -7,5 +7,6 @@
 
 .idea/
 
-workspace/*
-!workspace/*.toml
+example_workspace/*
+!example_workspace/*.toml
+!example_workspace/.gitkeep

CHANGELOG.md

@@ -1,6 +1,9 @@
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
 
+### Changed
+- Fixed a bug causing provider configurations to be duplicated when adding providers to multiple config files
+
 ## [0.5.0] - 2020-02-02
 
 ### Added

dev.sh

@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+GIT_WORKSPACE=$(pwd)/example_workspace cargo run -- "${@}"

example_workspace/workspace-github.toml

@@ -0,0 +1,6 @@
+[[provider]]
+provider = "github"
+name = "django"
+path = "github"
+env_var = "GITHUB_TOKEN"
+skip_forks = false

example_workspace/workspace-lock.toml

@@ -0,0 +1,130 @@
+[[repo]]
+path = "github/django/asgi_ipc"
+url = "[email protected]:django/asgi_ipc.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/asgiref"
+url = "[email protected]:django/asgiref.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/channels"
+url = "[email protected]:django/channels.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/channels_redis"
+url = "[email protected]:django/channels_redis.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/code-of-conduct"
+url = "[email protected]:django/code-of-conduct.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/code.djangoproject.com"
+url = "[email protected]:django/code.djangoproject.com.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/daphne"
+url = "[email protected]:django/daphne.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/deps"
+url = "[email protected]:django/deps.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/django"
+url = "[email protected]:django/django.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/django-contrib-comments"
+url = "[email protected]:django/django-contrib-comments.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/django-docker-box"
+url = "[email protected]:django/django-docker-box.git"
+upstream = "[email protected]:orf/django-docker-box.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/django-docs-translations"
+url = "[email protected]:django/django-docs-translations.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/django-localflavor"
+url = "[email protected]:django/django-localflavor.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/djangobench"
+url = "[email protected]:django/djangobench.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/djangopeople"
+url = "[email protected]:django/djangopeople.git"
+upstream = "[email protected]:simonw/djangopeople.net.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/djangoproject.com"
+url = "[email protected]:django/djangoproject.com.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/djangosnippets.org"
+url = "[email protected]:django/djangosnippets.org.git"
+branch = "master"
+
+[[repo]]
+path = "github/django/ticketbot"
+url = "[email protected]:django/ticketbot.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/backend-coding-challenge"
+url = "[email protected]:tom6/backend-coding-challenge.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/gitlab"
+url = "[email protected]:tom6/gitlab.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/jiri-gitlab"
+url = "[email protected]:tom6/jiri-gitlab.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/metrics-bug"
+url = "[email protected]:tom6/metrics-bug.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/minimal-ruby-app"
+url = "[email protected]:tom6/minimal-ruby-app.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/resizer"
+url = "[email protected]:tom6/resizer.git"
+branch = "master"
+
+[[repo]]
+path = "gitlab/tom6/test-empty-project"
+url = "[email protected]:tom6/test-empty-project.git"
+
+[[repo]]
+path = "gitlab/tom6/test-lfs"
+url = "[email protected]:tom6/test-lfs.git"
+branch = "master"

example_workspace/workspace.toml

@@ -0,0 +1,6 @@
+[[provider]]
+provider = "gitlab"
+name = "tom6"
+url = "https://gitlab.com"
+path = "gitlab"
+env_var = "GITLAB_TOKEN"

src/config.rs

@@ -17,36 +17,38 @@ struct ConfigContents {
 }
 
 pub struct Config {
-    workspace: PathBuf,
+    files: Vec<PathBuf>,
 }
 
-impl Config {
-    pub fn new(workspace: PathBuf) -> Config {
-        Config { workspace }
-    }
+pub fn all_config_files(workspace: &PathBuf) -> Result<Vec<PathBuf>, Error> {
+    let matcher = globset::GlobBuilder::new("workspace*.toml")
+        .literal_separator(true)
+        .build()?
+        .compile_matcher();
+    let entries: Vec<OsString> = fs::read_dir(&workspace)?
+        .map(|res| res.map(|e| e.file_name()))
+        .collect::<Result<Vec<_>, std::io::Error>>()?;
+    let mut entries_that_exist: Vec<PathBuf> = entries
+        .into_iter()
+        .filter(|p| p != "workspace-lock.toml" && matcher.is_match(p))
+        .map(|p| workspace.join(p))
+        .collect();
+    entries_that_exist.sort();
+    return Ok(entries_that_exist);
+}
 
-    fn all_config_files(&self) -> Result<Vec<OsString>, Error> {
-        let matcher = globset::GlobBuilder::new("workspace*.toml")
-            .literal_separator(true)
-            .build()?
-            .compile_matcher();
-        let entries: Vec<OsString> = fs::read_dir(&self.workspace)?
-            .map(|res| res.map(|e| e.file_name()))
-            .collect::<Result<Vec<_>, std::io::Error>>()?;
-        let mut entries_that_exist: Vec<OsString> = entries
-            .into_iter()
-            .filter(|p| p != "workspace-lock.toml" && matcher.is_match(p))
-            .collect();
-        entries_that_exist.sort();
-        return Ok(entries_that_exist);
+impl Config {
+    pub fn new(files: Vec<PathBuf>) -> Config {
+        Config { files }
     }
 
     pub fn read(&self) -> Result<Vec<ProviderSource>, Error> {
-        let all_configs = self.all_config_files()?;
         let mut all_providers = vec![];
 
-        for file_name in all_configs {
-            let path = self.workspace.join(file_name);
+        for path in &self.files {
+            if !path.exists() {
+                continue;
+            }
             let file_contents = fs::read_to_string(&path)
                 .context(format!("Cannot read file {}", path.display()))?;
             let contents: ConfigContents = toml::from_str(file_contents.as_str())
@@ -55,9 +57,12 @@ impl Config {
         }
         Ok(all_providers)
     }
-    pub fn write(&self, providers: Vec<ProviderSource>, config: &PathBuf) -> Result<(), Error> {
+    pub fn write(
+        &self,
+        providers: Vec<ProviderSource>,
+        config_path: &PathBuf,
+    ) -> Result<(), Error> {
         let toml = toml::to_string(&ConfigContents { providers })?;
-        let config_path = &self.workspace.join(config);
         fs::write(config_path, toml)
             .context(format!("Error writing to file {}", config_path.display()))?;
         Ok(())

src/main.rs

@@ -14,7 +14,7 @@ extern crate serde_json;
 extern crate structopt;
 extern crate walkdir;
 
-use crate::config::{Config, ProviderSource};
+use crate::config::{all_config_files, Config, ProviderSource};
 use crate::lockfile::Lockfile;
 use crate::repository::Repository;
 use atomic_counter::{AtomicCounter, RelaxedCounter};
@@ -169,9 +169,9 @@ fn add_provider_to_config(
     if !provider_source.correctly_configured() {
         bail!("Provider is not correctly configured")
     }
-
-    // Load and parse our configuration file
-    let config = Config::new(workspace.clone());
+    let path_to_config = workspace.join(file);
+    // Load and parse our configuration files
+    let config = Config::new(vec![path_to_config]);
     let mut sources = config.read().context("Error reading config file")?;
     // Ensure we don't add duplicates:
     if sources.iter().any(|s| s == &provider_source) {
@@ -181,7 +181,7 @@ fn add_provider_to_config(
         // Push the provider into the source and write it to the configuration file
         sources.push(provider_source);
         config
-            .write(sources, file)
+            .write(sources, &workspace.join(file))
             .context("Error writing config file")?;
     }
     Ok(())
@@ -262,8 +262,10 @@ fn fetch(workspace: &PathBuf, threads: usize) -> Result<(), Error> {
 
 /// Update our lockfile
 fn lock(workspace: &PathBuf) -> Result<(), Error> {
+    // Find all config files
+    let config_files = all_config_files(workspace)?;
     // Read the configuration sources
-    let config = Config::new(workspace.clone());
+    let config = Config::new(config_files);
     let sources = config.read()?;
     // For each source, in sequence, fetch the repositories
     let mut all_repositories = vec![];