Merge pull request #134 from rage/tmcprojectyml-inheritance

course refresh merges the root .tmcproject.yml with every exercise's

Author: Heliozoa

tmc-langs-framework/src/tmc_project_yml.rs

@@ -3,14 +3,16 @@
 use crate::TmcError;
 use serde::{
     de::{Error, Visitor},
-    Deserialize, Deserializer,
+    Deserialize, Deserializer, Serialize,
 };
 use std::fmt;
+use std::ops::Deref;
 use std::path::{Path, PathBuf};
-use tmc_langs_util::file_util;
+use tmc_langs_util::{file_util, FileError};
 
 /// Extra data from a `.tmcproject.yml` file.
-#[derive(Debug, Deserialize, Default)]
+// NOTE: when adding fields, remember to update the merge function as well
+#[derive(Debug, Serialize, Deserialize, Default, Clone)]
 pub struct TmcProjectYml {
     #[serde(default)]
     pub extra_student_files: Vec<PathBuf>,
@@ -36,9 +38,12 @@ pub struct TmcProjectYml {
 }
 
 impl TmcProjectYml {
+    fn path_in_dir(dir: &Path) -> PathBuf {
+        dir.join(".tmcproject.yml")
+    }
+
     pub fn from(project_dir: &Path) -> Result<Self, TmcError> {
-        let mut config_path = project_dir.to_owned();
-        config_path.push(".tmcproject.yml");
+        let config_path = Self::path_in_dir(project_dir);
 
         if !config_path.exists() {
             log::trace!("no config found at {}", config_path.display());
@@ -50,11 +55,47 @@ impl TmcProjectYml {
         log::trace!("read {:#?}", config);
         Ok(config)
     }
+
+    /// Merges the contents of `with` with `self`.
+    /// Empty or missing values in self are replaced with those from with. Other values are left unchanged.
+    pub fn merge(&mut self, with: Self) {
+        if self.extra_student_files.is_empty() {
+            self.extra_student_files = with.extra_student_files;
+        }
+        if self.extra_exercise_files.is_empty() {
+            self.extra_exercise_files = with.extra_exercise_files;
+        }
+        if self.force_update.is_empty() {
+            self.force_update = with.force_update;
+        }
+        if self.tests_timeout_ms.is_none() {
+            self.tests_timeout_ms = with.tests_timeout_ms;
+        }
+        if self.no_tests.is_none() {
+            self.no_tests = with.no_tests;
+        }
+        if self.fail_on_valgrind_error.is_none() {
+            self.fail_on_valgrind_error = with.fail_on_valgrind_error;
+        }
+        if self.minimum_python_version.is_none() {
+            self.minimum_python_version = with.minimum_python_version;
+        }
+    }
+
+    pub fn save_to_dir(&self, dir: &Path) -> Result<(), TmcError> {
+        let config_path = Self::path_in_dir(dir);
+        let mut file = file_util::create_file_lock(&config_path)?;
+        let guard = file
+            .lock()
+            .map_err(|e| FileError::FdLock(config_path.clone(), e))?;
+        serde_yaml::to_writer(guard.deref(), &self)?;
+        Ok(())
+    }
 }
 
 /// Minimum Python version requirement.
 /// TODO: if patch is Some minor is also guaranteed to be Some etc. encode this in the type system
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone, Copy, Serialize)]
 pub struct PythonVer {
     pub major: Option<usize>,
     pub minor: Option<usize>,
@@ -112,7 +153,7 @@ impl<'de> Deserialize<'de> for PythonVer {
 }
 
 /// Contents of the no-tests field.
-#[derive(Debug, Deserialize)]
+#[derive(Debug, Serialize, Deserialize, Clone)]
 #[serde(from = "NoTestsWrapper")]
 pub struct NoTests {
     pub flag: bool,
@@ -167,8 +208,16 @@ pub enum IntOrString {
 mod test {
     use super::*;
 
+    fn init() {
+        use log::*;
+        use simple_logger::*;
+        let _ = SimpleLogger::new().with_level(LevelFilter::Debug).init();
+    }
+
     #[test]
     fn deserialize_no_tests() {
+        init();
+
         let no_tests_yml = r#"no-tests:
   points:
     - 1
@@ -183,6 +232,8 @@ mod test {
 
     #[test]
     fn deserialize_python_ver() {
+        init();
+
         let python_ver: PythonVer = serde_yaml::from_str("1.2.3").unwrap();
         assert_eq!(python_ver.major, Some(1));
         assert_eq!(python_ver.minor, Some(2));
@@ -200,4 +251,42 @@ mod test {
 
         assert!(serde_yaml::from_str::<PythonVer>("asd").is_err())
     }
+
+    #[test]
+    fn merges() {
+        init();
+
+        let tpy_root = TmcProjectYml {
+            tests_timeout_ms: Some(123),
+            fail_on_valgrind_error: Some(true),
+            ..Default::default()
+        };
+        let mut tpy_exercise = TmcProjectYml {
+            tests_timeout_ms: Some(234),
+            ..Default::default()
+        };
+        tpy_exercise.merge(tpy_root);
+        assert_eq!(tpy_exercise.tests_timeout_ms, Some(234));
+        assert_eq!(tpy_exercise.fail_on_valgrind_error, Some(true));
+    }
+
+    #[test]
+    fn saves_to_dir() {
+        init();
+
+        let temp = tempfile::tempdir().unwrap();
+        let path = TmcProjectYml::path_in_dir(temp.path());
+
+        assert!(!path.exists());
+
+        let tpy = TmcProjectYml {
+            tests_timeout_ms: Some(1234),
+            ..Default::default()
+        };
+        tpy.save_to_dir(temp.path()).unwrap();
+
+        assert!(path.exists());
+        let tpy = TmcProjectYml::from(temp.path()).unwrap();
+        assert_eq!(tpy.tests_timeout_ms, Some(1234));
+    }
 }

tmc-langs-util/src/file_util.rs

@@ -112,7 +112,7 @@ pub fn create_file<P: AsRef<Path>>(path: P) -> Result<File, FileError> {
     File::create(path).map_err(|e| FileError::FileCreate(path.to_path_buf(), e))
 }
 
-/// Creates a file and immediately locks it. If a file already exists at the path, it acquires a lock on it first and then recreates it.
+/// Creates a file and wraps it in a lock. If a file already exists at the path, it acquires a lock on it first and then recreates it.
 /// Note: creates all intermediary directories if needed.
 pub fn create_file_lock<P: AsRef<Path>>(path: P) -> Result<FdLockWrapper, FileError> {
     log::debug!("locking file {}", path.as_ref().display());

tmc-langs/src/course_refresher.rs

@@ -7,7 +7,7 @@ use serde_yaml::Mapping;
 use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::time::Duration;
-use tmc_langs_framework::TmcCommand;
+use tmc_langs_framework::{TmcCommand, TmcProjectYml};
 use tmc_langs_util::file_util;
 use walkdir::WalkDir;
 
@@ -44,7 +44,7 @@ pub fn refresh_course(
     cache_root: PathBuf,
 ) -> Result<RefreshData, LangsError> {
     log::info!("refreshing course {}", course_name);
-    start_stage(9, "Refreshing course");
+    start_stage(10, "Refreshing course");
 
     // create new cache path
     let old_version = course_cache_path
@@ -84,7 +84,13 @@ pub fn refresh_course(
     let exercise_dirs = super::find_exercise_directories(&new_clone_path)?
         .into_iter()
         .map(|ed| ed.strip_prefix(&new_clone_path).unwrap().to_path_buf()) // safe
-        .collect();
+        .collect::<Vec<_>>();
+
+    // merge the root config with each exercise's, if any
+    if let Ok(root_tmcproject) = TmcProjectYml::from(&course_cache_path) {
+        merge_tmcproject_configs(root_tmcproject, &exercise_dirs)?;
+    }
+    progress_stage("Merged .tmcproject.yml files in exercise directories to the root file, if any");
 
     // make_solutions
     log::info!("preparing solutions to {}", new_solution_path.display());
@@ -215,6 +221,19 @@ fn check_directory_names(path: &Path) -> Result<(), LangsError> {
     Ok(())
 }
 
+fn merge_tmcproject_configs(
+    root_tmcproject: TmcProjectYml,
+    exercise_dirs: &[PathBuf],
+) -> Result<(), LangsError> {
+    for exercise_dir in exercise_dirs {
+        if let Ok(mut exercise_tmcproject) = TmcProjectYml::from(exercise_dir) {
+            exercise_tmcproject.merge(root_tmcproject.clone());
+            exercise_tmcproject.save_to_dir(exercise_dir)?;
+        }
+    }
+    Ok(())
+}
+
 /// Checks for a course_clone_path/course_options.yml
 /// If found, course-specific options are merged into it and it is returned.
 /// Else, an empty mapping is returned.
@@ -579,4 +598,42 @@ mod test {
                 .unwrap();
         assert_eq!(checksum, "6cacf02f21f9242674a876954132fb11");
     }
+
+    #[test]
+    fn merges_tmcproject_configs() {
+        init();
+
+        let temp = tempfile::tempdir().unwrap();
+        let exap = temp.path().join("exa");
+        file_util::create_dir(&exap).unwrap();
+        let exbp = temp.path().join("exb");
+        file_util::create_dir(&exbp).unwrap();
+
+        let root = TmcProjectYml {
+            tests_timeout_ms: Some(1234),
+            fail_on_valgrind_error: Some(true),
+            ..Default::default()
+        };
+        let tpya = TmcProjectYml {
+            tests_timeout_ms: Some(2345),
+            ..Default::default()
+        };
+        tpya.save_to_dir(&exap).unwrap();
+        let tpyb = TmcProjectYml {
+            fail_on_valgrind_error: Some(false),
+            ..Default::default()
+        };
+        tpyb.save_to_dir(&exbp).unwrap();
+        let exercise_dirs = &[exap.clone(), exbp.clone()];
+
+        merge_tmcproject_configs(root, exercise_dirs).unwrap();
+
+        let tpya = TmcProjectYml::from(&exap).unwrap();
+        assert_eq!(tpya.tests_timeout_ms, Some(2345));
+        assert_eq!(tpya.fail_on_valgrind_error, Some(true));
+
+        let tpyb = TmcProjectYml::from(&exbp).unwrap();
+        assert_eq!(tpyb.tests_timeout_ms, Some(1234));
+        assert_eq!(tpyb.fail_on_valgrind_error, Some(false));
+    }
 }