We currently say that `x..y` is "Ancestors of `y` that are not also
ancestors of `x`, both inclusive.". However, it's easy to think that
"both inclusive" means that both `x` and `y` are included in the set,
which is not the case. What we mean is more like "{Ancestors of `y`,
including `y` itself} that are not also {ancestors of `x`, including
`x` itself}.". Given that we already define ancestors and descendants
as being inclusive on the lines above, and we also give the equivalent
expressions using the `x:` and `:y` operators, it's probably best to
just skip the "both inclusive" parts.
There were two issues on my end:
1. `known_hosts` doesn't seem to be recognized
2. SSH Agent is ignored despite running
A workaround for 1. is to set the HOME environment variable on Windows, so I added a hint to suggest this. Ideally we would add a `certificate_check` callback to the remote callbacks, but the git2 crate doesn't expose whether the certificate check already succeeded, which makes it useless for this purpose (as we'd be prompting users to accept a certificate even though that certificate is already known to be valid).
As for 2., I changed the behavior from "check SSH Agent if some env variables exist" to "check SSH Agent and only fail if some env variables exist". On Windows SSH Agent doesn't use these env variables (but trying to communicate with it will still work), so now Windows properly works with SSH Agent.
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.
By setting a higher level by default, we open up to using the other
tracing levels without spamming the user.
I'm not sure if we should set it to ERROR or OFF by default, we let's
try ERROR for now. It shouldn't make any difference since we don't
have any ERROR-level events yet.
On "jj checkout", description of the working-copy commit is empty, and the
working-copy parent provides more information. It might be a bit verbose to
print parent summary on every history rewriting, but I think that's okay.
`workspace_root` is [canonicalized](41a2855b4f/lib/src/workspace.rs (L119)), but `cwd` [isn't](41a2855b4f/src/cli_util.rs (L581)). When [working with relative paths](41a2855b4f/src/cli_util.rs (L749-L760)), this leads to issues in Windows: canonicalized paths start with `\\?\`, whereas non-canonicalized paths do not. Therefore, paths were formatted canonicalized, and commands did not accept non-canonicalized paths:
```
$ jj st
Parent commit: bdb62c9 canonicalize cwd in cli_util.rs
Working copy : 059c104379cd (no description set)
Working copy changes:
A \\?\C:\github.com\71\jj\.vscode\launch.json
$ jj diff .vscode\launch.json
Error: Path ".vscode\launch.json" is not in the repo
$ jj diff \\?\C:\github.com\71\jj\.vscode\launch.json
Added regular file \\?\C:\github.com\71\jj\.vscode\launch.json:
...
```
With this change:
```
$ jj st
Parent commit: bdb62c9 canonicalize cwd in cli_util.rs
Working copy : 059c104379cd (no description set)
Working copy changes:
A .vscode\launch.json
$ jj diff .vscode\launch.json
Added regular file .vscode\launch.json:
...
```
This will make it easily available everywhere so we can get more
detailed information about arguments passed to commands. In
particular, I think it can be useful for displaying different hints
depending on whether the user passed `-r @` or if the `@` revision was
from the defaults.
We could add `walk.descendants(root_positions)` method, and apply
`.filter_by_generation(range)`, but queue-based `.descendants()` would be
slower than the one using reachable set. So I didn't add such method.
I also considered reimplementing non-lazy version of this function without
using the current RevWalkGenerationRange, but it appears the current iterator
version performs well even if we have to do .collect_vec() and .reverse().
This helps to extract a trait that abstracts CompositeIndex and descendants
map. Since the entry type E is a newtype wrapper, there wouldn't be runtime
cost.
I'm going to add a RevWalk method to walk descendants with generation filter,
which will use this helper method. RevWalk::take_until_roots() uses .min()
instead of .last() since RevWalk shouldn't know the order of the input set.
Before, the number of the generations to track would increase at each merge
point. This was really bad for queries like ':@--' in merge-heavy history,
but I didn't notice the problem because ancestors query is lazy and
the default log template is slow. Since I'm going to reuse RevWalk for
'roots++:' queries, which can't be lazy, I need to fix this problem first.
As we don't have a revset expression to specify exact generation range,
gen.end is initialized to either 1 or close to u32::MAX. So, this change
means long-lived generation ranges will eventually be merged into one.
If "jj op undo" doesn't roll back git refs (#1541), test_git_import_undo()
would get weird state. I think these tests are easier to follow than
test_git_fetch_undo() since no remote refs are involved.
This doc describes what we need to consider in a submodule storage
solution, some possible solutions and what criteria we should use to
decide on a future direction.
This is still a WIP:
- The solutions are still underdescribed
- The actual evaluation of solutions is missing
Suggestions for the above are welcome :)
