From a5c8cf43de300a3aed5b09be24c06cc33f06b3c6 Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Fri, 21 Feb 2020 22:29:27 +0000 Subject: [PATCH] Fix a bug when using multiple configuration files (#57) * Fix a bug when using multiple configuration files * Cargo fmt --- .gitignore | 5 +- CHANGELOG.md | 3 + dev.sh | 3 + example_workspace/.gitkeep | 0 example_workspace/workspace-github.toml | 6 ++ example_workspace/workspace-lock.toml | 130 ++++++++++++++++++++++++ example_workspace/workspace.toml | 6 ++ src/config.rs | 53 +++++----- src/main.rs | 14 +-- 9 files changed, 188 insertions(+), 32 deletions(-) create mode 100755 dev.sh create mode 100644 example_workspace/.gitkeep create mode 100644 example_workspace/workspace-github.toml create mode 100644 example_workspace/workspace-lock.toml create mode 100644 example_workspace/workspace.toml diff --git a/.gitignore b/.gitignore index 62da7d6..07ff806 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,6 @@ .idea/ -workspace/* -!workspace/*.toml +example_workspace/* +!example_workspace/*.toml +!example_workspace/.gitkeep diff --git a/CHANGELOG.md b/CHANGELOG.md index c9a85f3..5180700 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## [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 diff --git a/dev.sh b/dev.sh new file mode 100755 index 0000000..30d3cb9 --- /dev/null +++ b/dev.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +GIT_WORKSPACE=$(pwd)/example_workspace cargo run -- "${@}" diff --git a/example_workspace/.gitkeep b/example_workspace/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/example_workspace/workspace-github.toml b/example_workspace/workspace-github.toml new file mode 100644 index 0000000..e9bed49 --- /dev/null +++ b/example_workspace/workspace-github.toml @@ -0,0 +1,6 @@ +[[provider]] +provider = "github" +name = "django" +path = "github" +env_var = "GITHUB_TOKEN" +skip_forks = false diff --git a/example_workspace/workspace-lock.toml b/example_workspace/workspace-lock.toml new file mode 100644 index 0000000..3c1fefb --- /dev/null +++ b/example_workspace/workspace-lock.toml @@ -0,0 +1,130 @@ +[[repo]] +path = "github/django/asgi_ipc" +url = "git@github.com:django/asgi_ipc.git" +branch = "master" + +[[repo]] +path = "github/django/asgiref" +url = "git@github.com:django/asgiref.git" +branch = "master" + +[[repo]] +path = "github/django/channels" +url = "git@github.com:django/channels.git" +branch = "master" + +[[repo]] +path = "github/django/channels_redis" +url = "git@github.com:django/channels_redis.git" +branch = "master" + +[[repo]] +path = "github/django/code-of-conduct" +url = "git@github.com:django/code-of-conduct.git" +branch = "master" + +[[repo]] +path = "github/django/code.djangoproject.com" +url = "git@github.com:django/code.djangoproject.com.git" +branch = "master" + +[[repo]] +path = "github/django/daphne" +url = "git@github.com:django/daphne.git" +branch = "master" + +[[repo]] +path = "github/django/deps" +url = "git@github.com:django/deps.git" +branch = "master" + +[[repo]] +path = "github/django/django" +url = "git@github.com:django/django.git" +branch = "master" + +[[repo]] +path = "github/django/django-contrib-comments" +url = "git@github.com:django/django-contrib-comments.git" +branch = "master" + +[[repo]] +path = "github/django/django-docker-box" +url = "git@github.com:django/django-docker-box.git" +upstream = "git@github.com:orf/django-docker-box.git" +branch = "master" + +[[repo]] +path = "github/django/django-docs-translations" +url = "git@github.com:django/django-docs-translations.git" +branch = "master" + +[[repo]] +path = "github/django/django-localflavor" +url = "git@github.com:django/django-localflavor.git" +branch = "master" + +[[repo]] +path = "github/django/djangobench" +url = "git@github.com:django/djangobench.git" +branch = "master" + +[[repo]] +path = "github/django/djangopeople" +url = "git@github.com:django/djangopeople.git" +upstream = "git@github.com:simonw/djangopeople.net.git" +branch = "master" + +[[repo]] +path = "github/django/djangoproject.com" +url = "git@github.com:django/djangoproject.com.git" +branch = "master" + +[[repo]] +path = "github/django/djangosnippets.org" +url = "git@github.com:django/djangosnippets.org.git" +branch = "master" + +[[repo]] +path = "github/django/ticketbot" +url = "git@github.com:django/ticketbot.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/backend-coding-challenge" +url = "git@gitlab.com:tom6/backend-coding-challenge.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/gitlab" +url = "git@gitlab.com:tom6/gitlab.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/jiri-gitlab" +url = "git@gitlab.com:tom6/jiri-gitlab.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/metrics-bug" +url = "git@gitlab.com:tom6/metrics-bug.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/minimal-ruby-app" +url = "git@gitlab.com:tom6/minimal-ruby-app.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/resizer" +url = "git@gitlab.com:tom6/resizer.git" +branch = "master" + +[[repo]] +path = "gitlab/tom6/test-empty-project" +url = "git@gitlab.com:tom6/test-empty-project.git" + +[[repo]] +path = "gitlab/tom6/test-lfs" +url = "git@gitlab.com:tom6/test-lfs.git" +branch = "master" diff --git a/example_workspace/workspace.toml b/example_workspace/workspace.toml new file mode 100644 index 0000000..6c837f8 --- /dev/null +++ b/example_workspace/workspace.toml @@ -0,0 +1,6 @@ +[[provider]] +provider = "gitlab" +name = "tom6" +url = "https://gitlab.com" +path = "gitlab" +env_var = "GITLAB_TOKEN" diff --git a/src/config.rs b/src/config.rs index 79fa05d..d369730 100644 --- a/src/config.rs +++ b/src/config.rs @@ -17,36 +17,38 @@ struct ConfigContents { } pub struct Config { - workspace: PathBuf, + files: Vec, } -impl Config { - pub fn new(workspace: PathBuf) -> Config { - Config { workspace } - } +pub fn all_config_files(workspace: &PathBuf) -> Result, Error> { + let matcher = globset::GlobBuilder::new("workspace*.toml") + .literal_separator(true) + .build()? + .compile_matcher(); + let entries: Vec = fs::read_dir(&workspace)? + .map(|res| res.map(|e| e.file_name())) + .collect::, std::io::Error>>()?; + let mut entries_that_exist: Vec = 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, Error> { - let matcher = globset::GlobBuilder::new("workspace*.toml") - .literal_separator(true) - .build()? - .compile_matcher(); - let entries: Vec = fs::read_dir(&self.workspace)? - .map(|res| res.map(|e| e.file_name())) - .collect::, std::io::Error>>()?; - let mut entries_that_exist: Vec = 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) -> Config { + Config { files } } pub fn read(&self) -> Result, 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, config: &PathBuf) -> Result<(), Error> { + pub fn write( + &self, + providers: Vec, + 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(()) diff --git a/src/main.rs b/src/main.rs index 2c696f7..59142fc 100644 --- a/src/main.rs +++ b/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![];