Errors that may occur while loading backend would vary per backends, and
it's unlikely that these errors could be mapped to BackendError variants
other than BackendError::Other. So let's extract Other(_) of that kind as
a separate type to clarify there would be no other error variants.
Perhaps, Backend/Error will be renamed to CommitBackend/Error or
CommitStore/Error?, whereas I think BackendInit/LoadError can be shared
among store factories.
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.
This helps to map initialization error to BackendError without too general
From impl. I don't think io::Error (or our PathError) should be automatically
translated to BackendError::Other because BackendError has more specific
variants depending on context. If the error is specific to initialization,
it makes sense to translate it to Other variant.
I don't think we'll want to record a label for each term, because such
labels would get stale, and it seems hard to make them make sense
after transferring a remote to another repo. I think we'll probably
want to infer labels on demand instead (#1176).
Perhaps, this would handle patterns like ["a(b", "c)"] better. It might not
be correct to error out on "(", but should be better than building wrong
regexp pattern "a(b|c)".
Now that we've replaced `MergeHunk` by a `Conflict`, it makes sense to
convert the input `Conflict<FileId>` by mapping each term. Unlike
`Option::map()` I made `Conflict::map()` take a reference `self`,
because it's not uncommon to want to map the same conflict multiple
times. I'm going to use that for producing a
`Conflict<Option<TreeValue>>` from a `Conflict<Tree>` and a set of
paths.
Since `Conflict`s can represent the resolved state, so
`Conflict<ContentHunk>` can represent the states that we use
`MergeHunk` for. `MergeHunk` does force the user to handle the
resolved case, which may be useful. I suppose one could use the same
argument for making `Conflict` an enum, i.e. if we think that
`MergeHunk`'s two variants are beneficial, then we should consider
making `Conflict` an enum with those two variants.
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.
This only parses the fields relevant to us, i.e.:
- name: the stable identifier of the submodule
- path: the path to the submodule in the current commit
- url: the remote we can clone the submodule from
The full list of .gitmodules fields can be found at
https://git-scm.com/docs/gitmodules.
This eliminates indirect access through Vec<u8> and improves cache locality
while sorting the index entries. We can achieve a similar result by using
SmallVec<[u8; 24]> in place of Commit/ChangeId(Vec<u8>), but we would have
to determine a reasonable id length across backends. Indexing [u8; 4] performs
better, at the cost of the API and implementation complexity.
For temporary Commit/ChangeId allocation in general, I think a borrowed type
like Path/PathBuf will help.
Testing with my "linux" repo, this saves ~670ms needed to initialize both
change id index and disambiguation indexes.
I'll rewrite resolve_prefix_range() to branch depending on the prefix length,
and the easiest way to do that is passing iterator to continuation function
instead of returning iterator as an either (or boxed) type.
I'm going to rewrite IdIndex to store only first few bytes of the key. A
separate table helps there.
At this point, it wouldn't make sense to convert usize to u32, but the new
index will store ([u8; 4], u32) pairs.
It allows us to build multiple IdIndex instances within a single loop. As the
final sorting is heavy operation, I don't want to implement Default + Extend
for IdIndex to be compatible with Iterator::unzip().
With my colocated "linux" repo, this appears to save ~50ms startup overhead.
Since the repo has lots of indirect tags, we can't eliminate tag object
loading at all. But still, it's faster than falling back to peel_to_commit().
It was convenient that what the git backend stored in its "extras"
table is exactly a subset of the fields that local backend stores, but
it's bit ugly and limiting. For example, it makes it possible to
populate the `author` field in the git extras, but that would have no
effect. It's better that it's not possible to do that (we store the
author field in the git commit, of course).
What made me notice this now was that I'm working on tree-level
conflicts (#1624) and I'm thinking of adding a field to the git extras
saying "this commit has single tree, but it's still a new-style
commit", so we can know not to walking such trees to find path-level
conflicts. That's only needed for the git backend because we don't
care about compatibility for the local backend.
If we allow `Conflict::simplify()` to swap the removes and adds as
freely as we currently do, we may present the user with a conflict
marker with a diff that has never appeared anywhere before the
simplification. That seems very confusing. Let's instead preserve the
diffs when we simplify conflicts.
It's a bit weird to simplify a conflict like `A B->C D->E C->F` to `A
B->E D->F` because it changes which diffs are in the conflict, but
that's what we currently do. Let's have a test for that.
We actually already have tests showing how `A B->C D->A` gets
simplified to `C B->D`, but those are less obviously weird because
when rendered as `removes = [B], adds = [C, D]`, it doesn't look that
different from the reverse.
This commit fixes#1305
Before this commit, running `jj init --git-repo=./` in a folder that
does not have a .git would cause jj to panick and leave an unfinished corrupted jj repo.
This commit fixes that by changing the call chain to return an error
instead of calling .unwrap() and panicking. This commit also adds logic to delete the unfinished jj
repository when the git backend initialization failed.
Before this commit, running the above command would result in the following
```
Running `jj/target/debug/jj init --git-repo=./`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: -3, klass: 2, message: "failed to resolve path '/Users/kevincliao/github/jj/test-repo/.jj/repo/store/../../../.git': No such file or directory" }', lib/src/git_backend.rs:83:75
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
After this commit, the result is the following and the jj repo is deleted:
```
Running `jj/target/debug/jj init --git-repo=./`
Error: Failed to access the repository: Error: Failed to open git repository: failed to resolve path '/Users/kevincliao/github/jj/test-repo/.jj/repo/store/../../../.git': No such file or directory; class=Os (2); code=NotFound (-3)
```
..and other assorted boilerplate. These are just stubs for now, but now
that we've reserved the `submodule_store` subdirectory, we can start
adding more functionality.
This changes the behavior in one of the cases ilyagr@
[mentioned](https://github.com/martinvonz/jj/pull/1610#discussion_r1199823932)
to match his suggestion. After some more thinking while working on
tree-level conflicts, I now think it's clear that the added `+C-C`
terms should have no effect on the result. A very similar argument is
that `Conflict::simplify()` should not change the result of
`trivial_merge()`. I'll add tests for that next.
Before we had `conflicts::Conflict`, most of these functions took a
`backend::Conflict`. I think I didn't want to pollute the `backend`
module with this kind of logic, trying to keep it focused on
storage. Now that we have the type in `conflicts`, however, I think it
makes sense to move these functions onto it.
This gets rid of round-trip conversion from queries like "(main..)-". I have
such expression in my default log/disambiguation revset, and the query could
take ~150ms to convert head positions back and forth if the repository had
tons of unmerged commits.
Since unchanged refs should be pinned by new_git_heads, we only need to
consider about "changed" old_git_targets. This allows us to return early
if hidable_git_heads.is_empty().
In colocated mid-size "linux" repo, this saves ~450ms needed to do
enforce_view_invariants(). We could instead make add_head() to return early,
but the condition would be a bit weird since HEAD@git is typically a parent
of known heads, not a head itself.
While playing with perf.data captured with "jj log", I noticed RepoPath::join()
has measurable cost. The first half is small alloc()s for Vec and Strings, and
the latter is realloc() on Vec::push(). Removing realloc() is easy, so let's
do that.
We already have the new `Conflict::from_backend_conflict()` for
converting from a `backend::Conflict`, but we model conflicts in a
similar way in at least `RefTarget`. I'd like to be able to use
`conflicts::Conflict` there too. To prepare for that, let's extract
generic methods from `Conflict::from_backend_conflict()` and
`Conflict::to_backend_conflict()`.
I'm not sure I'll get around to making `RefTarget` use `Conflict` but
this commit seems like nice cleanup either way. It makes the tests
simpler if nothing else.
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.
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()`.
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.
For tree-level conflicts (#1624), I plan to remove `ConflictId`
completely. This commit removes `ConflictId` from
`update_conflict_from_content()` by instead making it take a
`Conflict<Option<TreeValue>>` and return a possibly different such
value.
I made the call site in `working_copy` avoid writing the conflict to
the store if it's unchanged, but I didn't make the same optimization
in `merge_tools` becuase it's much more likely to have changed there.
Use `br@git` instead.
Before, if there is not a local branch `br`, jj tried to resolve
it as a git ref `refs/heads/br`. Unchanged from before, `br` can
still be resolved as a tag `refs/tag/br`.
This doesn't change the way @git branches are stored in `git_refs` as opposed
to inside `BranchTarget` like normal remote-tracking branches. There are
subtle differences in behavior with e.g. `jj branch forget` and I'm not sure
how easy it is to rewrite `jj git import/export` to support a different
way of storage.
I've decided to call these "local-git tracking branches" since they track
branches in the local git repository. "local git-tracking" branches sounds a
bit more natural, but these could be confused with there are no remote
git-tracking branches. If one had the idea these might exist, they would be
confused with remote-tracking branches in the local git repo.
This addresses a portion of #1666
