feat: local changes set (PoC) #3546
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| @@ -1050,6 +1050,10 @@ fn build_predicate_fn( | |||
| let commit = store.get_commit(&entry.commit_id()).unwrap(); | |||
| commit.has_conflict().unwrap() | |||
| }), | |||
| RevsetFilterPredicate::Local => box_pure_predicate_fn(move |index, pos| { | |||
| let entry = index.entry_by_pos(pos); | |||
| store.is_local_change(&entry.change_id()).unwrap() | |||
There was a problem hiding this comment.
The local() is unusual as it doesn't depend (necessarily) on the content of the backend. Which makes it hard to fit it into existing framework of predicates.
I went for now with an approach of storing it as a standalone file, treating is more like a local UI/UX cache or a config file than part of the backend. It's unclear to me if it can be stored as a backend object - e.g. by writing it every time with Backend::write_file, and then updating a symlink with write_symlink or something like that. I don't understand desired featureset and requirements of the Backend.
| @@ -39,10 +41,14 @@ use crate::tree_builder::TreeBuilder; | |||
| /// Wraps the low-level backend and makes it return more convenient types. Also | |||
| /// adds caching. | |||
| pub struct Store { | |||
| repo_path: PathBuf, | |||
There was a problem hiding this comment.
To be able to access the right path, I sneaked repo_path in here. Probably not the best way to go about it. Possibly it could be all moved into it's own struct LocalChangesTracker and either injected here, or as a new argument to predicates, or just backend should be used directly for storage (see other comment).
| @@ -118,6 +127,75 @@ impl Store { | |||
| self.get_commit_async(id).block_on() | |||
| } | |||
|
|
|||
| pub fn is_local_change(self: &Arc<Self>, id: &ChangeId) -> BackendResult<bool> { | |||
There was a problem hiding this comment.
Implemented as a lazy-loaded json file, with a write-through cache. Given that it will not be modified very often writing the whole set every time seems fine. Happy to change any technical details.
I took a stab at #3529.