Revert "Fix --sync missing some recipes (#122)" (#123)

This reverts commit b1b37ab.

crates/brioche-core/src/recipe.rs

@@ -167,8 +167,9 @@ impl Recipe {
 pub async fn get_recipes(
     brioche: &Brioche,
     recipe_hashes: impl IntoIterator<Item = RecipeHash>,
-    recipes: &mut HashMap<RecipeHash, Recipe>,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<HashMap<RecipeHash, Recipe>> {
+    let mut recipes = HashMap::new();
+
     let cached_recipes = brioche.cached_recipes.read().await;
     let mut uncached_recipes = HashSet::new();
 
@@ -188,7 +189,7 @@ pub async fn get_recipes(
 
     // Return early if we have no uncached recipess to fetch
     if uncached_recipes.is_empty() {
-        return Ok(());
+        return Ok(recipes);
     }
 
     let mut db_conn = brioche.db_conn.lock().await;
@@ -250,12 +251,11 @@ pub async fn get_recipes(
         anyhow::bail!("recipes not found: {uncached_recipes:?}");
     }
 
-    Ok(())
+    Ok(recipes)
 }
 
 pub async fn get_recipe(brioche: &Brioche, recipe_hash: RecipeHash) -> anyhow::Result<Recipe> {
-    let mut recipes = HashMap::new();
-    get_recipes(brioche, [recipe_hash], &mut recipes).await?;
+    let mut recipes = get_recipes(brioche, [recipe_hash]).await?;
     let recipe = recipes
         .remove(&recipe_hash)
         .expect("recipe not returned in collection");
@@ -719,8 +719,7 @@ impl Directory {
     }
 
     pub async fn entries(&self, brioche: &Brioche) -> anyhow::Result<BTreeMap<BString, Artifact>> {
-        let mut entry_recipes = HashMap::new();
-        get_recipes(brioche, self.entries.values().copied(), &mut entry_recipes).await?;
+        let entry_recipes = get_recipes(brioche, self.entries.values().copied()).await?;
 
         let entries = self
             .entries

crates/brioche-core/src/references.rs

@@ -17,21 +17,6 @@ use crate::{
     Brioche,
 };
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub enum ReferencedRecipe {
-    Recipe(Recipe),
-    RecipeHash(RecipeHash),
-}
-
-impl ReferencedRecipe {
-    pub fn hash(&self) -> RecipeHash {
-        match self {
-            ReferencedRecipe::Recipe(recipe) => recipe.hash(),
-            ReferencedRecipe::RecipeHash(hash) => *hash,
-        }
-    }
-}
-
 #[derive(Debug, Default, Clone, serde::Serialize, serde::Deserialize)]
 pub struct RecipeReferences {
     pub blobs: HashSet<BlobHash>,
@@ -41,32 +26,18 @@ pub struct RecipeReferences {
 pub async fn recipe_references(
     brioche: &Brioche,
     references: &mut RecipeReferences,
-    recipes: impl IntoIterator<Item = ReferencedRecipe>,
+    recipes: impl IntoIterator<Item = RecipeHash>,
 ) -> anyhow::Result<()> {
     let mut unvisited = VecDeque::from_iter(recipes);
 
     loop {
-        unvisited.retain(|recipe| !references.recipes.contains_key(&recipe.hash()));
+        unvisited.retain(|recipe| !references.recipes.contains_key(recipe));
 
         if unvisited.is_empty() {
             break;
         }
 
-        let mut recipe_hashes = vec![];
-        let mut recipes = HashMap::new();
-
-        for referenced in unvisited.drain(..) {
-            match referenced {
-                ReferencedRecipe::Recipe(recipe) => {
-                    recipes.insert(recipe.hash(), recipe);
-                }
-                ReferencedRecipe::RecipeHash(hash) => {
-                    recipe_hashes.push(hash);
-                }
-            }
-        }
-
-        crate::recipe::get_recipes(brioche, recipe_hashes, &mut recipes).await?;
+        let recipes = crate::recipe::get_recipes(brioche, unvisited.drain(..)).await?;
 
         for recipe in recipes.values() {
             unvisited.extend(referenced_recipes(recipe));
@@ -93,7 +64,7 @@ pub async fn project_references(
     project_hashes: impl IntoIterator<Item = ProjectHash>,
 ) -> anyhow::Result<()> {
     let mut unvisited = VecDeque::from_iter(project_hashes);
-    let mut new_recipes = HashSet::<ReferencedRecipe>::new();
+    let mut new_recipes = HashSet::<RecipeHash>::new();
 
     loop {
         unvisited.retain(|project_hash| !references.projects.contains_key(project_hash));
@@ -130,7 +101,7 @@ pub async fn project_references(
                     .as_ref()
                     .with_context(|| format!("static not loaded for module {module_path}"))?;
                 let static_recipe_hash = static_.output_recipe_hash(output)?;
-                new_recipes.insert(ReferencedRecipe::RecipeHash(static_recipe_hash));
+                new_recipes.insert(static_recipe_hash);
             }
         }
     }
@@ -164,24 +135,17 @@ pub fn referenced_blobs(recipe: &Recipe) -> Vec<BlobHash> {
     }
 }
 
-pub fn referenced_recipes(recipe: &Recipe) -> Vec<ReferencedRecipe> {
+pub fn referenced_recipes(recipe: &Recipe) -> Vec<RecipeHash> {
     match recipe {
         Recipe::File {
             resources,
             content_blob: _,
             executable: _,
-        } => vec![ReferencedRecipe::Recipe(resources.value.clone())],
-        Recipe::Directory(directory) => directory
-            .entry_hashes()
-            .values()
-            .copied()
-            .map(ReferencedRecipe::RecipeHash)
-            .collect(),
+        } => referenced_recipes(resources),
+        Recipe::Directory(directory) => directory.entry_hashes().values().copied().collect(),
         Recipe::Symlink { .. } => vec![],
         Recipe::Download(_) => vec![],
-        Recipe::Unarchive(unarchive) => {
-            vec![ReferencedRecipe::Recipe(unarchive.file.value.clone())]
-        }
+        Recipe::Unarchive(unarchive) => referenced_recipes(&unarchive.file),
         Recipe::Process(process) => {
             let ProcessRecipe {
                 command,
@@ -199,28 +163,26 @@ pub fn referenced_recipes(recipe: &Recipe) -> Vec<ReferencedRecipe> {
 
             templates
                 .flat_map(|template| &template.components)
-                .filter_map(|component| match component {
-                    ProcessTemplateComponent::Input { recipe } => {
-                        Some(ReferencedRecipe::Recipe(recipe.value.clone()))
-                    }
+                .flat_map(|component| match component {
+                    ProcessTemplateComponent::Input { recipe } => referenced_recipes(recipe),
                     ProcessTemplateComponent::Literal { .. }
                     | ProcessTemplateComponent::OutputPath
                     | ProcessTemplateComponent::ResourceDir
                     | ProcessTemplateComponent::InputResourceDirs
                     | ProcessTemplateComponent::HomeDir
                     | ProcessTemplateComponent::WorkDir
-                    | ProcessTemplateComponent::TempDir => None,
+                    | ProcessTemplateComponent::TempDir => vec![],
                 })
                 .chain(
                     dependencies
                         .iter()
-                        .map(|recipe| ReferencedRecipe::Recipe(recipe.value.clone())),
+                        .flat_map(|dep| referenced_recipes(&dep.value)),
                 )
-                .chain([ReferencedRecipe::Recipe(work_dir.value.clone())])
+                .chain(referenced_recipes(work_dir))
                 .chain(
                     output_scaffold
-                        .as_ref()
-                        .map(|recipe| ReferencedRecipe::Recipe(recipe.value.clone())),
+                        .iter()
+                        .flat_map(|recipe| referenced_recipes(recipe)),
                 )
                 .collect()
         }
@@ -236,79 +198,71 @@ pub fn referenced_recipes(recipe: &Recipe) -> Vec<ReferencedRecipe> {
                 networking: _,
             } = process;
 
+            let work_dir = Recipe::from(work_dir.clone());
+            let output_scaffold = output_scaffold
+                .as_ref()
+                .map(|artifact| Recipe::from((**artifact).clone()));
+
             let templates = [command].into_iter().chain(args).chain(env.values());
 
             templates
                 .flat_map(|template| &template.components)
-                .filter_map(|component| match component {
+                .flat_map(|component| match component {
                     CompleteProcessTemplateComponent::Input { artifact } => {
-                        Some(ReferencedRecipe::Recipe(artifact.value.clone().into()))
+                        let recipe = Recipe::from(artifact.value.clone());
+                        referenced_recipes(&recipe)
                     }
                     CompleteProcessTemplateComponent::Literal { .. }
                     | CompleteProcessTemplateComponent::OutputPath
                     | CompleteProcessTemplateComponent::ResourceDir
                     | CompleteProcessTemplateComponent::InputResourceDirs
                     | CompleteProcessTemplateComponent::HomeDir
                     | CompleteProcessTemplateComponent::WorkDir
-                    | CompleteProcessTemplateComponent::TempDir => None,
+                    | CompleteProcessTemplateComponent::TempDir => vec![],
                 })
-                .chain([ReferencedRecipe::Recipe(Recipe::Directory(
-                    work_dir.clone(),
-                ))])
-                .chain(
-                    output_scaffold
-                        .as_deref()
-                        .map(|recipe| ReferencedRecipe::Recipe(recipe.clone().into())),
-                )
+                .chain(referenced_recipes(&work_dir))
+                .chain(output_scaffold.iter().flat_map(referenced_recipes))
                 .collect()
         }
         Recipe::CreateFile {
             content: _,
             executable: _,
             resources,
-        } => vec![ReferencedRecipe::Recipe(resources.value.clone())],
+        } => referenced_recipes(resources),
         Recipe::CreateDirectory(directory) => directory
             .entries
             .values()
-            .map(|entry| ReferencedRecipe::Recipe(entry.value.clone()))
+            .flat_map(|entry| referenced_recipes(entry))
             .collect(),
-        Recipe::Cast { recipe, to: _ } => vec![ReferencedRecipe::Recipe(recipe.value.clone())],
+        Recipe::Cast { recipe, to: _ } => referenced_recipes(recipe),
         Recipe::Merge { directories } => directories
             .iter()
-            .map(|dir| ReferencedRecipe::Recipe(dir.value.clone()))
+            .flat_map(|dir| referenced_recipes(dir))
             .collect(),
         Recipe::Peel {
             directory,
             depth: _,
-        } => vec![ReferencedRecipe::Recipe(directory.value.clone())],
-        Recipe::Get { directory, path: _ } => {
-            vec![ReferencedRecipe::Recipe(directory.value.clone())]
-        }
+        } => referenced_recipes(directory),
+        Recipe::Get { directory, path: _ } => referenced_recipes(directory),
         Recipe::Insert {
             directory,
             path: _,
             recipe,
-        } => [ReferencedRecipe::Recipe(directory.value.clone())]
+        } => referenced_recipes(directory)
             .into_iter()
-            .chain(
-                recipe
-                    .as_ref()
-                    .map(|recipe| ReferencedRecipe::Recipe(recipe.value.clone())),
-            )
+            .chain(recipe.iter().flat_map(|recipe| referenced_recipes(recipe)))
             .collect(),
         Recipe::Glob {
             directory,
             patterns: _,
-        } => vec![ReferencedRecipe::Recipe(directory.value.clone())],
+        } => referenced_recipes(directory),
         Recipe::SetPermissions {
             file,
             executable: _,
-        } => vec![ReferencedRecipe::Recipe(file.value.clone())],
-        Recipe::Proxy(proxy) => vec![ReferencedRecipe::RecipeHash(proxy.recipe)],
-        Recipe::CollectReferences { recipe } => {
-            vec![ReferencedRecipe::Recipe(recipe.value.clone())]
-        }
-        Recipe::Sync { recipe } => vec![ReferencedRecipe::Recipe(recipe.value.clone())],
+        } => referenced_recipes(file),
+        Recipe::Proxy(proxy) => vec![proxy.recipe],
+        Recipe::CollectReferences { recipe } => referenced_recipes(recipe),
+        Recipe::Sync { recipe } => referenced_recipes(recipe),
     }
 }
 

crates/brioche-core/src/registry.rs

@@ -12,7 +12,6 @@ use crate::{
     blob::BlobHash,
     project::{Project, ProjectHash},
     recipe::{Artifact, Recipe, RecipeHash},
-    references::ReferencedRecipe,
     Brioche,
 };
 
@@ -503,14 +502,7 @@ pub async fn fetch_recipes_deep(
 
         for recipe in &new_recipes {
             let referenced_recipes = crate::references::referenced_recipes(recipe);
-            let referenced_recipe_hashes =
-                referenced_recipes
-                    .into_iter()
-                    .filter_map(|reference| match reference {
-                        ReferencedRecipe::RecipeHash(hash) => Some(hash),
-                        ReferencedRecipe::Recipe(_) => None,
-                    });
-            pending_recipes.extend(referenced_recipe_hashes);
+            pending_recipes.extend(referenced_recipes);
         }
 
         checked_recipes.extend(new_recipes.iter().map(|recipe| recipe.hash()));

crates/brioche-core/src/sync.rs

@@ -4,7 +4,7 @@ use human_repr::HumanDuration;
 
 use crate::{
     project::ProjectHash,
-    references::{ProjectReferences, RecipeReferences, ReferencedRecipe},
+    references::{ProjectReferences, RecipeReferences},
     Brioche,
 };
 
@@ -55,12 +55,9 @@ pub async fn sync_bakes(
 
     let mut sync_references = RecipeReferences::default();
 
-    let recipe_hashes = bakes.iter().flat_map(|(input, output)| {
-        [
-            ReferencedRecipe::Recipe(input.clone()),
-            ReferencedRecipe::Recipe(output.clone().into()),
-        ]
-    });
+    let recipe_hashes = bakes
+        .iter()
+        .flat_map(|(input, output)| [input.hash(), output.hash()]);
     crate::references::recipe_references(brioche, &mut sync_references, recipe_hashes).await?;
 
     let num_recipe_refs = sync_references.recipes.len();

crates/brioche-core/tests/recipe.rs

@@ -114,13 +114,9 @@ async fn test_recipe_save_multiple() -> anyhow::Result<()> {
         brioche_core::recipe::save_recipes(&brioche, [file_2.clone(), file_3.clone()]).await?;
     assert_eq!(new_artifacts, 1);
 
-    let mut results = HashMap::new();
-    brioche_core::recipe::get_recipes(
-        &brioche,
-        [file_1.hash(), file_2.hash(), file_3.hash()],
-        &mut results,
-    )
-    .await?;
+    let results =
+        brioche_core::recipe::get_recipes(&brioche, [file_1.hash(), file_2.hash(), file_3.hash()])
+            .await?;
 
     assert_eq!(
         results,

crates/brioche-core/tests/sync_to_registry.rs

@@ -22,11 +22,8 @@ async fn test_sync_to_registry_process_and_complete_process() -> anyhow::Result<
         Recipe::CompleteProcess(complete_process_recipe.clone()).hash();
 
     // Create a mocked output for the complete_process recipe
-    let output_resource_dir = brioche_test_support::empty_dir_value();
-    let output_resource_dir_hash = Recipe::Directory(output_resource_dir.clone()).hash();
     let dummy_blob = brioche_test_support::blob(&brioche, "dummy value").await;
-    let mocked_output =
-        brioche_test_support::file_with_resources(dummy_blob, false, output_resource_dir);
+    let mocked_output = brioche_test_support::file(dummy_blob, false);
     brioche_test_support::mock_bake(
         &brioche,
         &Recipe::CompleteProcess(complete_process_recipe.clone()),
@@ -87,7 +84,6 @@ async fn test_sync_to_registry_process_and_complete_process() -> anyhow::Result<
                 process_recipe_hash,
                 complete_process_recipe_hash,
                 mocked_output.hash(),
-                output_resource_dir_hash,
             ])?)
             .create(),
     );