Closed tareknaser closed 1 month ago
By the way, let's use squashing and rebasing only before the merge into main
.
Otherwise, it's difficult to see what was changed after last review...
Actually, we need multiple values in the added
field... because we can have collisions/duplicates, so better to inform the user about it:
pub struct ModifiedPath {
path: PathBuf,
modified: SystemTime
}
pub struct IndexUpdate<Id: ResourceId> {
added: HashMap<Id, HashSet<UpdatedPath>>,
removed: HashSet<Id>,
}
And we should not duplicate ResourceId
in left and right sides, otherwise we'd need to maintain equality of them. So, auxiliary structure ModifiedPath
is handy.
Last actions points before merging:
added
field of IndexUpdate
Actually, we need multiple values in the
added
field... because we can have collisions/duplicates, so better to inform the user about it:
Thanks for catching that.
I've made the updates and added a unit test to verify the implementation's correctness. The test involves updating the content of a file and adding a new file with the same content, then confirming that update_all()
returns two entries for the new ID.
I also included some additional tests to cover corner cases we hadn't previously checked with update_all()
.
And we should not duplicate
ResourceId
in left and right sides, otherwise we'd need to maintain equality of them. So, auxiliary structureModifiedPath
is handy.
I renamed IndexEntry
to ResourceIdWithTimestamp
and created a separate struct called ResourcePathWithTimestamp
to prevent confusion with IndexedResource
and IndexEntry
. Do the names make things clearer?
I also ran into a bug where the root path prefix wasn't being stripped from the files in some cases. This issue has been fixed.
Now, all that's left is to fix the build. A couple of tests are failing only on windows for some reason. I tried to resolve it today but couldn't get it to work. I still need to spend some time on it, because I rely on CI logs for debugging.
Windows CI error fixed in e0f6e75
Description
This PR outlines a series of improvements and refactors to the
ResourceIndex
and its associated methods, as discussed in this comment.Changes
ResourceIndex
API to align with the proposed changes discussed in the linked comment.index.rs
.ResourceIndex::update_all()
to rebuild the index and compare it with the old index.id_to_resources
to be of typeHashMap<HashType, Vec<IndexedResource>>
instead ofHashMap<ResourceId, CanonicalPathBuf>
to handle cases where one ID maps to multiple resources due to collisions or identical content.Related Issues:
64: Collision Counting
20, #16, #31: Various enhancements and fixes related to the index functionality
Additional Changes:
rustfmt
to wrap long comments.rustfmt
experimental rules.target_os
totarget_family
.Todo:
ark-cli
crate to reflect the changes.fs-index
.README
forfs-index
.