Since Readonly/MutableIndexSegment no longer implement Index trait, there's
no ambiguity between segment-local and index-global operations. Let's shorten
the method names.
I'm thinking of changing some IndexSegment methods to return LocalPosition
instead of global IndexPosition, and using u32 there would be a source of bugs.
Both readonly and mutable segments know the commit ids to return, and the
caller only needs the ids. Since segment_commit_id(local_pos) scans the graph
entries, doing that would increase the chance of cache miss.
This requires creating a new public API as a substitute. I took the opportunity
to also add some comments to the
`MutRepo::record_rewritten_commit`/`record_abandoned_commit` functions.
I imade the simplest possible addition to the API; it is not a very elegant
one. Eventually, the entire `record_rewritten_commit` API should probably be
refactored again.
I also added some comments explaining what these functions do.
At some point, I tried `new_commit` instead of `rewrite_commit` in the split
command. That seemed to work, but messed up the dates in a subtle way.
This commit should prevents repeats of this mistake and emphasize the
importance of the author dates being preserved.
This partially reverts 6c627fb30d "cli: default to log when no subcommand is
provided." We could reject an empty alias at all, but we would still need to
ensure that the expanded alias contained a subcommand name.
The help output is a bit odd as the <COMMAND> can be omitted, but I think
that's acceptable. If we do care about that, maybe we can override_usage().
Spotted while adding error propagation there. This wouldn't likely be a real
problem because "jj debug reindex" removes all of the operation links.
The "} else {" condition is removed because it doesn't make sense to exclude
only the exact parent_op_id operation. This can be optimized to not walk
ancestors of the parent_op_id operation, but I don't see a motivation to add
tests covering such scenarios. It's pretty rare that an intermediate operation
link is missing.
There are many unit tests that call mutable_segment.save_in(), but I don't
think these callers expect that the segment file could be squashed depending
on the size. Let's make it caller's responsibility.
maybe_squash_with_ancestors() should be cheap if segment_num_commits() == 0,
so it's okay to call it before checking the emptiness.
I think "for" loop is easier to follow. Maybe it could be rewritten further to
.find_map() loop, but that would be too clever.
I also made ancestor_index_segments() pub(super) since it doesn't make sense
to only provide ancestor_files_without_local().
If the error is permanent (because the repo predates the no-gc-ref fix for
example), there's no easy way to recover. Still, panicking in this function
seems wrong.
This is basically the same as Mercurial's workaround. I don't know about Git,
but arguments order is very restricted in git, so -C path can be parsed prior
to alias expansion. In hg and jj, doing that would be messy and unreliable.
Closes#2414
It should be better to handle invalid -R path globally. The error message is
a bit worse, but I think it's still okay.
This helps to load temporary config from the cwd-relative path. If the command
processing continued with an invalid -R path, the temporary config would have
to be explicitly discarded.
This is the last use of Read/WriteBytesExt. The byteorder crate is great, but
we don't need an abstraction of endianness. Let's simply use the std functions.
If read_exact() or read_u32() reached to EOF, the index file should be
considered corrupted. File not found error is also treated as data corruption
because an invalid file name could be read from the child segment file. It
can't handle special file names like "..", though.
Index file name also applies to io::Error. New error type reuses io::Error to
represent data corruption. We could add an inner Corrupt|Io enum instead, but
we'll need to remap some io::Error variants (e.g. UnexpectedEof) to Corrupt
anyway.
The idea is that we don't have to reload parent files as we already have the
chain of the parent segments. The resulting readonly index will be constructed
from the loaded parent segments + local entries blob.
I thought IndexLoadError and DefaultIndexStoreError would represent "load" and
"store" failures respectively, but they aren't. Actually, DefaultIndexStoreError
is the store-level error, and IndexLoadError should be wrapped in it.
Make it clearer what the command does, make the error message when the file is
not ignored less of a surprise.
Also, I think it's nice to mention `.git/info/exclude`, since the path is
not trivial to remember.
Since OpStoreError can also include io::Error, it doesn't make much sense to
have Io variant at this level. Let's split it to context-specific errors, and
extract helper method that maps io::Error.
As far as I can see in the chat, there's no objection to changing the default,
and git.auto-local-branch = false is generally preferred.
docs/branches.md isn't updated as it would otherwise conflict with #2625. I
think the "Remotes" section will need a non-trivial rewrite.
#1136, #1862
