From c46a2ee433b6de462cea1d8782c73f76553ecc8a Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 7 Feb 2021 12:12:17 +0100 Subject: Fix: Do not borrow mutable twice This fixes a bug which caused butido to crash in case of `else` in the snippet changed here. This was because in the `else`-case, we borrow mutably what was already mutably borrowed, causing a panic. Signed-off-by: Matthias Beyer Tested-by: Matthias Beyer --- src/orchestrator/orchestrator.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) (limited to 'src/orchestrator/orchestrator.rs') diff --git a/src/orchestrator/orchestrator.rs b/src/orchestrator/orchestrator.rs index f5dd8cb..a1f8a99 100644 --- a/src/orchestrator/orchestrator.rs +++ b/src/orchestrator/orchestrator.rs @@ -268,20 +268,23 @@ impl<'a> Orchestrator<'a> { .filter(|j| j.1.jobdef.dependencies.contains(job.1.jobdef.job.uuid())) .map(|j| j.2.clone()) }); - } else { - *job.3.borrow_mut() = { - let depending_on_job = jobs.iter() - .filter(|j| j.1.jobdef.dependencies.contains(job.1.jobdef.job.uuid())) - .map(|j| j.2.clone()) - .collect::>>(); - - if depending_on_job.is_empty() { - None - } else { - Some(depending_on_job) - } - }; + + continue; } + + // else, but not in else {} because of borrowing + *job.3.borrow_mut() = { + let depending_on_job = jobs.iter() + .filter(|j| j.1.jobdef.dependencies.contains(job.1.jobdef.job.uuid())) + .map(|j| j.2.clone()) + .collect::>>(); + + if depending_on_job.is_empty() { + None + } else { + Some(depending_on_job) + } + }; } // Find the id of the root task -- cgit v1.2.3 From d319579fc89b5ac09f10abf4bfc77c21b0fc65ce Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 7 Feb 2021 12:35:33 +0100 Subject: Remove `Artifact` type This patch follows-up on the shrinking of the `Artifact` type and removes it entirely. The type is not needed. Only the `ArtifactPath` type is needed, which is a thin wrapper around `PathBuf`, ensuring that the path is relative to the store root. The `Artifact` type used `pom` to parse the name and version of the package from the `ArtifactPath` object it contained, which resulted in the restriction that the path must always be -... Which should not be a requirement and actually caused issues with a package named "foo-bar" (as an example). Signed-off-by: Matthias Beyer Tested-by: Matthias Beyer --- src/orchestrator/orchestrator.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'src/orchestrator/orchestrator.rs') diff --git a/src/orchestrator/orchestrator.rs b/src/orchestrator/orchestrator.rs index a1f8a99..79f769e 100644 --- a/src/orchestrator/orchestrator.rs +++ b/src/orchestrator/orchestrator.rs @@ -32,7 +32,7 @@ use crate::config::Configuration; use crate::db::models as dbmodels; use crate::endpoint::EndpointConfiguration; use crate::endpoint::EndpointScheduler; -use crate::filestore::Artifact; +use crate::filestore::ArtifactPath; use crate::filestore::MergedStores; use crate::filestore::ReleaseStore; use crate::filestore::StagingStore; @@ -203,16 +203,16 @@ impl<'a> OrchestratorSetup<'a> { /// It is either a list of artifacts with the UUID of the job they were produced by, /// or a UUID and an Error object, where the UUID is the job UUID and the error is the /// anyhow::Error that was issued. -type JobResult = std::result::Result>, HashMap>; +type JobResult = std::result::Result>, HashMap>; impl<'a> Orchestrator<'a> { - pub async fn run(self, output: &mut Vec) -> Result> { + pub async fn run(self, output: &mut Vec) -> Result> { let (results, errors) = self.run_tree().await?; output.extend(results.into_iter()); Ok(errors) } - async fn run_tree(self) -> Result<(Vec, HashMap)> { + async fn run_tree(self) -> Result<(Vec, HashMap)> { let multibar = Arc::new(indicatif::MultiProgress::new()); // For each job in the jobdag, built a tuple with @@ -403,7 +403,7 @@ impl<'a> JobTask<'a> { // A list of job run results from dependencies that were received from the tasks for the // dependencies - let mut received_dependencies: HashMap> = HashMap::new(); + let mut received_dependencies: HashMap> = HashMap::new(); // A list of errors that were received from the tasks for the dependencies let mut received_errors: HashMap = HashMap::with_capacity(self.jobdef.dependencies.len()); @@ -464,15 +464,15 @@ impl<'a> JobTask<'a> { } // Map the list of received dependencies from - // Vec<(Uuid, Vec)> + // Vec<(Uuid, Vec)> // to - // Vec + // Vec let dependency_artifacts = received_dependencies .values() .map(|v| v.iter()) .flatten() .cloned() - .collect(); + .collect::>(); trace!("[{}]: Dependency artifacts = {:?}", self.jobdef.job.uuid(), dependency_artifacts); self.bar.set_message(&format!("[{} {} {}]: Preparing...", self.jobdef.job.uuid(), @@ -531,11 +531,11 @@ impl<'a> JobTask<'a> { /// /// Return Ok(true) if we should continue operation /// Return Ok(false) if the channel is empty and we're done receiving - async fn perform_receive(&mut self, received_dependencies: &mut HashMap>, received_errors: &mut HashMap) -> Result { + async fn perform_receive(&mut self, received_dependencies: &mut HashMap>, received_errors: &mut HashMap) -> Result { match self.receiver.recv().await { Some(Ok(mut v)) => { // The task we depend on succeeded and returned an - // (uuid of the job, [Artifact]) + // (uuid of the job, [ArtifactPath]) trace!("[{}]: Received: {:?}", self.jobdef.job.uuid(), v); received_dependencies.extend(v); Ok(true) -- cgit v1.2.3