Fix case where lockfiles are added into a repo

.changeset/honest-wombats-pay.md

@@ -0,0 +1,5 @@
+---
+'@atlaspack/rust': patch
+---
+
+Fix VCS watcher handling of new yarn.lock files between revisions

crates/atlaspack_vcs/src/lib.rs

@@ -254,15 +254,18 @@ fn get_file_contents_at_commit(
   repo: &Repository,
   commit: &git2::Commit,
   path: &Path,
-) -> anyhow::Result<String> {
+) -> anyhow::Result<Option<String>> {
   let tree = commit.tree()?;
-  let entry = tree.get_path(path)?;
-  let blob = entry
-    .to_object(repo)?
-    .into_blob()
-    .map_err(|_| anyhow::anyhow!("Failed to read yarn.lock from git"))?;
-  let contents = blob.content();
-  Ok(String::from_utf8(contents.to_vec())?)
+  if let Ok(entry) = tree.get_path(path) {
+    let blob = entry
+      .to_object(repo)?
+      .into_blob()
+      .map_err(|_| anyhow::anyhow!("Failed to read yarn.lock from git"))?;
+    let contents = blob.content();
+    Ok(Some(String::from_utf8(contents.to_vec())?))
+  } else {
+    Ok(None)
+  }
 }
 
 #[derive(Debug, PartialEq)]
@@ -388,9 +391,19 @@ pub fn get_changed_files(
     let yarn_lock_path = yarn_lock_path.strip_prefix(repo_path)?;
     let node_modules_relative_path = yarn_lock_path.parent().unwrap().join("node_modules");
 
-    let old_yarn_lock_blob = get_file_contents_at_commit(&repo, &old_commit, yarn_lock_path)?;
-    let old_yarn_lock: YarnLock = parse_yarn_lock(&old_yarn_lock_blob)?;
-    let new_yarn_lock_blob = get_file_contents_at_commit(&repo, &new_commit, yarn_lock_path)?;
+    tracing::debug!(
+      "Reading yarn.lock ({}) from {:?} and {:?}",
+      yarn_lock_path.display(),
+      old_commit,
+      new_commit
+    );
+    let maybe_old_yarn_lock_blob = get_file_contents_at_commit(&repo, &old_commit, yarn_lock_path)?;
+    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 =
+      get_file_contents_at_commit(&repo, &new_commit, yarn_lock_path)?
+        .ok_or_else(|| anyhow!("Expected lockfile to exist in current revision"))?;
     let new_yarn_lock: YarnLock = parse_yarn_lock(&new_yarn_lock_blob)?;
 
     let node_modules_path = repo_path.join(&node_modules_relative_path);
@@ -413,7 +426,7 @@ pub fn get_changed_files(
     );
     let node_modules_changes = yarn_integration::generate_events(
       node_modules_path.parent().unwrap(),
-      &old_yarn_lock,
+      &maybe_old_yarn_lock,
       &new_yarn_lock,
       &yarn_state,
     );

crates/atlaspack_vcs/src/yarn_integration/mod.rs

@@ -39,6 +39,10 @@ impl YarnResolution {
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(transparent)]
 pub struct YarnLock {
+  /// Map of 'requirement' to 'resolution'
+  ///
+  /// * requirement -> `@atlaspack/core@^1.0.0`
+  /// * resolution -> `@atlaspack/core@^1.0.0: @atlaspack/[email protected]`
   inner: HashMap<String, YarnLockEntry>,
 }
 
@@ -132,13 +136,16 @@ pub fn parse_yarn_state_file(node_modules_directory: &Path) -> anyhow::Result<Ya
 
 pub fn generate_events(
   node_modules_parent_path: &Path,
-  old_yarn_lock: &YarnLock,
+  old_yarn_lock: &Option<YarnLock>,
   new_yarn_lock: &YarnLock,
   state: &YarnStateFile,
 ) -> Vec<PathBuf> {
   let changed_resolutions = new_yarn_lock
     .iter()
     .filter_map(|(package_name, new_resolution)| {
+      let Some(old_yarn_lock) = old_yarn_lock.as_ref() else {
+        return Some(new_resolution);
+      };
       let Some(old_resolution) = old_yarn_lock.get(package_name) else {
         return Some(new_resolution);
       };
@@ -156,6 +163,10 @@ pub fn generate_events(
 
     if let Some(dependency_state) = state.get(&resolution.resolution) {
       for location in &dependency_state.locations {
+        if location.is_empty() {
+          // the workspace is listed as a location, which we can skip
+          continue;
+        }
         changed_paths.push(node_modules_parent_path.join(location));
       }
     }
@@ -187,7 +198,36 @@ mod test {
 
     let events = generate_events(
       &node_modules_parent_path,
-      &old_yarn_lock,
+      &Some(old_yarn_lock),
+      &new_yarn_lock,
+      &yarn_state,
+    );
+    let events = events
+      .iter()
+      .map(|path| path.to_str().unwrap())
+      .collect::<Vec<_>>();
+
+    assert_eq!(events, vec!["node_modules/lodash"]);
+
+    Ok(())
+  }
+
+  #[test]
+  fn test_generate_events_when_old_file_is_missing() -> anyhow::Result<()> {
+    let node_modules_parent_path = PathBuf::from("");
+    let cargo_path = Path::new(env!("CARGO_MANIFEST_DIR"));
+    let samples_path = cargo_path.join("samples");
+    let new_yarn_lock_path = samples_path.join("new/yarn-lock");
+    let yarn_state_path = samples_path.join("new/yarn-state.yml");
+
+    let new_yarn_lock = parse_yarn_lock(&std::fs::read_to_string(new_yarn_lock_path)?)?;
+    let yarn_state: YarnStateFile =
+      serde_yaml::from_str(&std::fs::read_to_string(yarn_state_path)?)?;
+    yarn_state.validate()?;
+
+    let events = generate_events(
+      &node_modules_parent_path,
+      &None,
       &new_yarn_lock,
       &yarn_state,
     );

packages/core/fs/src/NodeVCSAwareFS.js

@@ -42,11 +42,8 @@ export class NodeVCSAwareFS extends NodeFS {
     const vcsEventsSince = instrument(
       'NodeVCSAwareFS::rust.getEventsSince',
       () => getEventsSince(this.#options.gitRepoPath, vcsState.gitHash),
-    );
-    this.#options.logEventDiff(
-      watcherEventsSince,
-      vcsEventsSince.map((e) => ({path: e.path, type: e.changeType})),
-    );
+    ).map((e) => ({path: e.path, type: e.changeType}));
+    this.#options.logEventDiff(watcherEventsSince, vcsEventsSince);
 
     return watcherEventsSince;
   }