We already have `create_random_commit()`, which returns a
`CommitBuilder`. Most callers directly write that to a
`MutableRepo`. That currently returns a `Commit`, but I'm about to
make it propagate errors from the backend. That would add an
`unwrap()` to this sequence, making it longer. Let's create a simple
helper for these callers to simplify this common pattern.
When you're done with the `CommitBuilder`, you're going to have to
call `write_to_repo()`, passing it a mutable `MutableRepo`
reference. It's a bit simpler to pass that reference when we create
the `CommitBuilder` instead, so that's what this patch does.
A drawback of passing in the mutable reference when we create the
builder is that we can't have multiple unfinished `CommitBuilder`
instance live at the same time. We don't have any such use cases yet,
and it's not hard to work around them, so I think this change is worth
it.
The next commit will introduce a newtype for -m/--message argument which
can be converted Into<String>.
Since CommitBuilder is a thin wrapper, code bloat caused by generic parameters
wouldn't matter. I have another set of commits that makes all builder methods
accept Into/IntoIterator, which will remove some of .clone() calls from tests.
I ran an upgraded Clippy on the codebase. All the changes seem to be
about using variables directly in format strings instead of passing
them as separate arguments.
This will be a building block of 'parents(base)' revset. 'base---' will
be .filter_by_generation(3..4) for example. I think 'ancestors(base)' can
also have an optional generation parameter, but I haven't considered any
particular syntax yet.
This basically transforms 's1 & (f() | s2)' to
's1.iter().filter(all && f || s2)'. Still the predicate part includes "all",
the filter function doesn't need to load commit data for every entry since
's1.iter().filter(all)' is tested first. To optimize "all" predicate out,
maybe we can add a wrapper that returns '|_: &IndexEntry| true'.
Instead of inserting AsFilter(_) node, I could add a recursive is_filter()
function. That would also work so long as the height of RevsetExpression tree
is limited. I chose node insertion just for ease of snapshot testing.
@yuja asked on #701 about the difference between the state in the
`git_export_view` and what we have in `mut_repo.view()`. It's true
that the branches in `mut_repo.view().git_refs()` should match what we
wrote to disk. We can therefore remove the on-disk storage and
simplify quite a bit. For now, I create the `last_export_view` from
the `mut_repo.view().git_refs()` before calling
`export_changes()`. I'll clean up a bit more next.
I think this is correct even considering e.g. undo. Let's consider
what would happen in a non-colocated Git repo (not because tricky
cases cannot happen there but because the explicit exports and imports
make it easier to discuss, and more cases can occur). If the user
moved a branch and then did `jj git export`, `jj undo`, and then `jj
git export` again, we would think on the second export that we should
perform the same changes to the Git repo, which should have no effect.
This patch also fixes the bug we were forced to work around in the
test case in the previous patch.
This removes one of our uses of Thrift.
This fixes the bugs shown by the tests added in the previous patch by
checking that the git branches we're about to update have not been
updated by git since our last export. If they have, we fail those
branches. The user can then re-import from the git repo and resolve
any conflicts before exporting again.
I had to update the `test_export_import_sequence` to make it
pass. That shows a new bug, which I'll fix next. The problem is that
the exported view doesn't get updated on import, so we would try to
export changes compared to an earlier export, even though we actually
knew (because of the `jj git import`) that the state in git had
changed.
If you update a branch using regular `git` (or some Git-based tool)
between two `jj git export`, we will overwrite that change if you had
also changed the branch in jj land. There's a similar problem if you
delete the branch in jj land. Let's have a test for that. I'm going to
make us not overwrite it soon. This patch adds a test for those cases,
plus many other cases in consistent way. Since the new test covers
some cases tested by existing tests, I removed those tests.
It seems that we didn't have a test for this simple case. I wrote this
test case while working on #111 but I don't know why I didn't push it
back then.
A new FileType, GitSubmodule is added which is ignored. Files or
directories having this type are not added to the work queue and
are ignored in snapshot. Submodules are not created by jujutsu
when resetting or checking out a tree, they should be currently
managed using git.
Because a unary negation node '~y' is more primitive than the corresponding
difference node 'x~y', '~y' is easier to deal with while rewriting the tree.
That's the main reason to add RevsetExpression::NotIn node.
As we have a NotIn node, it makes sense to add an operator for that. This
patch reuses '~' token, which I feel intuitive since the other set operators
looks like bitwise ops. Another option is '!'.
The unary '~' operator has the highest precedence among the set operators,
but they are lower than the ranges. This might be counter intuitive, but
useful because a prefix range ':x' can be negated without parens.
Maybe we can remove the redundant infix operator 'x ~ y', but it isn't
decided yet.
Let's acknowledge everyone's contributions by replacing "Google LLC"
in the copyright header by "The Jujutsu Authors". If I understand
correctly, it won't have any legal effect, but maybe it still helps
reduce concerns from contributors (though I haven't heard any
concerns).
Google employees can read about Google's policy at
go/releasing/contributions#copyright.
Follows up c5ed3e1477. Now change/commit ids are resolved at the same
precedence, which means there are at least three types of ambiguity.
I don't think we would need to discriminate these.
Because the use of the change id is recommended, any operation should abort
if a valid change id happens to match a commit id. We still try the commit
id lookup first as the change id lookup is more costly.
Ambiguous change/commit id is reported as AmbiguousCommitIdPrefix for now.
Maybe we can merge AmbiguousCommit/ChangeIdPrefix errors into one?
Closes#799
The CLI will load aliases from config, insert them one by one, and warn if
declaration part is invalid. That's why RevsetAliasesMap is a public struct
and needs to be instantiated by the caller.
To reduce conflicts between branches like `main` and `main/sub`, it's
better to first delete refs in git that have been deleted in jj, and
then add/update refs that have been added/updated in jj.
Since we now write a (partial) view object of the exported branches to
disk (since 7904474320), we can safely skip exporting some
branches. We already skip conflicted branches. This commit makes us
also skip branches that we fail to write to the backing Git repo,
instead of failing the whole operation (after possibly updating some
Git refs).
I made the `export_refs()` function return the branches that
failed. We should probably make that a struct later and have a
separate field for branches that we skipped due to conflicts.
Closes#493.
This adds a test for attempting to export both a branch called `main`
and one called `main/sub` (#493), as well as for exporting a branch
with an empty string as name (reported directly to me by @lkorinth).
The expression 'x ~ empty()' is identical to 'x & file(".")', but more
intuitive.
Note that 'x ~ empty()' is slower than 'x & file(".")' since the negative
intersection isn't optimized right now. I think that can be handled as
follows: 'x ~ filter(f)' -> 'x & filter(!f)' -> 'filter(!f, x)'
There are no "non-normal" files, so "normal" is not needed. We have
symlinks and conflicts, but they are not files, so I think just "file"
is unambiguous.
I left `testutils::write_normal_file()` because there it's used to
mean "not executable file" (there's also a `write_executable_file()`).
I left `working_copy::FileType::Normal` since renaming `Normal` there
to `File` would also suggest we should rename `FileType`, and I don't
know what would be a better name for that type.
We currently get the hostname and username from the `whoami` crate. We
do that in lib crate, without giving the caller a way to override
them. That seems wrong since it might be used in a server and
performing operations on behalf of some other user. This commit makes
the hostname and username configurable, so the calling crate can pass
them in. If they have not been passed in, we still default to the
values from the `whoami` crate.
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:
1. Import a branch pointing to commit A from Git
2. Modify the branch in jj to point to commit B
3. Export the branch to Git
4. Update the branch in Git to point to commit C
5. Import refs from Git
In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.
This commit fixes the bug by updating the refs in the `MutableRepo`.
Closes#463.
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
To fix#463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
These calls often appear in expressions long enough that not having to
qualify it means that we can sometimes avoid wrapping a line. I
noticed because IntelliJ told me that `test_git.rs` had some
unnecessary qualificiations (the function was already imported there).
The `testutils` module should ideally not be part of the library
dependencies. Since they're used by the integration tests (and the CLI
tests), we need to move them to a separate crate to achieve that.
If you remove all refs from the backing Git repo and then run `jj git
import`, we would see that all commits disappeared from the Git repo,
so we would remove them from the jj repo too. However, we do that by
doing a history walk from old heads to the new heads, which includes
the root commit when the new heads is an empty set. That means that we
mark the root commit as abandoned, which led to a crash in
`rewrite.rs` (when we try pick the root commit's first parent to use
as parent for rebased commits).
I was trying to create a reproduction script for #412, but the script
ran into another bug first. The script removed all the local and
remote branches from the backing Git repo. I noticed that we would
then try to abandon all commits. We should still count Git HEAD's
target as visible and not try to abandon it. This patch fixes that.
