On Windows, spawning a child process finds the command relative to the
parent's working directory. If a command is specified as
`["path/inside/repo/tool.exe"]`, then tool won't be found if `jj fix` is
run from a subdirectory of the workspace.
There doesn't seem to be a good way to change this behavior, nor does it
seem easy to convert `program` into an absolute path because
`["tool.exe"]` could refer either to a file on the PATH or a file
committed at the root of the repository.
So, we add a new variable `$root` that can be used in this situation.
Workspace-relative tools on Windows should be defined using this
variable:
```toml
# Tools on the PATH
command = ["tool.exe"]
# Workspace-relative tools
command = ["$root/tool.exe"]
command = ["$root/nested/dir/tool.exe"]
```
On Unix, the command is found relative to the working directory of the
child process, so this isn't an issue.
Fixes#7144
We ran into a stack overflow in `merge_trees()` at Google due to
limited stack space and large stack frames caused by async code. This
patch fixes that by making `merge_trees()` non-recursive. Since I was
rewriting the algortithm anyway, I also made it concurrent, addressing
a TODO.
I'll make RevsetGraphWalk eliminate transitive edges from intermediate edges to
mitigate out-of-memory issue. This means that edges_from*() functions will call
remove_transitive_edges(). Maybe we could rewrite the whole process to not use
machine stack, but doing that would be complicated. The process wouldn't be
trivially-deterministic on where the look-ahead can terminate either.
The performance problem introduced by this patch will be fixed by later patches.
In small repo:
```
% hyperfine --sort command --warmup 3 --runs 10 -L bin jj-1,jj-2,jj-3,jj-4,jj-5 \
'target/release-with-debug/{bin} -R ~/mirrors/git --ignore-working-copy log -r "tags()"'
Benchmark 1: target/release-with-debug/jj-1 -R ~/mirrors/git --ignore-working-copy log -r "tags()"
Time (mean ± σ): 1.511 s ± 0.091 s [User: 1.296 s, System: 0.214 s]
Range (min … max): 1.432 s … 1.674 s 10 runs
Benchmark 2: target/release-with-debug/jj-2 -R ~/mirrors/git --ignore-working-copy log -r "tags()"
Time (mean ± σ): 2.467 s ± 0.142 s [User: 2.271 s, System: 0.196 s]
Range (min … max): 2.246 s … 2.588 s 10 runs
Relative speed comparison
1.32 ± 0.10 target/release-with-debug/jj-1 -R ~/mirrors/git --ignore-working-copy log -r "tags()"
2.16 ± 0.16 target/release-with-debug/jj-2 -R ~/mirrors/git --ignore-working-copy log -r "tags()"
```
In mid-size repo:
```
% hyperfine --sort command --warmup 3 --runs 10 -L bin jj-1,jj-2,jj-3,jj-4,jj-5 \
'target/release-with-debug/{bin} -R ~/mirrors/linux --ignore-working-copy log -r "tags(v5)"'
Benchmark 1: target/release-with-debug/jj-1 -R ~/mirrors/linux --ignore-working-copy log -r "tags(v5)"
Time (mean ± σ): 937.7 ms ± 68.8 ms [User: 672.1 ms, System: 265.4 ms]
Range (min … max): 868.3 ms … 1025.4 ms 10 runs
Benchmark 2: target/release-with-debug/jj-2 -R ~/mirrors/linux --ignore-working-copy log -r "tags(v5)"
Time (mean ± σ): 1.185 s ± 0.066 s [User: 0.920 s, System: 0.265 s]
Range (min … max): 1.132 s … 1.310 s 10 runs
Relative speed comparison
1.07 ± 0.11 target/release-with-debug/jj-1 -R ~/mirrors/linux --ignore-working-copy log -r "tags(v5)"
1.35 ± 0.12 target/release-with-debug/jj-2 -R ~/mirrors/linux --ignore-working-copy log -r "tags(v5)"
```
I was initially planning to try adding an `nth` keyword argument to
`parents()` instead of adding `first_parent()`, and I noticed that based
on the current style, it would have been shown as one of the following
in the revset documentation:
* `parents(x[, depth[, [nth=]index]])`
* `parents(x[, depth][, [nth=]index])`
These both seem difficult to read to me, so I'm proposing changing the
style to something more like this:
* `parents(x, [depth], [[nth=]index])`
I also added a brief section explaining the syntax in case it isn't
immediately obvious what the square brackets mean.
We've had a few occurrences of people asking for `jj restore -r`,
as an analogous to `jj squash -r`, when the error message should already
be telling them to try `--changes-in`. This suggests that the message
is not being read, perhaps because it's too long, and the suggestions
get lost in the noise. This is an attempt to improve its readability.
- Reduce the error part of the message to its essence
- Label the hints that are presented as such
- Remove mention of `--into` for brevity, as it doesn't seem to be a
common interpretation of `-r`
When no progress has been made, indicate that as 0.0 progress rather
than dividing by zero. Avoids in particular a display issue where the
progress bar indicates NaN% progress when fetching from slow remotes.
Fixes#7155
Change-Id: I6a6a6964d6572d1c98b5fa25285d26c07ee27a40
This allows evaluating ancestors/ranges involving filters significantly
faster. For instance, naively evaluating `::mine()` requires reading
every commit in the repo to check `mine()` on each commit, then finding
the ancestors of that set. This optimization rewrites `::mine()` to
`::heads(mine())`, since `heads(mine())` can be evaluated more
efficiently by only reading commits until the first successful match.
If someone is unaware of how revsets are implemented, this case can come
up pretty easily, such as by including `~mine()` in `immutable_heads()`
to make other people's commits immutable. In my local `jj` repo, this
optimization reduces the runtime of `jj log` with `~mine()` in
`immutable_heads()` from about 800ms down to about 50ms.
Benchmark results on Git repo:
```
::(v1.0.0..v2.40.0) 55.5% faster
author(peff).. 96.8% faster
```
I think this can be useful in CLI tests. I noticed there are lots of "jj file
list" calls in https://github.com/jj-vcs/jj/pull/6141 which could be combined
with "jj log" outputs.
ReadonlyIndexLoadError could be a wrapper of PathError instead, but that isn't
compatible with the abstraction of the current load functions. The inner load
functions may be called with an in-memory buffer.
I'll add another directory for changed-path index. We could put them into the
same (content-addressed) directory, but that would just make debugging harder.
This will help extend stats() for changed-paths index. Since there are no
non-test callers who need to calculate stats of mutable index, it should be good
to move the implementation to DefaultReadonlyIndex.
DefaultMutableIndex will calculate diffs to index changed files if enabled. The
body of DefaultMutableIndex::add_commit() is moved to non-trait method. It will
become an async function.
This is technically wrong because multiple predecessors should be considered
integrated into the rewritten commit, but I don't have a good idea to mitigate
the "squashed" diff problem. In "op diff", content diffs from the first
predecessor is practically useful.
Closes#7090
We've discussed this a few times among the maintainers. Let's write it
down before it happens. The Mercurial steering committee has a similar
rule, FWIW.
With the current test samples, it seems we need to run ~1000 cases to catch
`heads(first_ancestors())` bug that existed in the original PR. This parameter
can be set by default, but I don't have a good idea about reasonable "cases"
value, prop_recursive() parameters, etc.
Negated `first_ancestors()` can't be converted to a range currently, so
it doesn't make sense to move it to the start when optimizing the order
of an intersection.
This optimization is already done for `heads(ancestors(x) & ...)`, so I
think it makes sense to extend it to `first_ancestors()` as well. This
is especially important if the expression involves a filter that
requires reading commits. For instance, in the nixpkgs repo, evaluating
`heads(first_ancestors(@) & description(jujutsu))` takes 2.6 seconds on
my machine before this optimization, and only 0.2 seconds after.
