Skip to content

Commit

Permalink
Fix issue in VCS node_modules change listing
Browse files Browse the repository at this point in the history
We must expand node_modules' file-paths for the invalidations to work properly.

Also we must read the lock-file state from disk rather than git, otherwise
we'll read the committed state only.

Test Plan: cargo build

Reviewers: MonicaOlejniczak, pancaspe87

Reviewed By: pancaspe87, MonicaOlejniczak

Pull Request: #360
  • Loading branch information
yamadapc authored Feb 28, 2025
1 parent 726b0b0 commit d3da6e6
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 31 deletions.
77 changes: 60 additions & 17 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ indoc = "2.0.5"
Inflector = "0.11.4"
is_elevated = "0.1.2"
itertools = "0.14.0"
jwalk = "0.8"
jemallocator = "0.5.4"
json = "0.12.4"
json5 = "0.4.1"
Expand Down Expand Up @@ -84,6 +85,7 @@ sourcemap = "9.1.2"
swc_core = "10.0.0"
swc_ecma_parser = "6.0.1"
swc_ecma_transforms_testing = "7.0.0"
tempfile = "3.17"
thiserror = "2.0.9"
thread_local = "1.1.8"
tinyvec = "1.8.1"
Expand Down
2 changes: 2 additions & 0 deletions crates/atlaspack_vcs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ serde = { workspace = true, features = ["derive"] }
serde_yaml = { workspace = true }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
jwalk = { workspace = true }

[dev-dependencies]
clap = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tempfile = { workspace = true }
2 changes: 1 addition & 1 deletion crates/atlaspack_vcs/examples/vcs_file_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn main() {
let changes = get_changed_files(
&repository_root,
&start_rev,
end_rev.as_deref().unwrap_or("HEAD"),
end_rev.as_deref(),
FailureMode::IgnoreMissingNodeModules,
)
.unwrap();
Expand Down
21 changes: 15 additions & 6 deletions crates/atlaspack_vcs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ pub fn list_yarn_states(

let node_modules_path = repo.join(&node_modules_relative_path);
let yarn_state = parse_yarn_state_file(&node_modules_path).map_err(|err| {
tracing::warn!(
tracing::debug!(
"Failed to read .yarn-state.yml {node_modules_relative_path:?} {}",
err
);
Expand Down Expand Up @@ -274,6 +274,7 @@ pub enum FailureMode {
FailOnMissingNodeModules,
}

#[derive(Debug)]
pub struct FileChangeEvent {
path: PathBuf,
change_type: FileChangeType,
Expand All @@ -297,6 +298,7 @@ impl FileChangeEvent {
}
}

#[derive(Debug)]
pub enum FileChangeType {
Create,
Update,
Expand All @@ -318,6 +320,7 @@ pub fn get_changed_files_from_git(
status_options.include_ignored(false);
status_options.include_untracked(true);
status_options.include_unmodified(false);
status_options.recurse_ignored_dirs(false);

let statuses = repo.statuses(Some(&mut status_options))?;
statuses.iter().for_each(|entry| {
Expand Down Expand Up @@ -396,16 +399,19 @@ pub fn get_changed_files_from_git(
pub fn get_changed_files(
repo_path: &Path,
old_rev: &str,
new_rev: &str,
new_rev: Option<&str>,
failure_mode: FailureMode,
) -> anyhow::Result<Vec<FileChangeEvent>> {
let repo = Repository::open(repo_path)?;
let old_commit = repo.revparse_single(old_rev)?.peel_to_commit()?;
let new_commit = repo.revparse_single(new_rev)?.peel_to_commit()?;
let new_commit = repo
.revparse_single(new_rev.unwrap_or("HEAD"))?
.peel_to_commit()?;

let mut changed_files = get_changed_files_from_git(repo_path, &repo, &old_commit, &new_commit)?;
tracing::trace!("Changed files: {:?}", changed_files);

tracing::debug!("Reading yarn.lock from {} and {}", old_rev, new_rev);
tracing::debug!("Reading yarn.lock from {} and {:?}", old_rev, new_rev);
let yarn_lock_changes = changed_files
.iter()
.filter(|file| file.path.file_name().unwrap() == "yarn.lock")
Expand All @@ -427,9 +433,12 @@ pub fn get_changed_files(
let maybe_old_yarn_lock: Option<YarnLock> = maybe_old_yarn_lock_blob
.map(|s| parse_yarn_lock(&s))
.transpose()?;
let new_yarn_lock_blob: String =
let new_yarn_lock_blob: String = if new_rev.is_some() {
get_file_contents_at_commit(&repo, &new_commit, yarn_lock_path)?
.ok_or_else(|| anyhow!("Expected lockfile to exist in current revision"))?;
.ok_or_else(|| anyhow!("Expected lockfile to exist in current revision"))?
} else {
std::fs::read_to_string(repo_path.join(yarn_lock_path))?
};
let new_yarn_lock: YarnLock = parse_yarn_lock(&new_yarn_lock_blob)?;

let node_modules_path = repo_path.join(&node_modules_relative_path);
Expand Down
Loading

0 comments on commit d3da6e6

Please sign in to comment.