Since we switched to the new `conflicts::Conflict` type, we represent
a missing tree entry by a `None` value in the conflict, not a missing
"add", so the condition removed in this commit will never happen, and
the case will be handled by the case just below it instead.
I don't know why I made it return an owned value. It seems like an
unnecessary restriction that the value implements `Clone`, so let's
return a reference instead.
This handles the basic case of where the matcher says that a whole
subtree is not matched. In the Linux repo, That's already enough to
speed up `jj --ignore-working-copy files samples` from 298 ms to 129
ms.
When using a sparse working copy (e.g. with no files at all) and
updating the working copy from the root commit to a commit with
millions of files, we shouldn't have to walk the parts of the diff
that doesn't match the sparse patterns. However, we still do the full
walk because our `Tree::diff()` currently doesn't care about what the
matcher tells us to visit, it only filters out unwanted files after
visiting them. This commit fixes that for the special (but common)
case of matching nothing in a directory.
I tried also adding special handling for when the matcher says that we
should only visit a few entries, but it wasn't clearly better in the
cases I tested it on. I'll keep that patch around and might send it if
I find some cases where it helps.
The function only needs the `TreeValue` so it makes more sense this
way, I think. That will also let the caller keep the rest of the
`Conflict` value owned (though there is nothing but the `value` field
in it right now).
It took a while before I realized that conflicts could be modeled as
simple algebraic expressions with positive and negative terms (they
were modeled as recursive 3-way conflicts initially). We've been
thinking of them that way for a while now, so let's make the
`ConflictPart` name match that model.
Let's acknowledge everyone's contributions by replacing "Google LLC"
in the copyright header by "The Jujutsu Authors". If I understand
correctly, it won't have any legal effect, but maybe it still helps
reduce concerns from contributors (though I haven't heard any
concerns).
Google employees can read about Google's policy at
go/releasing/contributions#copyright.
There are no "non-normal" files, so "normal" is not needed. We have
symlinks and conflicts, but they are not files, so I think just "file"
is unambiguous.
I left `testutils::write_normal_file()` because there it's used to
mean "not executable file" (there's also a `write_executable_file()`).
I left `working_copy::FileType::Normal` since renaming `Normal` there
to `File` would also suggest we should rename `FileType`, and I don't
know what would be a better name for that type.
Tree merges can currently fail because of a failure to look up an
object, or because of a failure to read its contents. Both results in
`BackendError` because of a `impl From<std::io::Error> for
BackendError`. That's kind of correct in this case, but it wasn't
intentional (that impl was from `local_backend`), and we need to
making errors more specific for better error handling.
When a directory is missing in one merge input (base or one side), we
would consider that a merge conflict. This patch changes that so we
instead merge trees by treating the missing tree as empty.
We do it for all the other kinds of objects already. It's useful to
have the path for backends that store objects by path (we don't have
any such backends yet). I think the reason I didn't do it from the
beginning was because we had separate `RepoPath` types for files and
directories back then.
This fixes a bug I've run into somewhat frequently. What happens is
that if you have a conflict on top of another conflict and you resolve
the conflict in the bottom commit, we just simplify the `Conflict`
object in the second commit, but we don't try to resolve the new
conflict. That shows up as an unexpected "conflict" in `jj log`
output, and when you check out the commit, there are actually no
conflicts, so you can just `jj squash` right away.
This patch fixes that bug. It also teaches the code to work with more
than 3 parts in the merge, so if there's a 5-way conflict, for
example, we still try to resolve it if possible.
This change teaches `Tree::diff()` to filter by a matcher. It only
filters the result so far; it does not restrict the tree walk to what
`Matcher::visit()` says is necessary yet. It also doesn't teach the
CLI to create a matcher and pass it in.
I had initially hoped that the type-safety provided by the separate
`FileRepoPath` and `DirRepoPath` types would help prevent bugs. I'm
not sure if it has prevented any bugs so far. It has turned out that
there are more cases than I had hoped where it's unknown whether a
path is for a directory or a file. One such example is for the path of
a conflict. Since it can be conflict between a directory and a file,
it doesn't make sense to use either. Instead we end up with quite a
bit of conversion between the types. I feel like they are not worth
the extra complexity. This patch therefore starts simplifying it by
replacing uses of `FileRepoPath` by `RepoPath`. `DirRepoPath` is a
little more complicated because its string form ends with a '/'. I'll
address that in separate patches.
This is yet another step towards making it easy to propagate
`BrokenPipe` errors. The `jj diff` code (naturally) diffs two trees
and prints the diffs. If the printing fails, we shouldn't just crash
like we do today.
The new code is probably slower since it does more copying (the
callback got references to the `FileRepoPath` and `TreeValue`). I hope
that won't make a noticeable difference. At least `jj diff -r
334afbc76fbd --summary` didn't seem to get measurably slower.
