Skip to content

Commit

Permalink
fix: missed .with_extension methods on the implementations of Transla…
Browse files Browse the repository at this point in the history
…tionUnit

fix: we can't use join for file_stem(s) that contains dots in their names, since it skips anything after the first point
  • Loading branch information
TheRustifyer committed Jun 24, 2024
1 parent f7c928c commit 48a95ee
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 50 deletions.
25 changes: 25 additions & 0 deletions zork++/src/lib/bounds/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,31 @@ pub trait ExecutableTarget<'a>: ExtraArgs<'a> {
pub trait TranslationUnit: Display + Debug {
/// Returns the file, being the addition of the path property plus the file stem plus
/// the extension property
///
/// # Examples
///
/// ```
/// use std::borrow::Cow;
/// use std::path::PathBuf;
/// use zork::bounds::TranslationUnit;
/// use zork::project_model::sourceset::SourceFile;
///
/// let source_file = SourceFile {
/// path: PathBuf::from("/usr/include"),
/// file_stem: Cow::from("std"),
/// extension: Cow::from("h"),
/// };
///
/// assert_eq!(source_file.file(), PathBuf::from("/usr/include/std.h"));
///
/// let source_file_compat = SourceFile {
/// path: PathBuf::from("/usr/include"),
/// file_stem: Cow::from("std.compat"),
/// extension: Cow::from("h"),
/// };
///
/// assert_eq!(source_file_compat.file(), PathBuf::from("/usr/include/std.compat.h"));
/// ```
fn file(&self) -> PathBuf;

/// Outputs the declared path for `self`, being self the translation unit
Expand Down
1 change: 0 additions & 1 deletion zork++/src/lib/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use std::{
fs::File,
path::{Path, PathBuf},
};
use std::borrow::Cow;

use crate::bounds::TranslationUnit;
use crate::cache::compile_commands::CompileCommands;
Expand Down
45 changes: 28 additions & 17 deletions zork++/src/lib/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ fn build_modular_stdlib(model: &ZorkModel<'_>, cache: &mut ZorkCache, commands:
let compiler = model.compiler.cpp_compiler;

// TODO: remaining ones: Clang, GCC
// TODO: try to abstract the procedures into just one entity
if compiler.eq(&CppCompiler::MSVC) {
let built_stdlib_path = &cache.compilers_metadata.msvc.stdlib_bmi_path;
let cpp_stdlib = if !built_stdlib_path.exists() {
Expand All @@ -93,8 +94,6 @@ fn build_modular_stdlib(model: &ZorkModel<'_>, cache: &mut ZorkCache, commands:
);
msvc_args::generate_std_cmd(model, cache, StdLibMode::Cpp) // TODO move mod msvc_args to commands
} else {
// TODO: p.ej: existe y además tiene status cached? modificar por &mut
// TODO: no será mejor sacarla de la caché?
let source_command_line = SourceCommandLine {
directory: built_stdlib_path.file_stem().unwrap().into(),
filename: built_stdlib_path
Expand All @@ -108,10 +107,11 @@ fn build_modular_stdlib(model: &ZorkModel<'_>, cache: &mut ZorkCache, commands:
};
source_command_line
};
log::info!("Generated std SourceCommandLine: {cpp_stdlib:?}");
commands.pre_tasks.push(cpp_stdlib);

let built_stdlib_compat_path = &cache.compilers_metadata.msvc.c_stdlib_bmi_path;
let c_cpp_stdlib = if !built_stdlib_path.exists() {
let c_compat = if !built_stdlib_path.exists() {
log::trace!("Building the {:?} C compat CPP std lib", compiler);
msvc_args::generate_std_cmd(model, cache, StdLibMode::CCompat)
} else {
Expand All @@ -128,7 +128,8 @@ fn build_modular_stdlib(model: &ZorkModel<'_>, cache: &mut ZorkCache, commands:
};
source_command_line
};
commands.pre_tasks.push(c_cpp_stdlib);
log::info!("Generated std SourceCommandLine: {c_compat:?}");
commands.pre_tasks.push(c_compat);
}
}

Expand Down Expand Up @@ -168,13 +169,13 @@ fn build_sources(
sources::generate_sources_arguments(model, commands, cache, &model.tests, src);
} else {
let command_line = SourceCommandLine::from_translation_unit(
src, Arguments::default(), true, CommandExecutionResult::Cached
src, Arguments::default(), true, CommandExecutionResult::Cached,
);

log::trace!("Source file: {:?} was not modified since the last iteration. No need to rebuilt it again.", &src.file());
commands.sources.push(command_line);
commands.add_linker_file_path_owned(helpers::generate_obj_file_path(
model.compiler.cpp_compiler, &model.build.output_dir, src
model.compiler.cpp_compiler, &model.build.output_dir, src,
))
});

Expand Down Expand Up @@ -225,12 +226,13 @@ fn process_module_interfaces<'a>(
let translation_unit_must_be_rebuilt = helpers::translation_unit_must_be_rebuilt(compiler, lpe, generated_cmd, &module_interface.file());
log::trace!("Source file: {:?} must be rebuilt: {translation_unit_must_be_rebuilt}", &module_interface.file());

if !translation_unit_must_be_rebuilt { log::trace!("Source file:{:?} was not modified since the last iteration. No need to rebuilt it again.", &module_interface.file());
if !translation_unit_must_be_rebuilt {
log::trace!("Source file:{:?} was not modified since the last iteration. No need to rebuilt it again.", &module_interface.file());
}
let mut cached_cmd_line = generated_cmd.clone(); // TODO: somehow, we should manage to solve this on the future
cached_cmd_line.need_to_build = translation_unit_must_be_rebuilt;
commands.linker.add_owned_buildable_at(helpers::generate_prebuilt_miu( // TODO: extremely provisional
model.compiler.cpp_compiler, &model.build.output_dir, module_interface
model.compiler.cpp_compiler, &model.build.output_dir, module_interface,
));
cached_cmd_line
} else {
Expand Down Expand Up @@ -265,7 +267,7 @@ fn process_module_implementations<'a>(
let mut cached_cmd_line = generated_cmd.clone(); // TODO: somehow, we should manage to solve this on the future
cached_cmd_line.need_to_build = translation_unit_must_be_rebuilt;
commands.linker.add_owned_buildable_at(helpers::generate_impl_obj_file( // TODO: extremely provisional
model.compiler.cpp_compiler, &model.build.output_dir, module_impl
model.compiler.cpp_compiler, &model.build.output_dir, module_impl,
));
cached_cmd_line
} else {
Expand Down Expand Up @@ -479,7 +481,9 @@ mod sources {
let compiler = model.compiler.cpp_compiler;
let out_dir: &Path = model.build.output_dir.as_ref();

let mut arguments = Arguments::default();
let mut arguments = Arguments::default(); // TODO: provisional while we're implementing the Flyweights
arguments.push(model.compiler.language_level_arg());
arguments.extend_from_slice(model.compiler.extra_args());

match compiler {
CppCompiler::CLANG => {
Expand Down Expand Up @@ -637,7 +641,7 @@ mod sources {
.join(compiler.as_ref())
.join("modules")
.join("implementations")
.join::<&str>(&*implementation.file_stem())
.join::<&str>(&implementation.file_stem())
.with_extension(compiler.get_obj_file_extension());

commands.add_linker_file_path(&obj_file_path);
Expand Down Expand Up @@ -868,11 +872,13 @@ mod helpers {
}

Check failure on line 872 in zork++/src/lib/compiler/mod.rs

View workflow job for this annotation

GitHub Actions / Verify code formatting

Diff in /home/runner/work/Zork/Zork/zork++/src/lib/compiler/mod.rs

/// TODO
pub(crate) fn translation_unit_must_be_rebuilt(
compiler: CppCompiler,
last_process_execution: &DateTime<Utc>,
cached_source_cmd: &SourceCommandLine,
file: &Path,
pub(crate) fn translation_unit_must_be_rebuilt( // TODO: separation of concerns? Please
// Just make two fns, the one that checks for the status and the one that checks for modifications
// then just use a template-factory design pattern by just abstracting away the two checks in one call
compiler: CppCompiler,
last_process_execution: &DateTime<Utc>,
cached_source_cmd: &SourceCommandLine,
file: &Path,
) -> bool {
if compiler.eq(&CppCompiler::CLANG) && cfg!(target_os = "windows") {
log::trace!("Module unit {:?} will be rebuilt since we've detected that you are using Clang in Windows", cached_source_cmd.path());
Expand All @@ -882,9 +888,14 @@ mod helpers {
let execution_result = cached_source_cmd.execution_result;
if execution_result != CommandExecutionResult::Success
&& execution_result != CommandExecutionResult::Cached
// TODO: Big one. What instead of having the boolean flag of need_to_built we modify the enumerated of execution result, change its name
// to TranslationUnitStatus or some other better name pls, and we introduce a variant for mark the files that must be built?
// TODO: also, introduce a variant like PendingToBuilt, that just will be check before executing the commands?
// TODO: and even better, what if we use the new enum type as a wrapper over execution result? So we can store inside the variants
// that contains build info the execution result, and in the others maybe nothing, or other interesting data?
{
log::trace!(
"File {file:?} build process failed previously with status: {:?}. It will be rebuilt again",
"File {file:?} build process failed previously with status: {:?}. It will be rebuilt again", // TODO: technically, it can be Unprocessed, which isn't a failure
execution_result
);
return true;
Expand Down
15 changes: 6 additions & 9 deletions zork++/src/lib/project_model/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ impl<'a> fmt::Display for ModuleInterfaceModel<'a> {

impl<'a> TranslationUnit for ModuleInterfaceModel<'a> {
fn file(&self) -> PathBuf {
self.path
.join::<&str>(&self.file_stem)
.join::<&str>(&self.extension)
let file_name = format!("{}.{}", self.file_stem, self.extension);
self.path().join(file_name)
}

fn path(&self) -> PathBuf {
Expand All @@ -64,9 +63,8 @@ impl<'a> TranslationUnit for ModuleInterfaceModel<'a> {

impl<'a> TranslationUnit for &'a ModuleInterfaceModel<'a> {
fn file(&self) -> PathBuf {
self.path
.join::<&str>(&self.file_stem)
.join::<&str>(&self.extension)
let file_name = format!("{}.{}", self.file_stem, self.extension);
self.path().join(file_name)
}

fn path(&self) -> PathBuf {
Expand Down Expand Up @@ -125,9 +123,8 @@ impl<'a> fmt::Display for ModuleImplementationModel<'a> {

impl<'a> TranslationUnit for &'a ModuleImplementationModel<'a> {
fn file(&self) -> PathBuf {
self.path
.join::<&str>(&self.file_stem)
.with_extension::<&str>(&self.extension)
let file_name = format!("{}.{}", self.file_stem, self.extension);
self.path().join(file_name)
}

fn path(&self) -> PathBuf {
Expand Down
10 changes: 4 additions & 6 deletions zork++/src/lib/project_model/sourceset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ pub struct SourceFile<'a> {

impl<'a> TranslationUnit for SourceFile<'a> {
fn file(&self) -> PathBuf {
self.path
.join::<&str>(&self.file_stem)
.with_extension::<&str>(&self.extension)
let file_name = format!("{}.{}", self.file_stem, self.extension);
self.path().join(file_name)
}

fn path(&self) -> PathBuf {
Expand All @@ -58,9 +57,8 @@ impl<'a> TranslationUnit for SourceFile<'a> {

impl<'a> TranslationUnit for &'a SourceFile<'a> {
fn file(&self) -> PathBuf {
self.path
.join::<&str>(&self.file_stem)
.with_extension::<&str>(&self.extension)
let file_name = format!("{}.{}", self.file_stem, self.extension);
self.path().join(file_name)
}

fn path(&self) -> PathBuf {
Expand Down
26 changes: 9 additions & 17 deletions zork++/src/lib/utils/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,27 +274,19 @@ fn assemble_module_interface_model<'a>(
let cfg_file = config.file;

let file_path = Path::new(project_root).join(base_path).join(cfg_file);
// let module_name = config.module_name.unwrap_or_else(move || {
// Path::new(&cfg_file)
// .file_stem()
// .unwrap_or_else(|| panic!("Found ill-formed path on: {cfg_file}"))
// .to_string_lossy()
// } // TODO: ensure that this one is valid with modules that contains dots in their name
let module_name = Cow::Borrowed(
config
.module_name
.unwrap_or_else(|| panic!("Found ill-formed module name on: {cfg_file}")),
);

let module_name = if let Some(mod_name) = config.module_name {
Cow::Borrowed(mod_name)
} else {
Path::new(cfg_file)
.file_stem()
.unwrap_or_else(|| panic!("Found ill-formed file_stem data for: {cfg_file}"))
.to_string_lossy()
};
let dependencies = config
.dependencies
.map(|deps| deps.into_iter().map(Cow::Borrowed).collect())
.unwrap_or_default();
let partition = if config.partition.is_none() {
None
} else {
Some(ModulePartitionModel::from(config.partition.unwrap()))
};
let partition = config.partition.map(ModulePartitionModel::from);

let file_details = utils::fs::get_file_details(&file_path).unwrap_or_else(|_| {
panic!("An unexpected error happened getting the file details for {file_path:?}")
Expand Down

0 comments on commit 48a95ee

Please sign in to comment.