If commit_id[..prefix_len] < prefix, commit_id < prefix is obviously true.
If commit_id[..prefix_len] == prefix, commit_id < prefix returns false. So
slicing isn't needed.
This makes commit_id_byte_prefix_to_pos() basically the same as
segment_commit_id_to_pos(), and these two functions can be merged.
matches() is called from resolve_change_id() loop right now, so it's better to
not allocate String there. Regarding new IdIndex integration, I'll probably make
IdIndex store raw byte ids instead of hexes, and use HexPrefix to look up
range and test prefixes. I think this is basically the same as prefix lookup
in MutableIndex, but I have no idea if we can factor out a common interface.
I made HexPrefix store (Vec<u8>, bool) instead of (Vec<u8>, Option<u8>) so
both min/partial prefixes can be borrowed as slice.
By inlining `wite_commit_internal()` into `write_commit()`, we can
avoid redoing some steps when we retry. This includes taking the mutex
lock, and reading the tree object and parent commits. It also means
that we avoid cloning the input commit object, which we otherwise
would even in the non-retrying case. I haven't measured if any of this
makes a significant difference, but I think it also slightly
simplifies the code, so it doesn't have to.
This is fast enough to be used on medium-sized repositories such as git/git.
It is a bit slow, but bearable, on huge repositories such as torvalds/linux.
There is 0 performance penalty if the display of unique prefixes is disabled
A trie-based implementation will be submitted for consideration in a
follow-up PR. It is faster, but more complicated.
**Update:** I also just discovered https://sapling-scm.com/docs/internals/indexedlog/
There are three important aspects of performance that seemed relevant:
1. Speed of computing the shortest unique prefix per id. It is worlds faster
than the naive implementation before this commit. It can be optimized
furher by using a trie or maybe the `fst` crate.
2. Speed of inital loading of the index that happens before the first commit is
shown. This is the part that's noticeable but bearable on torvalds/linux.
This could be optimized by storing a sorted list of commit and change ids on
disk. This would likely involve reworking the `Index`.
Failing that, the speed of inital loading doesn't change if a trie is used
and would likely be worse with the `fst` crate
3. Memory use is unremarkable here. I don't have good tools to measure it
precisely, but it does not balloon to gigabytes even on the linux repo.
This creates a templater function `short_underscore_prefix` for commit and
change ids. It is similar to `short` function, but shows one fewer hexadecimal
digit and inserts an underscore after the shortest unique prefix.
Highlighting with an underline and perhaps color/bold will be in a follow-up
PR.
The implementation is quadratic, a simple comparison of each id with every
other id. It is replaced in a subsequent commit. The problem with it is that,
while it works fine for a `jj`-sized repo, it becomes is painfully slow with a
repo the size of git/git.
Still, this naive implemenation is included here since it's simple, and could
be used as a reference implementation.
The `shortest_unique_prefix_length` function goes into `repo.rs` since that's
convenient for follow-up commits in this PR to have nicer diffs.
Fixes https://github.com/martinvonz/jj/issues/1050
Thanks to Martin for suggesting the exact fix.
The tests go into the new tests/test_duplicate_command.rs, which will be
expanded shortly with other tests depending on this bugfix.
You may use "abc\\" in .gitignore to ignore a file named "abc\". In this
case, removing training spaces on "abc\\ " must result in "abc\\" as the
trailing space is not escaped, the preceeding backslash being part of
the previous "\\" escaping sequence.
- branches has the signature branches([needle]), meaning the needle is optional (branches() is equivalent to branches("")) and it matches all branches whose name contains needle as a substring
- remote_branches has the signature remote_branches([branch_needle[, remote_needle]]), meaning it can be called with no arguments, or one argument (in which case, it's similar to branches), or two arguments where the first argument matches branch names and the second argument matches remote names (similar to branches, remote_branches(), remote_branches("") and remote_branches("", "") are all equivalent)
Dereferencing `self` as `*self` in order to perform patten-matching
using `ref` is unnecessary and will be done automatically by the
compiler (match ergonomics, introduced in Rust 1.26).
We don't care the ref content as long as it is unique, so using threaded
RNG should be fine.
This change means refs/jj/keep will now contain refs of the following
forms:
- new create_no_gc_ref(): 0f8d6cd9721823906cfb55dac99d7bf5
- old create_no_gc_ref(): 0f6d93fe-0507-4db8-ad0a-6317f02e27b9
- prevent_gc(commit_id): 0f9c15100b6f1373f38186357e274a829fb6c4e2
I don't think Workspace::load() should be permissive in that regard.
WorkspaceLoader could provide such function, but I feel it's more like
CLI business. CLI can also look for parent '.git' directory to suggest
'jj init --git-repo=..' if needed.
Since per-repo config may contain CLI settings, it must be visible to CLI.
Therefore, UserSettings::with_repo() -> RepoSettings isn't used, and its
implementation is nullified by this commit.
#616
It's unclear whether parse_args() or its caller should update LayeredConfigs.
--config-toml is processed by callee to apply --color early. -R/--repository
will be processed by caller since it will instantiate WorkspaceLoader.
Maybe --config-toml can be removed from EarlyArgs, and handle_early_args()
just updates ui state based on --color argument?
This will be needed to test functionality for showing shortest
unique prefix for commit and change ids. As a bonus, this also
allows us to test log output with change ids.
As another bonus, this will prevent occasional CI failures like
https://github.com/martinvonz/jj/actions/runs/3817554687/jobs/6493881468.
I needed this in the course of debugging an error. Before this commit, the error looked like this:
```
Error: Unexpected error from backend: Object not found
```
After this commit, it looks like this:
```
Error: Unexpected error from backend: Object with CommitId 8f59646bc9bb6bb44b5624f1248f4a708f37003c not found: object not found - no match for id (8f59646bc9bb6bb44b5624f1248f4a708f37003c); class=Odb (9); code=NotFound (-3)
```
Strictly speaking, we could rely on e.g. `git2::Oid::from_str` to produce an error, but I figure that having an explicit error for a mismatching hash length might demystify some error condition in the future, since commit IDs and change IDs and potentially other backends' IDs may have different lengths, so this could flag a mismatch earlier/more obviously.
We forgot to actually call `StoreFactories::load_op_heads_store()` to
load the right type of `OpHeadsStore` depending on the contents of
`.jj/repo/op_heads/type`. That shouldn't have any effect yet since we
only have one type so far, and there are no out-of-tree types yet
either (clearly, since they would not work).
A file entry is represented as a Dirs of is_file flag set. This might seem
odd at this point, but allows us to remove special case from PrefixMatcher.
PrefixMatcher::new(&[RepoPath::root()]) will set is_file to the root entry.
We already have `create_random_commit()`, which returns a
`CommitBuilder`. Most callers directly write that to a
`MutableRepo`. That currently returns a `Commit`, but I'm about to
make it propagate errors from the backend. That would add an
`unwrap()` to this sequence, making it longer. Let's create a simple
helper for these callers to simplify this common pattern.
When you're done with the `CommitBuilder`, you're going to have to
call `write_to_repo()`, passing it a mutable `MutableRepo`
reference. It's a bit simpler to pass that reference when we create
the `CommitBuilder` instead, so that's what this patch does.
A drawback of passing in the mutable reference when we create the
builder is that we can't have multiple unfinished `CommitBuilder`
instance live at the same time. We don't have any such use cases yet,
and it's not hard to work around them, so I think this change is worth
it.
When we fail to read the user's config, it seems obviously better to
use the default config than to not use it. It doesn't matter yet, but
it will matter when I've moved color configs out of `formatter.rs` and
into a `.toml` file. Without this change, we'd lose the default
coloring of the error message for config errors.
It's unlikely we'll need to customize these impls per type, so let's ensure
that these newtypes have identical implementations. This commit also adds
from_hex() to FileId, SymlinkId, and ConflictId.
Suggested by @arxanas.
Actually, it's easier to support these infix ops than erroring out, but I
don't want to make revset syntax more cryptic. "x- y" can't be handled by
this rule because "x-" is parsed as a parents expression.
The next commit will introduce a newtype for -m/--message argument which
can be converted Into<String>.
Since CommitBuilder is a thin wrapper, code bloat caused by generic parameters
wouldn't matter. I have another set of commits that makes all builder methods
accept Into/IntoIterator, which will remove some of .clone() calls from tests.
Performance on repositories with many commits is limited somewhat by repeatedly
stating the tablestore directory to work out what the head is. By caching the
table rather than looking it up from disk on every request, we can much more
rapidly satisfy requests.
This avoids the pathological case in #845 where jj operations take several
minutes to complete.
This patch doesn't change the normal flow of the write path: that will still
always call get_head() on the underlying TableStore, which will stat the
directory before writing out changes. It will however empty the cache when the
metadata has been written.
Fixes#845.
The implementation has some hoops to jump through because Rust does not allow
`self: &Arc<Self>` on trait methods, and two of the OpHeadsStore functions need
to return cloned selves. This is worked around by making the implementation type
itself a wrapper around Arc<>.
This is not particularly note worthy for the current implementation type where
the only data copied is a PathBuf, but for extensions it is likely to be more
critical that the lifetime management of the OpHeadsStore is properly
maintained.
Since we call `cargo_out_dir()` - which is the preferred way of using
`protobuf_codegen::Codegen` in `build.rs` - our call to `out_dir()`
has no effect.
I ran an upgraded Clippy on the codebase. All the changes seem to be
about using variables directly in format strings instead of passing
them as separate arguments.
