summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthias Beyer <matthias.beyer@atos.net>2021-04-09 10:23:09 +0200
committerMatthias Beyer <mail@beyermatthias.de>2021-04-09 12:39:46 +0200
commitf78df9eeb3c8d6aa6f74c0bb774282662b1ef635 (patch)
tree60e4e7098c0b166c36d712bfe06bf3a6ac24dcf5
parent8bf7702cf97a8d69c8d021f81d137aa009734ca9 (diff)
Fix: Patch file finding algorithm
Because commit 5c36c119f9448baf6bfe5245c6ebac1aa09d5b43 was not enough and was actually buggy, this fixes the patch-file-finding algorithm _again_. The approach changed a bit. It actually introduces even more overhead to the loading algorithm, because of constant type-conversions. But it's as good as it can be right now. So first of all, we're collecting the patches before the merge into a Vec<PathBuf>. Each of those are existing. Then we check whether the new patch exists, and if it not does, we check whether the file actually exists in the patches from before the merge (by filename). If it does, it seems that we dragged the entry from the previous recursion. In this case, the patches from before the merge were valid, and the recursion invalidated the path. I.E.: /pkg.toml with patches = [ "a" ] /a /sub/pkg.toml with no patches=[] in the recursion for /sub/pkg.toml, we get a ["a"] in the patches array, because that's how the layered configuration works. But because the path is invalid, we check whether before the merge (so, the patches array from /pkg.toml) has a file named equally - which it does. So the array before the merge is the correct one. I did some tests on this and it seems to work correctly, but more edge-cases may exist. Signed-off-by: Matthias Beyer <matthias.beyer@atos.net> Tested-by: Matthias Beyer <matthias.beyer@atos.net> Fixes: 5c36c119f9448baf6bfe5245c6ebac1aa09d5b43 ("Fix: Error out if a patch file is missing")
-rw-r--r--src/repository/repository.rs28
1 files changed, 25 insertions, 3 deletions
diff --git a/src/repository/repository.rs b/src/repository/repository.rs
index bae3faf..170c2b0 100644
--- a/src/repository/repository.rs
+++ b/src/repository/repository.rs
@@ -18,6 +18,7 @@ use anyhow::Error;
use anyhow::Result;
use log::trace;
use resiter::AndThen;
+use resiter::FilterMap;
use resiter::Map;
use crate::package::Package;
@@ -107,7 +108,16 @@ impl Repository {
// This is either the patches array from the last recursion or the newly set one,
// that doesn't matter here.
let patches_before_merge = match config.get_array("patches") {
- Ok(v) => v,
+ Ok(v) => {
+ v.into_iter()
+ .map(|p| {
+ p.into_str()
+ .map(PathBuf::from)
+ .with_context(|| anyhow!("patches must be strings"))
+ .map_err(Error::from)
+ })
+ .collect::<Result<Vec<_>>>()?
+ },
Err(config::ConfigError::NotFound(_)) => vec![],
Err(e) => return Err(e).map_err(Error::from),
};
@@ -145,22 +155,34 @@ impl Repository {
// Otherwise we have an error here, because we're refering to a non-existing file.
.and_then_ok(|patch| if patch.exists() {
trace!("Path to patch exists: {}", patch.display());
- Ok(config::Value::from(patch.display().to_string()))
+ Ok(Some(patch))
+ } else if patches_before_merge.iter().any(|pb| pb.file_name() == patch.file_name()) {
+ // We have a patch already in the array that is named equal to the patch
+ // we have in the current recursion.
+ // It seems like this patch was already in the list and we re-found it
+ // because we loaded a deeper pkg.toml file.
+ Ok(None)
} else {
trace!("Path to patch does not exist: {}", patch.display());
Err(anyhow!("Patch does not exist: {}", patch.display()))
})
+ .filter_map_ok(|o| o)
.collect::<Result<Vec<_>>>()?;
// If we found any patches, use them. Otherwise use the array from before the merge
// (which already has the correct pathes from the previous recursion).
- let patches = if !patches.is_empty() {
+ let patches = if !patches.is_empty() && patches.iter().all(|p| p.exists()) {
patches
} else {
patches_before_merge
};
trace!("Patches after postprocessing merge: {:?}", patches);
+ let patches = patches
+ .into_iter()
+ .map(|p| p.display().to_string())
+ .map(config::Value::from)
+ .collect::<Vec<_>>();
config.set_once("patches", config::Value::from(patches))?;
}