Commit graph

91 commits

Author SHA1 Message Date
Yuya Nishihara
dd26b7be40 merge: add Merge constructor that accepts interleaved values
Also migrated some callers of 3-way merge, where [left, base, right] order
looks okay.
2023-11-07 17:10:12 +09:00
Yuya Nishihara
895bbce8c0 files: use borrowed Merge iterator in merge()
Since the underlying Merge data type is no longer (Vec<T>, Vec<T>), it doesn't
make sense to build removes/adds Vecs and concatenate them.
2023-11-07 06:52:35 +09:00
Martin von Zweigbergk
309f1200d6 merge: introduce a type alias for Merge<Option<TreeValue>>
Reasons to introduce this alias:

* Reduces complexity of a type, to silence Clippy warnings in the
  future if we use this type as a type parameter

* The type is used quite frequently, so it makes sense to have a name
  for it

* It's easier to visually scan for the end of the type when you don't
  have to match opening and closing angle brackets
2023-10-26 06:20:56 -07:00
Martin von Zweigbergk
7fda80fc22 tree: simplify conflict before resolving at hunk level
I ran into a bug the other day where `jj status` said there was a
conflict in a file but there were no conflict markers in the working
copy. The commit was created when I squashed a conflict resolution
into the commit's parent. The rebased child commit then ended up in
this state. I.e., it looked something like this before squashing:

```
C (no conflict)
|
| B conflict
|/
A conflict
```

The conflict in B was different from the conflict in A. When I
squashed in C, jj would try to resolve the conflicts by first creating
a 7-way conflict (3 from A, 3 from B, 1 from C). Because of the exact
content-level changes, the 7-way conflict couldn't be automatically
resolved by `files::merge()` (the way it currently works
anyway). However, after simplifying the conflict, it could be
resolved. Because `MergedTree::merge()` does another round of conflict
simplification of the result at the end of the function, it was the
simplifed version that actually got stored in the commit. So when
inspecting the conflict later (e.g. in the working copy, as I did), it
could be automatically resolved.

I think there are at least two ways to solve this. One is to call
`merge_trees()` again after calling `tree.simplify()` in
`MergedTree::merge()`. However, I think it would only matter in the
case of content-level conflicts. Therefore, it seems better to make
the content-level resolution solve this case to start with. I've done
that by simplifying the conflict before passing it into
`files::merge()`. We could even do the fix in `files::merge()`, but
doing it before calling it has the advantage that we can avoid reading
some unchanged content from the backend.
2023-09-27 22:14:39 -07:00
Martin von Zweigbergk
af80e4e407 files: take Merge argument to merge()
All non-test callers already have a `Merge` object, so let's pass that
instead. We thereby simplify the callers a little, and we enforce the
"adds.len() == removes.len() + 1" constraint in the type.
2023-09-27 22:14:39 -07:00
Martin von Zweigbergk
9946e52fdf tree: leverage Merge::try_map() when reading file contents to merge 2023-09-22 19:33:48 -07:00
Martin von Zweigbergk
64b47bae56 tree: inline legacy_id() into its sole caller 2023-08-29 07:01:52 -07:00
Martin von Zweigbergk
f47da04a43 tree: delete recursive diff iterator, which is no longer used 2023-08-28 16:21:44 -07:00
Martin von Zweigbergk
1b24b522f6 tree: move diff_summary() to MergedTree 2023-08-28 16:21:44 -07:00
Martin von Zweigbergk
a7a2150328 tree: delete unused DiffSummary::is_empty() 2023-08-28 16:21:44 -07:00
Martin von Zweigbergk
1674a421ec commit_builder: take MergedTreeId for root id argument 2023-08-28 15:58:34 -07:00
Martin von Zweigbergk
adf9679d4c tree: inline simplify_conflict()
The function is just a few lines now. I don't think we need the long
documentation in it either since that's now in
docs/technical/conflicts.md.
2023-08-11 21:11:25 +00:00
Martin von Zweigbergk
0570963fe3 merge: add a Merge::into_resolved() to avoid cloning
I don't know if this has any measurable impact. It just seems like we
should be able to take a resolved value out of a `Merge` without
clonning.
2023-08-09 21:58:15 +00:00
Martin von Zweigbergk
ef5f97f8d7 conflicts: move Merge<T> to merge module
The `merge` module now seems like the obvious place for this type.
2023-08-06 22:08:09 +00:00
Martin von Zweigbergk
ecc030848d conflicts: rename Conflict<T> to Merge<T>
Since `Conflict<T>` can also represent a non-conflict state (a single
term), `Merge<T>` seems like better name.

Thanks to @ilyagr for the suggestion in
https://github.com/martinvonz/jj/pull/1774#discussion_r1257547709

Sorry about the churn. It would have been better if I thought of this
name before I introduced `Conflict<T>`.
2023-08-06 22:08:09 +00:00
Waleed Khan
7875656354 perf: add #[instrument] to all cmd_* functions 2023-07-28 09:28:01 -07:00
Martin von Zweigbergk
828d528361 merged_tree: add a function for resolving conflicts
This adds a function for resolving conflicts that can be automatically
resolved, i.e. like our current `merge_trees()` function. However, the
new function is written to merge an arbitrary number of trees and, in
case of unresolvable conflicts, to produce a `Conflict<TreeId>` as
result instead of writing path-level conflicts to the backend. Like
`merge_trees()`, it still leaves conflicts unresolved at the file
level if any hunks conflict, and it resolves paths that can be
trivially resolved even if there are other paths that do conflict.
2023-07-19 22:04:16 -07:00
Martin von Zweigbergk
e033f9139f cleanup: use let-else now that we're on Rust 1.65+ 2023-07-18 09:50:22 +01:00
Waleed Khan
54dba51a08 docs: warn about missing docs for jj-lib crate 2023-07-10 18:28:59 +03:00
Martin von Zweigbergk
2bc58ebacf tree: avoid "file" in name of variables that can be non-files 2023-07-07 05:15:39 +02:00
Martin von Zweigbergk
76b1d53b1d tree: avoid redoing a RepoPath::join()
I also renamed the variable from `file` to `path` to clarify.
2023-07-07 05:15:39 +02:00
Martin von Zweigbergk
7e48033a65 tree: remove dir argument from TreeDiffIterator::new()
The argument is always the root directory.
2023-07-07 05:15:39 +02:00
Martin von Zweigbergk
1d8e9ec215 tree: inline two trivial functions 2023-07-07 05:15:39 +02:00
Martin von Zweigbergk
5da131f937 tree: add a Diff::from_options() constructor
I'm not sure `Diff` is worth keeping, but as long as we have, it seems
that it should have this constructor.
2023-07-06 15:19:58 +02:00
Martin von Zweigbergk
b738f884c4 tree: drop Diff::as_options(), use Diff::into_options() instead
We don't have any current callers that only have a reference to a
`Diff`, so we don't need `.as_options()`.
2023-07-06 15:19:58 +02:00
Martin von Zweigbergk
99226bb96d tree: simplify diff iterator by leveraging Tree::value()
This is much simpler and I was slightly surprised that it doesn't have
much impact on performance. I tried `jj --ignore-working-copy diff -s
--from root --to v5.15` in the Linux kernel repo, and there was
perhaps a 1.5% slowdown (508 ms -> 515 ms). In more normal cases (like
diffing a single commit against its parent), I couldn't measure any
difference at all.
2023-07-06 11:21:21 +02:00
Yuya Nishihara
da3c03206c tree: remove useless pinning of Tree object
Since Tree is Unpin, Pin<Box<Tree>> can be moved and it's basically the same
as Box<Tree>. I heard using Box<T> still violates strict aliasing rule, but
Pin wouldn't give any additional guarantee.

https://morestina.net/blog/1868/self-referential-types-for-fun-and-profit
2023-07-01 17:56:00 +09:00
Martin von Zweigbergk
f32b67ac3d tree: leverage Conflict::flatten() etc 2023-06-30 14:43:58 +02:00
Martin von Zweigbergk
24c0190f74 conflicts: rename try_map() to maybe_map()
I'm going to add a `Result` version and it makes more sense to call
that `try_map()`.
2023-06-30 14:43:58 +02:00
Martin von Zweigbergk
6bd13382f4 backend: add a function for setting or removing a tree entry 2023-06-30 14:43:58 +02:00
Martin von Zweigbergk
c0ffce781e store: cache tree on write and return it
This matches what we do when writing commits.
2023-06-30 14:12:36 +02:00
Martin von Zweigbergk
35e4d5f205 conflicts: add take(), returning the removes and adds Vecs 2023-06-27 21:06:32 +02:00
Martin von Zweigbergk
b1f2e80349 files: add a newtype around Vec<u8> for content hunks
It's useful to have a more readable `Debug` format for `Vec<u8>`
(`"foo"` is better than `[102, 111, 111]`). It might also make types
in function signatures and elsewhere more readable.
2023-06-27 21:06:32 +02:00
Martin von Zweigbergk
b8221d4e21 conflicts: add try_map() method
This simplifies `to_file_conflict()` and `try_resolve_file_conflict()`
a bit.
2023-06-26 13:47:33 +02:00
Martin von Zweigbergk
9c486ebe45 conflicts: add a method returning the resolved value, if resolved 2023-06-26 13:47:33 +02:00
Yuya Nishihara
55f93bfa04 tree: reuse caller-provided path in known_sub_tree(), mark it as private 2023-06-15 00:45:07 +09:00
Yuya Nishihara
db7facb78e tree: rewrite sub_tree_recursive() to not use known_sub_tree()
This is the only place where the caller of known_sub_tree() doesn't know
the full repo path.
2023-06-15 00:45:07 +09:00
Martin von Zweigbergk
02bb3aecf3 tree: don't read file contents on contents/executable "conflict"
If one side changes the contents and one side changes the executable
bit, we get a non-trivial conflict in the `TreeValue`s, but once we've
split them up into `FileId`s and bools, we can trivially resolve them
separately, without having to read file contents.
2023-06-13 08:49:46 +02:00
Martin von Zweigbergk
6e6ca16fd4 tree: make try_resolve_file_conflict() write resolved file to store
The function takes a `&Conflict<Option<TreeValue>>` argument so it
seems it should return an `Option<TreeValue>` for symmetry.
2023-06-13 08:49:46 +02:00
Martin von Zweigbergk
1f1c6867c7 conflicts: add a simplify() method, taken from tree.rs
It seems generally useful to be able to simplify a conflict, and it's
not specific to merging trees, so let's move it to
`conflicts.rs`. Once we're done with the migration to tree-level
conflicts, I think `Conflict::simplify()` will remain but
`tree::simplify_conflict()` will be gone.

The tests I added there are quite similar to those of
`trivial_merge()`. I hope we can make `Conflict::simplify()` call
`trivial_merge()` later. I think it would also make sense to move
`trivial_merge()` onto `Conflict`, or at least have a
`Conflict::resolve_trivial()` calling `trivial_merge()`.
2023-06-13 08:49:46 +02:00
Martin von Zweigbergk
e5a28996b4 tree: specialize and inline tree_value_to_conflict()
It seems that this function didn't serve the purpose I intended it to;
we only passed in `TreeValue::Conflict` variants to it.
2023-06-13 08:49:46 +02:00
Martin von Zweigbergk
ee98d1eee8 tree: remove check for conflict-simplification resulting in deletion
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.
2023-06-13 08:49:46 +02:00
Martin von Zweigbergk
29b676f24f store: do conversion to/from backend::Conflict
We now convert to/from `backend::Conflict` right before/after calling
the `Store` methods, so we can simplify by having the `Store` do the
conversion.
2023-06-04 06:48:34 -07:00
Martin von Zweigbergk
43c55fcbc9 tree: replace uses of backend::Conflict 2023-06-04 06:48:34 -07:00
Martin von Zweigbergk
a5883eba15 merge: make trivial_merge() return a reference
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.
2023-05-31 06:28:01 -07:00
Martin von Zweigbergk
d047a8fbd4 tree: respect matcher.visit() when walking entries
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.
2023-05-28 07:28:16 -07:00
Martin von Zweigbergk
dbcecf7244 tree: rewrite recursive entries iterator to not use machine stack
In the same vein as c02c4168fe.
2023-05-28 07:28:16 -07:00
Martin von Zweigbergk
97c64d8071 tree: leverage trivial_merge() for file executable bit 2023-05-24 22:00:38 -07:00
Martin von Zweigbergk
451af92912 tree: leverage trivial_merge() in merge_trees() 2023-05-24 22:00:38 -07:00
Yuya Nishihara
770ea0c705 tree: rewrite recursive diff iterator to not use machine stack
While mutable borrow in the next() method is a bit tricky, I think it's
easier to follow than invoking iterators recursively.
2023-05-06 14:50:37 +09:00