Skip to content

Commit aa69e0c

Browse files
rossmacarthurrossmacarthur
committed
Fix not erroring when no files matched for plugin
This only affects plugins that need to apply templates that have the `each` flag set to `true`.
1 parent a972c35 commit aa69e0c

File tree

13 files changed

+313
-60
lines changed

13 files changed

+313
-60
lines changed

‎src/lock.rs‎

Lines changed: 145 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! This module handles the downloading of `Source`s and figuring out which
44
//! files to use for `Plugins`.
55
6-
use std::collections::{HashMap, HashSet};
6+
use std::collections::HashSet;
77
use std::fmt;
88
use std::fs;
99
use std::path::{Path, PathBuf};
@@ -374,36 +374,47 @@ impl ExternalPlugin {
374374
fn lock(
375375
self,
376376
ctx: &Context,
377-
source: LockedSource,
378-
matches: &[String],
379-
apply: &[String],
377+
templates: &IndexMap<String, Template>,
378+
locked_source: LockedSource,
379+
global_matches: &[String],
380+
global_apply: &[String],
380381
) -> Result<LockedExternalPlugin> {
381-
Ok(if let Source::Remote { .. } = self.source {
382-
let LockedSource { dir, file } = source;
382+
let Self {
383+
name,
384+
source,
385+
dir,
386+
uses,
387+
apply,
388+
} = self;
389+
390+
let apply = apply.unwrap_or_else(|| global_apply.to_vec());
391+
392+
Ok(if let Source::Remote { .. } = source {
393+
let LockedSource { dir, file } = locked_source;
383394
LockedExternalPlugin {
384-
name: self.name,
395+
name,
385396
source_dir: dir,
386397
plugin_dir: None,
387398
files: vec![file.unwrap()],
388-
apply: self.apply.unwrap_or_else(|| apply.to_vec()),
399+
apply,
389400
}
390401
} else {
391402
// Handlebars instance to do the rendering
392-
let mut templates = handlebars::Handlebars::new();
393-
templates.set_strict_mode(true);
403+
let mut hbs = handlebars::Handlebars::new();
404+
hbs.set_strict_mode(true);
394405

395406
// Data to use in template rendering
396407
let mut data = hashmap! {
397408
"root" => ctx
398409
.root()
399410
.to_str()
400411
.context("root directory is not valid UTF-8")?,
401-
"name" => &self.name
412+
"name" => &name
402413
};
403414

404-
let source_dir = source.dir;
405-
let plugin_dir = if let Some(dir) = self.dir {
406-
let rendered = templates
415+
let source_dir = locked_source.dir;
416+
let plugin_dir = if let Some(dir) = dir {
417+
let rendered = hbs
407418
.render_template(&dir, &data)
408419
.with_context(s!("failed to render template `{}`", dir))?;
409420
Some(source_dir.join(rendered))
@@ -420,9 +431,9 @@ impl ExternalPlugin {
420431
let mut files = Vec::new();
421432

422433
// If the plugin defined what files to use, we do all of them.
423-
if let Some(uses) = &self.uses {
434+
if let Some(uses) = &uses {
424435
for u in uses {
425-
let pattern = templates
436+
let pattern = hbs
426437
.render_template(u, &data)
427438
.with_context(s!("failed to render template `{}`", u))?;
428439
if !Self::match_globs(dir, &pattern, &mut files)? {
@@ -431,22 +442,29 @@ impl ExternalPlugin {
431442
}
432443
// Otherwise we try to figure out which files to use...
433444
} else {
434-
for g in matches {
435-
let pattern = templates
445+
for g in global_matches {
446+
let pattern = hbs
436447
.render_template(g, &data)
437448
.with_context(s!("failed to render template `{}`", g))?;
438449
if Self::match_globs(dir, &pattern, &mut files)? {
439450
break;
440451
}
441452
}
453+
if files.is_empty()
454+
&& templates
455+
.iter()
456+
.any(|(key, value)| apply.contains(key) && value.each)
457+
{
458+
bail!("no files matched for `{}`", &name);
459+
}
442460
}
443461

444462
LockedExternalPlugin {
445-
name: self.name,
463+
name,
446464
source_dir,
447465
plugin_dir,
448466
files,
449-
apply: self.apply.unwrap_or_else(|| apply.to_vec()),
467+
apply,
450468
}
451469
})
452470
}
@@ -459,17 +477,31 @@ impl Config {
459477
/// validates that local plugins are present, and checks that templates
460478
/// can compile.
461479
pub fn lock(self, ctx: &Context) -> Result<LockedConfig> {
462-
let shell = self.shell;
480+
let Self {
481+
shell,
482+
matches,
483+
apply,
484+
templates,
485+
plugins,
486+
} = self;
487+
488+
let templates = {
489+
let mut map = DEFAULT_TEMPLATES.clone();
490+
for (name, template) in templates {
491+
map.insert(name, template);
492+
}
493+
map
494+
};
463495

464496
// Partition the plugins into external and inline plugins.
465-
let (externals, inlines): (Vec<_>, Vec<_>) = self
466-
.plugins
467-
.into_iter()
468-
.enumerate()
469-
.partition_map(|(index, plugin)| match plugin {
470-
Plugin::External(plugin) => Either::Left((index, plugin)),
471-
Plugin::Inline(plugin) => Either::Right((index, LockedPlugin::Inline(plugin))),
472-
});
497+
let (externals, inlines): (Vec<_>, Vec<_>) =
498+
plugins
499+
.into_iter()
500+
.enumerate()
501+
.partition_map(|(index, plugin)| match plugin {
502+
Plugin::External(plugin) => Either::Left((index, plugin)),
503+
Plugin::Inline(plugin) => Either::Right((index, LockedPlugin::Inline(plugin))),
504+
});
473505

474506
// Create a map of unique `Source` to `Vec<Plugin>`
475507
let mut map = IndexMap::new();
@@ -479,11 +511,11 @@ impl Config {
479511
.push((index, plugin));
480512
}
481513

482-
let matches = &self.matches.as_ref().unwrap_or_else(|| match shell {
514+
let matches = &matches.as_ref().unwrap_or_else(|| match shell {
483515
Some(Shell::Bash) => &*DEFAULT_MATCHES_BASH,
484516
Some(Shell::Zsh) | None => &*DEFAULT_MATCHES_ZSH,
485517
});
486-
let apply = &self.apply.as_ref().unwrap_or(&*DEFAULT_APPLY);
518+
let apply = &apply.as_ref().unwrap_or(&*DEFAULT_APPLY);
487519
let count = map.len();
488520
let mut errors = Vec::new();
489521

@@ -507,7 +539,7 @@ impl Config {
507539
locked.push((
508540
index,
509541
plugin
510-
.lock(ctx, source.clone(), matches, apply)
542+
.lock(ctx, &templates, source.clone(), matches, apply)
511543
.with_context(s!("failed to install plugin `{}`", name)),
512544
));
513545
}
@@ -551,7 +583,7 @@ impl Config {
551583

552584
Ok(LockedConfig {
553585
settings: ctx.settings().clone(),
554-
templates: self.templates,
586+
templates,
555587
errors,
556588
plugins,
557589
})
@@ -685,20 +717,10 @@ impl LockedConfig {
685717

686718
/// Generate the script.
687719
pub fn source(&self, ctx: &Context) -> Result<String> {
688-
// Collaborate the default templates and the configured ones.
689-
let mut templates_map: HashMap<&str, &Template> =
690-
HashMap::with_capacity(DEFAULT_TEMPLATES.len() + self.templates.len());
691-
for (name, template) in DEFAULT_TEMPLATES.iter() {
692-
templates_map.insert(name, template);
693-
}
694-
for (name, template) in &self.templates {
695-
templates_map.insert(name, template);
696-
}
697-
698720
// Compile the templates
699721
let mut templates = handlebars::Handlebars::new();
700722
templates.set_strict_mode(true);
701-
for (name, template) in &templates_map {
723+
for (name, template) in &self.templates {
702724
templates
703725
.register_template_string(&name, &template.value)
704726
.with_context(s!("failed to compile template `{}`", name))?;
@@ -727,7 +749,7 @@ impl LockedConfig {
727749
"directory" => dir_as_str,
728750
};
729751

730-
if templates_map.get(name.as_str()).unwrap().each {
752+
if self.templates.get(name.as_str()).unwrap().each {
731753
for file in &plugin.files {
732754
let as_str =
733755
file.to_str().context("plugin file is not valid UTF-8")?;
@@ -1126,7 +1148,13 @@ mod tests {
11261148
let clone_dir = dir.join("repos/github.com/rossmacarthur/sheldon-test");
11271149

11281150
let locked = plugin
1129-
.lock(&ctx, locked_source, &[], &["hello".into()])
1151+
.lock(
1152+
&ctx,
1153+
&DEFAULT_TEMPLATES.clone(),
1154+
locked_source,
1155+
&[],
1156+
&["hello".into()],
1157+
)
11301158
.unwrap();
11311159

11321160
assert_eq!(locked.name, String::from("test"));
@@ -1162,6 +1190,7 @@ mod tests {
11621190
let locked = plugin
11631191
.lock(
11641192
&ctx,
1193+
&DEFAULT_TEMPLATES.clone(),
11651194
locked_source,
11661195
&["*.plugin.zsh".to_string()],
11671196
&["hello".to_string()],
@@ -1174,6 +1203,68 @@ mod tests {
11741203
assert_eq!(locked.apply, vec![String::from("hello")]);
11751204
}
11761205

1206+
#[test]
1207+
fn external_plugin_lock_git_with_matches_error() {
1208+
let temp = tempfile::tempdir().expect("create temporary directory");
1209+
let dir = temp.path();
1210+
let ctx = create_test_context(dir);
1211+
let plugin = ExternalPlugin {
1212+
name: "test".to_string(),
1213+
source: Source::Git {
1214+
url: Url::parse("https://github.com/rossmacarthur/sheldon-test").unwrap(),
1215+
reference: Some(GitReference::Tag("v0.1.0".to_string())),
1216+
},
1217+
dir: None,
1218+
uses: None,
1219+
apply: None,
1220+
};
1221+
let locked_source = plugin.source.clone().lock(&ctx).unwrap();
1222+
1223+
plugin
1224+
.lock(
1225+
&ctx,
1226+
&DEFAULT_TEMPLATES.clone(),
1227+
locked_source,
1228+
&["*doesnotexist*".to_string()],
1229+
&["source".to_string()],
1230+
)
1231+
.unwrap_err();
1232+
}
1233+
1234+
#[test]
1235+
fn external_plugin_lock_git_with_matches_not_each() {
1236+
let temp = tempfile::tempdir().expect("create temporary directory");
1237+
let dir = temp.path();
1238+
let ctx = create_test_context(dir);
1239+
let plugin = ExternalPlugin {
1240+
name: "test".to_string(),
1241+
source: Source::Git {
1242+
url: Url::parse("https://github.com/rossmacarthur/sheldon-test").unwrap(),
1243+
reference: Some(GitReference::Tag("v0.1.0".to_string())),
1244+
},
1245+
dir: None,
1246+
uses: None,
1247+
apply: None,
1248+
};
1249+
let locked_source = plugin.source.clone().lock(&ctx).unwrap();
1250+
let clone_dir = dir.join("repos/github.com/rossmacarthur/sheldon-test");
1251+
1252+
let locked = plugin
1253+
.lock(
1254+
&ctx,
1255+
&DEFAULT_TEMPLATES.clone(),
1256+
locked_source,
1257+
&["*doesnotexist*".to_string()],
1258+
&["PATH".to_string()],
1259+
)
1260+
.unwrap();
1261+
1262+
assert_eq!(locked.name, String::from("test"));
1263+
assert_eq!(locked.dir(), clone_dir);
1264+
assert!(locked.files.is_empty());
1265+
assert_eq!(locked.apply, vec![String::from("PATH")]);
1266+
}
1267+
11771268
#[test]
11781269
fn external_plugin_lock_remote() {
11791270
let temp = tempfile::tempdir().expect("create temporary directory");
@@ -1195,7 +1286,13 @@ mod tests {
11951286
let download_dir = dir.join("downloads/github.com/rossmacarthur/sheldon-test/raw/master");
11961287

11971288
let locked = plugin
1198-
.lock(&ctx, locked_source, &[], &["hello".to_string()])
1289+
.lock(
1290+
&ctx,
1291+
&DEFAULT_TEMPLATES.clone(),
1292+
locked_source,
1293+
&[],
1294+
&["hello".to_string()],
1295+
)
11991296
.unwrap();
12001297

12011298
assert_eq!(locked.name, String::from("test"));
@@ -1221,7 +1318,7 @@ mod tests {
12211318

12221319
assert_eq!(&locked.settings, ctx.settings());
12231320
assert_eq!(locked.plugins, Vec::new());
1224-
assert_eq!(locked.templates, IndexMap::new());
1321+
assert_eq!(locked.templates, DEFAULT_TEMPLATES.clone());
12251322
assert_eq!(locked.errors.len(), 0);
12261323
}
12271324

‎tests/cases/clean‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,21 @@ lock_file = "<root>/plugins.lock"
99
clone_dir = "<root>/repos"
1010
download_dir = "<root>/downloads"
1111
plugins = []
12+
[templates.PATH]
13+
value = "export PATH=\"{{ dir }}:$PATH\""
14+
each = false
1215

13-
[templates]
16+
[templates.path]
17+
value = "path=( \"{{ dir }}\" $path )"
18+
each = false
19+
20+
[templates.fpath]
21+
value = "fpath=( \"{{ dir }}\" $fpath )"
22+
each = false
23+
24+
[templates.source]
25+
value = "source \"{{ file }}\""
26+
each = true
1427

1528
# lock.stdout
1629

‎tests/cases/clean_permission_denied‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,21 @@ lock_file = "<root>/plugins.lock"
99
clone_dir = "<root>/repos"
1010
download_dir = "<root>/downloads"
1111
plugins = []
12+
[templates.PATH]
13+
value = "export PATH=\"{{ dir }}:$PATH\""
14+
each = false
1215

13-
[templates]
16+
[templates.path]
17+
value = "path=( \"{{ dir }}\" $path )"
18+
each = false
19+
20+
[templates.fpath]
21+
value = "fpath=( \"{{ dir }}\" $fpath )"
22+
each = false
23+
24+
[templates.source]
25+
value = "source \"{{ file }}\""
26+
each = true
1427

1528
# lock.stdout
1629

0 commit comments

Comments
 (0)