diff options
author | Matthias Beyer <mail@beyermatthias.de> | 2020-12-10 08:54:02 +0100 |
---|---|---|
committer | Matthias Beyer <mail@beyermatthias.de> | 2020-12-11 10:58:29 +0100 |
commit | b25131ed65496c9cf0f616f761517590aa16d120 (patch) | |
tree | bf0076fe1dc29e87ac4ea7b7b46ecd1339d47173 /src | |
parent | b7f6276b1d3b9678f9cafa377ec8307845c3446b (diff) |
Add feature: script linting
This patch adds script linting via a configurable script.
With this patch, the configuration can point to a script or program that
gets the packaging script on STDIN and might exit with a nonzero value
on error.
The simplest script I can think of that adds value is:
#!/bin/bash
shellcheck -
to run the packaging script through shellcheck.
stdout and stderr of the linting script are printed in case of non-zero
return value.
Linting can be disabled by not setting the script in the config or by
passing a commandline flag to skip linting (a warning will be printed in
this case).
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
Diffstat (limited to 'src')
-rw-r--r-- | src/cli.rs | 7 | ||||
-rw-r--r-- | src/commands/build.rs | 73 | ||||
-rw-r--r-- | src/config/not_validated.rs | 9 | ||||
-rw-r--r-- | src/package/script.rs | 45 |
4 files changed, 132 insertions, 2 deletions
@@ -231,6 +231,13 @@ pub fn cli<'a>() -> App<'a> { .long("no-verify") .about("Do not perform a hash sum check on all packages in the dependency tree before starting the build") ) + .arg(Arg::new("no_lint") + .required(false) + .multiple(false) + .takes_value(false) + .long("no-lint") + .about("Do not perform script linting before starting the build") + ) .arg(Arg::new("staging_dir") .required(false) diff --git a/src/commands/build.rs b/src/commands/build.rs index 5170a8f..466fe15 100644 --- a/src/commands/build.rs +++ b/src/commands/build.rs @@ -13,7 +13,7 @@ use diesel::ExpressionMethods; use diesel::PgConnection; use diesel::QueryDsl; use diesel::RunQueryDsl; -use log::{debug, info, warn, trace}; +use log::{debug, error, info, warn, trace}; use tokio::stream::StreamExt; use tokio::sync::RwLock; @@ -27,6 +27,7 @@ use crate::log::LogItem; use crate::orchestrator::OrchestratorSetup; use crate::package::PackageName; use crate::package::PackageVersion; +use crate::package::ScriptBuilder; use crate::package::Shebang; use crate::package::Tree; use crate::repository::Repository; @@ -190,6 +191,76 @@ pub async fn build(matches: &ArgMatches, .await?; } + // linting the package scripts + if matches.is_present("no_lint") { + warn!("No script linting will be performed!"); + } else { + if let Some(linter) = config.script_linter().as_ref() { + let shebang = Shebang::from(config.shebang().clone()); + + let all_packages = tree.all_packages(); + let bar = progressbars.bar(); + bar.set_length(all_packages.len() as u64); + bar.set_message("Linting package scripts..."); + + let lint_error = all_packages + .into_iter() + .map(|pkg| { + let shebang = shebang.clone(); + let bar = bar.clone(); + async move { + trace!("Linting script of {} {} with '{}'", pkg.name(), pkg.version(), linter.display()); + let cmd = tokio::process::Command::new(linter); + let script = ScriptBuilder::new(&shebang) + .build(pkg, config.available_phases(), *config.strict_script_interpolation())?; + + let (status, stdout, stderr) = script.lint(cmd).await?; + bar.inc(1); + Ok((pkg.name().clone(), pkg.version().clone(), status, stdout, stderr)) + } + }) + .collect::<futures::stream::FuturesUnordered<_>>() + .collect::<Result<Vec<_>>>() + .await? + .into_iter() + .any(|tpl| { + let pkg_name = tpl.0; + let pkg_vers = tpl.1; + let status = tpl.2; + let stdout = tpl.3; + let stderr = tpl.4; + + if status.success() { + info!("Linting {pkg_name} {pkg_vers} script (exit {status}):\nstdout:\n{stdout}\n\nstderr:\n\n{stderr}", + pkg_name = pkg_name, + pkg_vers = pkg_vers, + status = status, + stdout = stdout, + stderr = stderr); + false + } else { + error!("Linting {pkg_name} {pkg_vers} errored ({status}):\n\nstdout:\n{stdout}\n\nstderr:\n{stderr}\n\n", + pkg_name = pkg_name, + pkg_vers = pkg_vers, + status = status, + stdout = stdout, + stderr = stderr + ); + true + } + }); + + if lint_error { + bar.finish_with_message("Linting errored"); + return Err(anyhow!("Linting was not successful")) + } else { + bar.finish_with_message("Finished linting package scripts"); + } + } else { + warn!("No linter set in configuration, no script linting will be performed!"); + } + } // linting + trace!("Setting up database jobs for Package, GitHash, Image"); let db_package = async { Package::create_or_fetch(&database_connection, &package) }; let db_githash = async { GitHash::create_or_fetch(&database_connection, &hash_str) }; diff --git a/src/config/not_validated.rs b/src/config/not_validated.rs index bdbce89..9f99f80 100644 --- a/src/config/not_validated.rs +++ b/src/config/not_validated.rs @@ -38,6 +38,9 @@ pub struct NotValidatedConfiguration { #[getset(get = "pub")] script_highlight_theme: Option<String>, + #[getset(get = "pub")] + script_linter: Option<PathBuf>, + #[serde(default = "default_script_shebang")] #[getset(get = "pub")] shebang: String, @@ -86,6 +89,12 @@ pub struct NotValidatedConfiguration { impl NotValidatedConfiguration { pub fn validate(self) -> Result<Configuration> { + if let Some(linter) = self.script_linter.as_ref() { + if !linter.is_file() { + return Err(anyhow!("Lint script is not a file: {}", linter.display())) + } + } + if !self.staging_directory.is_dir() { return Err(anyhow!("Not a directory: staging = {}", self.staging_directory.display())) } diff --git a/src/package/script.rs b/src/package/script.rs index 4988e0e..f37dde8 100644 --- a/src/package/script.rs +++ b/src/package/script.rs @@ -1,13 +1,18 @@ -use anyhow::anyhow; +use std::process::ExitStatus; + use anyhow::Error; +use anyhow::Context as AnyhowContext; use anyhow::Result; +use anyhow::anyhow; use handlebars::{Handlebars, HelperDef, RenderContext, Helper, Context, JsonRender, HelperResult, Output, RenderError}; +use log::trace; use serde::Deserialize; use serde::Serialize; use syntect::easy::HighlightLines; use syntect::highlighting::{ThemeSet, Style}; use syntect::parsing::SyntaxSet; use syntect::util::{as_24_bit_terminal_escaped, LinesWithEndings}; +use tokio::process::Command; use crate::package::Package; use crate::phase::Phase; @@ -40,6 +45,43 @@ impl Script { pub fn lines_numbered(&self) -> impl Iterator<Item = (usize, &str)> { self.0.lines().enumerate() } + + pub async fn lint(&self, mut cmd: Command) -> Result<(ExitStatus, String, String)> { + use tokio::io::AsyncWriteExt; + use tokio::io::BufWriter; + + let mut child = cmd + .stderr(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stdin(std::process::Stdio::piped()) + .spawn() + .context("Spawning subprocess for linting package script")?; + + trace!("Child = {:?}", child); + + { + let stdin = child.stdin.take().ok_or_else(|| anyhow!("No stdin"))?; + let mut writer = BufWriter::new(stdin); + let _ = writer + .write_all(self.0.as_bytes()) + .await + .context("Writing package script to STDIN of subprocess")?; + + let _ = writer + .flush() + .await + .context("Flushing STDIN of subprocess")?; + trace!("Script written"); + } + + trace!("Waiting for child..."); + let out = child.wait_with_output() + .await + .context("Waiting for subprocess")?; + + Ok((out.status, String::from_utf8(out.stdout)?, String::from_utf8(out.stderr)?)) + } + } #[derive(Debug)] @@ -87,6 +129,7 @@ impl<'a> HighlightedScript<'a> { pub fn lines_numbered(&'a self) -> Result<impl Iterator<Item = (usize, String)> + 'a> { self.lines().map(|iter| iter.enumerate()) } + } impl From<String> for Shebang { |