Merge pull request #88 from rage/fix-refresh-course

redesign course-refresher

Author: Heliozoa

tmc-langs-cli/src/app.rs

@@ -206,86 +206,29 @@ pub fn create_app() -> App<'static, 'static> {
         .subcommand(SubCommand::with_name("refresh-course")
             .about("Refresh the given course")
             // .long_about(schema_leaked::<RefreshData>()) // can't format YAML mapping
-            .arg(Arg::with_name("course-name")
-                .help("The name of the course.")
-                .long("course-name")
-                .required(true)
-                .takes_value(true))
             .arg(Arg::with_name("cache-path")
                 .help("Path to the cached course.")
                 .long("cache-path")
                 .required(true)
                 .takes_value(true))
-            .arg(Arg::with_name("clone-path")
-                .help("Path to the course clone.")
-                .long("clone-path")
-                .required(true)
-                .takes_value(true))
-            .arg(Arg::with_name("stub-path")
-                .help("Path to the course stub.")
-                .long("stub-path")
-                .required(true)
-                .takes_value(true))
-            .arg(Arg::with_name("stub-zip-path")
-                .help("Path to the directory where the stub zips will be created.")
-                .long("stub-zip-path")
-                .required(true)
-                .takes_value(true))
-            .arg(Arg::with_name("solution-path")
-                .help("The name of the course solution.")
-                .long("solution-path")
-                .required(true)
-                .takes_value(true))
-            .arg(Arg::with_name("solution-zip-path")
-                .help("Path to the directory where the solution zips will be created.")
-                .long("solution-zip-path")
+            .arg(Arg::with_name("cache-root")
+                .help("The cache root.")
+                .long("cache-root")
                 .required(true)
                 .takes_value(true))
-            .arg(Arg::with_name("exercise")
-                .help("An exercise. Takes 3 values: the exercise's name, its relative path, and a comma separated list of its available points. Multiple exercises can be given.")
-                .long("exercise")
-                .takes_value(true)
-                .multiple(true)
-                .number_of_values(3)
-                .value_names(&["exercise name", "relative path", "available points"]))
-            .arg(Arg::with_name("source-backend")
-                .help("The version control used for the course.")
-                .long("source-backend")
-                .required(true)
-                .takes_value(true)
-                .possible_values(&["git"]))
-            .arg(Arg::with_name("source-url")
-                .help("Version control URL.")
-                .long("source-url")
+            .arg(Arg::with_name("course-name")
+                .help("The name of the course.")
+                .long("course-name")
                 .required(true)
                 .takes_value(true))
             .arg(Arg::with_name("git-branch")
                 .help("Version control branch.")
                 .long("git-branch")
                 .required(true)
                 .takes_value(true))
-            .arg(Arg::with_name("no-directory-changes")
-                .help("If set, the cache is not reset and the clone is not updated/cloned, and the solutions and stubs are not prepared.")
-                .long("no-directory-changes"))
-            .arg(Arg::with_name("no-background-operations")
-                .help("If set, will not update available points.")
-                .long("no-background-operations"))
-            .arg(Arg::with_name("chmod-bits")
-                .help("The unix file permission bits in octal notation used for the directories from the rails root to the cache root and the cache root's contents.")
-                .long("chmod-bits")
-                .takes_value(true))
-            .arg(Arg::with_name("chgrp-uid")
-                .help("The UID of the owner of the directories from the rails root to the cache root and the cache root's contents.")
-                .long("chgrp-uid")
-                .takes_value(true))
-            .arg(Arg::with_name("cache-root")
-                .help("The cache root.")
-                .long("cache-root")
-                .required(true)
-                .takes_value(true))
-            .arg(Arg::with_name("rails-root")
-                .help("The Rails root directory.")
-                .long("rails-root")
+            .arg(Arg::with_name("source-url")
+                .help("Version control URL.")
+                .long("source-url")
                 .required(true)
                 .takes_value(true)))
 
@@ -943,40 +886,12 @@ mod base_test {
             "path",
             "--cache-root",
             "path",
-            "--chgrp-uid",
-            "1234",
-            "--chmod-bits",
-            "1234",
-            "--clone-path",
-            "path",
             "--course-name",
             "name",
-            "--exercise",
-            "name",
-            "path",
-            "10,11,12",
-            "--exercise",
-            "second",
-            "path",
-            "20,21,22",
             "--git-branch",
             "main",
-            "--no-background-operations",
-            "--no-directory-changes",
-            "--rails-root",
-            "path",
-            "--solution-path",
-            "path",
-            "--solution-zip-path",
-            "path",
-            "--source-backend",
-            "git",
             "--source-url",
             "example.com",
-            "--stub-path",
-            "path",
-            "--stub-zip-path",
-            "path",
         ]);
     }
 

tmc-langs-cli/src/main.rs

@@ -36,9 +36,7 @@ use tmc_client::{ClientError, FeedbackAnswer, TmcClient, Token};
 use tmc_langs_framework::{domain::StyleValidationResult, error::CommandError, file_util};
 use tmc_langs_util::{
     progress_reporter::ProgressReporter,
-    task_executor::{
-        self, Course, GroupBits, ModeBits, Options, RefreshExercise, SourceBackend, TmcParams,
-    },
+    task_executor::{self, TmcParams},
     Language, OutputFormat,
 };
 use toml::{map::Map as TomlMap, Value as TomlValue};
@@ -559,84 +557,16 @@ fn run_app(matches: ArgMatches, pretty: bool, warnings: &mut Vec<anyhow::Error>)
         ("refresh-course", Some(matches)) => {
             let cache_path = matches.value_of("cache-path").unwrap();
             let cache_root = matches.value_of("cache-root").unwrap();
-            let chgrp_uid = matches.value_of("chgrp-uid");
-            let chmod_bits = matches.value_of("chmod-bits");
-            let clone_path = matches.value_of("clone-path").unwrap();
             let course_name = matches.value_of("course-name").unwrap();
-
-            let exercise_args = matches.values_of("exercise");
             let git_branch = matches.value_of("git-branch").unwrap();
-            let no_background_operations = matches.is_present("no-background-operations");
-            let no_directory_changes = matches.is_present("no-directory-changes");
-            let rails_root = matches.value_of("rails-root").unwrap();
-
-            let solution_path = matches.value_of("solution-path").unwrap();
-            let solution_zip_path = matches.value_of("solution-zip-path").unwrap();
-            let source_backend = matches.value_of("source-backend").unwrap();
             let source_url = matches.value_of("source-url").unwrap();
-            let stub_path = matches.value_of("stub-path").unwrap();
-            let stub_zip_path = matches.value_of("stub-zip-path").unwrap();
-
-            let mut exercises = vec![];
-            if let Some(mut exercise_args) = exercise_args {
-                while let Some(exercise_name) = exercise_args.next() {
-                    let relative_path = exercise_args.next().unwrap();
-                    let available_points: Vec<_> =
-                        exercise_args.next().unwrap().split(',').collect();
-                    exercises.push(RefreshExercise {
-                        name: exercise_name.to_string(),
-                        relative_path: PathBuf::from(relative_path),
-                        available_points: available_points
-                            .into_iter()
-                            .map(str::to_string)
-                            .filter(|s| !s.is_empty())
-                            .collect(),
-                    });
-                }
-            }
-            let source_backend = match source_backend {
-                "git" => SourceBackend::Git,
-                _ => unreachable!("validation error"),
-            };
-            let course = Course {
-                name: course_name.to_string(),
-                cache_path: PathBuf::from(cache_path),
-                clone_path: PathBuf::from(clone_path),
-                stub_path: PathBuf::from(stub_path),
-                stub_zip_path: PathBuf::from(stub_zip_path),
-                solution_path: PathBuf::from(solution_path),
-                solution_zip_path: solution_zip_path.into(),
-                exercises,
-                source_backend,
-                source_url: source_url.to_string(),
-                git_branch: git_branch.to_string(),
-            };
-            let options = Options {
-                no_background_operations,
-                no_directory_changes,
-            };
-            let chmod_bits = if let Some(chmod_bits) = chmod_bits {
-                Some(ModeBits::from_str_radix(chmod_bits, 8).with_context(|| {
-                    format!("Failed to convert chmod bits to an integer: {}", chmod_bits,)
-                })?)
-            } else {
-                None
-            };
-            let chgrp_uid = if let Some(chgrp_uid) = chgrp_uid {
-                Some(GroupBits::from_str_radix(chgrp_uid, 10).with_context(|| {
-                    format!("Failed to convert chgrp UID to an integer: {}", chgrp_uid,)
-                })?)
-            } else {
-                None
-            };
 
             let refresh_result = task_executor::refresh_course(
-                course,
-                options,
-                chmod_bits,
-                chgrp_uid,
+                course_name.to_string(),
+                PathBuf::from(cache_path),
+                source_url.to_string(),
+                git_branch.to_string(),
                 PathBuf::from(cache_root),
-                PathBuf::from(rails_root),
                 move |update| {
                     let output = Output::StatusUpdate(StatusUpdateData::None(update));
                     print_output(&output, pretty, &[])?;

tmc-langs-framework/src/tmc_project_yml.rs

@@ -41,7 +41,7 @@ impl TmcProjectYml {
         config_path.push(".tmcproject.yml");
 
         if !config_path.exists() {
-            log::debug!("no config found at {}", config_path.display());
+            log::trace!("no config found at {}", config_path.display());
             return Ok(Self::default());
         }
         log::debug!("reading .tmcproject.yml from {}", config_path.display());

tmc-langs-util/src/error.rs

@@ -1,11 +1,11 @@
 //! Contains the crate error type
 
-#[cfg(unix)]
-use crate::task_executor::ModeBits;
 use std::path::PathBuf;
 use thiserror::Error;
 use tmc_langs_framework::error::FileIo;
 
+use crate::task_executor::ModeBits;
+
 #[derive(Debug, Error)]
 pub enum UtilError {
     #[error("Failed to create temporary file")]
@@ -32,12 +32,8 @@ pub enum UtilError {
     #[error("Error while writing file to zip")]
     ZipWrite(#[source] std::io::Error),
 
-    #[error("Unsupported source backend")]
-    UnsupportedSourceBackend,
     #[error("Path {0} contained a dash '-' which is currently not allowed")]
     InvalidDirectory(PathBuf),
-    #[error("The cache path  ({0}) must be inside the rails root path ({1})")]
-    CacheNotInRailsRoot(PathBuf, PathBuf),
 
     #[error(transparent)]
     TmcError(#[from] tmc_langs_framework::TmcError),

tmc-langs-util/src/task_executor.rs

@@ -6,13 +6,10 @@ mod submission_processing;
 mod tar_helper;
 mod tmc_zip;
 
-pub use self::course_refresher::{
-    Course, GroupBits, ModeBits, Options, RefreshData, RefreshExercise, SourceBackend,
-};
+pub use self::course_refresher::{refresh_course, ModeBits, RefreshData, RefreshExercise};
 pub use self::submission_packaging::{OutputFormat, TmcParams};
 
 use crate::error::UtilError;
-use crate::progress_reporter::StatusUpdate;
 use crate::{ExerciseDesc, ExercisePackagingConfiguration, RunResult, StyleValidationResult};
 use std::path::{Path, PathBuf};
 use tmc_langs_csharp::CSharpPlugin;
@@ -213,8 +210,8 @@ pub fn find_exercise_directories(exercise_path: &Path) -> Result<Vec<PathBuf>, U
     let mut paths = vec![];
     for entry in WalkDir::new(exercise_path).into_iter().filter_entry(|e| {
         !submission_processing::is_hidden_dir(e)
-            || e.file_name() == "private"
-            || submission_processing::contains_tmcignore(e)
+            && e.file_name() != "private"
+            && !submission_processing::contains_tmcignore(e)
     }) {
         let entry = entry?;
         // TODO: Java implementation doesn't scan root directories
@@ -231,29 +228,6 @@ pub fn get_available_points(exercise_path: &Path) -> Result<Vec<String>, UtilErr
     Ok(points)
 }
 
-pub fn refresh_course(
-    course: Course,
-    options: Options,
-    chmod_bits: Option<ModeBits>,
-    chgrp_uid: Option<GroupBits>,
-    cache_root: PathBuf,
-    rails_root: PathBuf,
-    progress_reporter: impl 'static
-        + Sync
-        + Send
-        + Fn(StatusUpdate<()>) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>,
-) -> Result<RefreshData, UtilError> {
-    course_refresher::refresh_course(
-        course,
-        options,
-        chmod_bits,
-        chgrp_uid,
-        cache_root,
-        rails_root,
-        progress_reporter,
-    )
-}
-
 // enum containing all the plugins
 #[impl_enum::with_methods(
     fn clean(&self, path: &Path) -> Result<(), TmcError> {}
@@ -278,28 +252,56 @@ enum Plugin {
 // Get language plugin for the given path.
 fn get_language_plugin(path: &Path) -> Result<Plugin, TmcError> {
     if NoTestsPlugin::is_exercise_type_correct(path) {
-        log::info!("Detected project as {}", NoTestsPlugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            NoTestsPlugin::PLUGIN_NAME
+        );
         Ok(Plugin::NoTests(NoTestsPlugin::new()))
     } else if CSharpPlugin::is_exercise_type_correct(path) {
         let csharp = CSharpPlugin::new();
-        log::info!("Detected project as {}", CSharpPlugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            CSharpPlugin::PLUGIN_NAME
+        );
         Ok(Plugin::CSharp(csharp))
     } else if MakePlugin::is_exercise_type_correct(path) {
         let make = MakePlugin::new();
-        log::info!("Detected project as {}", MakePlugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            MakePlugin::PLUGIN_NAME
+        );
         Ok(Plugin::Make(make))
     } else if Python3Plugin::is_exercise_type_correct(path) {
-        log::info!("Detected project as {}", Python3Plugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            Python3Plugin::PLUGIN_NAME
+        );
         Ok(Plugin::Python3(Python3Plugin::new()))
     } else if RPlugin::is_exercise_type_correct(path) {
-        log::info!("Detected project as {}", RPlugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            RPlugin::PLUGIN_NAME
+        );
         Ok(Plugin::R(RPlugin::new()))
     } else if MavenPlugin::is_exercise_type_correct(path) {
-        log::info!("Detected project as {}", MavenPlugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            MavenPlugin::PLUGIN_NAME
+        );
         Ok(Plugin::Maven(MavenPlugin::new()?))
     } else if AntPlugin::is_exercise_type_correct(path) {
         // TODO: currently, ant needs to be last because any project with src and test are recognized as ant
-        log::info!("Detected project as {}", AntPlugin::PLUGIN_NAME);
+        log::info!(
+            "Detected project at {} as {}",
+            path.display(),
+            AntPlugin::PLUGIN_NAME
+        );
         Ok(Plugin::Ant(AntPlugin::new()?))
     } else {
         Err(TmcError::PluginNotFound(path.to_path_buf()))