Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed export package-definitions not including cached modules #3669

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion compiler-core/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::ast::{
CallArg, CustomType, DefinitionLocation, Pattern, TypeAst, TypedArg, TypedDefinition,
TypedExpr, TypedFunction, TypedPattern, TypedStatement,
};
use crate::package_interface;
use crate::type_::Type;
use crate::{
ast::{Definition, SrcSpan, TypedModule},
Expand Down Expand Up @@ -208,6 +209,7 @@ fn mode_includes_tests() {
pub struct Package {
pub config: PackageConfig,
pub modules: Vec<Module>,
pub cached_metadata: Vec<package_compiler::CacheMetadata>,
}

impl Package {
Expand All @@ -225,7 +227,7 @@ impl Package {
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
mine-tech-oficial marked this conversation as resolved.
Show resolved Hide resolved
pub struct Module {
pub name: EcoString,
pub code: EcoString,
Expand Down
2 changes: 1 addition & 1 deletion compiler-core/src/build/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ where
}
}

Ok(Input::Cached(self.cached(name, meta)))
Ok(Input::Cached(self.cached(name, meta.clone()), meta))
}

/// Read the timestamp file from the artefact directory for the given
Expand Down
3 changes: 3 additions & 0 deletions compiler-core/src/build/module_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{
build::SourceFingerprint,
io::{memory::InMemoryFileSystem, FileSystemWriter},
line_numbers::LineNumbers,
package_interface,
};
use std::time::Duration;

Expand Down Expand Up @@ -212,11 +213,13 @@ fn write_cache(
) {
let line_numbers = LineNumbers::new(source);
let cache_metadata = CacheMetadata {
name: "".into(),
mtime: SystemTime::UNIX_EPOCH + Duration::from_secs(seconds),
codegen_performed,
dependencies: vec![],
fingerprint: SourceFingerprint::new(source),
line_numbers,
interface: package_interface::ModuleInterface::default(),
};
let path = Utf8Path::new(path);
fs.write_bytes(&path, &cache_metadata.to_binary()).unwrap();
Expand Down
35 changes: 23 additions & 12 deletions compiler-core/src/build/package_compiler.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::analyse::{ModuleAnalyzerConstructor, TargetSupport};
use crate::line_numbers::{self, LineNumbers};
use crate::package_interface::{self, ModuleInterface};
use crate::type_::PRELUDE_MODULE_NAME;
use crate::{
ast::{SrcSpan, TypedModule, UntypedModule},
Expand All @@ -22,6 +23,7 @@ use crate::{
};
use askama::Template;
use ecow::EcoString;
use std::borrow::BorrowMut;
use std::collections::HashSet;
use std::{collections::HashMap, fmt::write, time::SystemTime};
use vec1::Vec1;
Expand Down Expand Up @@ -104,7 +106,7 @@ where
stale_modules: &mut StaleTracker,
incomplete_modules: &mut HashSet<EcoString>,
telemetry: &dyn Telemetry,
) -> Outcome<Vec<Module>, Error> {
) -> Outcome<(Vec<Module>, Vec<CacheMetadata>), Error> {
let span = tracing::info_span!("compile", package = %self.config.name.as_str());
let _enter = span.enter();

Expand Down Expand Up @@ -182,9 +184,12 @@ where
incomplete_modules,
);

let modules = match outcome {
let mut modules = match outcome {
Outcome::Ok(modules) => modules,
Outcome::PartialFailure(_, _) | Outcome::TotalFailure(_) => return outcome,
Outcome::PartialFailure(modules, err) => {
return Outcome::PartialFailure((modules, loaded.cached_metadata), err)
}
Outcome::TotalFailure(err) => return Outcome::TotalFailure(err),
};

tracing::debug!("performing_code_generation");
Expand All @@ -197,7 +202,7 @@ where
return error.into();
}

Outcome::Ok(modules)
Outcome::Ok((modules, loaded.cached_metadata))
}

fn compile_erlang_to_beam(&mut self, modules: &HashSet<Utf8PathBuf>) -> Result<(), Error> {
Expand Down Expand Up @@ -304,11 +309,13 @@ where
let name = format!("{}.cache_meta", &module_name);
let path = artefact_dir.join(name);
let info = CacheMetadata {
name: module.name.clone(),
mtime: module.mtime,
codegen_performed: self.perform_codegen,
dependencies: module.dependencies.clone(),
fingerprint: SourceFingerprint::new(&module.code),
line_numbers: module.ast.type_info.line_numbers.clone(),
interface: package_interface::ModuleInterface::from_module(module),
mine-tech-oficial marked this conversation as resolved.
Show resolved Hide resolved
};
self.io.write_bytes(&path, &info.to_binary())?;

Expand Down Expand Up @@ -594,28 +601,28 @@ pub(crate) fn module_name(package_path: &Utf8Path, full_module_path: &Utf8Path)
#[derive(Debug)]
pub(crate) enum Input {
New(UncompiledModule),
Cached(CachedModule),
Cached(CachedModule, CacheMetadata),
}

impl Input {
pub fn name(&self) -> &EcoString {
match self {
Input::New(m) => &m.name,
Input::Cached(m) => &m.name,
Input::Cached(m, _) => &m.name,
}
}

pub fn source_path(&self) -> &Utf8Path {
match self {
Input::New(m) => &m.path,
Input::Cached(m) => &m.source_path,
Input::Cached(m, _) => &m.source_path,
}
}

pub fn dependencies(&self) -> Vec<EcoString> {
match self {
Input::New(m) => m.dependencies.iter().map(|(n, _)| n.clone()).collect(),
Input::Cached(m) => m.dependencies.iter().map(|(n, _)| n.clone()).collect(),
Input::Cached(m, _) => m.dependencies.iter().map(|(n, _)| n.clone()).collect(),
}
}

Expand Down Expand Up @@ -645,13 +652,15 @@ pub(crate) struct CachedModule {
pub line_numbers: LineNumbers,
}

#[derive(Debug, serde::Serialize, serde::Deserialize)]
pub(crate) struct CacheMetadata {
#[derive(Debug, serde::Serialize, serde::Deserialize, Clone)]
pub struct CacheMetadata {
mine-tech-oficial marked this conversation as resolved.
Show resolved Hide resolved
pub name: EcoString,
pub mtime: SystemTime,
pub codegen_performed: bool,
pub dependencies: Vec<(EcoString, SrcSpan)>,
pub fingerprint: SourceFingerprint,
pub line_numbers: LineNumbers,
pub interface: package_interface::ModuleInterface,
}

impl CacheMetadata {
Expand All @@ -664,22 +673,24 @@ impl CacheMetadata {
}
}

#[derive(Debug, Default, PartialEq, Eq)]
#[derive(Debug, Default)]
pub(crate) struct Loaded {
pub to_compile: Vec<UncompiledModule>,
pub cached: Vec<type_::ModuleInterface>,
pub cached_metadata: Vec<CacheMetadata>,
}

impl Loaded {
fn empty() -> Self {
Self {
to_compile: vec![],
cached: vec![],
cached_metadata: vec![],
}
}
}

#[derive(Debug, PartialEq, Eq)]
#[derive(Debug, PartialEq, Eq, Clone)]
pub(crate) struct UncompiledModule {
pub path: Utf8PathBuf,
pub name: EcoString,
Expand Down
7 changes: 4 additions & 3 deletions compiler-core/src/build/package_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ where
// A cached module with dependencies that are stale must be
// recompiled as the changes in the dependencies may have affect
// the output, making the cache invalid.
Input::Cached(info) if self.stale_modules.includes_any(&info.dependencies) => {
Input::Cached(info, _) if self.stale_modules.includes_any(&info.dependencies) => {
tracing::debug!(module = %info.name, "module_to_be_compiled");
self.stale_modules.add(info.name.clone());
let module = self.load_and_parse(info)?;
Expand All @@ -157,10 +157,11 @@ where

// A cached module with no stale dependencies can be used as-is
// and does not need to be recompiled.
Input::Cached(info) => {
Input::Cached(info, meta) => {
tracing::debug!(module = %info.name, "module_to_load_from_cache");
let module = self.load_cached_module(info)?;
loaded.cached.push(module);
loaded.cached_metadata.push(meta);
}
}
}
Expand Down Expand Up @@ -324,7 +325,7 @@ where
src: module.code.clone(),
}
}
Input::Cached(cached_module) => {
Input::Cached(cached_module, _) => {
let (_, location) = cached_module
.dependencies
.iter()
Expand Down
6 changes: 4 additions & 2 deletions compiler-core/src/build/package_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::*;
use crate::{
build::SourceFingerprint,
io::{memory::InMemoryFileSystem, FileSystemWriter},
line_numbers,
line_numbers, package_interface,
parse::extra::ModuleExtra,
warning::NullWarningEmitterIO,
Warning,
Expand Down Expand Up @@ -39,11 +39,13 @@ fn write_cache(
let line_numbers = line_numbers::LineNumbers::new(src);
let mtime = SystemTime::UNIX_EPOCH + Duration::from_secs(seconds);
let cache_metadata = CacheMetadata {
name: name.into(),
mtime,
codegen_performed: true,
dependencies: deps,
fingerprint: SourceFingerprint::new(src),
line_numbers: line_numbers.clone(),
interface: package_interface::ModuleInterface::default(),
};
let path = Utf8Path::new("/artefact").join(format!("{name}.cache_meta"));
fs.write_bytes(&path, &cache_metadata.to_binary()).unwrap();
Expand All @@ -62,7 +64,7 @@ fn write_cache(
warnings: vec![],
minimum_required_version: Version::new(0, 1, 0),
};
let path = Utf8Path::new("/artefact").join(format!("{name}.cache"));
let path: Utf8PathBuf = Utf8Path::new("/artefact").join(format!("{name}.cache"));
fs.write_bytes(
&path,
&metadata::ModuleEncoder::new(&cache).encode().unwrap(),
Expand Down
22 changes: 15 additions & 7 deletions compiler-core/src/build/project_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
use ecow::EcoString;
use hexpm::version::Version;
use itertools::Itertools;
use pubgrub::range::Range;
use pubgrub::{package, range::Range};
use std::{
cmp,
collections::{HashMap, HashSet},
Expand Down Expand Up @@ -208,7 +208,11 @@ where
pub fn compile_root_package(&mut self) -> Outcome<Package, Error> {
let config = self.config.clone();
self.compile_gleam_package(&config, true, self.paths.root().to_path_buf())
.map(|modules| Package { config, modules })
.map(|(modules, cached_metadata)| Package {
config,
modules,
cached_metadata,
})
}

/// Checks that version file found in the build directory matches the
Expand Down Expand Up @@ -288,7 +292,9 @@ where
// longer need to have the package borrowed from self.packages.
let package = self.packages.get(name).expect("Missing package").clone();
let result = match usable_build_tools(&package)?.as_slice() {
&[BuildTool::Gleam] => self.compile_gleam_dep_package(&package),
&[BuildTool::Gleam] => self
.compile_gleam_dep_package(&package)
.map(|(modules, _)| modules),
&[BuildTool::Rebar3] => self.compile_rebar3_dep_package(&package).map(|_| vec![]),
&[BuildTool::Mix] => self.compile_mix_dep_package(&package).map(|_| vec![]),
&[BuildTool::Mix, BuildTool::Rebar3] => self
Expand Down Expand Up @@ -489,7 +495,7 @@ where
fn compile_gleam_dep_package(
&mut self,
package: &ManifestPackage,
) -> Result<Vec<Module>, Error> {
) -> Result<(Vec<Module>, Vec<package_compiler::CacheMetadata>), Error> {
// TODO: Test
let package_root = match &package.source {
// If the path is relative it is relative to the root of the
Expand Down Expand Up @@ -520,7 +526,7 @@ where
config: &PackageConfig,
is_root: bool,
root_path: Utf8PathBuf,
) -> Outcome<Vec<Module>, Error> {
) -> Outcome<(Vec<Module>, Vec<package_compiler::CacheMetadata>), Error> {
let out_path =
self.paths
.build_directory_for_package(self.mode(), self.target(), &config.name);
Expand Down Expand Up @@ -590,14 +596,16 @@ where
};

// Compile project to Erlang or JavaScript source code
compiler.compile(
let outcome = compiler.compile(
&mut self.warnings,
&mut self.importable_modules,
&mut self.defined_modules,
&mut self.stale_modules,
&mut self.incomplete_modules,
self.telemetry,
)
);

outcome
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler-core/src/docs/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ fn compile_with_markdown_pages(
)
.unwrap();

for module in &mut modules {
for module in &mut modules.0 {
module.attach_doc_and_module_comments();
}

Expand All @@ -79,7 +79,7 @@ fn compile_with_markdown_pages(
super::generate_html(
&paths,
&config,
&modules,
&modules.0,
&docs_pages,
pages_fs,
SystemTime::UNIX_EPOCH,
Expand Down
Loading
Loading