Having the checkout functionality in `LockedWorkingCopy` makes it a
little more flexible (one could imagine using it for udating working
copy files and then discarding the state changes, for example). It
also lets us reuse a few lines of code for locking. I left
`WorkingCopy::check_out()` for convenience because that's what all
current users want.
`WorkingCopy::check_out()` currently fails if the commit recorded on
disk has changed since it was last read. It fails with a "concurrent
checkout" error. That usually works well in practice, but one can
imagine cases where it's not correct. For an example where the current
behavior is wrong, consider this sequence of events:
1. Process A loads the repo and working copy.
2. Process B loads the repo at operation A. It has not loaded the
working copy yet.
3. Process A writes an operation and updates the working copy.
4. Process B loads the working copy and sees that it is checked out
to the commit process B set it to. We don't currently have any
checks that the working copy commit matches the view's checkout
(though I plan to add that).
5. Process B finishes its operation (which is now divergent with the
operation written by process A). It updates the working copy to
the checkout set in the repo view by process B. There's no data
loss here, but the behavior is surprising because we would usually
tell the user that we detected a concurrent update to the working
copy.
We should instead check that the working copy's commit on disk matches
what the previous repo view said, i.e. the view at the start of the
operation we just committed. This patch does that by having the caller
pass in the expected old commit ID.
We already have two usecases that can be modeled as updating the
`TreeState` without touching the working copy:
1. `jj untrack` can be implemented as removing paths from the tree
object and then doing a reset of the working copy state.
2. Importing Git HEAD when sharing the working copy with a Git repo.
This patch adds that functionality to `TreeState`.
I was surprised that we save the `TreeState` before
`LockedWorkingCopy::finish()`. That means that even if the caller
instead decides to discard the changes, some changes will already have
been written.
This patch changes the interface for making changes to the working
copy by replacing `write_tree()` and `untrack()` by a single
`start_mutation()` method. The two functions now live on the returned
`LockedWorkingCopy` object instead. That is more flexible because the
caller can make multiple changes while the working copy is locked. It
also helps us reduce the risk of buggy callers that read the commit ID
before taking the lock, because we can now make it accessible only on
`LockedWorkingCopy`.
The working copy object knows the currently checked out commit ID. It
is set to `None` when the object is initialized. It is also set to
`None` when an existing working copy is loaded. In that case, it's
used only to facilitate lazy loading. However, that means that
`WorkingCopy::current_commit_id()` fails if the working copy has been
initalized but no checkout has been specified. I've never run into
that case, but it's ugly that it can happen. This patch fixes it by
having `WorkingCopy::init()` take a `CommitId`.
`WorkingCopy::current_commit()` has been there from the beginning. It
has made less sense since we made the repo view keep track of the
current checkout. Let's remove it.
Before this patch, we got the old commit ID before we took the lock on
the working copy, which means we might unnecessarily create divergence
if another process just committed the working copy.
If you create a `dir/.gitignore` file with pattern "dir" in it, it'll
currently match the parent directory, making e.g. the `dir/.gitignore`
file itself ignored. That was quite confusing, and it doesn't match
how Git behaves. This patch fixes the bug.
I ran into a tool that produced a `.gitignore` file with CRLF line
endings. I had not considered that case when implementing support for
`.gitignore`, so we interpreted the CR as part of the string, which of
course made the files not match.
This patch fixes the bug by ignoring a single CR at EOL. That seems to
be what Git does (I didn't see any information about it in the
documentation).
It turns out that the `--help` option is "global", so the description
we set on the top-level command already applies to subcommands (and
subsubcommands, etc.).
A new Clippy version added a new warning when a function that returns
`Self` doesn't have `#[must_use]`. I feel like all the cases reported
by it were false positives. Most were functions on `CommitBuilder`,
where we take `mut self` and return `Self`. I don't think I've ever
forgotten to use the result of those.
I wanted to have all the documentation available on the command line,
but that makes it harder to maintain and link to. Let's move it to
markdown instead. We may later be able to add some way of presenting
the markdown in the terminal (or maybe by first converting it to
reStructuredText).
It makes sense to omit either of the arguments of the `..` operator,
even though `..x` is equivalent to `:x`. `x..`, with a implied right
argument of `heads()` is more useful.
