In "jj absorb", we'll need to calculate annotation from the parent tree. It's
usually identical to the tree of the parent commit, but this is not true for a
merge commit. Since I'm not sure how we'll process conflict trees in general,
this patch adds a minimal API to specify a single file content, not a
MergedTree.
The primary use case is to exclude immutable commits when calculating line
ranges to absorb. For example, "jj absorb" will build annotation of @ revision
with domain = mutable().
Due to the gradual rewrite to use the
`MutableRepo::transform_descendants` API, `DescendantRebaser` is no
longer used in `MutableRepo::rebase_descendants`, and is only used
(indirectly through `MutableRepo::rebase_descendants_return_map`) in
`rewrite::squash_commits`.
`DescendantRebaser` is removed since it contains a lot of logic
similar to `MutableRepo::transform_descendants`. Instead,
`MutableRepo::rebase_descendants_with_options_return_map` is rewritten
to use `MutableRepo::transform_descendants` directly, and the other
`MutableRepo::rebase_descendants_{return_map,with_options}` functions
call `MutableRepo::rebase_descendants_with_options_return_map` directly.
`MutableRepo::rebase_descendants_return_rebaser` is also removed.
https://rustsec.org/advisories/RUSTSEC-2024-0384 says to migrate off
of the `instant` crate because it's unmaintained. We depend on it only
via the `backoff` crate. That crate also seems unmaintained. So this
patch replaces our use of `backoff` by a custom implementation.
I initially intended to migrate to the `backon` crate, but that made
`lock::tests::lock_concurrent` tests fail. The test case spawns 72
threads (on my machine) and lets them all lock a file, and then it
waits 1 millisecond before releasing the file lock. I think the
problem is that their version of jitter is implemented as a random
addition of up to the initial backoff value. In our case, that means
we would add at most a millisecond. The `backoff` crate, on the other
hand does it by adding -50% to +50% of the current backoff value. I
think that leads to a much better distribution of backoffs, while
`backon`'s implementation means that only a few threads can lock the
file for each backoff exponent.
Both user and programmatic expressions use the same .evaluate() function now.
optimize() is applied globally after symbol resolution. The order shouldn't
matter, but it might be nicer because union of commit refs could be rewritten
to a single Commits(Vec<CommitId>) node.
This helps add library API that takes resolved revset expressions. For example,
"jj absorb" will first compute annotation within a user-specified ancestor range
such as "mutable()". Because the range expression may contain symbols, it should
be resolved by caller.
There are two ideas to check resolution state at compile time:
<https://github.com/martinvonz/jj/pull/4374>
a. add RevsetExpressionWrapper<PhantomState> and guarantee inner tree
consistency at public API boundary
b. parameterize RevsetExpression variant types in a way that invalid variants
can never be constructed
(a) is nice if we want to combine "resolved" and "unresolved" expressions. The
inner expression types are the same, so we can just calculate new state as
Resolved & Unresolved = Unresolved. (b) is stricter as the compiler can
guarantee invariants. This patch implements (b) because there are no existing
callers who need to construct "resolved" expression and convert it to "user"
expression.
.evaluate_programmatic() now requires that the expression is resolved.
This will become safe as I'm going to add static check that the expression does
never contain CommitRef(_)s. We could make Present(_) unconstructible, but the
existence of Present node is harmless.
I'm going to add RevsetExpression<State> type parameter, but the existing tree
transformer can't rewrite nodes to different state because the input and the
output must be of the same type. (If they were of different types, we couldn't
reuse the input subtree by Rc::clone().) The added visitor API will handle
state transitions by mapping RevsetExpression::<St1>::<Kind> to
RevsetExpression::<St2>::<Kind>.
CommitRef and AtOperation nodes are processed by specialized methods because
these nodes will depend on the State type. OTOH, Present node won't be
State-dependent, so it's inspected by the common fold_expression() method.
An input expression is not taken as an &Rc<RevsetExpression> but a &_ because
we can't reuse the allocation behind the Rc.
I originally considered adding deny-list-based implementation, but the Windows
compatibility rules are super confusing and I don't have a machine to find out
possible aliases. This patch instead adds directory equivalence tests.
In order to test file entity equivalence, we first need to create a file or
directory of the requested name. It's harmless to create an empty .jj or .git
directory, but materializing .git file or symlink can temporarily set up RCE
situation. That's why new empty file is created to test the path validity. We
might want to add some optimization for safe names (e.g. ASCII, not contain
"git" or "jj", not contain "~", etc.)
That being said, I'm not pretty sure if .git/.jj in sub directory must be
checked. It's not safe to cd into the directory and run "jj", but the same
thing can be said to other tools such as "cargo". Perhaps, our minimum
requirement is to protect our metadata (= the root .jj and .git) directories.
Despite the crate name (and internal use of std::fs::File),
same_file::is_same_file() can test equivalence of directories. This is
documented and tested, so I've removed my custom implementation, which was
slightly simpler but lacks Windows support.
If new file would overwrite an existing regular file, the file path is skipped.
It makes sense to apply the same rule to existing symlinks. Without this patch,
check out would fail if an existing path was a dead symlink or a symlink to
a directory.
I'm not sure if this was attackable before, but it should be better to not
try to remove file across symlinks.
The disk_path is now returned from create_parent_dirs() to clarify that the
path is identical.
This should be safer than relying on file open error. It's scary to continue
processing if the file was a symlink.
I'll add a few more sanity checks to remove_old_file(), so it's extracted as a
function.
I'm going to add "checked" version of to_fs_path(), but all callers can't be
migrated to it. For example, an error message should be produced even if the
path is malformed.
This patch also adds error variants to propagate InvalidRepoPathError. They
don't use ::Other { .. } so the errors can be distinguished in tests.
I'm going to replace the current .evaluate_programmatic() which does minimal
commit-ref resolution. The new .evaluate_programmatic() will be implemented on
a "resolved" expression.
For the same reason as the previous patch. It's nice if root() is considered
a "resolved" expression. With this change, most of the evaluate_programmatic()
callers won't have to do symbol resolution at all.
I'm going to add RevsetExpression<State = Resolved|User> type parameter to
detect API misuse at compile time. VisibleHeads is similar to All, and appears
in generic expression substitution function where a concrete State type
shouldn't be known.
This ensures that a symbol-resolved at_operation() expression won't be resolved
again when it's intersected with another expression, for example.
# in CLI
let expr1 = parse("at_operation(..)").resolve_user_symbol();
# in library
let expr2 = RevsetExpression::ancestors().intersection(&expr1);
expr2.evaluate_programmatic()
This will help construct file content based on diff hunks. For example, "jj
absorb" will first calculate annotation of the source parent (within mutable
ancestors), calculate diff, then "squash" hunks into ancestor commits of the
surrounding ranges.
This unblocks the use of TestBackend in long-running processes such as fuzzer.
It should also be safer because TempDir doesn't guarantee that the path is never
reused.
TestBackendData instances persist in memory right now, but they should be
discarded when the corresponding temp_dir gets dropped. The added struct will
manage the TestBackendData mapping.
We had documented that we support `git.auto-local-bookmark` but we
don't. The documentation has been incorrect since d9c68e08b1. This
patch fixes it by adding support for `git.auto-local-bookmark` with
fallback to the old/current `git.auto-local-branch`.
.
We might want to calculate (commit_id, range) pairs of consecutive lines in
order to "absorb" changes, for example.
This should also be cheaper since Vec<u8> doesn't have to be allocated per line.
Perhaps, get_same_line_map() could return an iterator, but implementing an
iterator to be "pull"-ed is much harder than writing a function to "push",
especially when lifetime is involved.
This function was short, and this change makes it clear that !.is_empty() was
redundant. Duplicated doc comment is also removed. I feel the inline comment is
easier to follow here.
