Both local and remote refs are backed by the same value type since we'll need
some kind of runtime abstraction to represent "branches" keyword (which is a
list of local + remote branches.) It's tedious to implement separate
local/remote/both ref types.
The "unsynced" flag is inverted just because the positive term is slightly
easier to document.
I'm going to change "branches" to return a list instead of formatted string,
and I don't think "if(branches, ..)" should be invalidated by that. Perhaps,
a container type like String, Vec<T>, Option<T> can implement the cast.
Makes sure that, at the point the commit summary for the new commit is written,
the original commit that is being rewritten is already abandoned. Otherwise,
once we show divergent change ids (in a subsequent commit) in the short commit
template, the commits would be shown as divergent.
This also has an effect on whether branches are displayed next to the commit;
the changes in test_resotre_command happen because, now, the branch is properly
propagated to the restored commit before its summary is displayed.
test_templater_alias(), test_templater_alias_override(), and
test_templater_bad_alias_decl() aren't moved since they also test config loading
and error formatting. The first test in test_templater_parse_error() is left for
the same reason. test_templater_upper_lower() depends on the commit templater.
I don't think many of the tests in test_templater.rs should use "jj log" command
as they check very specific template syntax and function behaviors. Let's move
them to in-module tests. We could add a separate test file, but we would have
to export a couple of templater macros.
test_templater_timestamp_method() is migrated as example.
I want to fix error propagation before I start using async in this
code. This makes the diff iterator propagate errors from reading tree
objects.
Errors include the path and don't stop the iteration. The idea is that
we should be able to show the user an error inline in diff output if
we failed to read a tree. That's going to be especially useful for
backends that can return `BackendError::AccessDenied`. That error
variant doesn't yet exist, but I plan to add it, and use it in
Google's internal backend.
Reasons to introduce this alias:
* Reduces complexity of a type, to silence Clippy warnings in the
future if we use this type as a type parameter
* The type is used quite frequently, so it makes sense to have a name
for it
* It's easier to visually scan for the end of the type when you don't
have to match opening and closing angle brackets
Since "jj git fetch --branch" supports glob patterns, users would expect that
"jj git push --branch glob:.." also works.
The error handling bits are copied from "branch" sub commands. We might want to
extract it to a common helper function, but I haven't figured out a reasonable
boundary point yet.
Thanks to @glencbz for noticing that VS Code works fine now as a
merge tool, and thanks to @solson for suggesting
`merge-tool-edits-conflict-markers = true`.
I'm about to make conflicts also get materialized in executable
files. We'll lose some of the test coverage in `test_chmod_command.rs`
then, because the those tests rely on the materialized content to
describe the executable bits. So this commit adds a debug command for
printing tree values and uses that in the tests.
If we add glob support, users will probably want to do something like
'jj branch untrack glob:"*@origin"'. It would be annoying if the command
failed just because one of the remote branches has already been untracked.
Since branch tracking/untracking is idempotent, it's safe to continue in
those cases.
The parse rule is lax compared to revset. We could require the pattern to be
quoted, but that would mean glob patterns have to be quoted like 'glob:"foo*"'.
find_forgettable_branches() is unchanged for now. I might want to rewrite it
to not remove untracked remote branches (because untracked branches aren't
associated with the local counterparts.)
We need to let async-ness propagate up from the backend because
`block_on()` doesn't like to be called recursively. The conflict
materialization code is a good place to make async because it doesn't
depends on anything that isn't already async-ready.
I personally don't mind if "jj branch list" showed all non-tracking branches,
but I agree it would be a mess if ~500 remote branches were listed. So let's
hide them by default as non-tracking branches aren't so interesting.
Closes#1136
This will be the option to include non-tracking remote branches. We could add
more fine-grained filtering flags, but I think --all is good enough and easier
to remember.
This patch also updates many of the test outputs to include synchronized remote
branches. I think verbose outputs will help catch future bugs.
This replaces our existing mechanism of adding `/.jj/` to
`.git/info/exclude` by adding `*` to `.jj/.gitignore`, as suggested by
@ppwwyyxx. That simplifies the code quite a bit, and it avoids the
problem with `.git/info/exclude` not existing (it apparently doesn't
exist when the user uses
https://git-scm.com/docs/git-init#_template_directory).
Closes#2385.
We can provide more actionable error message than "not fast-forwardable". If
the push was fast-forwardable, "jj branch track" should be able to merge the
remote branch without conflicts, so the added step would be minimal.
Although this is logically correct, the error message is a bit cryptic. It's
probably better to reject push if non-tracking remote branches exist.
#1136
We'll use remote_ref.tracking_target() to classify push action, but not all
callers of local_remote_branches() need tracking_target() instead of target.
This means that the commits previously pinned by remote branches are no longer
abandoned. I think that's more correct since "push" is the operation to
propagate local view to remote, and uninteresting commits should have been
locally abandoned.
Since pushed remote branches will share the common base targets with locals,
these branches should be marked as tracking. git::push_branches() will handle
that. It looks ugly that the public GitBranchPushTargets type keeps "force"-d
branches as a separate set, but we'll need to rework that anyway when we
implement --force-with-lease behavior. So let's leave it for now.
Some of the git::push_updates() tests have been migrated to the new function.
I left a couple of basic tests for git::push_updates() because push_updates()
will be used to implement a low-level "jj git push-refs" command.
This add support for custom `jj` binaries to use custom working-copy
backends. It works in the same way as with the other backends, i.e. we
write a `.jj/working_copy/type` file when the working copy is
initialized, and then we let that file control which implementation to
use (see previous commit).
I included an example of a (useless) working-copy implementation. I
hope we can figure out a way to test the examples some day.
This makes `Workspace::load()` look a new `.jj/working_copy/type` file
in order to load the right working copy implementation, just like
`Repo::load()` picks the right backends based on `.jj/store/type`,
`.jj/op_store/type`, etc. We don't write the file yet, and we don't
have a way of adding alternative working copy implementations, so it
will always be `LocalWorkingCopy` for now.
Our internal working copy implementations at Google will need the
commit so they can walk history backwards until they get to a "public"
commit. They'll then use that to tell build tools and virtual file
systems to present that as a base.
I'm not sure if we'll need to update `reset()` too. It's currently
only used by `jj untrack`, which doesn't change the commit's parent,
so it wouldn't affect any history walks.
This patch adds MutableRepo::track_remote_branch() as we'll probably need to
track the default branch on "jj git clone". untrack_remote_branch() is also
added for consistency.
I'm not sure if this is the best way to render non-tracking branches, but
it helps to write CLI tests. Maybe we can add some hint or decoration to
non-tracking branches, but I'd like to avoid bikeshedding at this point.
Since we haven't migrated the push function yet, a deleted branch can be
pushed to non-tracking remotes. This will be addressed later.
#1136
I'm about to make `LockedLocalWorkingCopy` not borrow from
`LocalWorkingCopy`. That will make it easier to forget to update any
`LocalWorkingCopy` variables when the modifications have been
committed. This patch introduces a wrapper around
`LockedLocalWorkingCopy` to help prevent that.
Thanks to Yuya for the suggestion.
`LocalWorkingCopy::check_out()` can be expressed using the planned
`WorkingCopy` trait, so it doesn't need to be in the trait itself
`WorkingCopy`. I wasn't sure if I should make it a free function in
`working_copy`, but I ended up moving it onto `Workspace`.
This isn't important, but I'm going to change remote_targets to store RemoteRef
instead of RefTarget, so I went ahead and change the other field types as well.
We could fix do_git_clone() instead, but it seemed a bit weird that the
git_repo_path is relative to the store path which is unknown to callers.
Fixes#2374
There's a subtle behavior change. Unlike the original remove_remote_branch(),
remote_views entry is not discarded when the branches map becomes empty. The
reasoning here is that the remote view can be added/removed when the remote
is added/removed respectively, though that's not implemented yet. Since the
serialized data cannot represent an empty remote, such view may generate
non-unique content hash.
Summary: This allows `workspace forget` to forget multiple workspaces in a
single action; it now behaves more consistently with other verbs like `abandon`
which can take multiple revisions at one time.
There's some hoop-jumping involved to ensure the oplog transaction description
looks nice, but as they say: small conveniences cost a lot.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Id91da269f87b145010c870b7dc043748
The `TreeStateError` type is specific to the current local-disk
working-copy backend, so it should not be part of the generic
working-copy interface I'm trying to create.
Summary: Workspaces are most useful to test different versions (commits) of
the tree within the same repository, but in many cases you want to check out a
specific commit within a workspace.
Make that trivial with a `--revision` option which will be used as the basis
for the new workspace. If no `-r` option is given, then the previous behavior
applies: the workspace is created with a working copy commit created on top of
the current working copy commit's parent.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I23549efe29bc23fb9f75437b6023c237
Before this patch, it was an error to run `jj config set --user foo
'[1]'` twice. But it's only been broken since the previous commit
because '[1]' was interpreted as a string before then.
Now we have a separate map for "git" tracking remote, we can always preserve
the last imported/exported git_refs. The option to restore git-tracking refs
has been removed. Perhaps, --what can be reorganized as --local and --remote
<NAME>.
Before this patch, when updating to a commit that has a file that's
currently an ignored file on disk, jj would crash. After this patch,
we instead leave the conflicting files or directories on disk. We
print a helpful message about how to inspect the differences between
the intended working copy and the actual working copy, and how to
discard the unintended changes.
Closes#976.
It's about time we make the working copy a pluggable backend like we
have for the other storage. We will use it at Google for at least two
reasons:
* To support our virtual file system. That will be a completely
separate working copy backend, which will interact with the virtual
file system to update and snapshot the working copy.
* On local disk, we need to tell our build system where to find the
paths that are not in the sparse patterns. We plan to do that by
wrapping the standard local working copy backend (the one moved in
this commit), writing a symlink that points to the mainline commit
where the "background" files can be read from.
Let's start by renaming the exising implementation to
`local_working_copy`.
I have used the tree-level conflict format for several weeks without
problem (after the fix in 51b5d168ae). Now - right after the 0.10.0
release - seems like a good time to enable the config by default.
I enabled the config in our default configs in the CLI crate to reduce
impact on tests (compared to changing the default in `settings.rs`).
I'll add a workaround for the root parent issue #1495 there. We can pass in
the wc parent id instead of the wc_commit object, but we might want to use
wc_commit.id() to generate a unique placeholder ref name.
While debugging git issues, I often ended up creating a deadlock by adding
debug prints. It's also not obvious that git::export_refs() works even if the
git_repo() has already been locked, whereas git::import_refs() wouldn't. Let's
consolidate lock handling to the backend implementation.
Apparently, it gets too verbose if the remote history is actively rewritten.
Let's summarize the output for now. The plan is to show the list of moved refs
instead of the full list of abandoned commits.
The codespell GitHub action fails because of the typo. I don't know
why it started failing now. The comment is 8 months old and the
codespell action hasn't been updated in 5 months.
The problem is that the first non-working-copy commit moves the unborn current
branch to that commit, but jj doesn't "export" the moved branch. Therefore,
the next jj invocation notices the "external" ref change, which was actually
made by jj.
I'm not sure why we play nice by setting the "current" HEAD, but I *think* it's
okay to set the "new" HEAD and reset to the same commit to clear Git index.
This will probably help to understand why you've got conflicts after fetching.
Maybe we can also report changed local refs.
I think the stats should be redirected to stderr, but we have many other similar
messages printed to stdout. I'll probably fix them all at once later.
I think most users who change the set of immutable heads away from
`trunk() | tags()` are going to also want to change the default log
revset to include the newly mutable commit and to exclude the newly
immutable commits. So let's update the default log revset to use
`immutable_heads()` instead.
`test_templater` changed because we have overridden the set of
immutable commits there so `jj log` now includes the remote branch.
`jj split` with no arguments operates interactively, but I am nonetheless constantly running `jj split -i` because I expect an `--interactive` flag to exist for consistency.
However, `jj split <paths>` before this commit always operates non-interactively, so this commit has the nice practical effect that you can restrict your interactive splitting to a certain set of paths.
All non-test callers already have a `Merge` object, so let's pass that
instead. We thereby simplify the callers a little, and we enforce the
"adds.len() == removes.len() + 1" constraint in the type.
When there's a single parent, we can determine if a commit is empty by
just comparing the tree ids. Also, when using tree-level conflicts, we
don't need to read the trees to determine if there's a conflict. This
patch adds both of those fast paths, speeding up `jj log -r ::main`
from 317 ms to 227 ms (-28.4%). It has much larger impact with our
cloud-based backend at Google (~5x faster).
I made the same fix in the revset engine and the Git push code (thanks
to Yuya for the suggestion).
This adds a new `revset-aliases.immutable_heads()s` config for
defining the set of immutable commits. The set is defined as the
configured revset, as well as its ancestors, and the root commit
commit (even if the configured set is empty).
This patch also adds enforcement of the config where we already had
checks preventing rewrite of the root commit. The working-copy commit
is implicitly assumed to be writable in most cases. Specifically, we
won't prevent amending the working copy even if the user includes it
in the config but we do prevent `jj edit @` in that case. That seems
good enough to me. Maybe we should emit a warning when the working
copy is in the set of immutable commits.
Maybe we should add support for something more like [Mercurial's
phases](https://wiki.mercurial-scm.org/Phases), which is propagated on
push and pull. There's already some affordance for that in the view
object's `public_heads` field. However, this is simpler, especially
since we can't propagate the phase to Git remotes, and seems like a
good start. Also, it lets you say that commits authored by other users
are immutable, for example.
For now, the functionality is in the CLI library. I'm not sure if we
want to move it into the library crate. I'm leaning towards letting
library users do whatever they want without being restricted by
immutable commits. I do think we should move the functionality into a
future `ui-lib` or `ui-util` crate. That crate would have most of the
functionality in the current `cli_util` module (but in a
non-CLI-specific form).
I'm going to make this function check against a configurable revset
indicating immutable commits. It's more efficient to do that by
evaluating the revset only once.
We may want to have a version of the function where we pass in an
unevaluated revset expression. That would allow us to error out if the
user accidentally tries to rebase a large set of commits, without
having to evaluate the whole set first.
Once we add support for immutable commits, `jj duplicate` should be
allowed to create duplicate of them. The reason it can't duplicate the
root commit is that it would mean there would be multiple root
commits, which would break the invariant that the single root commit
is the only root commit (and the backends refuse to write a commit
without parents). So let's have `jj duplicate` check specifically that
the user doesn't try to duplicate the root commit instead.
For `jj split --interactive`, the user will want to select changes from a subset of files. This means that we need to pass the `Matcher` object when materializing the list of changed files. I also updated the parameter lists so that the matcher always immediately follows the tree objects.
I don't think there's any reason to use the local backend in tests
instead of using the stricter test backend.
I think we should generally use the test backend in tests and only use
the local backend or git backend when there's a particular reason to
do so (such as in `test_bad_locking` where the on-disk directory
structure matters). But this patch only deals with the simpler cases
where we were only testing with the local backend.
This appears to be broken at db0d14569b "cli: wrap repo in a struct to
prepare for adding cached data." Testing this isn't easy since the operation
id recorded here will be overwritten immediately by snapshot_working_copy(),
and the snapshotting should work fine so long as the tree id matches.
The rest of the functions in this file are defined before they are used, so it confused me when trying to track down this function in the static call graph.
It makes the call sites clearer if we pass the `TestRepoBackend` enum
instead of the boolean `use_git` value. It's also more extensible (I
plan to add another backend for tests).
As we discussed in #1928, it seems better to print information about
abandoned commits in the context of the pre-abandon state. For
example, that means that we'll include any branches that pointed to
the now-abandoned commits.
This is a naive implementation, which cannot deal with multiple children
or parents stemming from merges.
Note: I gave each command separate a separate argument struct
for extensibility.
Fixes#878
Many failure to export refs to Git are not about conflicts between a
branch named `foo` and a branch named `foo/bar`, so don't give that
hint in most cases.
In `LockedWorkingCopy::drop()`, we panic if the caller had not called
`finish()`. IIRC, the idea was both to find bugs where we forgot to
call `finish()` and to prevent continuing with a modified
`WorkingCopy` instance. I don't think the former has been a problem in
practice. It has been a problem in practice to call `discard()` to
avoid the panic, though. To address that, we can make the `Drop`
implementation discard the changes (forcing a reload of the state if
the working copy is accessed again).
I also converted the error from `InternalError` to `UserError`. So far
I've intented to use `InternalError` only to indicate bugs or corrupt
repos. I'm not sure that's a good idea, and we can revisit it later.
If the path is too long to fit on the screen, this patch makes it so
we elide the first part of it. It goes a bit further and trims it down
to ~70% of the screen, giving some room for the stat. This seems
somewhat similar to what Git does.
We can still crash on terminals that are less than 4 characters wide
(maybe it doesn't matter if we do because the user can't tell the
crash report from a diffstat in such a terminal?). This patch fixes
the crash.
We would run into a panic due to "attempt to subtract with overflow"
if the path was long. This patch fixes that and adds tests showing the
current behavior when there are long paths and/or large diffs.
With the idea that less severe placeholders (like description) could
(and should) explicitly "opt out".
(Both email and name placeholders will be red with this change.)
I made it simply fail on explicit fetch/import, and ignored on implicit import.
Since the error mode is predictable and less likely to occur. I don't think it
makes sense to implement warning propagation just for this.
Closes#1690.
This switches the whole `diff_util` module to working with
`MergedTree`, `Merge<Option<TreeValue>>` etc., so it can support
tree-level conflicts.
Since we want to avoid using `ConflictId`s, I switched the hash we use
for conflicts in `--git` style diffs to use an all-'0' id instead of
using the conflict id.
This patch also extracts format_detailed_signature() function to deduplicate
the "show" template bits.
The added placeholder templates aren't labeled as "empty". If needed, I think
the whole template can be labeled as "empty" (or "empty_commit") just like
"working_copy".
Closes#2112
An alternative name for it would be `arity()`, but `num_sides()`
probably more clearly says that it's not about the number of removes
or the total number of terms.
I think this is less surprising than falling back to the default length.
i64-to-usize conversion can also overflow on 32 bit environment, but I'm not
bothered to handle overflow scenario.
To support tree-level conflicts, we're going to need to update the
working copy from one `MergedTree` to another. We're going need to
store multiple tree ids in the `tree_state` file. This patch gets us
closer to that by getting the diff from `MergedTree`s`, even though we
assume that they are legacy trees for now, so we can write to the
single-tree `tree_state` file.
This allows negative numbers, which also means functions which took numbers can now take negative numbers
Luckily, they all already handled this exactly as expected.
Many of the `TreeBuilder` users have an `Option<TreeValue>` and call
either `set()` or `remove()` or the builder depending on whether the
value is present. Let's centralize this logic in a new
`TreeBuilder::set_or_remove()`.
The way `jj git push` without arguments chooses branches pointing to
either `@` or `@-` is unusual and difficult to explain. Now that we
have `-r`, we could instead default it to `-r '@-::@'`. However, I
think it seems likely that users will want to push all local branches
leading up to `@` from the closest remote branch. That's typically
what I want. This patch changes the default to do that.
If there are branches in the revset that don't need to be pushed
because they already match the destination, we currently just print
`Nothing changed.` It seems consistent with that to also treat it as
success if there are no branches in the specified set to start
with. This patch makes the command print a warning in that case
instead.
`revset::parse()` already has a `RevsetWorkspaceContext` argument, so
I think it makes sense to put that and the other context arguments
into a larger `RevsetParseContext` object.
We resolve file paths into repo-relative paths while parsing the
revset expression, so I think it's consistent to also resolve which
workspace "@" refers to while parsing it. That means we won't need the
workspace context both while parsing and while resolving symbols.
In order to break things like `author("martinvonz@")` (thanks to @yuja
for catching this), I also changed the parsing of working-copy
expressions so they are not allowed to be
quoted. `author(martinvonz@)` will therefore be an error now. That
seems like a small improvement anyway, since we have recently talked
about making `root` and `[workspace]@` not parsed as other symbols.
Custom binaries will often want to provide e.g. additional command
aliases, additional revset aliases, custom colors, etc. This adds a
mechanism for them to do that.
This commit replaces the functions `UserSettings::user_name_placeholder()`` and
`UserSettings::user_email_placeholder()` with `const` `&str`s to emphasize that
the placeholder strings must not be changed to support commits without
names or email addresses made before this change.
Bright green really pops on my screen, and I don't think there is a reason
for the root commit to be attention-grabbing.
This follows up on https://github.com/martinvonz/jj/pull/2084.
One use case for `jj split` is when creating a new commit from some of
the changes in the working copy. If there's no description on the
working-copy commit in that case, it seems better to not ask the user
to provide one when they're splitting the commit either.
I've extracted the `builtin_log_root` template for users to customize the
default templates without fully overriding them, for example I would remove
the change_id/commit_id for myself - and we discussed in Discord that leaving
those makes sense for the user to be reminded/teached that the root commit has
a change id made from z's.
Similar to other boolean flags, such as "working_copy" or "empty".
We could test something like
`"0000000000000000000000000000000000000000".contains(commit_id)`
like I did for myself, but first of all this is ugly, and secondly the root
commit id is not guaranteed to be 40 zeroes as custom backend implementations
could have some other root.
