Commit graph

860 commits

Author SHA1 Message Date
Yuya Nishihara
9f4a7318c7 tests: compare git refs loaded from disk, not in-memory cache values
This addresses the test instability. The underlying problem still exists, but
it's unlikely to trigger user-facing issues because of that. A repo instance
won't be reused after gc() call.

Fixes #3537
2024-04-22 18:46:28 +09:00
Yuya Nishihara
527713a851 tests: fix potential mtime flakiness in git gc tests
Apparently, these gc() invocations rely on that the previous "git gc" packed
all refs so there are no loose refs to compare mtimes. If there were new (or
remaining) loose refs, mtime comparison could fail. I also added +1sec to
effectively turn off the keep_newer option, which isn't important in these
tests.
2024-04-22 18:46:28 +09:00
Evan Mesterhazy
f9a3021a7a Simplify calls to CommitRewriter::replace_parents()
Now that it takes `IntoIterator` the caller doesn't need to clone
the input `CommitIds`.
2024-04-21 23:31:17 -04:00
Martin von Zweigbergk
87c65ee0f9 rewrite: make CommitRewriter::replace_parents() remove repeats 2024-04-18 21:06:52 -07:00
Martin von Zweigbergk
96f5ca47d4 repo: add method for tranforming descendants, use in rebase_descendants()
There are several existing commands that would benefit from an API
that makes it easier to rewrite a whole graph of commits while
transforming them in some way.

`jj squash` is one example. When squashing into an ancestor, that
command currently rewrites the ancestor, then rebases descendants, and
then rewrites the rewritten source commit. It would be better to
rewrite the source commit (and any descendants) only once.

Another example is the future `jj fix`. That command will want to
rewrite a graph while updating the trees. There's currently no good
API for that; you have to manually iterate over descendants and
rewrite them.

This patch adds a new `MutableRepo::transform_descendants()` method
that takes a callback which gets a `CommitRewriter` passed to it. The
callback can then decide to change the parents, the tree, etc. The
callback is also free to leave the commit in place or to abandon it.

I updated the regular `rebase_descendants()` to use the new function
in order to exercise it. I hope we can replace all of the
`rebase_descendant_*()` flavors later.

I added a `replace_parent()` method that was a bit useful for the test
case. It could easily be hard-coded in the test case instead, but I
think the method will be useful for `jj git sync` and similar in the
future.
2024-04-18 21:06:52 -07:00
Martin von Zweigbergk
2859277941 rewrite: pass CommitRewriter into rebase_commit_with_options()
`CommitRewriter` wraps 3 of the arguments, so I think it makes sense
to pass it instead. More importantly, I hope to continue refactoring
so many of the callers already have a `CommitRewriter`.
2024-04-18 08:08:51 -07:00
Martin von Zweigbergk
b13cb8db26 rewrite: make EmptyBehavior implement Copy 2024-04-18 08:08:51 -07:00
Martin von Zweigbergk
93baff0b8a rewrite: pass just IDs of new parents into rewrite::rebase*()
It's cheap to look up commits again from the cache in `Store` but it
can be expensive to look up commits we didn't end up needing. This
will make it easier to refactor further and be able to cheaply set
preliminary parents for a rewritten commits and then let the caller
update them.
2024-04-17 06:13:54 -07:00
Martin von Zweigbergk
057b7c8d0b rewrite: take commit and new parents by value in rebase_commit()
I'm going to add a helper struct to help with rewriting commits. I
want to make that struct own the old commit and the new parents to
simplify lifetimes. This patch prepares for that by passing the
commits by value to `rebase_commit()`.
2024-04-17 06:13:54 -07:00
Yuya Nishihara
aaa2025dfc git: on fetch, pin visible untracked remote refs
This implements the other workaround described in 57167cefda "git: on
import_refs(), don't abandon ancestors of newly fetched refs":

> I think there are two ways to fix the problem:
>  a. pin non-tracking remote branches just like local refs
>  b. pin newly fetched refs in addition to local refs
> This patch implements (b) because it's simpler and more obvious that the
> fetched commits would never be abandoned immediately.

The idea of (a) is that untracked remote branches are independent read-only
refs, and read-only branches shouldn't be rewritten implicitly. Once the
branch gets rewritten or abandoned by user, these remote refs will be hidden,
and won't be pinned anymore.

Since (a) effectively supersedes (b), this patch also removes the original
workaround.

Fixes #3495
2024-04-14 11:38:21 +09:00
Yuya Nishihara
47150d2bb4 revset: migrate file() predicate to be based on FilesetExpression 2024-04-06 23:59:54 +09:00
Christoph Koehler
7bde6ddc29 revset: add working_copies() function
It includes the working copy commit of every workspace of the repo.

Implements #3384
2024-04-01 19:36:53 -06:00
Martin von Zweigbergk
5e8d7f8c6f rewrite: update references after rewriting all commits 2024-03-25 23:00:44 -07:00
Martin von Zweigbergk
d2043f069e repo: delete record_rewritten_commit()
I don't think we have any callers left that call
`record_rewritten_commit()` multiple times within a transaction and
expect it to result in divergence. I think we should consider it a bug
to do that.
2024-03-25 06:53:14 -07:00
Ilya Grigoriev
02a04d0d37 test_conflicts and test_resolve_command: use indoc! to indent conflict markers in tests
Apart from (IMO) looking nicer, this will also sidestep the potential problem
that if the file contains actual jj conflict markers (`>>>>>>>` in the beginning
of a line, for example), jj would currently have trouble materializing and
subsequently parsing conflicts in the file if it actually became conflicted.

I'll demo this bug in either this or a subsequent PR. It's the kind of bug that
sounds serious in theory but might never cause a problem in practice.

After this PR, only `docs/tutorial.md` has a conflict marker that's not indented.
There's only one there, so hopefully it won't be too much of a pain to deal with.

I also indented other strings in `test_conflicts.rs`. IMO, this looks nice and
more consistent with the `insta::assert_snapshot` output. I didn't spend the
time to do the same for `test_resolve_command`.
2024-03-22 23:27:25 -07:00
Yuya Nishihara
9207314173 revset: add substitution rule for "::x & ~(::y-)"
Suppose the generation value is usually small, it should be faster to do
bounded range look up first 'y-', then walk ancestors with the unwanted set
'y-..x'.
2024-03-17 14:50:48 +09:00
Yuya Nishihara
699707905c index: reorganize revset_graph_iterator as private module of default_index
The RevsetGraphIterator type is hidden so that the Iterator trait can be
implemented differently.
2024-03-14 10:07:19 +09:00
Yuya Nishihara
243675b793 index: turn CompositeIndex into transparent reference type
This helps to eliminate higher-ranked trait bounds from RevWalkRevset and
RevWalk combinators to be added. Since &CompositeIndex is now a real reference,
it can be passed to functions as index: &T.
2024-03-11 17:24:10 +09:00
Aleksey Kuznetsov
cd3d75ebf6 revset: introduce more performant way to check if a commit is in a revset
Initially we were thinking to have `Revset` return something like
`CachedRevset`:

```
pub trait CachedRevset {
  fn iter(&self) -> Box<dyn Iterator<Item = Commit>>;
  fn contains(&self, &CommitId) -> bool;
}
```

But we weren't sure what use case for `iter` would be, so we dropped the `iter`
method. `CachedRevset` with single `contains` method needed a better name. We
weren't able to come up with one, so we decided instead to have a method on
`Revset` that returns a closure to check if a commit is in a revset.
2024-03-11 08:27:35 +05:00
Yuya Nishihara
f5eb172769 tests: remove last use of walk_revs() from integration tests 2024-03-08 10:07:40 +09:00
Thomas Castiglione
d661f59f9d working_copy: implement symlinks on windows with a helper function
enables symlink tests on windows, ignoring failures due to disabled developer mode,
and updates windows.md
2024-03-05 15:16:38 +08:00
Yuya Nishihara
24868e5192 gpg_signing: handle early termination of gpg command in verify path
Also fixes missing wait() on I/O error. We have the same problem in several
places. I'll fix them in another batch.
2024-03-03 18:35:10 +09:00
Yuya Nishihara
ef9d22887c tests: disable gpg unknown_key() test on Windows as well
Follows up 7552f939c6 "tests: disable most gpg integration tests on Windows."
I couldn't find this test failing in a few samples before, but it does now.
2024-02-27 00:55:06 +09:00
Martin von Zweigbergk
1cbf2b4acf rewrite: allow working-copy to be abandoned
This removes the special handling of the working-copy commit. By
recording when an empty/emptied commit was abanoned, we rebase
descendants correctly and create a new empty working-copy commit on
top.
2024-02-25 16:39:05 -08:00
Yuya Nishihara
7552f939c6 tests: disable most gpg integration tests on Windows
These tests often stuck on Windows CI for unknown reasons. Let's mark them
ignored for the moment. The unknown_key test is allowed because it somehow
appears to pass.

https://github.com/martinvonz/jj/actions/runs/8009950119/job/21879789008?pr=3123#step:7:1487

#3140
2024-02-25 17:07:05 +09:00
Yuya Nishihara
ebf90384f6 operation: add shorthand for .store_operation().metadata 2024-02-25 09:00:56 +09:00
Julien Vincent
f97e929cbf sign: Skip gpg tests if gpg is not installed
This adds a guard to the gpg signing tests which will skip the test if
`gpg` is not installed on the system.

This is done in order to avoid requiring all collaborators to have setup
all the tools on their local machines that are required to test commit
signing.
2024-02-21 13:22:53 +00:00
Yuya Nishihara
9f05aa8c46 tests: fix fun typo "singing" -> "signing" 2024-02-21 22:04:41 +09:00
Austin Seipp
6c31bab0d3 fsmonitor: allow core.fsmonitor = "none" to disable
When doing things like testing snapshot performance differences,
this allows you to turn off the monitor, no matter what the enabled
user or repository configuration has, e.g.

    jj st --config-toml='core.fsmonitor="none"'

Signed-off-by: Austin Seipp <aseipp@pobox.com>
2024-02-20 20:19:47 -06:00
Ilya Grigoriev
106483ad6a clippy: run nightly cargo clippy --fix 2024-02-19 23:38:33 -08:00
Martin von Zweigbergk
11c67cf979 op_store: add metadata flag for ops representing working-copy snapshot
It should be useful at least in the presentation layer to know which
operations correspond to working-copy snapshots. They might be
rendered differently in the graph, for example. Or maybe an undo
command wants to warn if you just undid a snapshot operation. This
patch just introduces a field in the metadata to store the
information.
2024-02-19 22:44:38 -08:00
Julien Vincent
23e5fba737 sign: Add SSH backend tests 2024-02-20 00:02:08 +00:00
Julien Vincent
7c11a61c23 sign: GPG backend tests 2024-02-20 00:02:08 +00:00
Martin von Zweigbergk
a9d0300b11 rewrite: make simplification of ancestor merges optional
I think the conclusion from #2600 is that at least auto-rebasing
should not simplify merge commits that merge a commit with its
ancestor. Let's start by adding an option for that in the library.
2024-02-19 14:20:18 -08:00
Yuya Nishihara
3c7aa75b9b index: switch to persistent change id index
The shortest change id prefix will become a few digits longer, but I think
that's acceptable. Entries included in the "revsets.short-prefixes" set are
unaffected.

The reachable set is calculated eagerly, but this is still faster as we no
longer need to sort the reachable entries by change id. The lazy version will
save another ~100ms in mid-size repos.

"jj log" without working copy snapshot:
```
% hyperfine --sort command --warmup 3 --runs 20 -L bin jj-0,jj-1,jj-2 \
  -s "target/release-with-debug/{bin} -R ~/mirrors/linux debug reindex" \
  "target/release-with-debug/{bin} -R ~/mirrors/linux \
   --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=\"\"'"
Benchmark 1: target/release-with-debug/jj-0 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
  Time (mean ± σ):     353.6 ms ±  11.9 ms    [User: 266.7 ms, System: 87.0 ms]
  Range (min … max):   329.0 ms … 365.6 ms    20 runs

Benchmark 2: target/release-with-debug/jj-1 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
  Time (mean ± σ):     271.3 ms ±   9.9 ms    [User: 183.8 ms, System: 87.7 ms]
  Range (min … max):   250.5 ms … 282.7 ms    20 runs

Relative speed comparison
        1.99 ±  0.16  target/release-with-debug/jj-0 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
        1.53 ±  0.12  target/release-with-debug/jj-1 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
```

"jj status" with working copy snapshot (watchman enabled):
```
% hyperfine --sort command --warmup 3 --runs 20 -L bin jj-0,jj-1,jj-2 \
  -s "target/release-with-debug/{bin} -R ~/mirrors/linux debug reindex" \
  "target/release-with-debug/{bin} -R ~/mirrors/linux \
   status --config-toml='revsets.short-prefixes=\"\"'"
Benchmark 1: target/release-with-debug/jj-0 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
  Time (mean ± σ):     396.6 ms ±  10.1 ms    [User: 300.7 ms, System: 94.0 ms]
  Range (min … max):   373.6 ms … 408.0 ms    20 runs

Benchmark 2: target/release-with-debug/jj-1 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
  Time (mean ± σ):     318.6 ms ±  12.6 ms    [User: 219.1 ms, System: 94.1 ms]
  Range (min … max):   294.2 ms … 333.0 ms    20 runs

Relative speed comparison
        1.85 ±  0.14  target/release-with-debug/jj-0 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
        1.48 ±  0.12  target/release-with-debug/jj-1 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
```
2024-02-18 09:44:57 +09:00
Yuya Nishihara
8cdf6d752c index: move change ids to sstable, build change-id-to-pos lookup table
This basically means that the change ids are interned. We'll implement binary
search over the sorted change ids table. The table could be sorted differently
for better cache locality, but it is in lexicographical order for simplicity.
With my testing, the cost of the id lookup isn't dominant.

Unlike the parent entries, the size of the per-id overflow items isn't saved.
That's s because the number of the same-change-id commits is either 1 or many.
It doesn't make sense to allocate 8 bytes for each change id. Instead, we'll
pay extra indirection cost to determine the size.
2024-02-18 09:44:57 +09:00
Thomas Castiglione
aaa5d6bc4f working_copy: add Send supertrait
If WorkingCopy: Send, then Workspace is Send, which is useful for long-running
servers. All existing impls are Send already, so this is just a marker.
2024-02-17 15:13:25 +08:00
Yuya Nishihara
5eea88d26a tests: fix concurrent git read/write test to retry on ref lock contention
Apparently, gix has 100ms timeout. Since this test tries to create contended
situation, it's possible that the ref lock can't be acquired. I've added
upper bound to the retry loop at b37293fa68 "tests: add upper bound to
test_concurrent_read_write_commit() loop", so ignoring arbitrary errors
should be okay.

The problem can be reproduced on my Linux machine by inserting 10ms sleep() to
gix and increasing the concurrency.

Fixes #3069
2024-02-17 15:09:27 +09:00
Evan Mesterhazy
e1fd402d39 Fix the ContentHash implementations for std::Option, MergedTreeId, and RemoteRefState
The `ContentHash` documentation specifies that implementations for enums should
hash the ordinal number of the variant contained in the enum as a 32-bit
little-endian number and then hash the contents of the variant, if any.

The current implementations for `std::Option`, `MergedTreeId`, and
`RemoteRefState` are non-conformant since they hash the ordinal number as a u8
with platform specific endianness.


Fixes #3051
2024-02-16 09:27:32 -05:00
Yuya Nishihara
e2c8a8fabd index: fix change id resolution test to not depend on deterministic order
Since IdIndex sorts the entries by using .sort_unstable_by_key(), the order of
the same-key elements is undefined. Perhaps, it's stable for short arrays, and
the test passes because of that.
2024-02-14 23:22:23 +09:00
Yuya Nishihara
2905a70b18 doc, tests: drop use of deprecated revset dag range operator 2024-02-14 10:04:56 +09:00
Yuya Nishihara
b0e8e2a1af index: move segment files to sub directory, add version number
I'm going to introduce breaking changes in index format. Some of them will
affect the file size, so version number or signature won't be needed. However,
I think it's safer to detect the format change as early as possible.

I have no idea if embedded version number is the best way. Because segment
files are looked up through the operation links, the version number could be
stored there and/or the "segments" directory could be versioned. If we want to
support multiple format versions and clients, it might be better to split the
tables into data chunks (e.g. graph entries, commit id table, change id table),
and add per-chunk version/type tag. I choose the per-file version just because
it's simple and would be non-controversial.

As I'm going to introduce format change pretty soon, this patch doesn't
implement data migration. The existing index files will be deleted and new
files will be created from scratch.

Planned index format changes include:
 1. remove unused "flags" field
 2. inline commit parents up to two
 3. add sorted change ids table
2024-02-12 19:38:36 +09:00
Ilya Grigoriev
a9c3af8153 test_local_working_copy: use std::fs:write instead of OpenOptions 2024-02-10 16:06:28 -08:00
Ilya Grigoriev
b2e37d448b clippy: add truncate option as suggested by clippy
In the next commit, I replace the whole thing with
std::fs::write, but I'll leave this here in case
the next commit is somhow incorrect
2024-02-10 16:06:28 -08:00
Ilya Grigoriev
a88c06068e clippy: new nightly fixes
For some reason, clippy also suggested surrounding
`self.value` with parentheses. Not sure whether
that's a clippy bug.

Cc: https://github.com/rust-lang/rust-clippy/issues/12268
2024-02-10 16:06:28 -08:00
dependabot[bot]
6d1faf9b03 Update strsim (changes tests), clap, clap_complete
This is #3002 with tests rerun to account for changes
to `strsim`, as @thoughtpolice noticed in
https://github.com/martinvonz/jj/pull/3002#issuecomment-1936763101

The string similarity changes include an example that
seems better and one that seems worse. Decreasing
the threshold definitely makes things worse.
2024-02-10 00:01:47 -08:00
Austin Seipp
5b517b542e rust: bump MSRV to 1.76.0
Signed-off-by: Austin Seipp <aseipp@pobox.com>
2024-02-09 15:48:01 -06:00
Martin von Zweigbergk
6c1aeff7a9 working copy: materialize symlinks on Windows as regular files
I was a bit surprised to learn (or be reminded?) that checking out
symlinks on Windows leads to a panic. This patch fixes the crash by
materializing symlinks from the repo as regular files. It also updates
the snapshotting code so we preserve the symlink-ness of a path. The
user can update the symlink in the repo by updating the regular file
in the working copy. This seems to match Git's behavior on Windows
when symlinks are disabled.
2024-02-09 09:20:24 -08:00
jyn
d66fcf2ca0 compile integration tests as a single binary
this greatly speeds up the time to run all tests, at the cost of slightly larger recompile times for individual tests.

this unfortunately adds the requirement that all tests are listed in `runner.rs` for the crate.
to avoid forgetting, i've added a new test that ensures the directory is in sync with the file.

 ## benchmarks

before this change, recompiling all tests took 32-50 seconds and running a single test took 3.5 seconds:

```
; hyperfine 'touch lib/src/lib.rs && cargo t --test test_working_copy'
  Time (mean ± σ):      3.543 s ±  0.168 s    [User: 2.597 s, System: 1.262 s]
  Range (min … max):    3.400 s …  3.847 s    10 runs
```

after this change, recompiling all tests take 4 seconds:
```
;  hyperfine 'touch lib/src/lib.rs ; cargo t --test runner --no-run'
  Time (mean ± σ):      4.055 s ±  0.123 s    [User: 3.591 s, System: 1.593 s]
  Range (min … max):    3.804 s …  4.159 s    10 runs
```
and running a single test takes about the same:
```
; hyperfine 'touch lib/src/lib.rs && cargo t --test runner -- test_working_copy'
  Time (mean ± σ):      4.129 s ±  0.120 s    [User: 3.636 s, System: 1.593 s]
  Range (min … max):    3.933 s …  4.346 s    10 runs
```

about 1.4 seconds of that is the time for the runner, of which .4 is the time for the linker. so
there may be room for further improving the times.
2024-02-06 18:19:41 -08:00
Martin von Zweigbergk
b343289238 working_copy: make reset() take a commit instead of a tree
Our virtual file system at Google (CitC) would like to know the commit
so it can scan backwards and find the closest mainline tree based on
it. Since we always record an operation id (which resolves to a
working-copy commit) when we write the working-copy state, it doesn't
seem like a restriction to require a commit.
2024-02-06 12:41:09 -08:00