The tree-level conflicts have worked well in practice and we don't
want to allow users to use legacy trees for new commits. We don't
really support legacy trees very well since 0590f8beceb8 anyway.
This enables the creation of Repo objects in environments without standard filesystem support, by allowing the caller to load the store objects however they see fit. This confines interaction with the filesystem to the WorkingCopy abstractions.
If readonly_index() and index() returned Result, it would propagate to many
call sites. That seems bad for API ergonomics. Suppose most "repo" commands
depend on an index, I think it's okay to load index eagerly:
- "jj config" doesn't load repo (nor index)
- "jj workspace root" doesn't load repo (nor index)
- some other mutation commands load index when printing commit summary
- many other commands load index when resolving revset
As the doc comment says, it's called only from CommitBuilder. Let's clarify
that. I'm also planning to extract a builder that only writes to the store
(without mutably borrowing a mut_repo.) It will help implement description
template.
Forgetting a workspace removes its working-copy commit, so it makes
sense for it to be abandoned if it is discardable just like editing a
new commit will cause the old commit to be abandoned if it is
discardable.
Currently, if two workspaces are editing the same discardable commit and
one of them switches to editing a different commit, it is abandoned even
though the other workspace is still editing it. This commit treats
workspaces as referencing their working-copy commits so that they won't
be abandoned.
Since we've split (local, remotes) branches to (locals, remotes { branches }),
.has_branch() API no longer makes much sense. Callers often need to check if
a remote branch is tracked.
I've wanted to make the Git support optional for a long time. However,
since everyone uses the Git backend (and we want to support it even in
the custom binary at Google), there hasn't been much practical reason
to make Git support optional.
Since we now use jj-lib on the server at Google, it does make sense to
have the server not include Git support. In addition to making the
server binary smaller, it would make it easier for us (jj team at
Googlle) to prove that our server is not affected by some libgit2 or
Gitoxide vulnerability. But to be honest, neither of those problems
have come up, so it's more of an excuse to make the Git support
optional at this point.
It turned out to be much simpler than I expected to make Git support
in the lib crate optional. We have done a pretty good job of keeping
Git-related logic separated there.
If we make Git support optional in the lib crate, it's going to make
it a bit harder to move logic from the CLI crate into the lib crate
(as we have planned to do). Maybe that's good, though, since it helps
remind us to keep Git-related logic separated.
It's been more than 6 months since we added support for dynamically
selecting the working copy implementation. This patch drops support
for selecting the default implementation of that and other stores.
Some backends, like the one we have at Google, can restrict access to
certain files. For such files, if they return a regular
`BackendError::ReadObject`, then that will terminate iteration in many
cases (e.g. when diffing or listing files). This patch adds a new
error variant for them to return instead, plus handling of such errors
in diff output and in the working copy.
In order to test the feature, I added a new commit backend that
returns the new `ReadAccessDenied` error when the caller tries to read
certain objects.
I recently needed to test something on top of a two branches at the
same time, so I created a new commit on top of both of them (i.e. a
merge commit). I then ran tests and made some adjustments to the
code. These adjustments belonged in one of the parent branches, so I
used `jj squash --into` to squash it in there. Unfortunately, that
meant that my working copy became a single-parent commit based on one
of the branches only. We already had #2859 for tracking this issue.
This patch changes the behavior so we create a new working-copy commit
with all of the previous parents.
When I addded the workaround in 256988de65de, I missed the comment
just below explaining that heads removed by the other side were
already handled. Since that's not handled when using non-default
indexes now, we need to handle it in an `else` block.
The function now returns an iterator over `Result`s, matching
`Operation::parents()`.
I updated callers to also propagate the error where it was trivial.
We do a 3-way merge of repo views in a few different
scenarios. Perhaps the most obvious one is when merging concurrent
operations. Another case is when undoing an operation.
One step of the 3-way repo view merge is to find which added commits
are rewrites of which removed commits (by comparing change IDs). With
the high commit rate in the Google repo (combined with storing the
commits on a server), this step can get extremely slow.
This patch adds a hack to disable the slow step when using a
non-standard `Index` implementation. The Google index is the only
non-standard implementation I'm aware of, so I don't think this will
affect anyone else. The patch is also small enough that I don't think
it will cause much maintenance overhead for the project.
`RewriteType::Rewritten` must have exactly one replacement. I think
it's better to encode that in the type by attaching the value to the
enum variant. I also renamed the type to just `Rewrite` since it now
has attached data and `Type` sounds like a traditional data-free enum
to me.
Looks like I forgot this in some recent refactoring.
I don't really see any harm in making the type public later. I might
want to make `rebase_descendants()` not clear `parent_mapping` and
instead provide a way of accessing it afterwards (removing the need
for the `_return_map()` flavors). We'll see if that ends up
happening. For now it can be private anyway.
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.
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.
The functions now depend only on `MutableRepo`, so I think they belong
on that type. This gets us closer to being able to make
`parent_mapping` private again.
If we ever implement some sort of ABI for dynamic extension loading, we'll need these underlying APIs to support multiple extensions, so we might as well do that first.
I don't think we have any transactions that mark commit as abandoned
and then later mark it as rewritten or divergent. But if we ever do, I
think it should be considered just rewritten/divergent. So let's
enforce that invariant by removing the old value from the set of
abandoned commits.
When `rebase_commit_with_options()` decides to abandons a commit, it
records the new parents in the `MutableRepo`, but it's currently the
caller's responsibility to remember to mark it as abandoned. Let's
move that logic into the function to reduce the risk of future bugs.
By adding the abandoned commit's parents to `parent_mapping`, we can
remove a bit more of the special handling of abandoned commitsin
`DescendantRebaser`.
A subset of the state in `DescendantRebaser` now matches exactly what
`MutableRepo` already stores, so we can avoid copying that state and
have `DescendantRebaser` use it directly instead. Having a single
source of truth for the state will enable further simplifications and
improvements.
This is just to match `DescendantRebaser`, to make the next commit a
bit simpler. I think `MutableRepo` still has few enough fields that
just `abandoned` is clear enough. Maybe we'll move the three
rewrite-related fields into a new struct at some point.
With this patch, `MutableRepo` has the same tracking of rewritten
commits as `DescendantRebaser`, so we can simply pass that state into
`DescendantRebaser` when we create it. The next step is to remove the
state from `DescendantRebaser`.
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.
MutableRepo and CommitBuilder both define public (now crate-public) functions
which should only be called by each other. This commit adds documentation and
restricts visibility of these functions to the jj_lib crate. It might be even
better to move CommitBuilder to the same module as MutableRepo so that these
codependent functions can be private to the module to avoid misuse.
Since the operation log has a root operation, we don't need to create
the repo-initialization operation in order to create a valid
`ReadonlyRepo` instance. I think it's conceptually simpler to create
the instance at the root operation id and then add the initial
operation using the usual `Transaction` API. That's what this patch
does.
Doing that also brought two issues to light:
1. The empty view object doesn't have the root commit as head.
2. The initialized `OpHeadsStore` doesn't have the root operation as
head.
Both of those seem somewhat reasonable, but maybe we should change
them. For now, I just made the initial repo (before the initial
operation) have a single op head (to compensate for (2)). It might be
worth addressing both issues so the repo is in a better state before
we create the initial operation. Until we do, we probably shouldn't
drop the initial operation.
