From 80ce3e8c664e0e4a30adafcd05334d06e61d48b0 Mon Sep 17 00:00:00 2001 From: Daniel Martinez Date: Fri, 29 Jan 2021 20:06:51 +0200 Subject: [PATCH 1/3] redesign course-refresher --- tmc-langs-cli/src/app.rs | 103 +- tmc-langs-cli/src/main.rs | 80 +- tmc-langs-framework/src/tmc_project_yml.rs | 2 +- tmc-langs-util/src/error.rs | 3 +- tmc-langs-util/src/task_executor.rs | 74 +- .../src/task_executor/course_refresher.rs | 1098 +++++------------ 6 files changed, 348 insertions(+), 1012 deletions(-) diff --git a/tmc-langs-cli/src/app.rs b/tmc-langs-cli/src/app.rs index fd9b16004d9..bcb068ec3dc 100644 --- a/tmc-langs-cli/src/app.rs +++ b/tmc-langs-cli/src/app.rs @@ -206,57 +206,19 @@ pub fn create_app() -> App<'static, 'static> { .subcommand(SubCommand::with_name("refresh-course") .about("Refresh the given course") // .long_about(schema_leaked::()) // 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") @@ -264,28 +226,9 @@ pub fn create_app() -> App<'static, 'static> { .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", ]); } diff --git a/tmc-langs-cli/src/main.rs b/tmc-langs-cli/src/main.rs index 9abbd4758c0..f11f2b13058 100644 --- a/tmc-langs-cli/src/main.rs +++ b/tmc-langs-cli/src/main.rs @@ -33,9 +33,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}; @@ -556,84 +554,16 @@ fn run_app(matches: ArgMatches, pretty: bool, warnings: &mut Vec) ("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, &[])?; diff --git a/tmc-langs-framework/src/tmc_project_yml.rs b/tmc-langs-framework/src/tmc_project_yml.rs index 4661f2ddc71..8696f0107c2 100644 --- a/tmc-langs-framework/src/tmc_project_yml.rs +++ b/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()); diff --git a/tmc-langs-util/src/error.rs b/tmc-langs-util/src/error.rs index 1ead03e5425..3fe8824c6d9 100644 --- a/tmc-langs-util/src/error.rs +++ b/tmc-langs-util/src/error.rs @@ -1,7 +1,6 @@ //! 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; @@ -53,7 +52,7 @@ pub enum UtilError { NixPermissionChange(PathBuf, #[source] nix::Error), #[cfg(unix)] #[error("Invalid chmod flag: {0}")] - NixFlag(ModeBits), + NixFlag(u32), #[error(transparent)] DynError(#[from] Box), diff --git a/tmc-langs-util/src/task_executor.rs b/tmc-langs-util/src/task_executor.rs index 3c721b3f8fb..6a1121c4cc5 100644 --- a/tmc-langs-util/src/task_executor.rs +++ b/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, 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, 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, UtilErr Ok(points) } -pub fn refresh_course( - course: Course, - options: Options, - chmod_bits: Option, - chgrp_uid: Option, - cache_root: PathBuf, - rails_root: PathBuf, - progress_reporter: impl 'static - + Sync - + Send - + Fn(StatusUpdate<()>) -> Result<(), Box>, -) -> Result { - 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 { 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())) diff --git a/tmc-langs-util/src/task_executor/course_refresher.rs b/tmc-langs-util/src/task_executor/course_refresher.rs index fac4679a4c6..0a167ab6cc9 100644 --- a/tmc-langs-util/src/task_executor/course_refresher.rs +++ b/tmc-langs-util/src/task_executor/course_refresher.rs @@ -6,79 +6,28 @@ use crate::{ task_executor, }; use md5::Context; -use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use serde_yaml::{Mapping, Value}; -use std::collections::{HashMap, HashSet}; +use serde_yaml::Mapping; use std::io::Write; use std::path::{Path, PathBuf}; use tmc_langs_framework::{command::TmcCommand, file_util, subprocess::Redirection}; use walkdir::WalkDir; -#[cfg(unix)] -pub type ModeBits = nix::sys::stat::mode_t; -#[cfg(not(unix))] -pub type ModeBits = u32; - -#[cfg(unix)] -pub type GroupBits = nix::libc::uid_t; -#[cfg(not(unix))] -pub type GroupBits = u32; - -#[derive(Debug, PartialEq, Eq)] -pub enum SourceBackend { - Git, -} - -#[derive(Debug)] -pub struct RefreshExercise { - pub name: String, - pub relative_path: PathBuf, - pub available_points: Vec, -} - -#[derive(Debug)] -pub struct Course { - pub name: String, - pub cache_path: PathBuf, - pub clone_path: PathBuf, - pub stub_path: PathBuf, - pub stub_zip_path: PathBuf, - pub solution_path: PathBuf, - pub solution_zip_path: PathBuf, - pub exercises: Vec, - pub source_backend: SourceBackend, - pub source_url: String, - pub git_branch: String, -} - -#[derive(Debug)] -pub struct Options { - pub no_directory_changes: bool, - pub no_background_operations: bool, -} - #[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] pub struct RefreshData { - pub new_exercises: Vec, - pub removed_exercises: Vec, - pub review_points: HashMap>, - pub metadata: HashMap, - pub checksum_stubs: HashMap, + pub new_cache_path: PathBuf, pub course_options: Mapping, - pub update_points: HashMap, -} - -#[derive(Debug)] -struct ExerciseOptions { - review_points: HashMap>, - metadata_map: HashMap, + pub exercises: Vec, } -#[derive(Debug, Serialize, Deserialize, JsonSchema)] -pub struct UpdatePoints { - added: Vec, - removed: Vec, +#[derive(Debug, Serialize, Deserialize)] +pub struct RefreshExercise { + name: String, + checksum: String, + points: Vec, + #[serde(skip)] + path: PathBuf, } struct CourseRefresher { @@ -99,158 +48,105 @@ impl CourseRefresher { pub fn refresh_course( self, - course: Course, - options: Options, - git_repos_chmod: Option, - git_repos_chgrp: Option, + course_name: String, + course_cache_path: PathBuf, + source_url: String, + git_branch: String, cache_root: PathBuf, - rails_root: PathBuf, ) -> Result { - log::info!("refreshing course {}", course.name); + log::info!("refreshing course {}", course_name); self.progress_reporter.start_timer(); // sets the total amount of progress steps properly - self.progress_reporter.increment_progress_steps(4); - if !options.no_directory_changes { - self.progress_reporter.increment_progress_steps(8); - } - if !options.no_background_operations { - self.progress_reporter.increment_progress_steps(1); - } + self.progress_reporter.increment_progress_steps(13); - let old_cache_path = &course.cache_path; - - // increment_cached_version not implemented - - if !options.no_directory_changes { - log::info!("clearing cache at {}", course.cache_path.display()); - file_util::remove_dir_all(&course.cache_path)?; - file_util::create_dir_all(&course.cache_path)?; - self.progress_reporter - .finish_step("Cleared cache".to_string(), None)?; - - log::info!("updating repository at {}", course.clone_path.display()); - update_or_clone_repository( - &course.source_backend, - &course.clone_path, - &course.source_url, - &course.git_branch, - &old_cache_path, - )?; - check_directory_names(&course.clone_path)?; - self.progress_reporter - .finish_step("Updated repository".to_string(), None)?; + // create new cache path + let old_version = course_cache_path + .to_str() + .unwrap() + .split('-') + .last() + .unwrap() + .parse::() + .unwrap(); + let new_cache_path = cache_root.join(format!("{}-{}", course_name, old_version + 1)); + if new_cache_path.exists() { + log::info!("clearing new cache path at {}", new_cache_path.display()); + file_util::remove_dir_all(&new_cache_path)?; } + file_util::create_dir_all(&course_cache_path)?; + self.progress_reporter + .finish_step("Created new cache dir".to_string(), None)?; + + // initialize new clone path and verify directory names + let new_clone_path = new_cache_path.join("clone"); + log::info!("updating repository to {}", new_clone_path.display()); + let old_clone_path = course_cache_path.join("clone"); + update_or_clone_repository(&new_clone_path, &old_clone_path, &source_url, &git_branch)?; + check_directory_names(&new_clone_path)?; + self.progress_reporter + .finish_step("Updated repository".to_string(), None)?; log::info!("updating course options"); - let course_options = get_course_options(&course.clone_path, &course.name)?; + let course_options = get_course_options(&new_clone_path, &course_name)?; self.progress_reporter .finish_step("Updated course options".to_string(), None)?; - // add_records_for_new_exercises & delete_records_for_removed_exercises - log::info!("updating exercises"); - let (new_exercises, removed_exercises) = - get_exercise_changes(&course.clone_path, &course.exercises)?; + // make_solutions + let new_solution_path = new_cache_path.join("solution"); + log::info!("preparing solution to {}", new_solution_path.display()); + task_executor::prepare_solution(&new_clone_path, &new_solution_path)?; self.progress_reporter - .finish_step("Updated exercises".to_string(), None)?; - log::info!("updating exercise options"); - let ExerciseOptions { - review_points, - metadata_map: metadata, - } = update_exercise_options(&course.exercises, &course.clone_path, &course.name)?; + .finish_step("Prepared solutions".to_string(), None)?; + + // make_stubs + let new_stub_path = new_cache_path.join("stub"); + log::info!("preparing stubs to {}", new_stub_path.display()); + task_executor::prepare_stub(&new_clone_path, &new_stub_path)?; self.progress_reporter - .finish_step("Updated exercise options".to_string(), None)?; - - // set_has_tests_flags not implemented here - - let update_points = if !options.no_background_operations { - log::info!("updating available points"); - let result = - update_available_points(&course.exercises, &course.clone_path, &review_points)?; - self.progress_reporter - .finish_step("Updated available points".to_string(), None)?; - result - } else { - HashMap::new() - }; + .finish_step("Prepared stubs".to_string(), None)?; - if !options.no_directory_changes { - // make_solutions - log::info!("preparing solution"); - task_executor::prepare_solution(&course.clone_path, &course.solution_path)?; - self.progress_reporter - .finish_step("Prepared solutions".to_string(), None)?; - - // make_stubs - log::info!("preparing stubs"); - let exercise_dirs = task_executor::find_exercise_directories(&course.clone_path)?; - for exercise_dir in exercise_dirs { - task_executor::prepare_stub(&exercise_dir, &course.stub_path)?; - } - self.progress_reporter - .finish_step("Prepared stubs".to_string(), None)?; - } + // find exercises in new clone path + log::info!("finding exercises"); + // (exercise name, exercise path) + let exercises = get_exercises(&new_clone_path, &new_stub_path)?; + self.progress_reporter + .finish_step("Located exercises".to_string(), None)?; - log::info!("calculating checksums"); - let checksum_stubs = checksum_stubs(&course.exercises, &course.stub_path)?; + // make_zips_of_solutions + let new_solution_zip_path = new_cache_path.join("solution_zip"); + log::info!("compressing solutions"); + execute_zip(&exercises, &new_solution_path, &new_solution_zip_path)?; self.progress_reporter - .finish_step("Calculated checksums".to_string(), None)?; - - if !options.no_directory_changes { - // make_zips_of_stubs - log::info!("compressing stubs"); - execute_zip(&course.exercises, &course.stub_path, &course.stub_zip_path)?; - self.progress_reporter - .finish_step("Compressed stubs".to_string(), None)?; - - // make_zips_of_solutions - log::info!("compressing solutions"); - execute_zip( - &course.exercises, - &course.solution_path, - &course.solution_zip_path, - )?; - self.progress_reporter - .finish_step("Compressed solutions".to_string(), None)?; - - // set_permissions - log::info!("setting permissions"); - set_permissions( - &course.cache_path, - git_repos_chmod, - git_repos_chgrp, - &cache_root, - rails_root, - )?; - self.progress_reporter - .finish_step("Set permissions".to_string(), None)?; - } + .finish_step("Compressed solutions".to_string(), None)?; + + // make_zips_of_stubs + let new_stub_zip_path = new_cache_path.join("stub_zip"); + log::info!("compressing stubs"); + execute_zip(&exercises, &new_stub_path, &new_stub_zip_path)?; + self.progress_reporter + .finish_step("Compressed stubs".to_string(), None)?; - // invalidate_unlocks not implemented - // kafka_publish_exercises not implemented + // make sure the new cache path is readable by anyone + set_permissions(&new_cache_path)?; self.progress_reporter .finish_step("Refreshed course".to_string(), None)?; Ok(RefreshData { - new_exercises, - removed_exercises, - review_points, - metadata, - checksum_stubs, + new_cache_path, course_options, - update_points, + exercises, }) } } /// Refreshes a course... pub fn refresh_course( - course: Course, - options: Options, - git_repos_chmod: Option, - git_repos_chgrp: Option, + course_name: String, + course_cache_path: PathBuf, + source_url: String, + git_branch: String, cache_root: PathBuf, - rails_root: PathBuf, progress_reporter: impl 'static + Sync + Send @@ -258,12 +154,11 @@ pub fn refresh_course( ) -> Result { let course_refresher = CourseRefresher::new(progress_reporter); course_refresher.refresh_course( - course, - options, - git_repos_chmod, - git_repos_chgrp, + course_name, + course_cache_path, + source_url, + git_branch, cache_root, - rails_root, ) } @@ -272,26 +167,22 @@ pub fn refresh_course( /// If not found or found but one of the git commands causes an error, deletes course_clone_path and clones course_git_branch from course_source_url there. /// NOP during testing. fn update_or_clone_repository( - course_source_backend: &SourceBackend, - course_clone_path: &Path, + new_clone_path: &Path, + old_clone_path: &Path, course_source_url: &str, course_git_branch: &str, - old_cache_path: &Path, ) -> Result<(), UtilError> { - if course_source_backend != &SourceBackend::Git { - log::error!("Source types other than git not yet implemented"); - return Err(UtilError::UnsupportedSourceBackend); - } - if old_cache_path.join("clone").join(".git").exists() { + if old_clone_path.join(".git").exists() { // Try a fast path: copy old clone and git fetch new stuff + + // closure to collect any error that occurs during the process let copy_and_update_repository = || -> Result<(), UtilError> { - let old_clone_path = old_cache_path.join("clone"); - file_util::copy(old_clone_path, course_clone_path)?; + file_util::copy(old_clone_path, new_clone_path)?; let run_git = |args: &[&str]| { TmcCommand::new("git".to_string()) .with(|e| { - e.cwd(course_clone_path) + e.cwd(new_clone_path) .args(args) .stdout(Redirection::Pipe) .stderr(Redirection::Pipe) @@ -309,63 +200,32 @@ fn update_or_clone_repository( if let Err(error) = copy_and_update_repository() { log::warn!("failed to update repository: {}", error); - file_util::remove_dir_all(course_clone_path)?; - // clone_repository - TmcCommand::new("git".to_string()) - .with(|e| { - e.args(&["clone", "-q", "-b"]) - .arg(course_git_branch) - .arg(course_source_url) - .arg(course_clone_path) - .stdout(Redirection::Pipe) - .stderr(Redirection::Pipe) - }) - .output_checked()?; + file_util::remove_dir_all(new_clone_path)?; } - } else { - // clone_repository - TmcCommand::new("git".to_string()) - .with(|e| { - e.args(&["clone", "-q", "-b"]) - .arg(course_git_branch) - .arg(course_source_url) - .arg(course_clone_path) - .stdout(Redirection::Pipe) - .stderr(Redirection::Pipe) - }) - .output_checked()?; - } + }; + + // clone_repository + TmcCommand::new("git".to_string()) + .with(|e| { + e.args(&["clone", "-q", "-b"]) + .arg(course_git_branch) + .arg(course_source_url) + .arg(new_clone_path) + .stdout(Redirection::Pipe) + .stderr(Redirection::Pipe) + }) + .output_checked()?; Ok(()) } /// Makes sure no directory directly under path is an exercise directory containing a dash in the relative path from path to the dir. -/// (For some reason) +/// A dash is used as a special delimiter. fn check_directory_names(path: &Path) -> Result<(), UtilError> { // exercise directories in canonicalized form - let exercise_dirs = { - let mut canon_dirs = vec![]; - for path in task_executor::find_exercise_directories(path)? { - let canon = path - .canonicalize() - .map_err(|e| UtilError::Canonicalize(path, e))?; - canon_dirs.push(canon); - } - canon_dirs - }; - for entry in WalkDir::new(path).min_depth(1) { - let entry = entry?; - let canon_path = entry - .path() - .canonicalize() - .map_err(|e| UtilError::Canonicalize(entry.path().to_path_buf(), e))?; - let relpath = entry.path().strip_prefix(path).unwrap(); - let rel_contains_dash = relpath.to_string_lossy().contains('-'); - if canon_path.is_dir() - && exercise_dirs - .iter() - .any(|exdir| exdir.starts_with(&canon_path) && rel_contains_dash) - { - return Err(UtilError::InvalidDirectory(canon_path)); + for exercise_dir in task_executor::find_exercise_directories(path)? { + let relative = exercise_dir.strip_prefix(path).unwrap(); + if relative.to_string_lossy().contains('-') { + return Err(UtilError::InvalidDirectory(exercise_dir)); } } Ok(()) @@ -379,235 +239,77 @@ fn get_course_options(course_clone_path: &Path, course_name: &str) -> Result Result<(Vec, Vec), UtilError> { - let exercise_dirs = task_executor::find_exercise_directories(course_clone_path)?; - let exercise_names = exercise_dirs + course_stub_path: &Path, +) -> Result, UtilError> { + let exercises = task_executor::find_exercise_directories(course_clone_path)? .into_iter() - .map(|ed| { - ed.strip_prefix(course_clone_path) - .unwrap_or(&ed) - .to_string_lossy() - .replace("/", "-") - }) - .collect::>(); - - let mut new_exercises = vec![]; - for exercise_name in &exercise_names { - if !course_exercises.iter().any(|e| &e.name == exercise_name) { - log::info!("Added new exercise {}", exercise_name); - new_exercises.push(exercise_name.clone()); - } - } - - let mut removed_exercises = vec![]; - for exercise in course_exercises { - if !exercise_names.contains(&exercise.name) { - log::info!("Removed exercise {}", exercise.name); - removed_exercises.push(exercise.name.clone()); - } - } - Ok((new_exercises, removed_exercises)) -} - -fn update_exercise_options( - course_exercises: &[RefreshExercise], - course_clone_path: &Path, - course_name: &str, -) -> Result { - let exercise_default_metadata = { - use Value::{Bool, Null, String}; - let mut defaults = Mapping::new(); - defaults.insert(String("deadline".to_string()), Null); - defaults.insert(String("soft_deadline".to_string()), Null); - defaults.insert(String("publish_time".to_string()), Null); - defaults.insert(String("gdocs_sheet".to_string()), Null); - defaults.insert(String("points_visible".to_string()), Bool(true)); - defaults.insert(String("hidden".to_string()), Bool(false)); - defaults.insert(String("returnable".to_string()), Null); - defaults.insert(String("solution_visible_after".to_string()), Null); - defaults.insert( - String("valgrind_strategy".to_string()), - String("fail".to_string()), - ); - defaults.insert(String("runtime_params".to_string()), Null); - defaults.insert( - String("code_review_requests_enabled".to_string()), - Bool(true), - ); - defaults.insert( - String("run_tests_locally_action_enabled".to_string()), - Bool(true), - ); - defaults - }; - - let mut review_points = HashMap::new(); - let mut metadata_map = HashMap::new(); - for exercise in course_exercises { - let mut metadata = exercise_default_metadata.clone(); - let mut try_merge_metadata_in_dir = |path: &Path| -> Result<(), UtilError> { - let metadata_path = path.join("metadata.yml"); - log::debug!("checking for metadata file {}", metadata_path.display()); - if metadata_path.exists() { - let file = file_util::open_file(metadata_path)?; - if let Ok(mut yaml) = serde_yaml::from_reader::<_, Mapping>(file) { - merge_course_options(course_name, &mut yaml); - recursive_merge(yaml, &mut metadata); + .map(|exercise_dir| { + let exercise_dir = exercise_dir.strip_prefix(course_clone_path).unwrap(); + log::debug!("{}", exercise_dir.display()); + let name = exercise_dir.to_string_lossy().replace("/", "-"); + + // checksum + let mut digest = Context::new(); + for entry in WalkDir::new(course_stub_path.join(&exercise_dir)) + .sort_by(|a, b| a.file_name().cmp(b.file_name())) + .into_iter() + .filter_entry(|e| { + !e.file_name() + .to_str() + .map(|e| e.starts_with('.')) + .unwrap_or_default() + }) + { + let entry = entry?; + println!("{}", entry.path().display()); + if entry.path().is_file() { + let relative = entry.path().strip_prefix(course_stub_path).unwrap(); + digest.consume(relative.as_os_str().to_string_lossy().into_owned()); + let file = file_util::read_file(entry.path())?; + digest.consume(file); } } - Ok(()) - }; - - try_merge_metadata_in_dir(&course_clone_path)?; - let mut rel_path = PathBuf::from("."); - // traverse - for component in exercise.relative_path.components() { - rel_path = rel_path.join(component); - try_merge_metadata_in_dir(&course_clone_path.join(&rel_path))?; - } - - let exercise_review_points = match metadata.get(&Value::String("review_points".to_string())) - { - Some(Value::String(string)) => { - string.split_whitespace().map(|s| s.to_string()).collect() - } - Some(Value::Sequence(seq)) => seq - .iter() - .filter_map(|v| v.as_str()) - .map(|s| s.to_string()) - .collect(), - _ => vec![], // todo: empty vec is correct for null, but what to do with other values? - }; - review_points.insert(exercise.name.clone(), exercise_review_points); - metadata_map.insert(exercise.name.clone(), metadata); - } - - Ok(ExerciseOptions { - review_points, - metadata_map, - }) -} - -fn update_available_points( - course_exercises: &[RefreshExercise], - course_clone_path: &Path, - review_points: &HashMap>, -) -> Result, UtilError> { - // scans the exercise directory and compares the points found (and review points) with what was given in the arguments - // to find new and removed points - let mut update_points = HashMap::new(); - for exercise in course_exercises { - let review_points = review_points.get(&exercise.name).unwrap(); // safe, previous steps guarantee each exercise has review points - let mut point_names = HashSet::new(); - - let points_data = { - // if options.no_directory_changes { - // optimization not implemented - // } - let path = course_clone_path.join(&exercise.relative_path); - task_executor::get_available_points(&path)? - }; - point_names.extend(points_data); - point_names.extend(review_points.clone()); - - let mut added = vec![]; - let mut removed = vec![]; - - for name in &point_names { - if !exercise.available_points.contains(name) { - added.push(name.clone()); - } - } - for point in &exercise.available_points { - if !point_names.contains(point) { - removed.push(point.clone()); - } - } + // convert the digest into a hex string + let digest = digest.compute(); + let checksum = format!("{:x}", digest); - if !added.is_empty() { - log::info!( - "Added points to exercise {}: {}", - exercise.name, - added.join(", ") - ); - } - if !removed.is_empty() { - log::info!( - "Removed points from exercise {}: {}", - exercise.name, - removed.join(", ") - ); - } - update_points.insert(exercise.name.clone(), UpdatePoints { added, removed }); - } - Ok(update_points) -} + let exercise_path = course_clone_path.join(&exercise_dir); + let points = task_executor::get_available_points(&exercise_path)?; -fn checksum_stubs( - course_exercises: &[RefreshExercise], - course_stub_path: &Path, -) -> Result, UtilError> { - let mut checksum_stubs = HashMap::new(); - for e in course_exercises { - let mut digest = Context::new(); - for entry in WalkDir::new(course_stub_path.join(&e.relative_path)) - .sort_by(|a, b| a.file_name().cmp(b.file_name())) - .into_iter() - .filter_entry(|e| { - !e.file_name() - .to_str() - .map(|e| e.starts_with('.')) - .unwrap_or_default() + Ok(RefreshExercise { + name, + points, + checksum, + path: exercise_dir.to_path_buf(), }) - // filter hidden - { - let entry = entry?; - if entry.path().is_file() { - let relative = entry.path().strip_prefix(course_stub_path).unwrap(); // safe - digest.consume(relative.as_os_str().to_string_lossy().into_owned()); - let file = file_util::read_file(dbg!(entry.path()))?; - digest.consume(file); - } - } - - // convert the digest into a hex string - let digest = digest.compute(); - let hex = format!("{:x}", digest); - checksum_stubs.insert(e.name.clone(), hex); - } - Ok(checksum_stubs) + }) + .collect::>()?; + Ok(exercises) } fn execute_zip( @@ -616,9 +318,9 @@ fn execute_zip( zip_dir: &Path, ) -> Result<(), UtilError> { file_util::create_dir_all(zip_dir)?; - for e in course_exercises { - let exercise_root = root_path.join(&e.relative_path); - let zip_file_path = zip_dir.join(format!("{}.zip", e.name)); + for exercise in course_exercises { + let exercise_root = root_path.join(&exercise.path); + let zip_file_path = zip_dir.join(format!("{}.zip", exercise.name)); let mut writer = zip::ZipWriter::new(file_util::create_file(zip_file_path)?); for entry in WalkDir::new(&exercise_root).into_iter().filter_entry(|e| { @@ -634,7 +336,7 @@ fn execute_zip( let relative_path = entry.path().strip_prefix(&exercise_root).unwrap(); // safe writer .start_file( - e.relative_path.join(relative_path).to_string_lossy(), + relative_path.to_string_lossy(), zip::write::FileOptions::default(), ) .unwrap(); @@ -647,96 +349,37 @@ fn execute_zip( } #[cfg(not(unix))] -fn set_permissions( - _course_cache_path: &Path, - _chmod: Option, - _chgrp: Option, - _cache_root: &Path, - _rails_root: PathBuf, -) -> Result<(), UtilError> { +fn set_permissions(path: &Path) -> Result<(), UtilError> { // NOP on non-Unix platforms Ok(()) } #[cfg(unix)] -fn set_permissions( - course_cache_path: &Path, - chmod: Option, - chgrp: Option, - cache_root: &Path, - rails_root: PathBuf, -) -> Result<(), UtilError> { +fn set_permissions(path: &Path) -> Result<(), UtilError> { use nix::sys::stat; - use nix::unistd::{self, Uid}; use std::os::unix::io::AsRawFd; - // verify that the cache root is inside the rails root - if !cache_root.starts_with(&rails_root) { - return Err(UtilError::CacheNotInRailsRoot( - cache_root.to_path_buf(), - rails_root, - )); - }; - - let run_chmod = |path: &Path| -> Result<(), UtilError> { - if let Some(chmod) = chmod { - let file = file_util::open_file(path)?; - stat::fchmod( - file.as_raw_fd(), - stat::Mode::from_bits(chmod).ok_or(UtilError::NixFlag(chmod))?, - ) - .map_err(|e| UtilError::NixPermissionChange(path.to_path_buf(), e))?; - } - Ok(()) - }; - let run_chgrp = |path: &Path| -> Result<(), UtilError> { - if let Some(chgrp) = chgrp { - unistd::chown(path, Some(Uid::from_raw(chgrp)), None) - .map_err(|e| UtilError::NixPermissionChange(path.to_path_buf(), e))?; - } - Ok(()) - }; - - // mod all directories from cache root up to rails root - let mut next = cache_root; - run_chmod(next)?; - while let Some(parent) = next.parent() { - run_chmod(parent)?; - run_chgrp(parent)?; - if parent == rails_root { - break; - } - next = parent; - } - - for entry in WalkDir::new(&course_cache_path) { + let chmod = 0o555; // octal, read and execute permissions for all users + for entry in WalkDir::new(path) { let entry = entry?; - run_chmod(entry.path())?; - run_chgrp(entry.path())?; + let file = file_util::open_file(entry.path())?; + stat::fchmod( + file.as_raw_fd(), + stat::Mode::from_bits(chmod).ok_or(UtilError::NixFlag(chmod))?, + ) + .map_err(|e| UtilError::NixPermissionChange(path.to_path_buf(), e))?; } - Ok(()) -} -fn recursive_merge(from: Mapping, into: &mut Mapping) { - for (key, value) in from { - if let Value::Mapping(from_mapping) = value { - if let Some(Value::Mapping(into_mapping)) = into.get_mut(&key) { - // combine mappings - recursive_merge(from_mapping, into_mapping); - } else { - into.insert(key, Value::Mapping(from_mapping)); - } - } else { - into.insert(key.clone(), value); - } - } + Ok(()) } #[cfg(test)] mod test { - use super::*; + use std::io::Read; - const GIT_REPO: &str = "https://github.com/rage/rfcs"; + use super::*; + use serde_yaml::Value; + use tempfile::tempdir; fn init() { use log::*; @@ -744,6 +387,20 @@ mod test { let _ = SimpleLogger::new().with_level(LevelFilter::Debug).init(); } + fn file_to( + target_dir: impl AsRef, + target_relative: impl AsRef, + contents: impl AsRef<[u8]>, + ) -> PathBuf { + let target = target_dir.as_ref().join(target_relative); + if let Some(parent) = target.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&target, contents.as_ref()).unwrap(); + target + } + + /* #[test] #[ignore = "uses git"] fn updates_repository() { @@ -769,14 +426,8 @@ mod test { run_git(&["init"], &clone.path()); run_git(&["remote", "add", "origin", ""], &clone.path()); - update_or_clone_repository( - &SourceBackend::Git, - clone.path(), - GIT_REPO, - "master", - cache.path(), - ) - .unwrap(); + update_or_clone_repository(clone.path(), Path::new(GIT_REPO), "master", cache.path()) + .unwrap(); assert!(clone.path().join("texts").exists()); } @@ -789,241 +440,87 @@ mod test { assert!(!clone.path().join(".git").exists()); let old_cache_path = Path::new("nonexistent"); - update_or_clone_repository( - &SourceBackend::Git, - clone.path(), - GIT_REPO, - "master", - old_cache_path, - ) - .unwrap(); + update_or_clone_repository(clone.path(), Path::new(GIT_REPO), "master", old_cache_path) + .unwrap(); assert!(clone.path().join("texts").exists()); } + */ #[test] fn checks_directory_names() { init(); - assert!( - check_directory_names(Path::new("tests/data/course_refresher/valid_exercises")).is_ok() - ); - assert!( - check_directory_names(Path::new("tests/data/course_refresher/invalid_exercises")) - .is_err() - ); - } + let temp = tempfile::tempdir().unwrap(); + file_to(&temp, "course/valid_part/valid_ex/setup.py", ""); + assert!(check_directory_names(&temp.path().join("course")).is_ok()); - #[test] - fn updates_course_options_empty() { - init(); + let course = tempfile::tempdir().unwrap(); + file_to(course.path(), "course/part1/invalid-ex1/setup.py", ""); + assert!(check_directory_names(&course.path().join("course")).is_err()); - let clone = tempfile::TempDir::new().unwrap(); - let name = "name"; - let options = get_course_options(clone.path(), name).unwrap(); - assert!(options.is_empty()); + let course = tempfile::tempdir().unwrap(); + file_to(course.path(), "course/invalid-part/valid_ex/setup.py", ""); + assert!(check_directory_names(&course.path().join("course")).is_err()); } #[test] - fn updates_course_options_non_empty() { + fn updates_course_options() { init(); - let clone_path = Path::new("tests/data/course_refresher"); - let name = "course-name"; - let options = get_course_options(clone_path, name).unwrap(); - assert!(!options.is_empty()); - - let b = options - .get(&Value::String("inner_value".to_string())) + let temp = tempfile::tempdir().unwrap(); + file_to(&temp, "course_options.yml", "option: true"); + let options = get_course_options(temp.path(), "some course").unwrap(); + assert_eq!(options.len(), 1); + assert!(options + .get(&Value::String("option".to_string())) .unwrap() .as_bool() - .unwrap(); - assert!(b); - - let val = options - .get(&Value::String("inner_map".to_string())) - .unwrap() - .as_mapping() - .unwrap(); - let val = val - .get(&Value::String("param".to_string())) - .unwrap() - .as_i64() - .unwrap(); - assert_eq!(val, 1) - } - - #[test] - fn updates_exercises() { - init(); - - let clone_path = Path::new("tests/data/course_refresher/empty"); - let (new, removed) = get_exercise_changes(clone_path, &[]).unwrap(); - assert!(new.is_empty()); - assert!(removed.is_empty()); - - let clone_path = Path::new("tests/data/course_refresher/valid_exercises"); - let (new, removed) = get_exercise_changes( - clone_path, - &[ - RefreshExercise { - available_points: vec![], - relative_path: PathBuf::new(), - name: "ex2".to_string(), - }, - RefreshExercise { - available_points: vec![], - relative_path: PathBuf::new(), - name: "ex3".to_string(), - }, - ], - ) - .unwrap(); - assert_eq!(new, &["ex1"]); - assert_eq!(removed, &["ex3"]); + .unwrap()) } #[test] - fn updates_exercise_options() { + fn updates_course_options_merged() { init(); - let opts = update_exercise_options(&[], Path::new(""), "").unwrap(); - assert!(opts.review_points.is_empty()); - assert!(opts.metadata_map.is_empty()); - - let opts = dbg!(update_exercise_options( - &[ - RefreshExercise { - available_points: vec![], - name: "defaults".to_string(), - relative_path: PathBuf::from("ex1"), - }, - RefreshExercise { - available_points: vec![], - name: "deep".to_string(), - relative_path: PathBuf::from("deep/deeper"), - }, - ], - Path::new("tests/data/course_refresher/valid_exercises"), - "course-name-PART1", - ) - .unwrap()); - assert!(!opts.review_points.is_empty()); - assert_eq!(opts.metadata_map.len(), 2); - - let val = opts.metadata_map.get("defaults").unwrap(); - assert!(val - .get(&Value::String("points_visible".to_string())) - .unwrap() - .as_bool() - .unwrap()); - - let val = opts.metadata_map.get("deep").unwrap(); - assert!(!val - .get(&Value::String("points_visible".to_string())) - .unwrap() - .as_bool() - .unwrap()); - assert!(val - .get(&Value::String("true_in_inner_false_in_outer".to_string())) - .unwrap() - .as_bool() - .unwrap()); - let inner_map = val - .get(&Value::String("inner_map".to_string())) - .unwrap() - .as_mapping() - .unwrap(); - assert!(inner_map - .get(&Value::String("true_in_inner_false_in_outer".to_string())) + let temp = tempfile::tempdir().unwrap(); + file_to( + &temp, + "course_options.yml", + r#" +courses: + course_1: + option: true + course_2: + other_option: true +"#, + ); + let options = get_course_options(temp.path(), "course_1").unwrap(); + assert_eq!(options.len(), 1); + assert!(options + .get(&Value::String("option".to_string())) .unwrap() .as_bool() - .unwrap()); + .unwrap()) } #[test] - fn updates_available_points() { + fn gets_exercises() { init(); - let mut review_points = HashMap::new(); - review_points.insert( - "ex1".to_string(), - vec!["rev_point".to_string(), "ex_and_rev_point".to_string()], + let temp = tempfile::tempdir().unwrap(); + file_to(&temp, "course/part1/ex1/setup.py", ""); + file_to( + &temp, + "course/part1/ex1/test/test.py", + "@points('1') @points('2')", ); - - let update_points = dbg!(update_available_points( - &[RefreshExercise { - available_points: vec![ - "ex_point".to_string(), - "ex_and_rev_point".to_string(), - "ex_and_test_point".to_string() - ], - name: "ex1".to_string(), - relative_path: PathBuf::from("ex1"), - }], - Path::new("tests/data/course_refresher/valid_exercises"), - &review_points, - ) - .unwrap()); - let pts = update_points.get("ex1").unwrap(); - assert_eq!(pts.added.len(), 2); - assert!(pts.added.contains(&"rev_point".to_string())); - assert!(pts.added.contains(&"test_point".to_string())); - assert_eq!(pts.removed, &["ex_point"]); - } - - #[test] - fn checksums_stubs() { - init(); - - let first = tempfile::tempdir().unwrap(); - std::fs::create_dir(first.path().join("dir")).unwrap(); - std::fs::write(first.path().join("dir/file"), b"hello").unwrap(); - std::fs::write(first.path().join("dir/.hidden"), b"hello").unwrap(); - - let second = tempfile::tempdir().unwrap(); - std::fs::create_dir(second.path().join("dir")).unwrap(); - std::fs::write(second.path().join("dir/file"), b"hello").unwrap(); - std::fs::write(second.path().join("dir/.hidden"), b"bye").unwrap(); - - let third = tempfile::tempdir().unwrap(); - std::fs::create_dir(third.path().join("dir")).unwrap(); - std::fs::write(third.path().join("dir/file"), b"bye").unwrap(); - - let cks = dbg!(checksum_stubs( - &[RefreshExercise { - available_points: vec![], - name: "first".to_string(), - relative_path: PathBuf::from("dir") - }], - first.path(), - ) - .unwrap()); - let f = cks.get("first").unwrap(); - - let cks = dbg!(checksum_stubs( - &[RefreshExercise { - available_points: vec![], - name: "second".to_string(), - relative_path: PathBuf::from("dir") - }], - second.path(), - ) - .unwrap()); - let s = cks.get("second").unwrap(); - - let cks = dbg!(checksum_stubs( - &[RefreshExercise { - available_points: vec![], - name: "third".to_string(), - relative_path: PathBuf::from("dir") - }], - third.path(), - ) - .unwrap()); - let t = cks.get("third").unwrap(); - - assert_eq!(f, s); - assert_ne!(f, t); + let exercises = + get_exercises(&temp.path().join("course"), &temp.path().join("course")).unwrap(); + assert_eq!(exercises.len(), 1); + assert_eq!(exercises[0].points.len(), 2); + assert_eq!(exercises[0].points[0], "1"); + assert_eq!(exercises[0].points[1], "2"); + assert_eq!(exercises[0].checksum, "043fb4832da4e3fbf5babd13ed9fa732"); } #[test] @@ -1031,45 +528,43 @@ mod test { init(); let temp = tempfile::tempdir().unwrap(); - execute_zip( - &[ - RefreshExercise { - available_points: vec![], - name: "first".to_string(), - relative_path: PathBuf::from("ex1"), - }, - RefreshExercise { - available_points: vec![], - name: "second".to_string(), - relative_path: PathBuf::from("ex2"), - }, - ], - Path::new("tests/data/course_refresher/valid_exercises"), - temp.path(), - ) - .unwrap(); - - let first_zip = temp.path().join("first.zip"); - assert!(first_zip.exists()); - let mut fz = zip::ZipArchive::new(file_util::open_file(first_zip).unwrap()).unwrap(); - assert!(fz - .by_name( - &Path::new("ex1") - .join("test") - .join("test.py") - .to_string_lossy() - ) - .is_ok()); - - let second_zip = temp.path().join("second.zip"); - assert!(second_zip.exists()); - let mut sz = zip::ZipArchive::new(file_util::open_file(second_zip).unwrap()).unwrap(); - assert!(sz - .by_name(&Path::new("ex2").join("setup.py").to_string_lossy()) - .is_ok()); - assert!(sz - .by_name(&Path::new("ex2").join(".hiddenfile").to_string_lossy()) - .is_err()); + file_to(&temp, "clone/part1/ex1/setup.py", ""); + file_to(&temp, "clone/part1/ex2/setup.py", ""); + file_to(&temp, "clone/part2/ex1/setup.py", ""); + file_to(&temp, "clone/part2/ex2/setup.py", ""); + file_to(&temp, "clone/part2/ex2/dir/subdir/file", ""); + file_to(&temp, "clone/part2/ex2/dir/subdir/.hidden", ""); + file_to(&temp, "stub/part1/ex1/setup.py", ""); + file_to(&temp, "stub/part1/ex2/setup.py", ""); + file_to(&temp, "stub/part2/ex1/setup.py", ""); + file_to(&temp, "stub/part2/ex2/setup.py", ""); + file_to(&temp, "stub/part2/ex2/dir/subdir/file", "some file"); + file_to(&temp, "stub/part2/ex2/dir/subdir/.hidden", "hidden file"); + + let exercises = + get_exercises(&temp.path().join("clone"), &temp.path().join("stub")).unwrap(); + + execute_zip(&exercises, &temp.path().join("stub"), temp.path()).unwrap(); + + let zip = temp.path().join("part1-ex1.zip"); + assert!(zip.exists()); + let zip = temp.path().join("part1-ex2.zip"); + assert!(zip.exists()); + let zip = temp.path().join("part2-ex1.zip"); + assert!(zip.exists()); + let zip = temp.path().join("part2-ex2.zip"); + assert!(zip.exists()); + + let mut fz = zip::ZipArchive::new(file_util::open_file(&zip).unwrap()).unwrap(); + for i in fz.file_names() { + log::debug!("{}", i); + } + assert!(fz.by_name("setup.py").is_ok()); + assert!(fz.by_name("dir/subdir/.hidden").is_err()); + let mut file = fz.by_name("dir/subdir/file").unwrap(); + let mut buf = String::new(); + file.read_to_string(&mut buf).unwrap(); + assert_eq!(buf, "some file"); } #[cfg(unix)] @@ -1078,14 +573,9 @@ mod test { fn sets_permissions() { init(); - let rails_root = Path::new("tests/data/course_refresher/rails_root"); - set_permissions( - &rails_root.join("dir/cache_root"), - Some(0o0777), - Some(1000), - &rails_root.join("dir/cache_root"), - rails_root.to_path_buf(), - ) - .unwrap(); + let temp = tempdir().unwrap(); + file_to(&temp, "file", "contents"); + + set_permissions(temp.path()).unwrap(); } } From 854f829580a137ab797666627da9c4353642d457 Mon Sep 17 00:00:00 2001 From: Daniel Martinez Date: Fri, 29 Jan 2021 20:21:51 +0200 Subject: [PATCH 2/3] fixes --- tmc-langs-util/src/error.rs | 9 +++------ tmc-langs-util/src/task_executor.rs | 2 +- tmc-langs-util/src/task_executor/course_refresher.rs | 7 ++++++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tmc-langs-util/src/error.rs b/tmc-langs-util/src/error.rs index 3fe8824c6d9..6cc04361643 100644 --- a/tmc-langs-util/src/error.rs +++ b/tmc-langs-util/src/error.rs @@ -1,10 +1,11 @@ //! Contains the crate error type -#[cfg(unix)] 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")] @@ -31,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), @@ -52,7 +49,7 @@ pub enum UtilError { NixPermissionChange(PathBuf, #[source] nix::Error), #[cfg(unix)] #[error("Invalid chmod flag: {0}")] - NixFlag(u32), + NixFlag(ModeBits), #[error(transparent)] DynError(#[from] Box), diff --git a/tmc-langs-util/src/task_executor.rs b/tmc-langs-util/src/task_executor.rs index 6a1121c4cc5..71ac49c7b34 100644 --- a/tmc-langs-util/src/task_executor.rs +++ b/tmc-langs-util/src/task_executor.rs @@ -6,7 +6,7 @@ mod submission_processing; mod tar_helper; mod tmc_zip; -pub use self::course_refresher::{refresh_course, RefreshData, RefreshExercise}; +pub use self::course_refresher::{refresh_course, ModeBits, RefreshData, RefreshExercise}; pub use self::submission_packaging::{OutputFormat, TmcParams}; use crate::error::UtilError; diff --git a/tmc-langs-util/src/task_executor/course_refresher.rs b/tmc-langs-util/src/task_executor/course_refresher.rs index 0a167ab6cc9..ab882e30a55 100644 --- a/tmc-langs-util/src/task_executor/course_refresher.rs +++ b/tmc-langs-util/src/task_executor/course_refresher.rs @@ -13,6 +13,11 @@ use std::path::{Path, PathBuf}; use tmc_langs_framework::{command::TmcCommand, file_util, subprocess::Redirection}; use walkdir::WalkDir; +#[cfg(unix)] +pub type ModeBits = nix::sys::stat::mode_t; +#[cfg(not(unix))] +pub type ModeBits = u32; + #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct RefreshData { @@ -359,7 +364,7 @@ fn set_permissions(path: &Path) -> Result<(), UtilError> { use nix::sys::stat; use std::os::unix::io::AsRawFd; - let chmod = 0o555; // octal, read and execute permissions for all users + let chmod: ModeBits = 0o555; // octal, read and execute permissions for all users for entry in WalkDir::new(path) { let entry = entry?; let file = file_util::open_file(entry.path())?; From 194263571cfb4b9ab4760a8ace4af2ba926e2132 Mon Sep 17 00:00:00 2001 From: Daniel Martinez Date: Fri, 29 Jan 2021 20:57:37 +0200 Subject: [PATCH 3/3] disable tests on windows --- .../src/task_executor/course_refresher.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tmc-langs-util/src/task_executor/course_refresher.rs b/tmc-langs-util/src/task_executor/course_refresher.rs index ab882e30a55..82cc0d4af89 100644 --- a/tmc-langs-util/src/task_executor/course_refresher.rs +++ b/tmc-langs-util/src/task_executor/course_refresher.rs @@ -379,6 +379,7 @@ fn set_permissions(path: &Path) -> Result<(), UtilError> { } #[cfg(test)] +#[cfg(unix)] // not used on windows mod test { use std::io::Read; @@ -565,14 +566,27 @@ courses: log::debug!("{}", i); } assert!(fz.by_name("setup.py").is_ok()); - assert!(fz.by_name("dir/subdir/.hidden").is_err()); - let mut file = fz.by_name("dir/subdir/file").unwrap(); + assert!(fz + .by_name( + &Path::new("dir") + .join("subdir") + .join(".hidden") + .to_string_lossy() + ) + .is_err()); + let mut file = fz + .by_name( + &Path::new("dir") + .join("subdir") + .join("file") + .to_string_lossy(), + ) + .unwrap(); let mut buf = String::new(); file.read_to_string(&mut buf).unwrap(); assert_eq!(buf, "some file"); } - #[cfg(unix)] #[test] #[ignore = "issues in CI, maybe due to the user ID"] fn sets_permissions() {