This patch replaces single-char L type aliases with P, and renames the L aliases
where that makes sense. Many of the P aliases will be removed later by
introducing generic wrap<T>() trait.
I think this was remainder of the old design where wrap_*() functions took
&TemplateLanguage as self argument. Since the wrapper type implements accessor
functions like .try_into_boolean(), it makes sense that the same type implements
::wrap_boolean(), etc.
These .wrap_<T>() functions will become generic over T.
It seems like a small usability improvement if users don't need to
enter the "$schema" link manually when they create a new config file.
This doesn't help existing users.
This change, from an enum to a struct, is a more accurate representation
of the actual way that a ConfigPath works; additionally, it lets us add
different information without modifying every single enumeration field.
I'm trying to refactor property wrapping functions, and noticed that it's odd
that .wrap_<property>() does boxing internally whereas .wrap_template() doesn't.
Also, it sometimes makes sense to turn property into trait object earlier. For
example, we can deduplicate L::wrap_boolean() in build_binary_operation().
This also adds a test case for the completion of arguments following
multi-argument aliases, to cover the bug reported in issue #5377.
The default command is like a special kind of alias, one which is
expanded from zero tokens. Consequently, it also triggers the bug
#5377 on bash/zsh, even if the `default-command` is just a single token.
The fix is along the lines sketched out by the "TODO" comment. Bash and
Zsh don't behave identical, so the padding ([""]) still needs to be
applied (and removed) conditionally in a disciplined manner.
The completion mechanism works differently in different shells:
For example, when the command line `jj aaa bb ccc` is completed at the
end of the `bb` token, bash and zsh pass the completer the whole line
`-- jj aaa bb ccc` and an index of 2 which refers to the `bb` token;
they are then expected to complete `bb`. Meanwhile, fish and Powershell
only pass the command up to the completion point, so `-- jj aaa bb`;
the completer is always expected to complete the last token. In all
cases, the shell ultimately decides what to do with the completions,
e.g. to complete up to a common prefix (bash), to show an interactive
picker (zsh, fish), or to insert the completion if it is the only one
(all shells). Remaining tokens (`ccc`) are also always appended by the
shell and not part of the completion.
While this is mostly handled by the clap_complete crate, we do expand
aliases and present clap_complete with a fake view of the real command
line, thereby reaching into its internals by wrapping the interface
between the completion shell script that is provided by clap_complete
and its Rust code in `CommandEnv::try_complete()`. If we get this wrong,
completion might yield unexpected results, so it is worth testing
completion for both flavors of shells whenever aliases are potentially
in the mix.
To avoid redundancy, the shell-specific invocation of `jj` is factored
into a `complete_at()` function of the test fixture. The `test-case`
crate is then used to instantiate each test case for different values of
clap_complete's `Shell` enum.
filter
bash/zsh specific behavior
move impl
The `CliRunner` lets a custom binary add processing that should happen
before running a command. This patch replaces that by a hook that can
do processing before and/or after. I thought I would want to use this
for adding telemetry (such as timings) to our custom binary at
Google. I ended up adding that logging outside of `CliRunner::run()`
instead, so it gives a more accurate timing of the whole invocation. I
think this patch is still an improvement, as it's more generic than
the start hook.
In builtin diff editor, we materializes conflicts, so we need to parse them
back to reproduce the original (or partially-resolved) contents. OTOH, the
merge editor should write the merged contents transparently.
This change also revealed that binary hunks wouldn't be processed correctly in
the merge editor.
make_diff_files() will be async function that uses materialized_diff_stream()
internally. apply_diff_builtin() will take callbacks to handle binary/conflict
files.
If `git.fetch` contains remotes that are not available, we currently error even
if other remotes are available. For common fork workflows with separate
`upstream` and `origin` remotes (for example), this requires a user to either
set both remotes in their user config and override single-remote repos or set
only one in their user config and override all multi-remote repos to fetch from
`upstream` (or both).
This change updates fetching to only *warn* about unknown remotes **if** other
remotes are available. If none of the configured remotes are available, an error
is still raised as before.
These days, we usually think of conflicts as one base state and series
of diffs between other states. The base state is normally the parent
when rebasing.
Also, we're deprecated `jj backout` in favor of `jj revert`, so let's
use that terminology.
When we ask the user to prodive a commit description, we currently
write a file to `.jj/repo/` with the draft description and then pass
that to the editor. If the editor exits with an error status, we leave
the file in place and tell the user about the path so they can recover
the description. I'm not sure I've ever used one of these files. I
have certainly never used a file that's not from the most recent
edit. I have, however, cleaned up old such files. This patch changes
the code so we write them to /tmp instead, so we get the cleanup for
free.
A pattern has emerged where a integration tests check for the
availability of an external tool (`git`, `taplo`, `gpg`, ...) and skip
the test (by simply passing it) when it is not available. To check this,
the program is run with the `--version` flag.
Some tests require that the program be available at least when running
in CI, by calling `ensure_running_outside_ci` conditionally on the
outcome. The decision is up to each test, though, the utility merely
returns a `bool`.
The `CommandNameAndArgs` struct is used in multiple places to specify
external tools. Previously, the schema only allowed for this in
`ui.pager`.
This commit adds a few sample configs which define variables editors and
fix tools as commands with env vars.
The schema has also been updated to make these valid.
Not sure if that is intentional or should rather be considered a bug,
but currently the "structured" option for specifying an external tool
requires that both the command "command" and the "env" keys are
specified. The "command" key makes sense; for the "env" key it came as
a surprise, but it can be argued that this form should only be used when
environment variables need to be specified and the plain string or array
form should be used otherwise.
Either way, the schema did not accurately reflect the current behavior;
now it does. Two sample configs have been added as schema test cases.
Anytime an external tool is referenced in the config, the command can be
provided as a string or as a token array. In the latter case, the array
must not be empty; at least the command name must be provided.
The schema didn't previously object to an empty array, though; this has
now been rectified. I've added more sample configs to cover this case.
Those same configs can also be used to illustrate that this is indeed
jj's current behavior:
$ jj --config-file cli/tests/sample-configs/invalid/ui.pager_empty_array.toml show
Config error: Invalid type or value for ui.pager
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.pager.command_empty_array.toml show
Config error: Invalid type or value for ui.pager
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.editor_empty_array.toml config edit --user
Config error: Invalid type or value for ui.editor
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.diff-editor_empty_array.toml split
Error: Failed to load tool configuration
Caused by:
1: Invalid type or value for ui.diff-editor
2: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.merge-editor_empty_array.toml resolve
Error: Failed to load tool configuration
Caused by:
1: Invalid type or value for ui.merge-editor
2: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.diff.tool_empty_array.toml diff
Config error: Invalid type or value for ui.diff.tool
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/fix.tools.command_empty_array.toml fix
Config error: Invalid type or value for fix.tools.black
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
in `command`
As a notable exception, `ui.default-command` *is* allowed to be an empty
array. In that case, `jj` will print a usage message. This is also
covered by a valid sample config.
While `ui.pager` can be a string which will be tokenized on whitespace,
and argument token array, or a command/env table, the `command` key
within that table currently must be an array. The schema previously
explicitly also allowed it to be a string but that does not actually
work, as exemplified by running:
```sh
$ jj --config-file cli/tests/sample-configs/invalid/ui.pager_command-env_string.toml config list
Config error: Invalid type or value for ui.pager
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
```
`CommandNameAndArgs` should potentially be changed to allow strings.
For now, the schema has been updated to reflect the status quo. A new
sample toml has been added to the `invalid` directory to cover this;
prior to updating the schema, this new test case failed. Once the
behavior is changed to allow string, the file merely needs to be moved
to `valid`.
These are two more instances where the default values were wrong and in
fact not even consistent with the schema itself.
I've found these by running
```sh
jq -r 'paths(type == "object" and has("default")) as $p | getpath($p).default | tojson as $v | $p | map("\"\(select(. != "properties"))\"") | join(".") as $k | "\($k) = \($v)"' cli/src/config-schema.json | taplo check --schema=file://$PWD/cli/src/config-schema.json -
```
which uses `jq` to filter the default values from the schema definition
to create a rudimentary TOML file containing all the defaults according
to the schema and then uses `taplo` the validate this TOML against the
schema.
This approach could be developed further to also parse the intermediate
TOML file and compare the result with the default config (from parsing
an empty config). That would not only test for self-consistency of the
schema's proclaimed defaults but also for consistency with the actual
defaults as assumed by jj.
Adds a bunch of additional sample config toml files. Via the
`datatest_runner`, these each correspond to a test case to check that
the toml is correctly (in-)validated according to the schema.
The `valid/*.toml` files typically define multiple related config
options at once. Where there's some overlap with the default configs in
`cli/src/config`, the aim was to choose different allowed values, e.g.
hex colors, file size in bytes (numeric), etc.
The `invalid/*.toml` files typically only define a single offending
property such as to not obscure individual false negatives. All of the
"invalid" files are still valid toml as the aim is not to test the
`toml_edit` crate or Taplo.
The sample files all contain a Taplo schema directive. This allows them
to be validated against the schema on the fly by Taplo's LSP and derived
IDE plugins to speed up editing and immediately highlight offending
options.
Closes#5695.
The `datatest-stable` crate allows to dynamically instantiate test cases
based on available files. This is applied to `test_config_schema` to
create one test case per config file. As case in point, the test case
for `hints.toml` was missing previously, hence the total number of tests
is up one.
This will become useful when adding more config examples to somewhat
exhaust the schema.
`datatest-stable` uses a custom test harness and thus cannot be used in
the same integration test binary that all of the other test modules run
in. However, if data-driven tests are to be used for other applications,
they can share in the same binary, so the module structure is already
set up to mirror the central "runner" approach.
The previous implementation of `assert_no_forgotten_test_files`
hard-coded the name of the `runner` integration test and required all
other source files to appear in matching `mod` declarations. Thus, this
approach cannot handle multiple integration tests.
However, additional integration tests may be desirable
- to support tests using a custom test harness (see upcoming commits)
- to balance the trade-off between test run time and compile time as
the test suite grows in the future.
The new implementation first uses `taplo` to parse the `[[test]]`
sections of the manifest to identify integration test main modules,
and then searches in those for `mod` declarations. This is then compared
to the list of source files in the tests directory. Like the previous
implementation, the new one does not attempt to recurse into submodules
or to handle directory-style modules; just like before it only treats
source files without a module declaration as an error and relies on the
compiler to complain about the other way around.
When `taplo` is not installed, the check is skipped unless it is running
in CI where we require `taplo` to be available.
This allows text typed into the search box to persist across page navigation, which is
useful when users look up a term but aren't sure on which page they will eventually find
the relevant information. See the [`mkdocs-material` docs][mm-docs] for more
information.
[mm-docs]:
https://squidfunk.github.io/mkdocs-material/setup/setting-up-navigation/#instant-loading
Since we now have the template alias, it's easy to migrate the config value
programatically. However, there are a couple of minor behavior changes:
a. default description won't be used if user customized the
draft_commit_description template.
b. default description won't be inserted if trailer is added
For (a), I assumed user would inline the default description in the draft
template if they customized the template. (b) should be okay because the
trailers template is a new feature.
I don't have any plan to implement incremental UI for file annotation, but I
think the new API is nicer in that they have fewer function arguments.
Note that this wouldn't help implement interactive UI to go ancestor annotation
by clicking annotated line. To achieve that cheaply, we'll need weave-like data.
This makes it easier to override just the default description without
having copy the whole default template (and having to keep it up to
date with new versions).
An existing "push-*" bookmark should usually be in position. If it wasn't
because of e.g split, I think the user should be aware of that and take an
explicit action.
We were not calling `view.mark_dirty()` after adding heads, which
means that we would not enforce the usual view invariants, such that
ancestors of heads should not also be in the set of heads. This patch
fixes that and updates the test (which actually shows the broken
invariant in its current form).
A not-insignificant amount of our PR traffic is from Dependabot, even
with the grouped update feature (something like 20% of all PRs in total
are from Dependabot, last I checked.)
We don't really need daily updates, and with the the current crate
dependency graph we practically get updates *every* day. Bump it to
weekly instead to stem the tide a little.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
We ran into a crash on our server at Google today because we
accidentally called `RepoPathBuf::from_internal_string()` with a
string starting with a '/', which resulted in a the assertion in that
function failing. This patch changes that constructor and its siblings
to return a `Result` instead.
namely Signed-off-by and Change-Id
`format_signed_off_by_trailer` will be formatted properly if the author
name is not set, but will contain the email placeholder if the author
email is not set, as I haven't found a way to make the template
generation fail.
`format_gerrit_change_id_trailer` is based on jj's change id, but it
needed to be padded to reach 40 characters. Zero-padding is kind of
boring so I've used `6a6a6964`, the hexadecimal representation of `jjid`
in ascii.
Because the trailer value runs up to the end of the line, they are
all terminated with a new line. This way it's also convenient to
define these trailers in the `commit_trailers` template:
[templates]
commit_trailers = '''
format_signed_off_by_trailer(self)
++ format_gerrit_change_id_trailer(self)
'''
The logic is similar to the color-words diff's. We first resolve trivial
conflicts, then compare each hunk of Merge<&BStr> type. We also apply the same
optimization as the resolved case to minimize lines to be merged and diffed.
This allows the customization of the duplicated commit descriptions.
An ideal use case for this is emulating `git cherry-pick -x`, as
illustrated in the tests.
namely Signed-off-by and cherry-pick trailers. The cherry-pick trailer
doesn't appear in the parsed trailer though, to match the
`git interpret-trailers` implementation.
see https://github.com/git/git/blob/master/trailer.c for reference
Add a new `template.commit_trailer` configuration option. This template
is used to add some trailers to the commit description.
A new trailer paragraph is created if no trailer paragraph is found in
the commit description.
The trailer is not added to the trailer paragraph when the trailer is
already present, or if the commit description is empty.
We call `RevsetEvaluationError::expect_backend_error()` in lots of
places. If the revset engine had returned a non-`BackendError`, it
would result in a panic. For example, a revset engine might return
something like `RevsetEvaluationError::Other("not supported".into())`
for some revsets. We shouldn't crash when this happens.
We may want to create some higher-level `RepoError` to return here,
but for now let's just avoid the crash.
This reverts a01d0bf7738f "cli: resolve: leave executable bit unchanged when
using external tool", and updates handling of resolved content.
Fixes#6250
As Martin suggested, "we should instead make it an error to try to resolve the
content-level conflict before resolving the executable-bit-level conflict. I
think we should do the same when merging copy records."
https://github.com/jj-vcs/jj/pull/6288#pullrequestreview-2751367755
We're going to make "jj resolve" error out on exec bit conflict. This behavior
should be more consistent with tree::try_resolve_file_conflict(), which exits
early if the tree value had absent entries.
Since unchanged exec bit shouldn't be lost when resolving change-delete content
conflict, the resolution function falls back to the original state if the
exec-bit conflict is resolved to None.
This seems a bit easier to follow than nesting Option types.
try_resolve_file_conflict() doesn't have this problem, but it is the only caller
of maybe_map(), so I've gone ahead and replaced it, too.
The return type is unchanged because it looked equally bad if to_tree_merge()
returned Result<Result<Merge<Tree>, ()>, BackendError>.
try_materialize_file_conflict_value() and to_file_merge() shouldn't fail because
of exec bit changes. And error.to_string() shouldn't include trailing newline.
This will probably help debug weird problem like #6287. File names are a bit
redundant for ReadObject errors (which include ObjectId), but that should be
okay.
This helps propagate error from tx.write(). I made the error variants
non-transparent because it's easier than manually implementing From<> that maps
each inner error to the other inner error.
Latest rustc nightlies seem to have broken incremental compilation [0].
I apparently have the worst luck in this department, as I've gotten multiple
ICEs within the last few hours alone. So, I'm putting future me out of
her misery.
[0]: https://github.com/rust-lang/rust/issues/139110
When read/writing commits from the git-backend, populate the git commit
header with a backwards hash of the `change-id`. This should enable
preserving change identity across various git remotes assuming a
cooperative git server that doesn't strip the git header.
This feature is behind a `git.write-change-id-header` configuration flag
at least to start.
This uses `zlib-rs`, a native Rust library that is comparable in
performance to `zlib-ng`. Since there’s no complicated C build
and gitoxide only has one hashing backend now, this lets us drop our
`packaging` feature without adding any awkward build requirements.
`zlib-rs` is generally faster at decompression than
`zlib-ng`, and faster at compression on levels 6 and 9; see
<https://trifectatech.org/blog/zlib-rs-is-faster-than-c/>
for details.
I couldn’t get reliable‐looking benchmark results out of my
temperamental laptop; `hyperfine` seemed to think that some random
`jj` workloads I tested might be slightly slower than with `zlib-ng`,
but it wasn’t unambiguously distinguishable from noise, so I’d
like to see measurements from others.
It’s certainly a lot faster than the previous default, and I
think it’s likely that `zlib-rs` will continue to get faster
and that it’s more than worth avoiding the headaches of a native
library with a CMake build dependency. (Though on the other hand,
if distributions move in the direction of shipping `zlib-ng` by
default, maybe there will be more motivation to make `libz-ng-sys`
support system libraries.)
The original idea was to flatten left/right conflict trees and pair up adjacent
negative/positive terms. For example, diff(A, B-C+D) could be rendered as
diff(A, B) and diff(C, D). The problem of this formalization is that one of the
diff pairs is often empty (because e.g. A=B), so the context is fully omitted.
The resulting diff(C, D) doesn't provide any notion why the hunk is conflicted,
and how it is different from A.
Instead, this patch implements diffs in which each left/right pair is compared.
In the example above, the left terms are padded, and the diffs are rendered as
diff(A, B), diff(-A, -C), diff(A, D). This appears to be working reasonably well
so long as either side is resolved or both sides have the same numbers of terms.
Closes#4062
This helps extract hunk rendering function for non-materialized color-words
diffs. In conflict hunk, an identical diff pair will be omitted with "..."
marker.
This patch introduces a subtle behavior change when "ignore whitespace" options
are used. Before, "..." wouldn't be printed if contents differ only in
whitespace. I don't think the new behavior is bad because the file header says
"Modified regular file" in that case.
I originally thought we could remove args.reset_author tests in later pass, but
we can't because author timestamp may be updated if a commit is discardable.
Still it's nice that we can deduplicate some logic.
The version that returns Merge<_> will be used in diff functions. The added
helper functions will also be used in order to apply word-level merging.
Since we cannot express FnOnce(impl IntoIterator<..>) where the argument type is
controlled by callee, this patch adds helper trait to bridge collect_*()
functions.
This helps implement variants of file::merge() that return Merge<BString> or
Option<BString>. collect_hunks() could be implemented as FromIterator for
MergeResult. It would enable .collect(), but I don't think this abstraction
would generally be useful. The other types to which we want to collect the
outcome is Option<BString> and Merge<BString>. They could technically implement
FromIterator, but it would be weird if .collect() into Merge<BString> panicked
because of incompatible merge shapes.
Merge::resolved() is marked as const fn to clarify it is a trivial constructor.
We haven't updated `prost` in a while, and the reason for that is
probably because the code generation output slightly changed, which
would have caused dependabot to exclude that package from its updates as
a failure. (At least, I suspect that's what happened.)
This lets us drop a dependency on `itertools 0.12.x` because the prost
0.13.5 release weakened its constraints, allowing `itertools 0.13.x`.
Now we only depend on two major versions of itertools (0.10 + 0.12)
instead of three (previously four).
Signed-off-by: Austin Seipp <aseipp@pobox.com>
It can be useful for fixers to record some information about the
inputs they processed. For example, the `FileFixer` we use in our
server at Google get more detailed error information back from its
formatter tools. We may want to record these errors in our
`FileFixer`'s state so we can then return that to the client.
This should fix git::import_refs() issue with gix 0.71.0. Old commits could be
repopulated by importing stale refs stored in packed-refs.
https://github.com/GitoxideLabs/gitoxide/issues/1928
The Zlib license is added to the allow list because foldhash appears in the
dependency chain.
This was taken out of the "Jujutsu from first principles" doc in another PR. It represents some of the common ideas
which the project had around a year ago.
I think this can be modernized if the maintainers want it.
This should fix git::import_refs() issue with gix 0.71.0. Old commits could be
repopulated by importing stale refs stored in packed-refs.
https://github.com/GitoxideLabs/gitoxide/issues/1928
The Zlib license is added to the allow list because foldhash appears in the
dependency chain.
I think this check will only be useful if it actually blocks the
build. Since the resolution is simple (just bump the limit), I think
it’s okay to add a small amount of friction and ask people to take
a moment to consider other options first.
This function is similar to `transform_descendants_with_options`, but
only rewrites the given commits and not their descendants. This will
allow `rewrite::move_commits` to transform a given set of commits
without iterating through their descendants twice (once when computing
new parents and once when rewriting commits).
The `new_parents_map` will allow the parents of each commit to be
customized before the `transform_descendents` callback is invoked. This
is useful when the order of commits needs to be changed, whereby setting
the new parents in the default callback might lead to repeated rebasing
and cycles if the new parent has not been visited in the reverse
topological order traversal.
Follows up on https://github.com/jj-vcs/jj/pull/5698 and
https://github.com/jj-vcs/jj/pull/6236.
The behavior of `--named` in corner cases differs. This wasn't detected
in #5698 because of a bug in the test that was fixed in #6236. This
commit better illustrates the more important case where the behavior is
the same, and the less important case where it differs.
Follow-up to b1bb5e1
This creates a `.github/scripts/count-cargo-lock-packages` script to
count packages with our methodology that one can run outside CI.
I also renamed the check so that it's clearer what it does.
After some discussion on Discord yesterday, Emily floated this idea to
have a check that fails if `Cargo.lock` has too many dependencies, where
"too many" means "more than a random number I made up and sounds good."
This implements that, as a non-required check, and to do so it invokes
the power of an extremely evil and annoying Dragon. We could also ask
this Dragon to do other things too I suppose (pending future contract
negotiations).
Signed-off-by: Austin Seipp <aseipp@pobox.com>
In docs/templates.md, the Commit type and Operation type looked like:
`method() -> ReturnType`
rather than everything else, which looks like:
`.method() -> ReturnType`
This commit changes the Commit and Operation types to look more like the
rest of the documentation.
Fix GHSA-794x-2rpg-rfgr.
`gix::Repository::work_dir` was renamed to `workdir` (though strangely
not the `gix::ThreadSafeRepository` version), and `lossy_config`
is now off by default in all configurations.
These merge tools didn't work properly on conflicted trees with more
than two sides since they didn't simplify the conflict before resolving.
The new implementation is more similar to how external merge tools are
executed, which should give a more consistent behavior.
Previously, this was calling `git_repository_open()`,
which is equivalent to `git_repository_open_ext()` with
`flags = GIT_REPOSITORY_OPEN_NO_SEARCH` and `ceiling_dirs =
NULL`. This changes `ceiling_dirs` to an empty string, and adds
`GIT_REPOSITORY_OPEN_FROM_ENV` to `flags` when we’re in test code.
`GIT_REPOSITORY_OPEN_FROM_ENV` is used to respect the Git configuration
path environment variables, which is what we want for the test
hermeticity code. It works like this:
* `config_path_system` will use `$GIT_CONFIG_SYSTEM` because `use_env`
will be set.
* `config_path_global` will use `$GIT_CONFIG_GLOBAL` because `use_env`
will be set.
* `git_config__find_xdg` and `git_config__find_programdata` will find
impure system paths and load them even when `$GIT_CONFIG_GLOBAL` is
set, contrary to Git behaviour, so we need to set `$XDG_CONFIG_HOME`
and `$PROGAMDATA`.
It has a few other effects, which I will exhaustively enumerate to
show that they are benign:
* It respects `$GIT_WORK_TREE` and `$GIT_COMMON_DIR`. These would
already break our tests, I think, so we’re assuming they’re
not set. (Possibly we should set them explicitly.)
* When opening a repository, it will:
* Set the starting path for the search to `$GIT_DIR` if it’s
`NULL`, but we do set it, so no change.
* Initialize `ceiling_dirs` to `$GIT_CEILING_DIRECTORIES` if it’s
`NULL`, but we do set it, so no change.
* Respect `$GIT_DISCOVERY_ACROSS_FILESYSTEM` and set the
`GIT_REPOSITORY_OPEN_CROSS_FS` flag appropriately. However,
this is only checked on subsequent iterations of the loop in
`find_repo_traverse`, and we set `GIT_REPOSITORY_OPEN_NO_SEARCH`
which causes it to never enter a second iteration.
* Use `ceiling_dirs` in `find_ceiling_dir_offset`, but the result is
ignored when `GIT_REPOSITORY_OPEN_NO_SEARCH` is set, so changing
from `NULL` to the empty string doesn’t affect behaviour. (It
also would always return the same result for either value, anyway.)
- Now computes the latest tag automatically in the first suggested command.
- Now excludes dependabot.
I used the command to generate the list of contributors in the v0.28.0 release commit.
If a commit needs fixing but its descendant does not, we currently
skip rewriting the descendant, which means it will instead be rebased
when the transactions finished. That usually results in
conflicts. This adds a test case for that case.
With this template, a 'git format-patch' compatible
email message can be generated using something like:
jj show --git --template git_format_patch_email_headers <rev>
Since 7618b52b "cli: consider 'JJ:' lines as comments also when not followed by
space", lines starting with "JJ:" (without space) are also ignored. We can
simply add "JJ:" prefix to empty intro/instruction lines.
Closes#5484
This helps detect whether the last line is "JJ:" instruction or not. It seems
also nice that I don't have to insert newline to reflow the edited paragraph.
This reverts commit b8ca9ae and adds a comment.
In commit 4d42604 (a year ago), we started writing the trees involved in
conflicted commits to the Git commit object in addition to the proto
storage. We validated that the two record matched when reading commits
until as recently as f7b14be (about a week ago). Just after that, in
commit b8ca9ae, we stopped writing the tree ids to the proto storage.
That means that any version of jj between 4d42604 and f7b14be would
error fail when reading a conflicted commit created by new versions.
While we don't guarantee forward compatibility, it's easy to do so here,
so let's be friendly to older versions by rolling back b8ca9ae so we
continue to write the conflicted tree ids to both places for a while
more.
(Thanks to Martin for the detailed explanation!)
Fixes https://github.com/jj-vcs/jj/issues/6185
A subset of cli tests could fail if the system /etc/gitconfig had
configuration interfering with the tests. The cause seems to be running
of `jj` commands that would in turn use a `git` subprocess.
Fix this by setting `GIT_CONFIG_SYSTEM` and `GIT_CONFIG_GLOBAL`, like
in `hermetic_git`.
Fixes#6159
When rendering diff of conflicts, it might make sense to show diff of negative
(or base) terms with "removed"/"added" labels swapped. For example, diffs
between A and B-C+D can be rendered as
{left, right} = {A (-), B (+)}, {A (+), C (-)}, {A (-), D (+)}
by padding -A+A to the left side. To achieve that, I'm thinking of adding
labels: [&str; 2] parameter. This patch will help keep function arguments more
consistent there.
FWIW, I also tried rendering in {A (-), B (+)}, {C (-), D (+)} forms. This
seemed not intuitive because, when diffing, we compare two distinct states. In
the example above, the diff {C, D} should be considered an internal state at the
right side, not the states pair to be diffed.
I think this makes more sense because WorkspaceId is currently a human-readable
name. In error/status messages, workspace names are now printed in revset
syntax.
New WorkspaceId types do not implement Default. It would be weird if string-like
type had non-empty Default::default(). The DEFAULT constant is provided instead.
It seemed inconsistent that only GitExportError was translated to an internal
error. There are various reasons that triggers git::export_refs()/reset_head()
failure, and some of them are environmental error.
Wrapped errors are usually displayed after "Failed to import refs ...", etc., so
the context is obvious. I also removed "Internal"/"Error" for consistency.
The option will be used to prioritize branches of the `jj log` graph to
be displayed on the left.
This can make them more readable in some situations.
An example would be
```
[revsets]
log-graph-prioritize = "coalesce(description("megamerge\n"), trunk())"
```
In the diagram, E has parent B, and D has parents B and C, so the fork point of E and D
(the most downstream common ancestor of these commits) should be B rather than A.
This avoids having to escape double and single quotes used within page names. One page has double quotes in its name in the current state, and another will have a single quote in its name in the next commit.
As discussed several times before, we want to restrict permission to
approve PRs to the maintainers only. This patch adds a GitHub
CODEOWNERS file for that purpose. Once this has been merged, I'm going
to update the rulesets to make PRs requires approval from a
maintainer.
It's deprecated since 1aad25042029 "Shorten the git push branch when possible
using the short change ID hash" (2023-01-12). I don't think we need the
fallback. I also removed the check for ambiguous prefixes as I believe it
wouldn't practically matter. If needed, we can add a check for existing bookmark
pointing to different commit. We can also make it templated with default
"'push-' ++ change_id.shortest(12)".
Previously, the completions suggested the literal string:
`{f_not_yet_renamed => f_renamed}`. Instead, the old and new file names
should be completed separately.
Previously, the "renamed" file wasn't deleted at its old location, so it
wasn't renamed at all. Correcting this reveals a bug in the completions
of renamed paths. The completions suggest the literal string:
`{f_not_yet_renamed => f_renamed}`. Instead, the old and new file names
should be completed separately.
With the default config (if global `conflict-marker-style` is not
customized), this reverts the behavior to that before 7f57866332.
`vimdiff` config already shows three panes with the three snapshots for
each conflict, so it's helpful to show jj's diff view in the editing
pane.
In other words, previously the conflict was always shown in the
"snapshot" format in the main pane:
```
<<<<<<< Conflict 1 of 1
+++++++ Contents of side #1
fn has_tracked_remote_bookmarks(view: &View, bookmark: &RefName) -> bool {
------- Contents of base
fn has_tracked_remote_bookmarks(view: &View, bookmark: &str) -> bool {
+++++++ Contents of side #2
pub fn has_tracked_remote_bookmarks(view: &View, bookmark: &str) -> bool {
>>>>>>> Conflict 1 of 1 ends
```
and now it is shown in whatever format the user set as the default. If
the user didn't pick a default, the "diff" format is used in the main
pane:
```
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
-fn has_tracked_remote_bookmarks(view: &View, bookmark: &str) -> bool {
+fn has_tracked_remote_bookmarks(view: &View, bookmark: &RefName) -> bool {
+++++++ Contents of side #2
pub fn has_tracked_remote_bookmarks(view: &View, bookmark: &str) -> bool {
>>>>>>> Conflict 1 of 1 ends
```
See also the screenshot in https://github.com/jj-vcs/jj/pull/6147 to see
the main pane in the context of the three other panes that always
display the snapshots.
This allows callers to mutate RevsetParseContext if needed.
RevsetParseContext was changed to an opaque struct at 4e0abf06317f "revset: make
RevsetParseContext opaque." I think the intent there was to hide implementation
details from revset extension functions. This is now achieved by opaque
LoweringContext.
Another reason of this change is that aliases_map is no longer needed when
transforming AST to UserRevsetExpression.
I tried to minimize this patch, but it seemed rather complicated than porting
most callers all at once. Remote management functions in git.rs are unchanged.
They'll be ported separately.
With this change, many non-template bookmark/remote name outputs should be
rendered in revset syntax.
It's a bit weird that Deref is required, but this helps deduce the query type.
Maybe we can add typed StringPattern wrapper, but I'm not sure if that's a good
idea.
Alternatively, RefNameBuf could implement Borrow<str>, but that means the map
could be looked up by weakly-typed string keys.
These types are akin to ObjectId types, but for refs. Perhaps, WorkspaceId can
be ported to impl_name_type!().
These types don't implement Display because it would be source of subtle bugs
if a string-like type could be formatted differently. For example, we would
have to be careful to not format!("refs/heads/{name}") if the name implemented
a default Display in revset syntax.
We've been finding that a lot of bug reports on `jj git push` come from
sub-standard error reporting on the reasons the failure happens.
It can come from a number of places:
- hook failure
- remote branch protection
- git config
This commit forwards the reason as explained by the ouptut of git push
to help users figure out what is happening.
We need to report more complicated errors on push.
Firstly, we can have a mix of unexpected ref locations and remote
rejections. We should report both at the same time.
Second, git gives us a reason for why a push failed.
For this to work, it's relevant to refactor the current error reporting
path to allow us to inject this information.
This is to ensure two things:
1. That the evolog isn't missing the history from the target commit.
2. That the evolog doesn't include extra "temporary" commits due to
the way `jj split` is implemented.
The impetus for this is the discussion in https://github.com/jj-vcs/jj/pull/5926.
The changes added here detect the extra temporary commit added to the evolog
when `split --parallel` is run using the implementation in #5926.
In colocated repos, we set up the Git index to make `git diff` closely
match `jj diff`. However, `git diff` will not include new files. We're
long talked about using the `git add --intent-to-add` feature to make
the match closer. This patch implements that. It does so both after
updating the working copy and after snapshotting. After updating the
working copy, the new file in the working-copy commit (compared to the
parent(s)) are marked as intent-to-add. After snapshotting, newly
snapshotted files are marked as intent-to-add, and deleted ones that
were previously marked as intent-to-add are removed from the index.
Closes#6122
I'm about to set the intent-to-add flag for newly added files. It's
also somewhat useful to see the index flags for conflict stages (the
different stages).
It is important for this case to be an error, because otherwise it would
be possible to construct a non-conflicted commit which appears to have a
different tree when viewed using `jj` than when viewed using Git. This
could potentially be used to hide malicious code in commits in such a
way that on GitHub, the code would appear normal, but it would become
malicious when cloned using `jj`.
Prior to f7b14beacc678a9b351ae65224df43a96aa26392, if a commit had a
`jj:trees` header with only a single tree, it would result in a panic of
"root tree should have been initialized to a legacy id". This commit
restores the error behavior by adding an explicit check for this case.
The git remote sideband adds a dummy suffix of 8 spaces to attempt to clear
any leftover data. This is done to help with cases where the line is
rewritten.
However, a common option in a lot of editors removes trailing whitespace.
This means that anyone with that option that opens this file would make the
following snapshot fail. Using the insta filter here normalizes the
output.
It wasn't immediately obvious to me what was happening here, but once I
understood it it seemed pretty simple. Perhaps it's worth a comment to explain
it to the next reader.
I have no immediate plan, but I think we can make "jj git export" show exported
refs. The FailedRefExport type is replaced with a plain tuple since we have a
named type wrapping (symbol, reason) pairs now.
We have been writing conflicted tree ids to the `jj:trees` commit
header since 4d426049 (just over a year ago) and we have been able to
read them from there since the same commit. This patch updates the
write path so we no longer redundantly write the trees to the proto
storage.
We write the `jj:trees` commit header since 4d426049. If the
information is written to that header, we should not require it to
also be in the proto storage.
I'm going to change the key type from RefName to RemoteSymbolBuf, but the std
BTreeMap/HashMap doesn't support lookup by un-Borrow-able ref types. We can use
hashbrown::HashMap, but there aren't popular alternative for ordered maps.
Since we don't need random insertion and lookup, we can simply use Vec.
I usually don't read these messages carefully, and the red "(conflict)" label
looked scary. Suppose we don't have to take further action on resolved commits,
I don't think we need to print commit summary for each resolved commit.
This moves the default template to `builtin_draft_commit_description` and
points `draft_commit_description` to it. This makes it easier to override
the template while still being able to refer to the default.
In test_git_clone.rs, it's common to execute "jj git clone" or do file operation
at the root directory. This patch adds root_dir: &TestWorkDir references for
that reason. We might want to somehow merge root_dir and test_env later.
Like the CI runners, I've occasionally seen spikes where binary builds
on 'main' were failing. Bump a little bit to avoid that.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
We're well under the 15 minute limit at this point at the p90 case, but it
seems like the p99 build tends to run into ~10minute spikes due to underlying
runner anomalies (oversaturation?)
Let's go ahead and use a timeout of 20 minutes to give it a little more slack.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
This might be the cause of dependabot PRs not having automerge. Let's
just revert it and see what happens when the next PRs roll in.
This backs out commit 47cd10669de28ecc36f0d7dbbb9964945124b730.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
When used inside GUI application, enabling GPG or SSH signing causes console window to appear temporarily.
std::process::Command needs special flags to prevent that. More details: https://stackoverflow.com/a/60958956
When used inside GUI application, git.subprocess=true option causes console window to appear temporarily.
std::process::Command needs special flags to prevent that. More details: https://stackoverflow.com/a/60958956
This ensures that remote symbols are printed with quoting as needed. Local
bookmark names aren't quoted, which will be fixed separately by introducing
RefName(str) newtype.
This shaves over 3½ minutes off the tests, taking this from the
slowest job to faster than Windows again. The macOS 13 builders
presumably won’t be around forever, and the newer free builders are
all Apple Silicon, but this is an easy substantial improvement for now.
AFAICT, this option was needed when we're going to abandon hundreds of commits.
However, I typically notice that I had to use --summary to suppress the output
after the fact. Since the list is now truncated up to 10 commits, I don't think
the --summary flag is useful anymore. This patch also removes the special case
for 1 item, which existed primarily for overriding --summary.
The choice of the upper limit is arbitrary. We use 5 in "multiple revisions"
error, but I feel 5 would be too small for "jj absorb", where the stack of
mutable commits may be ~10, but wouldn't likely be ~100s.
Since prompt_choice_with() is now public, it should handle the default value
more consistently. parse() shouldn't be a function that can potentially feed a
default value.
We no longer print "unrecognized response" on empty input. I think this is
correct because an empty input is noop.
I think "jj next/prev" prompt can also support selection by change ID prefix.
Maybe we can also add ui.prompt_choice_range(prompt, range, default) if that's
common?
This is barely worth doing, but since I did it once...
I feel that some people might treat the simple backend as an easter egg, and
this will help them know what to expect.
This removes warning about unused SimpleBackend on non-dev build. Since
Workspace::init_simple() isn't feature-gated, it doesn't make sense to disable
the backend loading.
In recent bugs, it's been really hard to triage the behaviour from the
existing debug logs.
In particular, there have been situations where the `git` command is not
enough to triage, because the bug stems from a particular
environment/config issue. Moreover, these bugs are either transient, and
as such hard to replicate, or they only manifest when spawning `git`
from `jj` (and as such re-running the `git` command does not yield
useful information).
In those cases, seeing what the subprocessing code is seeing becomes
very much useful. This commit adds some debug logging to help with this
problem, by logging some parts of the `git(1)` output and the config.
This shaves off something like 20 to 40 seconds for Linux. It’s
seemingly within the margin of error for Windows, so not sure if
we’ll want to keep it there.
This configuration is for the CI builders, so by moving it to another
file we can add more opinionated things without risking breaking
builds for anyone else.
To avoid redundant computation, users who run `COMPLETE=fish jj | source`
in their ~/.config/fish/config.fish should eventually remove that, or move it
to ~/.config/fish/completions/jj.fish which overrides the default completions.
Ref: https://github.com/fish-shell/fish-shell/pull/11046
This adds a revert command which is similar to backout, but adds the
`--destination`, `--insert-after`, and `--insert-before` optoins to
customize the location of the new reverted commits.
`jj backout` will subsequently be deprecated.
Closes#5688.
Suppose we add "jj file annotate" option to specify the search domain or depth,
the root commit within the range can be displayed as the boundary commit. "git
blame" for example displays boundary commits with ^ prefix.
This follows up 85e0a8b0687e "annotate: add option to not search all ancestors
of starting commit."
The code in this area gets significantly simpler, since there's no
longer any ambiguity to resolve. The scm-record changes expect that any
mode changes are represented by file mode sections, and adds UI hooks to
simplify working with them (e.g. automatically de-selecting a deletion
mode change if any lines in the file are de-selected, since the file
still needs to exist to hold those lines).
If the user selects a file mode change, the new mode comes back as part
of .get_selected_contents(), so we can use this directly to figure out
whether the file was removed or just changed.
* The lib part should not deal with tools, or tool config, or running stuff in subprocesses. That stays in the CLI.
* Adds a fix_files function. This is the entry point.
* Adds a FileFixer trait with a single method to process a set of files. This is a high-level plugin point.
* Adds a ParallelFileFixer which implements the FileFixer trait and invokes a function to fix one file a time, in parallel.
* The CLI uses ParallelFileFixer.
The FileFixer trait allows environments (other than jj CLI) to plug in their own implementation; for example, a server could implement a FileFixer that sends RPCs to a "formatting service".
Just using the fastest platform should be fine for this. Hopefully it
shouldn’t slow down CI too much since it’s an independent build
job and only temporary, though a potential alternative would be to
just check the build instead. (It wouldn’t catch build regressions
in the test code, though.)
The current behaviour means (transitively) dependent crates' debug logs end up
in our logs when `--debug` is active.
The biggest offender here is `globset`, imported from the `ignore
crate`. These logs are extremely noisy and mostly irrelevant from our usecase.
This commit changes this behaviour to whitelist `jj` related debug logs,
while allowing more debugging to come from env vars
If a signing key is not configured, the user's email will be
used as the signing key. This aligns with `git`'s behavior
and allows the users to not specify the key in their configs
given that they have a key associated with their email.
Multiple user configs are now supported and are loaded in the following precedence order:
- `$HOME/.jjconfig.toml`
- `$XDG_CONFIG_HOME/jj/config.toml`
- `$XDG_CONFIG_HOME/jj/conf.d/*.toml`
Which removes the need to set `JJ_CONFIG` for a multi-file approach.
Later files override earlier files and the `JJ_CONFIG` environment
variable can be used to override the default paths.
The `JJ_CONFIG` environment variable can now contain multiple paths separated
by a colon (or semicolon on Windows).
When using old git versions, some of the options we use
(namely, --no-write-fetch-head #5933), we want to report that the error
comes from the outdated git, instead of cryptically failing.
Closes#5933
This code has been unused since 8e6e04b92.
I also deleted the associated tests. I considered porting them to
`MergedTree` but it looks like we have good coverage of these cases in
the existing `MergedTree` tests combined with the low-level `Merge`
tests.
I'm thinking of adding RefName(str) and RemoteName(str) newtypes, and the
templater type name would conflict with that. Since the templater RefName type
is basically a (name, target) pair, I think it should be called a "Ref", and I
added "Commit" prefix for disambiguation.
This isn't a breaking change since template type names only appear in docs and
error messages.
For `builtin_log_compact` you can specify how you want signatures to be
displayed via `format_short_signature`, but the same wasn't possible for
`builtin_log_compact_oneline` which always chose `email().local()`.
When we push a ref to a git remote, we use --force-with-lease
Our understanding was that this could fail iff the expected location of
the ref on the remote was not that of the local tracking ref
However, if the ref is from a protected branch (e.g., main) it will be
rejected by the remote for a different reason.
This commit solves this, by detecting this difference.
Adds a `templates.config.list` config option to control whether the
detailed list is shown or not.
The `builtin_config_list_detailed` template adds the config origin to
the end of the line for each config value in the list. Options coming
from files will show the file path.
It turns out that merge queues bundle auto‐merge for free, so we
can just do this the old way.
This backs out commit eba4257ab9a0268460108c234dc0d046efcd6c33.
Ideally we’d just cancel workflows entirely when they’re ejected
from the merge queue but apparently that’s kinda hard for some
inexplicable reason. This should be a cheap and harmless win. I
guess it gives you slightly less information if something fails
because of changes in the queue but you can just rebase your PR to
get everything running.
I don’t think `github.event.merge_queue.head_ref` actually does
anything useful, but I’m not sure if `github.ref` works for merge
groups and it seems better to have a fallback than not. I’d really
like to cancel PR workflows when a PR enters the merge queue, and
cancel queue workflows when a PR is ejected from the merge queue,
but apparently this simple task is high‐level wizardry when you’re
using GitHub Actions, so I’m postponing that for later.
This was running all the checks twice for pushes to pull requests,
unnecessarily limiting our concurrency and clogging up the status
report. The only benefit is if someone is pushing to a branch that
they don’t have a PR for and waiting for the checks to run. I
suspect nobody is doing this with regularity, but if it turns out
the functionality is important, we could just ask people to add this
back to the `.github/workflows/ci.yml` on the branches they want
GitHub to test, or add logic to try and cancel `push` runs that match
`pull_request` ones.
This allows the user to select a particular file when using multiple
configs. In the event that a prompt cannot be displayed, the first
file will be automatically selected.
std::fs::rename() and symlink() aren't added to TestWorkDir. There are few
instances of rename(). work_dir.symlink() would be a bit unclear whether the
link content path is normalized, which usually shouldn't.
This reports the status of the checks that are currently set as
required in the repository configuration, but only in the merge
queue. The advantages are twofold:
1. We can adjust required checks with our normal CI process rather
than having to bug Martin for it. (This doesn’t really place any
more trust in anyone than we do currently, because a malicious PR
could always just replace the jobs with ones that unconditionally
succeed anyway.)
2. We can make the job only ever fail in the merge queue. Currently,
we can only submit a PR into the merge queue after all its checks
pass. Those checks then immediately get run again in the merge
queue. If you do one final fix‐up to an approved PR, it takes
half an hour after that to merge instead of fifteen minutes. We
make this less painful by using auto‐merges, but it’s silly
to block on the PR checks when the actual guarantees are provided
by the merge queue.
Unfortunately GitHub demands the same jobs be required for putting
something into the merge queue and taking it out. We can work
around this by only requiring the `required-checks` job and having
it report its status depending on the context.
Tasks for Martin:
1. Go to <https://github.com/jj-vcs/jj/settings> and disable “Allow
auto-merge”. This would now only block on PR approval / discussion
resolution, so it’s probably more confusing than helpful; I’ve
hit the button and then come back an hour later to discover that
I forgot to resolve discussions.
Go to <https://github.com/jj-vcs/jj/settings/rules/3400426> and
replace all of the required GitHub Actions checks under “Require
status checks to pass” with the single `required-checks` check.
3. While you’re at it, go to
<https://github.com/jj-vcs/jj/settings/actions> and ensure that
“Workflow permissions” is set to “Read repository contents
and packages permissions”. We already handle this correctly in
all our workflows but the default is to allow write permissions
for legacy reasons.
As described in #5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.
However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.
Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This sets the
empty commit as carrying the blame for that line.
All "short" formats should be able to be combined with -p/--patch. It was also
weird that "short" diff stats followed "long" color-words/git diffs.
Fixes#5986
`jj resolve` will currently try to resolve the executable bit and will
clear it if it can't be resolved. I don't think the executable bit
should be affected by the external merge tool. We could preset the
flags on the files we pass to the merge tool and then expect the merge
tool to update the flags, but we don't do that. I'm not sure that's a
good idea (few merge tools probably expect that), but since we don't
do it, I think it's better to leave the executable bits
unchanged. This patch makes it so by calling
`Merge::with_new_file_ids()` on the original conflict whether or not
the content-level conflict was resolved.
I noticed this was while working on the copy-tracking proposal in PR
#4988, which involves adding copy information in the `TreeValue`. If
we do implement that, then we should preserve copy information here
too (in addition to the executable flag). That will automatically work
after this patch.
If we add copy info to `TreeValue::File`, we will want to preserve
that when the user runs `jj chmod`. It's easier to preserve it by
updating the `Merge<Option<TreeValue>>` in place, so that's what this
patch does.
We could replace clap's "SuggestedSubcommand" instead of removing it,
but then the user might not notice that this is a custom suggestion and
not pay enough attention to it.
This uses the just-released https://github.com/clap-rs/clap/pull/5941.
There are alternative ways to solving the problem addressed by the next
commit, but they don't seem to preserve color well. So, let's have a
test to make sure future refactorings can tell whether this issue is
still relevant.
This removes the special case for RefName::LocalBranch(_), which can be
processed as a remote bookmark.
"jj git import" now prints local bookmarks and tags with @git suffix. I think
this is more correct since these refs are imported from the backing Git
repository.
The FAQ entry explaining why `jj log` doesn't show all commits
explained that the behavior is configurable but it didn't explain what
the rationale for not showing all commits is. Users coming from Git
are used to seeing all commits and probably read this FAQ entry to
find an answer. We don't want them to just update their config without
understanding why we have the default we have.
Suppose revsets and filesets are primarily used in command shell, it would be
annoying if quoting is required in addition to the shell quoting. We might also
want to relax the revset parser to allow bare * in glob string.
Closes#2101
- Swapped `file:"path"` and `cwd-file:"path"` for consistency with the first line
- Created a labeled section for quoting rules
- Elaborated on the quoting rules in the beginning of file pattern section.
If `--to` is going to become a required argument, it should
have a short alias as it will be used quite frequently.
Given that `--to` has a short alias it only makes sense to
allow `-f` for `--from` as this is consistent with other
commands and nothing makes this particular command special.
It's super common to pair test_env with repo_path.
Some of the tests still use absolute paths because it seemed rather confusing
to mix paths relative to the repo_root and to the env_root.
I'm going to reimplement git_ref_filter to process translated remote bookmark
names, and "git" remote will mean the local Git-tracking remote there. The
reserved remote name is checked prior to filtering because refs in that remote
cannot be represented as remote symbols.
I originally implemented the error handling the other way because we didn't
have a machinery to report partial import failure. Now we have stats, it's
easy to report skipped ref names.
I think this will help implement sorting options. Untracked remote bookmarks
should be sorted independently, whereas tracked remote bookmarks should always
be listed after the associated local bookmarks.
Some of these will be called from "jj bookmark list". The template RefName type
is useful as an abstract local/remote ref object. Maybe it should be renamed to
RefEntry or CommitRef.
I'm about to move the `create_commit()` helper to a common
place. However, this version of `create_commit()` is different from
the others in that it put a prefix of "descr_for_" in the
description. This patch removes that and instead updates the template
so it's still clear what's a description and what's a bookmark name.
This is hidden gem of a feature, especially in combination.
`JJ_CONFIG` being a directory allows loading multiple TOML config files.
`--when` can be used on the top level, which can lend itself to a much
cleaner config than `[[--scope]]` tables.
ui.progress_output() doesn't borrow anything from &Ui, and there would be no
reason to extend the borrow. Write ops on Ui don't require mutable instance.
The paste crate hasn't had a maintainer since Oct. 2024. We don't use it
directly, but it's used by ratatui, and there's no direct replacement for it
AFAIK.
Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0436
This is currently blocking PRs because ci fails:
```
error[unmaintained]: paste - no longer maintained
┌─ /github/workspace/Cargo.lock:262:1
│
262 │ paste 1.0.15 registry+https://github.com/rust-lang/crates.io-index
│ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ unmaintained advisory detected
│
├ ID: RUSTSEC-2024-0436
├ Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0436
├ The creator of the crate `paste` has stated in the [`README.md`](https://github.com/dtolnay/paste/blob/master/README.md)
that this project is not longer maintained as well as archived the repository
├ Announcement: https://github.com/dtolnay/paste
├ Solution: No safe upgrade is available!
├ paste v1.0.15
└── ratatui v0.29.0
└── scm-record v0.5.0
└── jj-cli v0.27.0
└── (dev) jj-cli v0.27.0 (*)
advisories FAILED
```
It's odd that "foo/bar" is tested earlier in push_branches() whereas "git" is
rejected by push_updates(). Suppose push_updates() is a low-level function to
push arbitrary refs without updating jj's repo view, it's probably okay to allow
unsupported remote name here.
This change moves the code for prompting the user to select changes for the
first commit to a helper function. There are no functional changes.
Resulting from the discussion in https://github.com/jj-vcs/jj/pull/5828
we are planning to add several new flags (-B, -A, and -d) to jj split.
Since this may change how the user selects changes (i.e. they may select
changes to remove from the target commit, rather than what stays in the target
commit), I want to move the selection logic to a function to make the main
split function more readable.
This is part of a refactor of cmd_split in preparation for adding new -A, -B,
and -d arguments to control where the changes split from the target commit end
up.
The goal is to simplify the flow in the main cmd_split function and manage
complexity introduced by the new options. In this case, the raw arguments will
be parsed once at the beginning of the command in resolve_args, after which the
raw SplitArgs are no longer used.
The function is pretty simple now, but will evolve once the new arguments are
added.
Also resolves one TODO made possible by the new MSRV
Most of this was done by enabling the lint forbidding `allow` directives
using `cargo cranky`, running `cargo cranky --workspace
--all-featuers --fix`, and fixing up the result.
The CI seems to correctly use rustc 1.84.1 (and not 1.84.0) with this.
For reference, we last updated the MSRV to 1.76 in 5b517b5. According to
https://releases.rs/docs/1.76.0/, this was when it barely became stable.
`flake.nix` seems to be using a nightly toolchain now, so it seems there
is no need to update the version there.
The more precise clippy command used was:
cargo clippy --workspace --all-targets --fix
It was requested on Discord once.
The result is almost a tutorial; we could consider actually moving it into the
tutorial and providing a link in the FAQ. That could be done separately.
The `jj` commands outputs are faked in a few, hopefully inconspicuous, ways.
Hopefully, this is not distracting.
Typo in signing.behavior shouldn't be ignored.
The idea is the same as [user] and [operation] tables. It's easier if all
parameters needed to create a commit is parsed by UserSettings constructor.
Another option is to make repo.start_transaction() fail on config error.
It is not needed to gate `jj debug init-local` on an option now that the
command is no longer called `jj init`.
This also includes a separate fixup to #5845 in demos/
The main goal here is so that a completely new user that types in `jj
init` or `jj clone` gets a pointer to the correct command needed to try
out `jj`.
Now that we depend on the `git` executable being available for `jj git
fetch/push` tests, we might as well use it for `jj util gc` tests
too.
I also switched to using the Git backend in
`cli/tests/test_file_track_untrack_commands.rs`, which seemed to be
using the local backend for no good reason.
The gix feature "blocking-network-client" was configured in cd6141693
(cli/tests: add gitoxide helpers, 2025-02-03) most likely because
we needed to clone using gix in tests, but 071e724c1 (cli/tests:
move test_git_colocated_fetch_deleted_or_moved_bookmark to gitoxide,
2025-02-25) changed the test to clone by subprocessing out to system git
(not by using gitoxide, as a cursory read of the commit description may
indicate), meaning that we no longer need that feature.
Therefore, remove that feature.
The warning this suppresses is shown by `mkdocs serve`:
```
INFO - The following pages exist in the docs directory, but are not included in the "nav" configuration:
- paid_contributors.md
```
We can alternatively add that doc to nav.
This will be used to compute the location to place commits from the
`--destination`, `--insert-after`, and `--insert-before` options. This
is used across the `new`, `duplicate`, and `rebase` commands, and can be
used for further commands as well.
The previous implementation of `new --insert-after` excluded children
that were ancestors of the new commit. This was done to avoid creating
cycles in the commit graph. However, this does not align with the
behavior of the `--insert-after` in the other commands, which explicitly
error if inserting a new commit after the given `--insert-after` commits
would create a cycle.
For consistency, and given that the desired behavior can be achieved
using by using `--insert-after` along with `--insert-before`, this
behavior is removed.
After system crash, file contents are often truncated or filled with zeros. If
a file was truncated to empty, it can be decoded successfully and we'll get
cryptic "is a directory" error because of an empty view ID. We should instead
report data corruption with the ID of the corrupted file.
#4423
The consensus about this change, if one ever existed, seems to have dissolved
into two cohorts, one which is happy with the change, and one which thinks we
should move both the bookmark and change id to the child commit instead of
leaving them on the parent.
We may decide to add flags to allow users to choose between the two behaviors,
but there are also other concerns such as where @ should go (parent or child).
Until we agree on a path forward it seems reasonable to delay the breaking
change by disabling it via the config option we added. I don't think it's
necessary to fully revert the code and new tests since we aren't announcing the
option.
#3419
This has bothered me for a while... It's useful to describe the target of the
split command as the "target commit", and it's a little confusing to have a
variable named "commit" in the code when we are dealing with several different
commits in the command.
I think this improves readability by being more precise.
Clearing a sign key would require calling `set_sign_key(None)`, which
makes the API slightly awkward.
Instead, we introduce `clear_sign_key()` and make `set_sign_key()` only
accept a `String`. This makes clearing the sign key explicit.
Co-Authored-By: Yuya Nishihara <yuya@tcha.org>
When signing commits with `jj sign`, one might want to use a workflow
like:
```bash
jj fix && jj sign .. && jj git push
```
Making the default value for `-r`/`--revisions` configurable, will allow
such a workflow.
Co-Authored-By: Yuya Nishihara <yuya@tcha.org>
This will probably mitigate problems of concurrent updates. If two concurrent
processes tried to import + reset HEAD, one of them should fail.
Closes#3754
I think I likely found the issue. zizmor seems OK
with persisting credentials, see
https://github.com/jj-vcs/jj/actions/runs/13559693565/job/37900455060?pr=5820
Both of these workflows run only on commits in `main`,
so this doesn't seem like a huge security hole, but
we could consider other, better solutions in the
future.
Follow up to 78177ff. See #5819 for a failed attempt.
cc @thoughtpolice @neongreen @martinvonz
It's been broken since 514a009. Since 0e2d079,
the error was:
```
error: failed to push branch gh-pages to origin:
"fatal: could not read Username for 'https://github.com': No such device or address"
```
This is hard to test outside of prod, but follows a guess after
reading some Github docs and `gh auth login` docs from
<https://cli.github.com/manual/gh_auth_login>. I could not
find docs specifically about using the `git` CLI, but I'm guessing
that if `gh auth login` makes `git push` work on user machines by
setting `GH_TOKEN`, doing the same might help.
🤞
Cc @thoughtpolice, @martinvonz
I'm going to fix some CLI outputs to print remote symbols in revset syntax, and
it's easier if the view API took/returned type that represents a ref name.
.as_ref() and .to_owned() are implemented as non-trait methods because
RemoteRefSymbol can't be a true reference type. I think it's similar to
Option::as_ref(). I couldn't find any std examples of non-trait .to_owned(),
though.
The output of `jj unsign` is based on that of `jj abandon`.
We output warnings when unsigning commits, which are not authored by the
user. This is encouraging to use `jj undo`, in case one unintentionally
drops signatures of others.
---
Co-authored-by: julienvincent <m@julienvincent.io>
Co-authored-by: necauqua <him@necauq.ua>
We always sign commits. This means commits, which are already signed,
will be resigned. While this is cumbersome for people using hardware
devices for signatures, we cannot reliably check if a commit is already
signed at the moment (see https://github.com/jj-vcs/jj/issues/5786).
We output warnings when signing commits, which are not authored by the
user. This is encouraging to use `jj undo`, in case one unintentionally
signs commits of others.
The output of `jj sign` is based on that of `jj abandon`.
---
Co-authored-by: julienvincent <m@julienvincent.io>
Co-authored-by: necauqua <him@necauq.ua>
In https://github.com/jj-vcs/jj/actions/runs/13490171888, we got this error:
```
Invalid workflow file: .github/workflows/scorecards.yml#L48
The workflow is not valid. .github/workflows/scorecards.yml (Line: 48, Col: 9): Unexpected value 'permissions'
```
It looks like permissions can be set per job but not per step, so I
moved the permission setting to the job level.
This means that watchman is partially disabled until the user cleans up
untracked files. This isn't nice, but should be better than hiding untracked
files forever.
Fixes#5728Fixes#5602
There have been a number of users confused about why
their commits are immutable, or what to do about it, ex.
[https://github.com/jj-vcs/jj/discussions/5659].
Separately, I feel that the cli is too quick to suggest
`--ignore-immutable`, without context of the consequences. A new user
could see that the command is failing, see a helpful hint to make it not
fail, apply it and move on. This has wildly different consequences, from
`jj squash --into someone_elses_branch@origin` rewriting a single commit,
to `jj edit 'root()+'` rewriting your entire history.
This commit changes the immutable hint by doing the following:
* Adds a short description of what immutable commits are used for, and a
link to the relevant docs, to the hint message.
* Shows the number of immutable commits that would be rewritten if
the operation had succeeded.
* Removes the suggestion to use `--ignore-immutable`.
Previously, tools invoked by `jj fix` did not have their
`.current_dir()` set, and would just run from whatever directory the
user was in.
Now, the tools will always be invoked from the same directory that
`jj root` gives.
As a motivating example, consider a configuration path that's always at
the workspace root:
```toml
[fix.tools.something]
# previous:
command = ["cmd", "--config", "$root/tool.toml"]
# ^^^^^^ not possible
# now:
command = ["cmd", "--config", "./tool.toml"]
# ^^ now, just use a relative path
```
Aliases and `ui.default-command` are loaded before `--config` and
`--config-file` arguments are parsed (#5282).
There is supposed to be a warning when the result of these arguments
would change the parsed args: #5286.
However, that warning was not being printed if the arguments failed to
parse due to an unrecognized subcommand. Example:
```bash
# correct:
$ jj --config ui.default-command=nonsense
Warning: Command aliases cannot be loaded from -R/--repository path or --config/--config-file arguments.
# previous, confusing:
$ jj --config aliases.some-alias=nonsense some-alias
error: unrecognized subcommand 'some-alias'
# now:
$ jj --config aliases.some-alias=nonsense some-alias
Warning: Command aliases cannot be loaded from -R/--repository path or --config/--config-file arguments.
error: unrecognized subcommand 'some-alias'
```
Move the `track` specific logic to track.rs, let print_snapshot_stats
handle just the default case.
Previously print_snapshot_stats would get called twice and could warn
twice about a file (if that file was both in the auto-track fileset as
well as in the fileset passed to `jj file track`), so now we merge the
snapshot stats and display appropriate hints for each file.
Snapshot stats were automatically printed when calling
`WorkspaceCommandHelper::snapshot_working_copy`. This has been moved up
the call chain to allow more choice in when to actually print stats.
These functions used to trigger printing of the snapshot stats via
(direct or indirect) calls to `snapshot_working_copy`:
- CommandHelper::workspace_helper_with_stats
- CommandHelper::recover_stale_working_copy
- WorkspaceCommandHelper::maybe_snapshot_impl
This means we can now chose not to print the snapshot stats if we know
we will perform another snapshot operation during the same command, to
reduce the number of warnings and hints shown to the user.
Calling `workspace_helper`, or `maybe_snapshot` instead of
`workspace_helper_with_stats` and `maybe_snapshot_impl` still prints
stats automatically.
This fixes real dependency cycle that would expose slightly different versions
of jj_lib types to unit tests, one from super::* and another from testutils::*.
The testutils dependency can be removed by splitting lib/tests to a separate
crate.
This should help prevent misuse of "jj bookmark set" with e.g. remote bookmark
name. A remote-looking bookmark can be created by quoting the name if needed.
This patch doesn't change the parsing of name patterns. For patterns, I think
it's better to add support for compound expressions (e.g. `glob:foo* & ~bar`)
rather than adding revset::parse_string_pattern(text) -> StringPattern.
Closes#5705
These helpers are going to be needed to port the git2 code in the lib
tests to gitoxide. Since the cli tests already depend on testutils, this
helps with avoiding duplicating the code
It's easier to review insta snapshot diffs than scanning panic log containing
lengthy output. I think this will also help printf debugging.
test_global_opts.rs is migrated to new API as an example.
This was originally a deprecation until we made it the default behavior in
https://github.com/jj-vcs/jj/pull/5697. Since the default behavior has changed,
we should list this a breaking change.
The "conflict" label is supposed to highlight "(conflict)" markers in commit
template. This patch just removes the label as we don't colorize the same
message in update_working_copy().
I'm going to add "[EOF]" marker to test that command output is terminated by
newline char. This patch ensures that callers who expect a raw output string
would never be affected by any normalization passes.
Some common normalization functions are extracted as CommandOutputString
methods.
Although this behaviour is accepted by git, it's a degenerate case.
Especially because we implicitely rely on being able to parse out the
remote from the refname (i.e., `refs/remotes/<remote>/<branch>`).
Branches can have forward slashes, but if remotes can also have them,
parsing the refname becomes ambiguous, and a pain. Especially because it
would be totally legal to have a branch "c" on remote "a/b" and a branch
"b" on remote "a".
Fixes#5731
In the case where the user has set `snapshot.auto-track` to something
other than `all()`, running `jj st` with a higher file size set just for
that command will not actually fix the user's problem, `jj file track`
needs to be called instead.
Some working-copy implementations, like the Google CitC one, may want
to update the VFS's state even if the tree didn't change. In
particular, when updating between two commits with the same tree but
different mainline base, we want to update the base commit. Otherwise
a future snapshot, which we do relative to the mainline commit, may
notice an incorrect set of changed file.
When updating the working copy results in no changes, we currently
return `None` from `cli_util::update_working_copy()`. However, a no-op
update is still an update, so I think it makes more sense to always
return a `CheckoutStats`. We already skip printing stats if they're
zero.
Forgetting remote bookmarks can lead to surprising behavior since it
causes the repo state to become out-of-sync with the remote until the
next `jj git fetch`. Untracking the bookmarks should be a simpler and
more intuitive default behavior. The old behavior is still available
with the `--include-remotes` flag.
I also changed the displayed number of forgotten branches. Previously
when forgetting "bookmark", "bookmark@remote", and "bookmark@git" it
would display `Forgot 1 bookmarks`, but I think this would be confusing
with the new flag since the user might think that `--include-remotes`
didn't work. Now it shows separate `Forgot N local bookmarks` and
`Forgot M remote bookmarks` messages when applicable.
The condition we were checking (which I suggested during review) is
very broken. It checks if the new view is equal to view before the
current operation, but that's supposed to always be true (except for
operation that update remote bookmarks). The reason the warning didn't
trigger all the time was that we did the comparison after calling
`merge_view()` but before calling `rebase_descendants()` (via
`tx.finish()`). The former only records commits as abandoned without
actually hiding them. However, because of [the hack][hack] in
`merge_view()` to make it work reasonably well in huge repos, every
undo operation at Google would print the warning.
This patch fixes the bug by checking if the to-be-undone operation is
an undo by comparing its view to its grandparent's view.
[hack]: ec6f8278fd/lib/src/repo.rs (L1693-L1708)
After even more discussion on Discord, we decided to use the new bookmark
behavior immediately and only print a warning during `jj split` if the user has
opted out of the new behavior using the `split.legacy-bookmark-behavior` config
setting.
The reasoning is that if the behavior change breaks someone's workflow or is
very disruptive, they are likely to check the changelog and learn about the
config option. For users that are not adversely impacted, printing a warning
that can only be silenced by changing their config is also disruptive.
#3419
Allows:
* self.commit()
* self.line_number()
* self.first_line_in_hunk()
Certain pagers (like `delta`), when used for `git blame`, only show the
commit information for the first line in a hunk. This would be a nice
addition to `jj file annotate`.
`jj file annotate` already uses a template to control the rendering of
commit information --- `templates.annotate_commit_summary`. Instead of
a custom CLI flag, the tools necessary to do this should be available in
the template language.
If `1 % 2` or `1.is_even()` was available in the template language, this
would also allow alternating colors (using `raw_escape_sequence`).
Example:
```toml
[templates]
# only show commit info for the first line of each hunk
annotate_commit_summary = '''
if(first_line_in_hunk,
show_commit_info(commit),
pad_end(20, " "),
)
'''
```
This removes some dependencies on very old versions of the `dirs`
crate and a few others. Thanks to @quark-zju for getting a new
`sapling-streampager` released.
There's currently no way to tell what `jj fix` is doing. This adds some
traces such that `jj fix --debug` will at least tell you what's being
run and what it's exit code is, similar to other subprocessing code.
This was discussed in the Discord a while ago, and this is the logical and consistent
conclusion. Implementing it as such makes it consistent with both `jj edit` and `jj new`
which make hidden commits such as predecessors visible.
This actually was Martins work, I just added the tests.
Co-Authored-by: martinvonz <martinvonz@google.com>
In theory this could be exploited by a contributor who pushes a tag with a
specifically crafted name in order to exfiltrate a `GITHUB_TOKEN` that has
permissions on the jj repository. This token would be scoped narrowly ideally,
but...
Today, Martin does releases manually, so this workflow currently does not e.g.
have the ability to upload a compromised package to crates.io. However in the
future we'd like to have the release done automatically via workflow as well,
which would make this potential injection vector catastrophic if the injection
was possible in a step with a crates.io API token available.
Found by `zizmor`.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
The dependabot workflow already specifies the exact permissions it needs within
the workflow steps, so there's no need to enable any default permissions.
Found by `zizmor`.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
This stops issuing overly broad permissions to the entire workflow and instead
scopes them to the necessary steps in the job.
Found by `zizmor`.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
It only occurred to me after I rebuilt jj and actually had to run the command
that we should probably recommend a change to the user config instead of the
repo config.
#3419
This adds a record of all contributing employees of companies who pay
for contributions. The purpose is to help make possible conflicts of
interest easier to spot. As far as we know, only Google currently pays
right now.
Currently, `jj split` moves bookmarks from the target revision to the second
revision created by the split. Since the first revision inherits the change id
of the target revision, moving the bookmarks to the first revision is less
surprising (i.e. the bookmarks stay with the change id). This no-implicit-move
behavior also aligns with how `jj abandon` drops bookmarks instead of moving
them to the parent revision.
Two releases from now, `jj split` will no longer move bookmarks to the second
revision created by the split. Instead, local bookmarks associated with the
target revision will move to the first revision created by the split (which
inherits the target revision's change id). You can opt out of this change by
setting `split.legacy-bookmark-behavior = true`, but this will likely be
removed in a future release. You can also try the new behavior now by setting
`split.legacy-bookmark-behavior = false`.
Users who have not opted into the new behavior via the config setting will see
a warning when they run `jj split` informing them about the change. The default
behavior be changed in the future.
The `jj split` tests for bookmarks are updated to run in all three configurations:
- Config setting enabled
- Config setting disabled
- Config setting unset
#3419
It's been about five months, and it seems best to occasionally try
updating. Let's see if this causes any problems.
There was also a minor breaking change to
`mkdocs-include-markdown-plugin`, I added an option to keep
the old behavior (which seems nice in this case).
To do these kinds of updates, `uv tree --outdated` is useful.
`gix::Repository` instances are larger than 1KB on 64-bit machines. This
makes Clippy warn about an unbalance between variants of `GitFetchImpl`:
the `Git2` variant requires 8 bytes, while the `Subprocess` variant
requires 1128 bytes.
Boxing the `gix::Repository` makes `GitFetchImpl` instances smaller, as
well as the other structs embedding them such as `GitFetch`.
The fix settings weren't indented consistent with the rest of the settings,
causing them to appear in the redacted snapshot in the test_util_config_schema
test.
In addition to a single string, the `ui.editor` and `ui.diff.tool` options also
allow arrays of strings. `ui.pager` allows arrays of strings as well as a nested
table describing both the command and the environment.
Update config-schema.json to allow all of these types of values.
Fixes#5617
We are planning to add a config option that controls how bookmarks and change
ids move during `jj split` based on feedback in https://github.com/jj-vcs/jj/pull/5618.
I think the tests will be more readable after the config option is added if we
move the bookmark testing to its own test.
#3419
Since git CLI appends " " to sideband messages to overwrite the previous
line, we need to undo that. Our GitSidebandProgressMessageWriter expects
original sideband messages.
"$comment" is not officially supported by draft-4, but it is in newer
drafts, and JSON schema readers are supposed to skip fields that
are not known to them.
This makes the schema a bit less smart, but there's a good chance
`taplo` could not use "propertyNames" anyway because that comes
from a newer version of the schema than `taplo` officially supports
(see the child of this commit).
Fixes#5607
With this change a warning is shown if the user does not explicitly specify the target revision, but the behavior is unchanged (it still defaults to the working copy).
In the future the warning will be turned into an error. In other words, it will be required to specify target revision.
The bulk of the changes here are to prepare tests for the upcoming change, to make the transition easier.
For additional details please see:
* https://github.com/jj-vcs/jj/issues/5374
* https://github.com/jj-vcs/jj/discussions/5363
The CI system is variously hitting the limits of the existing 15 minute
build mark; in particular, Windows seems to be consistently hitting ~14m
so the chance of a runner timing out gets high if the underlying load is
also high.
While I'm working on this, let's just (regrettably) increase the slack
on the timeout so that everyone else isn't disturbed.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
'ci' is much more high-level and accurately explains what the jobs in
this test should be for. It's much more applicable than 'test' (testing
what? on what platforms?) or 'build' (does it only build? or test? or
run linters?)
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Just a rename; now that `build` and `build-nix` are unified, this makes
it more clear this workflow is conceptually separate. NFC.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
There's no real reason to keep these separate and it makes the build UX
easier to navigate, too; as well as see what the actual "global" slowest
builds across all workflows easier to see.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
This makes the methods and the madness a little more consistent with
each other, so it's easier to read and categorize build reports in the
GHA UX.
No functional change, but requires updating rulesets upstream.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Given the previously‐stated intention of making this default
for the 0.27 release, prepare for that decision ahead of time by
enabling subprocessing by default on trunk. This will help surface
any regressions and workflow incompatibilities and therefore give
us more information to decide whether to keep or revert this commit,
without inconveniencing any users who haven’t already opted in to
the bleeding edge.
Please feel free to revert without hesitation if any major issues
arise; this is not intended as a strong commitment to enable this
option for the next stable release if it turns out to not be ready. In
that case, it’s better that we learn that early on in the cycle,
rather than having to revert at the last minute or, worse, cutting
a stable release that we later find contains a serious regression.
This is the result of a lot of back and forth, the weekly efforts of the
governance working group, consisting of:
- Martin von Zweigbergk (martinvonz)
- Waleed Khan (arxanas)
- Emily Shaffer (nasamuffin)
- Austin Seipp (thoughtpolice; yours truly)
Many thanks as well to emeritus member Khionu Sybiern, who helped kickstart this
whole process.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
When a commit is split, the second commit produced by the split becomes the
working copy commit for all workspaces whose working copy commit was the target
of the split.
This commit adds two tests for this behavior:
1. Split a commit which is the working copy commit for two workspaces.
2. Split a commit which is the working copy commit for only one of the two
workspaces.
These tests check that the working copy commits for the workspaces are updated
correctly. Both parallel and non-parallel splits are tested.
See the discussion in #5623. This prevents future accidents if member variables
are added to RewriteRootCommit which need to be propagated to EditCommitError
during conversion.
The example became incorrect when `jj config set` stopped promoting
values with apostrophes to strings in 3f115cb. Use a TOML literal
string to fix it.
The previous name violated the "snake case" convention for variable names. I
suspect the intention was to use `RewriteRootCommit` as a type declaration, not
the argument name. Since the argument isn't used, we can use an underscore as
the name and leave the type declaration.
#cleanup
This makes the pre-release header bar into a steely blue-grey color and
changes its label, while the header for the stable docs (and past
release docs) remains indigo (blue). The goal is to make it easier to
tell prerelease docs and regular docs apart with a glance. I also
considered using a favicon with a different background, but perhaps
that's not as useful as changing the header color.
See https://github.com/jj-vcs/jj/pull/5614 for screenshots and
https://squidfunk.github.io/mkdocs-material/setup/changing-the-colors/#primary-color
for some more possibilities. "Deep purple" or "brown" are the closest
runners up to "blue grey" in my mind.
This can be tested with
```
MKDOCS_PRIMARY_COLOR="blue grey" MKDOCS_SITE_NAME="Jujutsu docs (prerelease)" uv run mkdocs serve
```
-----------
We could also want to make the historical versions more easily
distinguishable from the `latest` docs, but that would be more
difficult. We'd either need to rely on javascript or to rebuild the
*previous* version of the docs in addition to the new one on every
release.
See [mkdocs-material banner docs] for a javascript apporach (which would
still need some care in implementing, since we probably want the
prerelease version to either not have a banner or to have a different
banner).
[mkdocs-material banner docs]:
https://squidfunk.github.io/mkdocs-material/setup/setting-up-versioning/?h=version#version-warning
there are many ways to implement `heads` for your custom backend:
one involves calling `self.evaluate_revset` with a proper revset.
However, it can return an error, but `heads` does not allow it.
In our implementation of the index backend we do exactly the above
and we ended up with several unwraps which we are trying to avoid
as it could potentially crash our prod jobs.
P.S. The same logic applies to many methods in this trait, but I am doing one at a time.
One common issue that users run into with `jj undo` is that they expect
it to behave like an "undo stack", in which every consecutive `jj undo`
execution moves the current state further back in history. This is
(currently) an incorrect intuition - running multiple `jj undo` commands
back-to-back will only result in the previous non-undo change being
reverted, then un-reverted, then reverted, and so on.
This change adds a hint when the user runs `jj undo` multiple times
consecutively, suggesting that they may be looking for `jj op restore`,
which allows them to revert the whole state of the repository back to
a previous snapshot.
The previous "on this page" statement is wrong more often than not.
Unfortunately there is no "Report a vulnerability" button on
https://github.com/jj-vcs/jj/security/policy, and looking for such a
button from https://github.com/jj-vcs/jj?tab=security-ov-file leads to
confusion.
This is not the end of the world, but I don't see much security downside
to clarifying it (that is, I don't think *not* having a link protects
against phishing in any real way).
If an ellipsis arg is given to the truncate_* template functions, append (or
prepend) the ellipsis when the template content is truncated to fit the maximum
width.
Fixes#5085.
We no longer need to do cross compilation, and these instances seem
pretty fast. Enable them everywhere across the board.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Since we need to scan directory entries recursively in order to detect new
untracked paths, it doesn't make sense to reuse the .gitignore code path.
This change means all untracked file paths are listed in "jj status" even if
the whole directory is untracked. It might be a bit verbose, but should be
okay. Directories like node_modules should be excluded by .gitignore, not by
auto-track pattern.
Fixes#5389
The commit guidelines feel like they're relevant before documentation
about the code review process - they could conceivably apply while
developing a change.
These often seem to get missed and raised during reviews - giving them
their own section may increase visibility.
In the future, we could consider adding a few more
special configs, e.g. `:pagerenv` that strictly uses
the `PAGER` environment variable, and `:unix` that
mimics Git's algorithm of trying `PAGER`, defaulting
to `less`, and setting `LESS`.
I originally considered adding `stats() -> DiffStats` which returns an
unprintable object, with deprecation of `.stat(width)` in favor of
`.stats().<method_to_render>(width)`. However, I couldn't find a good name for
the rendering function. This patch instead made the width parameter optional. I
think that's good because template language doesn't have to be overly strict.
Closes#4154
This will help write template based on diff.stat() result. #4154
show_diff_stats() isn't reimplemented as DiffStats method because I think
DiffStats can be moved to jj-lib.
Total aadded/removed counts will be provided as DiffStats methods. This change
might result in slightly bad perf, but that wouldn't matter in practice.
Add a new pad_center function that centers content within a minimum
width. If an odd number of fill characters is required, the trailing
fill will be one character longer than the leading fill.
Fixes#5066.
The original description suggests jj somehow converts line endings, when in fact all it
does is commit files as is (as described in the previous paragraph), whether the line
endings are CRLF or LF.
We've been using `uv 0.4` syntax, but we only support `uv` 0.5.1+. The
newer `uv 0.5` syntax follows an approved PEP, and the older syntax will
probably be removed at some point.
I also updated a few comments.
`jj amend` is discouraged and no longer a visible alias of `jj squash` since 6d78d92. This
is the only remaining occurrence of it in the docs. We should recommend using `jj
squash` directly instead.
With git.subprocess = true, it's more important to skip unneeded remote
operations. Each remote command may involve user intervention if authentication
requires manual step.
This change also means that the remote connection is no longer reused in git2
impl. I think the added cost is acceptable. The git2 impl will hopefully be
removed soon, and the remote branch name is needed only when cloning new repo.
E.g. when the user doesn't have the default command set, you don't want
to get
```
Warning: Cannot define an alias that overrides the built-in command 'log'
```
printed on every completion.
This helps sideband writer overwrite the progress bar. Suppose progress
information is less important than sideband messages, it should be okay to
always overwrite the progress bar. The sideband writer will erase the trailing
characters and move the cursor back to the first column, and/or put "\n"
accordingly.
The current comment uses `[noeol]`, which can be difficult to
understand. In this commit, the brackets are changed to parentheses to
make it clear that there is no semantic meaning to the comment, and the
wording is changed to be more clear to the user.
Git doesn't show this information in their "diff3" style either, and it
looks a bit strange having two different bracketed sections of text next
to each other, so I think it would be better to remove it.
When we have e.g. a bookmark name or a remote name and we want to
produce a revset from it, we may need to quote and escape it. This
patch implements a function for that.
Perhaps we'll want to more generally be able to format a whole
`RevsetExpression` later.
With this and "git --progress" PR #5519, remote git progress will be displayed.
The sideband callback is disabled in git2 code path if progress is not enabled.
If this were enabled, most "git fetch" tests would fail with git2 due to remote
progress messages. Since there's no git2 API to turn the remote progress off,
and the git2 code path is supposed to be phased out, I just inserted ad-hoc
workaround.
This patch adds thread-based implementation. On Unix, we can poll pipes without
using thread, but I don't think platform-dependent optimization is needed.
The meaning of `[noeol]` might not be immediately clear to a user, so it
would be good to document it on our documentation page for conflicts. We
may also want to improve the conflict marker comments we use for this
case in the future (possibly after receiving user feedback).
This will be more robust long term since e.g. native ARM runners use
`ubuntu-24.04-arm` as their runner image, and the version number really
isn't a big deal in this case.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Overall, building in the test profile should significantly speed up
the overall build pipeline because so many less cycles are spent (on
GHA runners that are certainly at high load). The goal here is to help
reduce CI flake outs due to things timing out; I suspect part of the
problem may be a lot of the ~15 minute time limit being used up just
compiling things.
This is a partial revert of b714592952400ab, which removed this previous
override of the Flake `checks`.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Some of our builds have been timing out, and one angle I want to look at
is whether any tests are hanging. Nextest can help us keep track of slow
tests in CI where the underlying hosts will have significantly higher
load. In general this should also help keep our tests healthy, IMO.
I don't see any reason why any existing test should take over 20
seconds, but it should at least help control for really slow runners
that have huge latency spikes. we can start there and see how it goes
in practice.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
* Make the new `GitFetch` api public.
* Move `git::fetch` to `lib/tests/test_git.rs` as `git_fetch`, to minimize
churn in the tests. Update test call sites to use `git_fetch`
* Delete the `git::fetch` from `lib/src/git.rs`.
* Update `jj git clone` and `git_fetch` in `cli/src/git_utils.rs` to use
the new api directly. Removing one redundant layer of indirection.
* This fixes#4920 as it first fetches from all remotes before `import_refs()`
is called, so there is no race condition if the same commit is treated
differently in different remotes specified in the same command.
Original commit by @essiene
The primary goal is to control the version of the `codespell` Python
package that we run via `uv.lock`. See also
https://github.com/jj-vcs/jj/pull/5425.
Also move the action next to the `cargo fmt` action. It might make sense
to split both of them out of `build.yml` if it gets too long, but I
think they should be next to each other since they are so similar in
spirit.
Fixes#5490 (we can catch other instances of this manually)
Note that it links to the `latest` even if you are looking at the
`prerelease` docs. This is not ideal, but seems annoying to fix.
`ui.prompt_password()` already adds ": " to the end of the prompt.
Remove the extra at the (only) callsite, to keep similarity to the above
`terminal_get_pw()`.
A missing newline would cause the following conflict markers to become
invalid when materializing the conflict, so we add a newline to prevent
this from happening. When parsing, if we parsed a conflict at the end of
the file which ended in a newline, we can check whether the file
actually did end in a newline, and then remove the newline if it didn't.
File conflicts have been simplified during materialization since 0.19.0,
so it should be safe to remove support for unsimplified conflicts now.
This will make implementing the next commit easier, since it won't have
to support unsimplified conflicts.
I originally thought we could just reinitialize RepoLoader, but it wasn't
enough. Since LocalWorkingCopy holds Arc<Store>, we need to reinstantiate both
working copy and repo loader.
As I said, I don't have strong feeling about the current behavior, and appears
that "log | head | reverse" is preferred over "log | reverse | head".
"jj evolog" already behaves differently, so I just updated the doc.
Note that the new behavior might be slightly different from git, but nobody
would care. (iirc, in git, topological sorting comes later.)
Closes#5403
Currently, the Git subprocess tests only work on Linux due to a default
path being used for the `git` executable when `$PATH` is unset. This can
break if Git isn't installed at the expected path. Also, I believe it is
currently necessary to set the `$TEST_GIT_EXECUTABLE_PATH` environment
variable on Windows for tests to pass. Instead, we should use the user's
`$PATH` to locate the `git` executable, as well as any other executables
that are needed. This also makes `$TEST_GIT_EXECUTABLE_PATH` no longer
necessary, so it can be removed.
Other links in CHANGELOG.md have the issue number as the text, and the
full URL as a link. They're swapped here, meaning the link points to a
non-existent anchor in the CHANGELOG.md file.
Apparently, Magic Nix Cache will stop working on Feb 1st, because
the underlying APIs are being sunset by GitHub, and there are no open
replacements for it quite yet. Since it's just an optimization, we'll
have to live with it.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Before this commit, both VS Code's "Even better TOML" and `taplo`
reported an error saying `["status","--no-pager"] is not of type
"string"` for
```toml
"$schema" = "https://jj-vcs.github.io/jj/latest/config-schema.json"
ui.default-command = ["status", "--no-pager"]
```
I believe the schema was either invalid or just ignored `oneOf` because
it was trivially satisfied whenever the `type` restriction was
satisfied.
This can be used in order to count the number of added files by
diff.files().filter(..).len(), for example. If we add more advanced fileset
predicates, it can also be expressed as diff("added()").files().len(). The
latter would be cheaper to compute.
For #4154, I'll probably add DiffStats template type. It might be doable by
extending diff.files() method, but I don't think it's good idea to write stats
calculation logic in template language.
Closes#5272
bacon 3.6.0 -> 3.8.0, shows output with nextest 0.9.86
https://github.com/Canop/bacon/issues/280
Add python3 to the devshell to be able to run tools installed with uv
Closes#5217
Motivating use case:
[[--scope]]
--when.command = ["log"]
[--scope.ui]
pager = "less"
This adds a new (optional) field to `--when` config conditions, to
inspect the current command.
`--when.commands` is a list of space-separated values that matches a
space-separated command. To be specific:
--when.command = ["file"] # matches `jj file show`, `jj file list`, etc
--when.command = ["file show"] # matches `jj file show`, but *NOT* `jj file list`
--when.command = ["file", "log"] # matches `jj file` *OR* `jj log` (or subcommand of either)
When both `--when.commands` and `--when.repositories` are set, the
intersection is used.
In some cases, there are non trivial codepaths for fetching multiple
branches explicitly. In particular, it might be the case that fetching
works for n = 2 but not n = 3.
This commit changes a cli test to have 3 remotes and 3 branches, and a
lib test with 3 branches, only one of them succeds.
Fully qualified git refs (e.g., `+refs/head/main:refs/remotes/origin/main`)
are commonly used throughout the git glue code.
It is laborious and error prone to keep parsing refspecs when we need
only a segment of it.
This commits adds a type, `RefSpec`, which does exactly that
The --edit flag forces the editor to be shown, even if it would have
been hidden due to the --no-edit flag or another flag that implies it
(such as --message or --stdin).
I'm going to add "file list" template, and I think it's better to provide a file
path as "path", not "self". This patch also adds some tree value properties
which seemed useful.
Tests will be added later.
This will be used in "file list" template. Option<RepoPath> type isn't
needed for that, but I think it'll appear somewhere in custom diff template.
The path.parent() method is added mainly for testing Option<RepoPath>.
Tests will be added later.
This one is easy, but porting other "jj git remote" sub commands would require
non-trivial work.
The empty [remote "<name>"] case is covered by existing tests.
It's not uncommon to insert new table at a certain path. This could be modelled
as set_table(name, new_table), but the caller might want to merge items with
the existing table if any. This function can be used for both cases.
This was needed in order to migrate fix.tool-command to [fix.tools] table. We
don't have other use cases right now, but there will be something in future.
I had to guess whether to complete all revisions or just mutable ones. I'm
guessing that `-r` is sometimes used with immutable revisions (as part of a
revset, or for creating special-purpose branches), while `-c` is almost always
used to push a mutable revision. We can change it later if my guess is wrong.
Based on the discussion in #3505, I think the sliding behavior isn't favored at
least for "jj abandon". This patch changes the default to delete bookmarks.
"jj rebase --skip-emptied" can also be updated if needed. It might be good for
consistency. However, I'm skeptical about changing the default of the internal
API. It's not easy to add generic reporting mechanism for deleted/abandoned
bookmarks. If we added one in report_repo_changes(), redundant message would be
printed on "jj git fetch". So I think callers should enable the deletion
explicitly.
Closes#3505
I'll make "jj abandon" delete bookmarks by default. This could be handled by
cmd_abandon(), but we'll need a repo API if we also want to change the behavior
of "jj rebase --skip-emptied".
I noticed that some config structures do not use their "Default" implementation, and have their default specified in a TOML file. I think specifying defaults in TOML is great, and it'd be good to delete those Default implementations, so that it does not diverge from the default in the TOML file.
There's only one non-test caller who doesn't need a complete map of rebased
commits.
FWIW, I tried to rewrite squash_commits() based on transform_descendants() API,
but it wasn't easy because of A-B+A=A rule. The merge order matters here.
Cryptographic signature support in templates was added in
c99c97c6467fee3f3269d2934c10c758e041f834 (#4853), but has to be manually
configured. This adds some defaults to the built-in config.
Instead of having separate `builtin_*_with_sig` aliases, this adds to
the aliases that actually format commits. Since signature verification
is slow, this is disabled by default. To enable it, override
`ui.show-cryptographic-signatures`:
[ui]
show-cryptographic-signatures = true
[template-aliases]
'format_short_cryptographic_signature(signature)' = ...
'format_detailed_cryptographic_signature(signature)' = ...
Note that the two formatting functions take
`Option<CryptographicSignature>`, not `CryptographicSignature`. This
allows you to display a custom message if a signature is not found, but
will emit an error if you do not check for signature presence.
[template-aliases]
'format_detailed_cryptographic_signature(signature)' = '''
if(signature,
"message if present",
"message if missing",
)
'''
Tag and bookmark names are usually ASCII, but they occasionally include Latin
or Han characters.
This doesn't fix the serialization problem, but should mitigate #5359.
The "identifier" rule will be changed to accept unicode characters. It's not
super important to reject non-ASCII string pattern prefixes or alias symbols,
but this is more consistent with templater and fileset. We can relax the rule
later if needed.
This goes against our rule that we shouldn't add config knob that changes the
command behavior, but I don't have any other idea to work around the problem.
Apparently, there are two parties, one who always wants to push new bookmarks,
and the other who mildly prefers to push&track new bookmarks explicitly.
Perhaps, for the former, creation of bookmarks means that the target branches
are marked to be pushed.
The added flag is a simple boolean. "non-tracking-only" behavior #5173 could be
implemented, but I don't want to complicate things. It's a failed attempt to
address the issue without introducing config knob.
Closes#5094Closes#5173
According to the discussion on Discord, streampager is still maintained.
It brings more dependencies, but seems more reliable in our use case. For
example, the streampager doesn't consume inputs indefinitely whereas minus
does. We can also use OS-level pipe to redirect child stderr to the pager.
The WTFPL license is added to the allow list. I've never heard about this
license, but it's basically the same as public domain according to wikipedia.
Perhaps, the "windows" feature was enabled through "minus" or "scm-record". If I
removed "minus" in earlier revision, the Windows build of crossterm 0.27.0
failed.
Adds an optional `fix.tools.TOOL.enabled` config that disables use of a fix
tool (if omitted, the tool is enabled). This is useful for defining tools in
the user's configuration without enabling them for all repositories:
```toml
# ~/.jjconfig.toml
[fix.tools.rustfmt]
enabled = false
command = ["rustfmt", "--emit", "stdout"]
patterns = ["glob:'**/*.rs'"]
```
Then to enable it in a repository:
```shell
$ jj config set --repo fix.tools.rustfmt.enabled true
```
Now `jj --config ui.con<TAB><TAB>` will autocomplete
```
ui.conflict-marker-style=diff
ui.conflict-marker-style=snapshot
ui.conflict-marker-style=git
```
Booleans are completed as `false`, `true`, the remaining types are left
without completions.
the trailing `=` is especially nice to have because otherwise fish will
complete the suggestion and insert a space before the cursor.
With the `=`, `jj config --config ui.pagin<TAB>never` works.
The "git" CLI chdir()s to the work tree root, so paths in config file are
usually resolved relative to the workspace root. OTOH, jj doesn't modify the
process environment, so libgit2 resolves remote paths relative to cwd, not to
the workspace root. To mitigate the problem, this patch makes "jj git remote"
sub commands to store resolved path in .git/config. It would be nice if we can
reconfigure in-memory git2 remote object to use absolute paths (or set up
in-memory named remote without writing a config file), but there's no usable
API afaik.
This behavior is different from "git remote add"/"set-url". I don't know the
rationale, but these commands don't resolve relative paths, whereas "git clone"
writes resolved path to .git/config. I think it's more consistent to make all
"jj git" sub commands resolve relative paths.
When running `cmd.spawn()` rust will by default inherit
the stderr of the parent, so `jj log test<TAB>`, would
print `There is no jj repo in "."` into the prompt.
UserSettings will be obtained from there. If operation template supported
"self.repo() -> Repo" method, RepoLoader would be needed. It's equivalent to
Repo in CommitTemplateLanguage.
This makes the graph compact. Short change ids are usually printed as a part
of commit summary. The --no-graph output is a bit harder to parse, but we can
still discriminate entries by change ids.
It contained a check for `repo_owner == 'martinvonz'` which meant that it hasn't run in 4 weeks.
This is another great example showing why current CI systems suck, as it always was "successful" in
doing nothing with no warning whatsoever.
I also only noticed it because I wanted to check my latest doc change on the prerelease site.
In recent times multiple users showed up and were confused _why_ some
completions didn't show up, so make it explicit that they're opt-in
until the Clap upstream has made it so.
Also remove the link to the dynamic completions improvements issue, as it was closed
a while ago.
The `RUSTFLAGS` enviroment variable needs to be set in a `shellHook` to
allow the command to be interpreted by the shell. Otherwise, it will
just be passed as a string into the `rustc` binary:
```
cargo nextest run --workspace
error: failed to run `rustc` to learn about target-specific information
Caused by:
process didn't exit successfully: `rustc - --crate-name ___ --print=file-names -Zthreads=0 -C 'link-arg=--ld-path=$(unset' 'DEVELOPER_DIR;' /usr/bin/xcrun --find 'ld)' -C link-arg=-ld_new --target aarch64-apple-darwin --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=crate-name --print=cfg` (exit status: 1)
--- stderr
error: Unrecognized option: 'find'
```
This is a middle ground. An inline table can still be overwritten or deleted
because it's syntactically a value. It would be annoying if "jj config set
colors.<name> '{ .. }'" couldn't be executed more than once because the existing
item was a table.
#5255
Like `main`, we don't want to double build these branches since they will be
handled by `merge_group` already.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
The list of migration rules is managed by CliRunner. I don't know if that's
needed, but in theory, an extension may insert migration rules as well as
default config layers.
Migration could be handled by ConfigEnv::resolve_config(), but it seemed rather
complicated because Vec<ConfigMigrationRule> cannot be cloned, and the scope of
these variables are a bit different.
Instead of resolving deprecated variables by callers or config object, this
patch adds a function that rewrites deprecated variables globally (and emits
warnings accordingly.) It's simpler because StackedConfig doesn't have to deal
with renamed variables internally. OTOH, warnings will be issued no matter if
variables are used or not, which might be a bit noisy.
Maybe we can also add "jj config migrate" command that updates user and repo
configs.
I have come to think of conflicts more and more as one positive
followed by a series of diffs and less as two separate sets of adds
and removes. We've already changed the implmentation of `Merge` to be
a single list of interleaved positive and negative terms. I think it's
simpler to use the same format in `trivial_merge()` too.
To keep the diff simple, I just preserved the test case values and
order as is for now even if that means that some of them are now
unintuitive. I'll clean that up in the next commit.
If many files are conflicted, it would be nice to be able to resolve all
conflicts at once without having to run `jj resolve` multiple times.
This is especially nice for merge tools which try to automatically
resolve conflicts without user input, but it is also good for regular
merge editors like VS Code.
This change makes the behavior of `jj resolve` more consistent with
other commands which accept filesets since it will use the entire
fileset instead of picking an arbitrary file from the fileset.
Since we don't support passing directories to merge tools yet, the
current implementation just calls the merge tool repeatedly in a loop
until every file is resolved, or until an error occurs. If an error
occurs after successfully resolving at least one file, the transaction
is committed with all of the successful changes before returning the
error. This means the user can just close the editor at any point to
cancel resolution on all remaining files.
The new `merge_group` event will already attach all the CI events from the
various workflows where it applies. If the merge queue is drained and merges to
`main`, having `push` on these workflows will cause them to be run again, doubling
the number of CI checks. We don't need `push` anymore, basically.
Also, because checks can be cancelled now, the current double running can
probably lead to an awkward setup like:
- Merge Queue merges A to main, starts `push` workflows
- Merge Queue then merges B and C to main, starts `push` workflows
- `A`'s workflows get cancelled if it's not yet done
- This makes it look like `A` has failing tests somehow, but it doesn't
Therefore, just removing the double builds is the first step to cut down on the
CI times for our repo.
We also run the build workflows on all pushes to NOT main, because that will
help people who want to run CI before opening an actual PR.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
This will make sure the build on the main listing still looks clean, instead of
previous builds being cancelled and making it look like things failed.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
There are some experiments to try and compile `jj` to WebAssembly, so that we
might be able to do things like interactive web tutorials. One step for that
is making `git` support in `jj-cli` optional, because we can stub it out for
something more appropriate and it's otherwise a lot of porting annoyance for
both gitoxide and libgit2.
(On top of that, it might be a useful build configuration for other experiments
of mine where removing the need for the large libgit2 depchain is useful.)
As part of this, we need to mark `jj-lib` as having `default-features = false`
in the workspace dependency configuration; otherwise, the default behavior
for Cargo is to compile with all its default features, i.e. with git support
enabled, ignoring the `jj-cli` features clauses.
Other than that, it is fairly straightforward junk; it largely just sprinkles
some `#[cfg]` around liberally in order to make things work. It also adjusts the
CI pipeline so this is tested there, too, so we can progressively clean it up.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
`zstd` is only used to write files in the native backend right now. For now,
jettison it, to unbundle some C code that we don't really need.
(Ideally, a future compression library would be pure Rust, but we'll cross that
bridge when we get to it...)
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Certain tools (`diff`, `delta`) exit with code 1 to indicate there was
a difference. This allows selectively suppressing the "Tool exited with
... status" warning from jj when generating a diff.
example:
```toml
[merge.tools.delta]
diff-expected-exit-codes = [0, 1]
```
This backs out commit 0de36918e4c020e0e54816f29c47cb57cc9cfbf5. Documentation,
tests, and comments are updated accordingly. I also add ConfigTableLike type
alias as we decided to abstract table-like items away.
Closes#5255
Our workload is pretty CPU bound, but `nextest` does seem to make a small ~5s
difference for me. Let's see how it works on the CI runners.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
These are the workflows that run on PRs, so they need to have the `merge_group`
event added to them if we want to use the merge queue.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
This gets rid of an annoying warning in the Actions tab, but we already rely on
this for the build-binaries workflow, and it's probably better to be explicit
about this anyway.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
If the parent tree contains conflicts, we want the index to also contain
a conflict to ensure that the use can't accidentally commit conflict
markers using `git commit`. Since we can't represent conflicts with more
than 2 sides in the Git index, we need to add a dummy conflict in this
case. We use ".jj-do-not-resolve-this-conflict" as the dummy conflict to
indicate to the user that they should not attempt to resolve this
conflict using Git.
Instead of setting the index to match the tree of HEAD, we now set the
index to the merged parent tree of the working copy commit. This means
that if you edit a merge commit, it will make the Git index look like it
would in the middle of a `git merge` operation (with all of the
successfully-merged files staged in the index).
If there are any 2-sided conflicts in the merged parent tree, then they
will be added to the index as conflicts. Since Git doesn't support
conflicts with more than 2 sides, many-sided conflicts are staged as the
first side of the conflict. The following commit will improve this.
This will give us more fine-grained control over what files we put in
the index, allowing us to create conflicted index states. We also still
need to use git2 to clean up the merge/rebase state, since gix doesn't
have any function for this currently.
These tests should still pass after we switch to gix for resetting the
index, so we need to make sure they don't rely on the cached index from
the `git2::Repository` instance.
There should be no difference, but it's more consistent that all workspace/repo
commands use workspace_command.settings(), etc. than command.settings().
This ensures that WorkspaceCommandHelper created for initialized/cloned repo
refers to the settings object for that repo, not the settings loaded for the
cwd repo.
I also removed WorkspaceCommandEnvironment::settings() because it doesn't own
a workspace object. It's implementation detail that the env object holds a
copy of workspace.settings(), and the caller should be accessible to the
workspace object.
This patch doesn't fix DiffEditor::edit() API, which is fundamentally broken
if matcher argument is specified. I'm not sure if the builtin behavior is
correct or not. Suppose we add "jj diffedit FILESETS..", it would probably make
sense to leave unmatched paths unmodified because it is the command to edit the
destination (or right) tree. This is the edit_diff_external() behavior.
Fixes#5252
Disclaimer: this is the work of @necauqua and @julienvincent (see
#3141). I simply materialized the changes by rebasing them on latest
`main` and making the necessary adjustments to pass CI.
---
I had to fix an issue in `TestSignatureBackend::sign()`.
The following test was failing:
```
---- test_signature_templates::test_signature_templates stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot: signature_templates
Source: cli/tests/test_signature_templates.rs:28
────────────────────────────────────────────────────────────────────────────────
Expression: stdout
────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────
0 0 │ @ Commit ID: 05ac066d05701071af20e77506a0f2195194cbc9
1 1 │ │ Change ID: qpvuntsmwlqtpsluzzsnyyzlmlwvmlnu
2 2 │ │ Author: Test User <test.user@example.com> (2001-02-03 08:05:07)
3 3 │ │ Committer: Test User <test.user@example.com> (2001-02-03 08:05:07)
4 │-│ Signature: Good test signature
4 │+│ Signature: Bad test signature
5 5 │ │
6 6 │ │ (no description set)
7 7 │ │
8 8 │ ◆ Commit ID: 0000000000000000000000000000000000000000
────────────┴───────────────────────────────────────────────────────────────────
```
Print debugging revealed that the signature was bad, because of a
missing trailing `\n` in `TestSignatureBackend::sign()`.
```diff
diff --git a/lib/src/test_signing_backend.rs b/lib/src/test_signing_backend.rs
index d47fef1086..0ba249e358 100644
--- a/lib/src/test_signing_backend.rs
+++ b/lib/src/test_signing_backend.rs
@@ -59,6 +59,8 @@
let key = (!key.is_empty()).then_some(std::str::from_utf8(key).unwrap().to_owned());
let sig = self.sign(data, key.as_deref())?;
+ dbg!(&std::str::from_utf8(&signature).unwrap());
+ dbg!(&std::str::from_utf8(&sig).unwrap());
if sig == signature {
Ok(Verification::new(
SigStatus::Good,
```
```
[lib/src/test_signing_backend.rs:62:9] &std::str::from_utf8(&signature).unwrap() = \"--- JJ-TEST-SIGNATURE ---\\nKEY: \\n5300977ff3ecda4555bd86d383b070afac7b7459c07f762af918943975394a8261d244629e430c8554258904f16dd9c18d737f8969f2e7d849246db0d93cc004\\n\"
[lib/src/test_signing_backend.rs:63:9] &std::str::from_utf8(&sig).unwrap() = \"--- JJ-TEST-SIGNATURE ---\\nKEY: \\n5300977ff3ecda4555bd86d383b070afac7b7459c07f762af918943975394a8261d244629e430c8554258904f16dd9c18d737f8969f2e7d849246db0d93cc004\"
```
Thankfully, @yuja pointed out that libgit2 appends a trailing newline
(see bfb7613d5d192d3c4dc533afa4f2ff0d6b9016c5).
Co-authored-by: necauqua <him@necauq.ua>
Co-authored-by: julienvincent <m@julienvincent.io>
We need to make `TestSigningBackend` available for cli tests, as we will
add cli tests for signing related functionality (templates for
displaying commit signatures, `jj sign`) in upcoming commits.
Co-authored-by: julienvincent <m@julienvincent.io>
If jj is compiled against musl libc, not all name services are available, and
getpwuid() can return null even if the system is configured properly. That's
the problem reported as #5231. Suppose operation.username exists mainly for
logging/tracing purposes, it should be better to include something less reliable
than leaving the field empty.
This patch also removes TODO comment about empty hostname/username. It's
unlikely that the hostname is invalid (as that would cause panic on older jj
versions), and $USER would probably be set on Unix.
The following deprecated commands have been removed for their `jj files`
alternatives:
- `jj cat` was deprecated for `jj file show` in 47bd6f4 and e6c2108.
- `jj chmod` was deprecated for `jj file chmod` in 47bd6f4.
- `jj files` was deprecated for `jj file list` in 5d307e6.
The idea is the same as diff_editor()/selector() API. This object will be passed
in to edit_*description() functions in place of (repo_path, settings) pair.
"ui.editor" isn't specific to editing commit descriptions, but it's mainly used
for that purpose. So I put the wrapper type in description_util.rs.
It's not super important that operation log has a valid user/host name. Let's
allow invalid system configuration. In jj 0.24.0, invalid hostname was panic,
and invalid username was mapped to "Unknown".
Fixes#5231
This reverts commit b7ba3fc0be734cfa39bb7d6cfc50ed67eee15dce.
As per discussion in https://github.com/martinvonz/jj/pull/5047, I would like to make it so that the default build configuration doesn't need `cmake`.
Using `gix/max-performance` requires `cmake` as a build-time dependency, which could be a significant barrier for contributors (including existing ones, like me, who already work on jj but didn't have `cmake` installed thus far).
This commit switches back to using `gix/max-performance-safe`, which doesn't have the `cmake` dependency, and adds `gix/max-performance` behind the `gix-max-performance` feature for `jj-lib`.
It also adds `gix-max-performance` to the `packaging` feature group, since I'm assuming that packagers will want maximum performance, and are more likely to have `cmake` at hand.
Tested with
```
$ cargo build --workspace
$ cargo build --workspace --features packaging
```
(and the `--features packaging` build failed until I installed `cmake`)
I'm going to remove the settings argument from start_transaction(),
new_commit(), and rewrite_commit(), and these functions will use the settings
passed in to the constructor.
The idea is that ReadonlyRepo/MutableRepo hold UserSettings to accomplish
their operations (such as transaction commit.) Later patches will remove
the "settings" argument from a couple of repo methods, which will greatly
reduce the amount of settings refs we had to pass around.
The current UserSettings is set up for the repo, so we wouldn't need a separate
RepoSettings type.
I think the issue #5144 can be fixed cleanly if UserSettings object is
loaded/resolved per repo/workspace, not per CommandHelper.
This change makes it clear that UserSettings is supposed to be shared
immutably.
This fixes the output of "jj config list --include-overridden --include-defaults
ui.pager" and ".. ui.pager.command". The default ui.pager.* should be marked as
overridden by ui.pager if set in user configuration.
It has seriously gone out of date, so update it for 2025 and mention the new `jj-vcs` org.
The suggestion came from melutovich in the #jujutsu Libera IRC chat.
It's nice that "jj config list --include-defaults" can show these default
values.
I just copied the jj-cli directory structure, but it's unlikely we'll add more
config/*.toml files.
This implements "scissor" lines. For example:
this text is included in the commit message
JJ: ignore-rest
this text is not, and is encouraged to be rendered as a diff
JJ: ignore-rest
this text is *still not* included in the commit message
When editing multiple commit messages, the `JJ: describe {}` lines
are parsed before the description is cleaned up. That means that the
following will correctly add descriptions to multiple commits:
JJ: describe aaaaaaaaaaaa
this text is included in the first commit message
JJ: ignore-rest
scissored...
JJ: describe bbbbbbbbbbbb
this text is included in the first commit message
JJ: ignore-rest
scissored...
Git supports passing the conflict marker length to merge drivers using
"%L". It would be useful if we also had a way to pass the marker length
to merge tools, since it would allow Git merge drivers to be used with
`jj resolve` in more cases. Without this variable, any merge tool that
parses or generates conflict markers could fail on files which require
conflict markers longer than 7 characters.
https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver
I often do "jj log -rREV" to preview the commits to abandon, and it's annoying
that I have to remove -r or insert space to "jj abandon ..".
The implementation is basically the same as b0c7d0a7e262.
Note that infallible version of whoami::username() would return "Unknown" on
error. I just made it error out, but it's also an option to fall back to an
empty string.
Since source.contains(':') can't be used for non-local path detection on
Windows, we now use gix::url for parsing. It might be stricter, but I assume it
would be more reliable.
Closes#4188
ReverseGraphIterator will be inlined. It doesn't make sense the iterator yields
Item = Result<..> whereas all possible errors are confined by constructor.
Currently, `jj resolve` always sets the file to be non-executable
whenever a conflict is fully resolved. This is confusing, because it can
cause the executable bit to be lost even if every side agrees that the
file is executable.
These paths may be printed, compared with user inputs, or passed to external
programs. It's probably better to avoid unusual "\\?\C:\" paths on Windows.
Fixes#5143
Storing the conflict marker length in the working copy makes conflict
parsing more consistent, and it allows us to parse valid conflict hunks
even if the user left some invalid conflict markers in the file while
resolving the conflicts.
If a file contains lines which look like conflict markers, then we need
to make the real conflict markers longer so that the materialized
conflicts can be parsed unambiguously.
When parsing the conflict, we require that the conflict markers are at
least as long as the materialized conflict markers based on the current
tree. This can lead to some unintuitive edge cases which will be solved
in the next commit.
For instance, if we have a file explaining the differences between
Jujutsu's conflict markers and Git's conflict markers, it could produce
a conflict with long markers like this:
```
<<<<<<<<<<< Conflict 1 of 1
%%%%%%%%%%% Changes from base to side #1
Jujutsu uses different conflict markers than Git, which just shows the
-sides of a conflict without a diff.
+sides of a conflict without a diff:
+
+<<<<<<<
+left
+|||||||
+base
+=======
+right
+>>>>>>>
+++++++++++ Contents of side #2
Jujutsu uses different conflict markers than Git:
<<<<<<<
%%%%%%%
-base
+left
+++++++
right
>>>>>>>
>>>>>>>>>>> Conflict 1 of 1 ends
```
We should support these options for "git" conflict marker style as well,
since Git actually does support producing longer conflict markers in
some cases through .gitattributes:
https://git-scm.com/docs/gitattributes#_conflict_marker_size
We may also want to support passing the conflict marker length to merge
tools as well in the future, since Git supports a "%L" parameter to pass
the conflict marker length to merge drivers:
https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver
I don't think we need --keep-emptied flag. IIRC, "jj squash" has that flag in
order not to squash commit description to the destination commits. Since
"jj absorb" never moves commit description, the source commit is preserved in
that situation.
Closes#5141
It's convenient to test if rewriter.reparent()-ed commit is empty than
implementing the same check based on rewriter.new_parents(). CommitRewriter
is an intermediate state, and it doesn't know whether the commit will be rebased
or reparented.
This patch moves max_new_file_size() and conflict_marker_style() to CLI, but
there isn't a clear boundary whether the configuration should be managed by
UserSettings or not. I decided to move them to CLI just because we can eliminate
.optional() handling. The default parameters are defined in config/misc.toml.
I originally added the check so we would never canonicalize path relative to
random cwd, but Windows CI failed because "/" is a relative path. Suppose user
would want to share the same configuration file between native Windows and WSL,
this check would be too nitpicky.
This is an alternative way to achieve includeIf of Git without adding "include"
directive. Conditional include (or include in general) is a bit trickier to
implement than loading all files and filtering the contents.
Closes#616
The next patch will introduce the resolution stage of conditional tables.
Since it wasn't easy to detect misuse of "raw" StackedConfig, I added a thin
newtype.
I'm going to add config post-processing stage in order to enable config
variables conditionally, and handle/parse_early_args() doesn't have enough
information to evaluate the condition.
The deprecation warning could be emitted by parse_early_args(), but it's
probably better to print it after --color option is applied.
This will simplify path comparison in config resolver. <cwd>/.. shouldn't match
path prefix <cwd>. Maybe we should do path canonicalization globally (or never
do canonicalization), but I'm not sure where that should be made.
This provides an inline version of Git's includeIf. The idea is that each config
layer has an optional key "--when" to enable the layer, and a layer may have
multiple sub-layer tables "--scope" to inline stuff. Layers can be nested, but
that wouldn't be useful in practice.
I choose "--" prefix to make these meta keys look special (and are placed
earlier in lexicographical order), but I don't have strong opinion about that.
We can use whatever names that are unlikely to conflict with the other config
sections.
resolve() isn't exported as a StackedConfig method because it doesn't make
sense to call resolve() on "resolved" StackedConfig. I'll add a newtype in
CLI layer to distinguish raw config from resolved one. Most consumers will
just see the "resolved" config.
#616
ConfigLayers will be cloned to new StackedConfig instance at resolution stage.
ConfigFile could own ConfigLayer instead of Arc<_>, but copy-on-write behavior
is probably better for the current users.
We can usually omit quotes in --config=NAME=VALUE, but an RFC3339 string is a
valid TOML date-time expression. It's weird that quoting is required to specify
a date-time value.
I looked through the code for the `ignore` crate, and this optional path
is not used anywhere. The only reason to pass it would be to be able to
get the path from the `Glob` when we call `Gitignore::matched` or
`Gitignore::matched_path_or_any_parents`, but we ignore the returned
`Glob` completely anyway. Passing the path would require an unnecessary
clone of the path for each line in every .gitignore file, so it's better
not to pass it since we don't need it.
This will simplify handling of multiple ConfigArg items of the same type.
Consecutive --config NAME=VALUE arguments will be inserted to the same
ConfigLayer.
The same parsing function will be used for --config NAME=VALUE.
I don't think we'll add schema-based type inference anytime soon, so I moved
the value parsing to clap layer.
The `Signature.email()` method is also updated to return the new Email
type. The `Signature.username()` method is deprecated for
`Signature.email().local()`.
I don't think the new behavior is strictly better, but it's more consistent
with the other "jj config" commands so we can remove the special case for
"jj config edit".
If the user config path was an empty directory, these commands would fail with
EISDIR instead of "can't set config in path (dirs not supported)". I think this
can be changed later to create new "$dir/config.toml" file. Maybe we can also
change the default path to ~/.config/jj directory, and load all *.toml files
from there?
This patch adds simpler user/repo_config_path() accessors. existing_*_path()
are kinda implementation details (for testing), so they are now private methods.
new_user_config_path() will be removed later.
They are short, and it wouldn't make much sense to do load, mutate one entry,
and save in one function.
In later patches, "jj config set"/"unset" will be changed to reuse the loaded
ConfigLayer if the layer can be unambiguously selected.
Since .get("path.to.non-table.children") returns NotFound, I made
.delete_value() not fail in that case. The path doesn't exist, so
.delete_value() should be noop.
remove_config_value_from_file() will be inlined to callers.
I followed the recommendation in the `jj new` doc to use `jj new main @`
to make a merge commit and ended up with a merge commit that GitHub did
not like. The PR diff included both the relevant changes from that
branch plus everything I merged in. @papertigers pointed out that
swapping the two args produces a merge commit GitHub understands better.
Happy to add a line explaining that the order matters, but it might be
too much detail for this spot. The linked doc
https://martinvonz.github.io/jj/latest/working-copy/ also does not
explain this.
This would be useful for scripting purpose. Maybe we can also replace the
current --config-toml=<TOML> use cases by --config-file=<PATH> and simpler
--config=<KEY>=<VALUE>.
https://github.com/martinvonz/jj/issues/4926#issuecomment-2506672165
If we want to add more source variants (such as fd number), it might be better
to add --config-from=<type>:<path|fd|..>. In any case, we'll probably want
--config=<KEY>=<VALUE>, and therefore, we'll need to merge more than one
--config* arguments.
This backs out commit 89d57ffb29577c99f0f5187b76bff369f19c5886.
This is causing a CI failure because we can't build musl binaries,
presumably because the rust-toolchain file overriding the chosen
musl toolchain for some reason. Backout until we can reapply
a proper fix.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Before, "jj config get"/"list" and .get() functions processed inline tables as
tables (or directories in filesystem analogy), whereas "set"/"unset" processed
ones as values (or files.) This patch makes all commands and functions process
inline tables as values. We rarely use the inline table syntax, and it's very
hard to pack many (unrelated) values into an inline table. TOML doesn't allow
newlines between { .. }. Our common use case is to define color styles, which
wouldn't be meant to inherit attributes from the default settings.
The default pager setting is flattened in case user overrides pager.env without
changing the command args.
As these tests show, we sometimes print an error when trying to rebase
an empty set, and sometimes we don't say anything at all. It seems to
me like we should say "Nothing changed" in all of these cases.
Follow-up from discussion at https://discord.com/channels/968932220549103686/1288926971719323762
I don't think we achieved consensus in that thread. We could use `stable` or `nightly` here instead if we prefer. Note that user `rustup` overrides are still respected in the presence of a `rust-toolchain.toml`.
We enabled GitHub's private vulnerability reporting a few weeks or
months ago (for CVE-2024-51990), so there's no need to email about
vulnerabilities anymore.
As Martin spotted, the original code can't prevent "1.0GiB, maximum size allowed
is ~1.0GiB." I personally don't mind if the error message contained the exact
size, so I simply let it print both exact and human byte sizes unconditionally.
I think this provides a better UX than refusing any operation due to large
files. Because untracked files won't be overwritten, it's usually safe to
continue operation ignoring the untracked files. One caveat is that new large
files can become tracked if files of the same name checked out. (see the test
case)
FWIW, the warning will be printed only once if watchman is enabled. If we use
the snapshot stats to print untracked paths in "jj status", this will be a
problem.
Closes#3616, #3912
It wasn't obvious which io::Error was mapped to a "file creation" error.
Perhaps, file creation will be moved to caller, but let's make the error
handling explicit so we'll remove the unused error variant later.
This patch does not change the handling of inline tables yet. Both inline and
non-inline tables are merged as before. OTOH, .set_value() is strict about table
types because it should refuse to overwrite a table whereas an inline table
should be overwritten as a value. This matches "jj config set"/"unset"
semantics. rules_from_config() in formatter.rs uses .as_inline_table(), which is
valid because toml_edit::Value type never contains non-inline table.
Since toml_edit::Value doesn't implement PartialEq, stacking tests now use
insta::assert_snapshot!().
It's annoying that rustfmt indents a large code block depending on the length
of parameters and method calls needed to configure the parallel iterator.
I think the idea of the code was try do 30 updates per second even if
events arrive at, say, every 20 milliseconds. If we had reset the
timer every time we printed, we would otherwise reset the timer every
40 milliseconds and end up with 25 updates per second. However, a bug
in the code caused it to print every update because it always set the
threshold to print the next update to `now`. I tried to keep what I
think was the intent of the original code while fixing the bug.
The progress bar is supposed to update only 30 times per second, but
as was reported in #5057, that doesn't seem to be what's
happening. This patch adds tests showing how we update the progress
bar for every event.
.get_table() callers will be migrated to .table_keys() + .get() to attach source
indication to error message. It might be a bit more expensive, but we can get by
without introducing table wrapper that tracks source config layers.
Both Mercurial and Git (xdiff) have a special case for empty hunks.
https://repo.mercurial-scm.org/hg/rev/2b1ec74c961f
I also changed the internal line numbers to start from 0 so we wouldn't have
to think about whether "N - 1" would underflow.
Fixes#5049
Appears that "cargo test" parses indented text as a code block, and fails to
run doc tests. Spotted by running "cargo insta test". This doc comment is a CLI
help which is usually rendered to console, so I think markdown annotation should
be minimal.
This backs out commit ed84468cb8f3, "docs: in `jj help util exec`, use Markdown
`warning` admonition."
Since config::Value type was lax about data types, an integer value could be
deserialized through a string. This won't apply to new toml_edit-based value
types.
I'm going to rewrite the deserialization constructor to accept either integer
or string, but the from_str() API is generally useful.
This patch also fixes lifetime of the parse error message.
The serde::Deserialize interface isn't always useful. Suppose we're going to
make parsing of color tables stricter, we would have to process a string|table
value. It could be encoded as an untagged enum in serde, but that means the
error message has to be static str (e.g. "expected string or color table") and a
detailed error (e.g. "invalid color name: xxx") would be lost. It's way easier
to dispatch based on the ConfigValue type, than on serde data model.
We're scraping the CLI help text and rendering it as markdown, so we can use an "admonition" to have this warning text render nicer in the web documentation.
You could argue that `!!! warning` is a little weird to see on the CLI. Some alternatives:
- We could opt to not design the CLI help text around markdown and skip the change to the `jj util exec` help in this commit.
- We could adopt some kind of format that can be rendered well in both contexts.
- Could sticking to specific formatting constructs by convention.
- Could use/create an actual translation tool from CLI format to Markdwon.
- We could keep separate versions of web and CLI documentation. (Seems like a bad idea for the foreseeable future, because we don't have the resources to constantly keep both up-to-date and sync.)
I'm in favor of just writing Markdown in the CLI help text for now.
The current implementation is silly, which will be reimplemented to be using
toml_edit::DocumentMut. "jj config set" will probably be ported to this API.
This patch removes pre-merge steps which depend on the ConfigBuilder API.
Some of Vec<ConfigLayer> could be changed to Vec<config::Config> (or
Vec<ConfigTable>) to drop the layer parameter, but I'm not sure which is
better. parse_config_args() will have to construct ConfigLayer (or ConfigTable +
Option<PathBuf>) if we add support for --config-file=PATH argument.
The `NO_COLOR` spec says that user-specified config is supposed to
override the `$NO_COLOR` environment variable, and we do correctly use
the `ColorFormatter` when `ui.color= "always"` is set in the user's
config. However, it turns out that `NO_COLOR=1` still resulted in no
color because `crossterm` also respects the variable, so the color
codes the `ColorFormatter` requested had no effect. Since `crossterm`
doesn't know about our user configs, it cannot decide whether to
respect `$NO_COLOR`, so let's tell `crossterm` to always use the
colors we tell it to use.
I noticed miniz_oxide appears in perf samples. While miniz_oxide is safer, I
think zlib-ng is pretty reliable.
https://docs.rs/gix/latest/gix/#performance
libz-ng-sys is downgraded to 1.1.16 due to the Windows linking issue. The
benchmark result is obtained with libz-ng-sys 1.1.20.
https://github.com/rust-lang/libz-sys/issues/225
```
% hyperfine --sort command --warmup 3 --runs 10 -L bin jj-0,jj-1 \
'target/release-with-debug/{bin} --ignore-working-copy log README.md'
Benchmark 1: target/release-with-debug/jj-0 ..
Time (mean ± σ): 256.6 ms ± 4.3 ms [User: 214.1 ms, System: 38.6 ms]
Range (min … max): 245.4 ms … 261.2 ms 10 runs
Benchmark 2: target/release-with-debug/jj-1 ..
Time (mean ± σ): 223.0 ms ± 4.2 ms [User: 174.7 ms, System: 44.4 ms]
Range (min … max): 212.4 ms … 225.8 ms 10 runs
```
Since most callers don't need to handle loading/parsing errors, it's probably
better to add a separate error type for "get" operations. The other uses of
ConfigError will be migrated later.
Since ConfigGetError can preserve the source name/path information, this patch
also removes some ad-hock error handling codes.
The added function is not "get_table(name) -> Result<Table, Error>" because
callers need to know whether the value was missing or shadowed by non-table
value. We just don't have this problem in template/revset-aliases because these
tables are top-level items.
As per a [Discord discussion](https://discord.com/channels/968932220549103686/968932220549103689/1314291772893171784), we thought it might be nice to include additional information in the changelog/release notes, similar to certain other open-source projects.
For example: In [the Rust 1.82 release notes](https://blog.rust-lang.org/2024/10/17/Rust-1.82.0.html), they include a one-line description of the project as well as installation instructions, and then go over several release highlights.
Possible future process:
- We can put the release highlights into `CHANGELOG.md` itself, so that it can undergo the normal review process.
- We'll add the static description/installation text to each release as we publish it (which doesn't need to be duplicated for each version in `CHANGELOG.md`).
We currently ignore lines prefixed with "JJ: " (including the space)
in commit messages and in the list of sparse paths from `jj sparse
edit`. I think I included the trailing space in the prefix simply
because that's how we render comments line we insert before we ask the
user to edit the file. However, as #5004 says, Git doesn't require a
space after their "#" prefix. Neither does Mercurial after their "HG:"
prefix. So let's follow their lead and not require the trailing
space. Seems useful especially for people who have their editor
configured to strip trailing spaces.
Some Git merge drivers can partially resolve conflicts, leaving some
conflict markers in the output file. In that case, they exit with a code
between 1 and 127 to indicate that there was a conflict remaining in the
file (since Git uses a shell to run the merge drivers, and shells often
use exit codes above 127 for special meanings such as exiting due to a
signal):
https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver
We should support this convention as well, since it will allow many Git
merge drivers to be used with Jujutsu, but we don't run our merge tools
through a shell, so there is no reason to treat exit codes 1 through 127
specially. Instead, we let the user specify which exact exit codes
should indicate conflicts. This is also better for cross-platform
support, since Windows may use different exit codes than Linux for
instance.
When I wrote the original lookup function, I didn't notice that the root config
value could be accessed as &config.cache without cloning. That's the only reason
I added split_safe_prefix().
I would like to delete this alias altogether. By moving it to the
config to start with, users can start overriding it themselves, as
`commit` or whatever they prefer.
This backs out commit fd271d39ad54 "cli: make `jj desc` and `jj st` aliases
hidden."
As we discussed in https://github.com/martinvonz/jj/pull/4811, completion of
flags doesn't work for hidden aliases.
While this is a debug option, it doesn't make sense to store an integer value
as a string. We can parse the environment variable instead. The variable is
temporarily parsed into i64 because i64 is the integer type of TOML.
toml_edit::Value doesn't implement any other integer conversion functions.
The goal is to remove dependency on config::Config and replace the underlying
table type to toml_edit::Table. Other StackedConfig::merge() users will be
migrated in the next batch.
This patch adds separate .get<T>(), .get_table(), and .get_value() methods
because generic value types in toml_edit don't implement Deserialize.
There's a bit of naming inconsistency right now. Low-level generic object is
called an "item", whereas mid-level generic object is called a "value". These
objects will become different types when we migrate to toml_edit.
We usually look up config objects with a static name, but commands like "jj
config get" has to parse a user input as a config key. We could consolidate the
name type to &ConfigNamePathBuf, but it would be inconvenient if all callers had
to do config.get(&"..".parse()?). It's also undesirable to pass a raw user input
in to .get() as a string.
I originally considered making it support fallible conversion, but doing that
would complicate error handling path. Not all functions should return an error.
It's better to do fallible parsing at the call sites instead.
A string config name is parsed into a temporary ConfigNamePathBuf object. If the
allocation cost matters, the output path type can be abstracted.
This will help migrate away from config::Config. We'll probably need a better
abstraction to preserve error source indication, but let's leave that for now.
.get_table() isn't forwarded to .get::<T>() because ConfigTable won't implement
Deserialize if we migrate to toml_edit.
* GitFetch{} separates `fetch()` from `import_refs()`.
* This allows more control over the stages of the fetch so multiple fetches
can happen before `import_refs` resolves the changes in the local jj repo.
* Implement `git::fetch` in terms of the new api.
* Add a test case for initial fetch from a repo without `HEAD` set. This tests
the default branch retrieving behaviour.
Issue: #4923
If there are multiple files in a subdirectory that are candidates for
completion, only complete the common directory prefix to reduce the number of
completion candidates shown at once.
This matches the normal shell completion of file paths.
UserSettings::get_*() will be changed to look up a merged value from
StackedConfig, not from a merged config::Value. This will help migrate away
from the config crate.
Not all tests are ported to ConfigLayer::parse() because it seemed a bit odd
to format!() a TOML document and parse it to build a table of configuration
variables.
This is heavily based on Benjamin Tan's fish completions:
https://gist.github.com/bnjmnt4n/9f47082b8b6e6ed2b2a805a1516090c8
Some differences include:
- The end of a `--from`, `--to` ranges is also considered.
- `jj log` is not completed (yet). It has a different `--revisions` argument
that requires some special handling.
As reported by @lilyball on Discord, the recent nixpkgs update this week brought
with it an entirely new Darwin stdenv, which had a behavioral change versus
the prior one, causing builds inside the developer shell to fail, as the new
super-duper parallel linker could not be used.
However, rather than giving up and throwing away *billions* of precious CPU
cycles, @emilazy and @lilyball instead doubled down and diagnosed the issue. The
result is a new impenetrable line of Nix code, which is fully explained by the
comments within.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
I left the "merge-tool-edits-conflict-markers" option unchanged,
since removing it would likely break some existing configurations. It
also seems like it could be useful to have a merge tool use the default
conflict markers instead of requiring the conflict marker style to
always be set for the merge tool (e.g. if a merge tool allows the user
to manually edit the conflicts).
This will help replace cli LayeredConfigs with new StackedConfig. I originally
considered moving this function to jj-lib, but the current API is very specific
to the CLI use case, and wouldn't be reused for building a merged sub table. We
might instead want to extract some bits as a library function.
This adds a new `revsets.simplify-parents` configuration option (similar
to `revsets.fix`) which serves as the default revset for `jj
simplify-parents` if no `--source` or `--revisions` arguments are
provided.
Layers are now constructed per file, not per source type. This will allow us
to report precise position where bad configuration variable is set. Because
layers are now created per file, it makes sense to require existence of the
file, instead of ignoring missing files which would leave an empty layer in the
stack. The path existence is tested by ConfigEnv::existing_config_path(), so I
simply made the new load_file/dir() methods stricter. However, we still need
config::File::required(false) flag in order to accept /dev/null as an empty
TOML file.
The lib type is renamed to StackedConfig to avoid name conflicts. The cli
LayeredConfigs will probably be reorganized as an environment object that
builds a StackedConfig.
The current docs for conflict markers start out by introducing Git
"diff3" conflict markers, and then discussing how Jujutsu's conflict
markers are different from Git's. I don't think this is the best way to
explain it, because it requires the reader to first understand Git's
conflict markers before they go on to learn Jujutsu's conflict markers.
Instead, I think it's better to explain the conflict markers from
scratch with a more detailed example so that we don't assume any
particular knowledge of Git.
This structure also works better with the new config option, since we
can talk about the default option first, and then go on to explain
alternatives later.
Adds a new "git" conflict marker style option. This option matches Git's
"diff3" conflict style, allowing these conflicts to be parsed by some
external tools that don't support JJ-style conflicts. If a conflict has
more than 2 sides, then it falls back to the similar "snapshot" conflict
marker style.
The conflict parsing code now supports parsing Git-style conflict
markers in addition to the normal JJ-style conflict markers, regardless
of the conflict marker style setting. This has the benefit of allowing
the user to switch the conflict marker style while they already have
conflicts checked out, and their old conflicts will still be parsed
correctly.
Example of "git" conflict markers:
```
<<<<<<< Side #1 (Conflict 1 of 1)
fn example(word: String) {
println!("word is {word}");
||||||| Base
fn example(w: String) {
println!("word is {w}");
=======
fn example(w: &str) {
println!("word is {w}");
>>>>>>> Side #2 (Conflict 1 of 1 ends)
}
```
Adds a new "snapshot" conflict marker style which returns a series of
snapshots, similar to Git's "diff3" conflict style. The "snapshot"
option uses a subset of the conflict hunk headers as the "diff" option
(it just doesn't use "%%%%%%%"), meaning that the two options are
trivially compatible with each other (i.e. a file materialized with
"snapshot" can be parsed with "diff" and vice versa).
Example of "snapshot" conflict markers:
```
<<<<<<< Conflict 1 of 1
+++++++ Contents of side #1
fn example(word: String) {
println!("word is {word}");
------- Contents of base
fn example(w: String) {
println!("word is {w}");
+++++++ Contents of side #2
fn example(w: &str) {
println!("word is {w}");
>>>>>>> Conflict 1 of 1 ends
}
```
Adds a new "ui.conflict-marker-style" config option. The "diff" option
is the default jj-style conflict markers with a snapshot and a series of
diffs to apply to the snapshot. New conflict marker style options will
be added in later commits.
The majority of the changes in this commit are from passing the config
option down to the code that materializes the conflicts.
Example of "diff" conflict markers:
```
<<<<<<< Conflict 1 of 1
+++++++ Contents of side #1
fn example(word: String) {
println!("word is {word}");
%%%%%%% Changes from base to side #2
-fn example(w: String) {
+fn example(w: &str) {
println!("word is {w}");
>>>>>>> Conflict 1 of 1 ends
}
```
Since commit e716fe7, the function no longer returns a tree id. That
commit updated the function documentation to match the new behavior,
but then the old documentation was restored in commit ef83f2b, perhaps
by a bad merge conflict resolution.
There's a subtle difference in error message, but the conversion function to be
called is the same. The error message now includes "for key <key>", which is
nice.
.get_table() isn't implemented because it isn't cheap to build a HashMap,
and a table of an abstract Value type wouldn't be useful. Maybe we'll
instead provide an iterator of table keys.
.config() is renamed to .raw_config() to break existing callers.
Ui::with_config() is unchanged because I'm not sure if UserSettings should be
constructed earlier. I assume UserSettings will hold an immutable copy of
LayerdConfigs object, whereas Ui has to be initialized before all config layers
get loaded.
I'm planning to rewrite config store layer by leveraging toml_edit instead of
the config crate. It will allow us to merge config overlays in a way that
deprecated keys are resolved within a layer prior to merging, for example.
This patch moves ConfigNamePathBuf to jj-lib where new config API will be
hosted. We'll probably extract LayeredConfigs to this module, but we'll first
need to split environment dependencies from it.
Some editors strip trailing whitespace on save, which breaks any diffs
which have context lines, since the parsing function expects them to
start with a space. There's no visual difference between " \n" and "\n",
so it seems reasonable to accept both.
Currently, conflict markers ending in CRLF line endings aren't allowed.
I don't see any reason why we should reject them, since some
editors/tools might produce CRLF automatically on Windows when saving
files, which would break the conflicts otherwise.
Use `jj new && jj undo` instead of `jj new @-` at the end of converting to a
co-located repository, because the latter "stashes" newly added working copy
changes into a sibling commit.
See also: https://github.com/martinvonz/jj/discussions/4945
Signed-off-by: Tim Janik <timj@gnu.org>
The previous iteration of `jj simplify-parents` would only reparent the
commits in the target set in the `MutableRepo::transform_descendants`
callback. The subsequent `MutableRepo::rebase_descendants` call invoked
when the transaction is committed would then rebase all descendants of
the target set, which includes the commits in the target set again.
This commit updates the `MutableRepo::transform_descendants` callback to
perform rebasing of all descendants within the callback. All descendants
of the target set will only be rebased at most once.
Because the output of diff.hunks() is a list of [&BStr]s, a Merge object
reconstructed from a diff hunk will be Merge<&BStr>. I'm not pretty sure if
we'll implement conflict diffs in that way, but this change should be harmless
anyway.
I believe this was an oversight. "jj duplicate" should duplicate commits (=
patches), not trees.
This patch adds a separate test file because test_rewrite.rs is pretty big, and
we'll probably want to migrate CLI tests to jj-lib.
Our docs are built with MkDocs, which requires Python and several deps.
Previously those deps were managed with Poetry, which is also written in Python.
This commit replaces Poetry with `uv`, a Rust-based Python
project/package manager, and thus removes several steps from the docs
build process.
Before:
<install Python>
<install pipx>
pipx install poetry
poetry install
poetry run -- mkdocs serve
After:
<install uv>
uv run mkdocs serve
The working-copy revision is usually the latest commit, but it's not always
true. This patch ensures that the wc branch is emitted first so the graph node
order is less dependent on rewrites.
This isn't always fast because it increases the chance of cache miss, but in
practice, it makes "jj file annotate" faster. It's still slower than
"git blame", though.
Maybe we should also change the hash function.
```
group new old
----- --- ---
bench_diff_git_git_read_tree_c 1.00 45.2±0.38µs 1.29 58.4±0.32µs
bench_diff_lines/modified/10k 1.00 32.7±0.24ms 1.05 34.4±0.17ms
bench_diff_lines/modified/1k 1.00 2.9±0.00ms 1.06 3.1±0.01ms
bench_diff_lines/reversed/10k 1.00 22.7±0.18ms 1.02 23.2±0.29ms
bench_diff_lines/reversed/1k 1.00 439.0±9.46µs 1.19 523.9±6.05µs
bench_diff_lines/unchanged/10k 1.00 2.9±0.06ms 1.20 3.5±0.02ms
bench_diff_lines/unchanged/1k 1.00 240.8±1.03µs 1.30 312.1±1.05µs
```
```
% hyperfine --sort command --warmup 3 --runs 10 -L bin jj-0,jj-1 \
'target/release-with-debug/{bin} --ignore-working-copy file annotate lib/src/revset.rs'
Benchmark 1: target/release-with-debug/jj-0 ..
Time (mean ± σ): 1.604 s ± 0.259 s [User: 1.543 s, System: 0.057 s]
Range (min … max): 1.348 s … 1.917 s 10 runs
Benchmark 2: target/release-with-debug/jj-1 ..
Time (mean ± σ): 1.183 s ± 0.026 s [User: 1.118 s, System: 0.062 s]
Range (min … max): 1.155 s … 1.237 s 10 runs
```
Since patience diff is recursive, it makes some sense to reuse precomputed
hash values. This patch migrates Histogram to remembering hashed values. The
precomputed values will be cached globally by DiffSource.
Technically, Histogram doesn't have to keep a separate copy of hash values, but
this appears to give better perf than slicing text and hash value from two Vecs.
If you have multiple remotes to push to, you might want to keep some changes
(such as security patches) in your private fork. Git CLI has one upstream remote
per branch, but jj supports multiple tracking remotes, and therefore "jj git
push" can start tracking new remotes automatically.
This patch makes new bookmarks not eligible for push by default. I considered
adding a warning, but it's not always possible to interrupt the push shortly
after a warning is emitted.
--all implies --allow-new because otherwise it's equivalent to --tracked. It's
also easier to write a conflict rule with --all/--deleted/--tracked than with
two of them.
-c/--change doesn't require --allow-new because it is the flag to create new
tracking bookmark.
#1278
I hope we'll have support for copies and renames in not too long. It's
good to have as many versions before that as possible without support
for `jj move`, in case we want to later use that to record a moved
file (maybe as an alias for `jj file move`).
There are two known limitations right now:
- Only statically known keys are suggested.
- Keys that the user did not set are still suggested for `jj config get`.
Running that suggestion may result in an error. The error message will be
appropriate though and there is some value in letting the user know that
this config value theoretically exists. Some users may try to explore what
configurations are available via the completions.
This is a workaround for command name completion. On zsh, the complete position
is specified by $_CLAP_COMPLETE_INDEX, and an empty argument isn't padded. So,
for "jj <TAB>", { args = ["jj"], index = 1 } is provided, and our expand_args()
helpfully fills in the default command "log". Since args[index] is now "log",
no other commands are listed.
Perhaps it would be nice to point to the revset docs from every
argument that takes a revset, or perhaps that would be too
verbose. `jj log` is perhaps where most people first run into revset
syntax, so I hope pointing to the docs from there is a good start.
This can be used to find the fork point (best common ancestors) of a
revset with an arbitrary number of commits, which cannot be expressed
currently in the revset language.
This forces us to think about which repo should be used. It doesn't matter in
find_bookmarks_to_push() because moved "push-{change_id}" bookmarks are
prioritized. It doesn't matter in print_commits_ready_to_push() either, which
uses the repo only for ancestry lookup, but I think tx.repo() is better here.
This makes completions suggest aliases from the user or repository
configuration. It's more useful for long aliases that aren't used very often,
but serve the purpose of "executable documentation" of complex and useful jj
commands.
An earlier patch "resolved" aliases, meaning that any arguments following an
alias would be completed as if the normal command had been used. So it only
made sure that using aliases doesn't degrade the rest of the completions.
Commit ID: 325402dc9463ccaa70822069b3e94716d9cc7417
The most relevant part (and the reason for this change) is the addition
of the `mainProgram` attribute. This allows getting the executable name
from inside nix expressions with ease:
```
# before
lib.getExe' jujutsu "jj"
# or
"${jujutsu}/bin/jj"
```
```
# now
lib.getExe jujutsu
```
- gix::object::tree::diff::change::Event::Rewrite is flattened
- diff options are extracted to separate type
2b81e6c8bd
- signature text now includes a trailing newline
4a6bbb1b79
The last change means that our SecureSig { sig } returned by GitBackend is now
terminated by '\n'. I think this is harmless since textual signature is usually
ends with '\n'.
I don't think we need to declare these dependencies separately because both
are the optional dependencies enabled by the git feature.
We'll also need the gix's "attributes" feature at some point.
> attributes - Query attributes and excludes. Enables access to pathspecs,
> worktree checkouts, filter-pipelines and submodules.
https://docs.rs/gix/0.67.0/gix/index.html#feature-flags
There is now an updated version of `esl01-renderdag` published from
the Sapling repo, so let's use that. That lets us remove the
`bitflags` 1.x dependency and the `itertools` 0.10.x non-dev
dependency.
It was not clear if users should activate both sets of completions if they
wanted the dynamic ones.
The shell commands for activating the dynamic completions are aligned with the
static version. Users can choose / are responsible themselves to add the
activation to their shell config if they want them in every shell instance.
For fish specifically, `script | source` is more idiomatic than
`source (script | psub)`.
This test reliably failed if I dropped tv_nsec part from statx().
Since we reload the repo now, several assertions get "fixed". I've added
index().has_id() test to clarify that it's still broken.
This commit modifies the commit graph used in the `rebase --after
--before` test case to add an additional branch of commits for use in
tests for `rebase -b` along with `--after --before`.
This commit extracts out the common code printing out the
`MoveCommitsStats` information into a shared function. The printed
output was also inconsistent between `-r` and `-s`/`-b` code paths, so I
standardized it to say "Rebased ? commits onto destination" for both
cases.
This is different from skipped paths because the file state has to remain as
FileType::GitSubmodule in order to ignore the submodule directory when
snapshotting.
Fixes#4825.
The destination commits are selected based on annotation, which I think is
basically the same as "hg absorb" (except for handling of consecutive hunks.)
However, we don't compute a full interleaved delta right now, and the hunks are
merged in the same way as "jj squash". This means absorbed hunks might produce
conflicts if no context lines exist. Still I think this is more intuitive than
selecting destination commits based on patch commutativity.
I've left inline comments to the tests where behavior is different from "hg
absorb", but these aren't exhaustively checked.
Closes#170
In "jj absorb", we'll need to calculate annotation from the parent tree. It's
usually identical to the tree of the parent commit, but this is not true for a
merge commit. Since I'm not sure how we'll process conflict trees in general,
this patch adds a minimal API to specify a single file content, not a
MergedTree.
The primary use case is to exclude immutable commits when calculating line
ranges to absorb. For example, "jj absorb" will build annotation of @ revision
with domain = mutable().
Unlike other documented aliases, it can be redefined by the user.
However, I think it's important for people to be informed that `b`
exists.
I left `amend`, `unamend`, and `ci` undocumented, since I think we were
considering removing or at least discouraging them.
Due to the gradual rewrite to use the
`MutableRepo::transform_descendants` API, `DescendantRebaser` is no
longer used in `MutableRepo::rebase_descendants`, and is only used
(indirectly through `MutableRepo::rebase_descendants_return_map`) in
`rewrite::squash_commits`.
`DescendantRebaser` is removed since it contains a lot of logic
similar to `MutableRepo::transform_descendants`. Instead,
`MutableRepo::rebase_descendants_with_options_return_map` is rewritten
to use `MutableRepo::transform_descendants` directly, and the other
`MutableRepo::rebase_descendants_{return_map,with_options}` functions
call `MutableRepo::rebase_descendants_with_options_return_map` directly.
`MutableRepo::rebase_descendants_return_rebaser` is also removed.
Previously, attempting to modify an immutable commit only showed the
ID of the commit being modified, which wasn't very helpful when trying
to figure out which immutable commit is being modified at a quick
glance.
This commit prints the commit summary as a hint to make it simpler for
the user to see what the immutable commit is without having to run
`jj show <commit-id>`.
https://rustsec.org/advisories/RUSTSEC-2024-0384 says to migrate off
of the `instant` crate because it's unmaintained. We depend on it only
via the `backoff` crate. That crate also seems unmaintained. So this
patch replaces our use of `backoff` by a custom implementation.
I initially intended to migrate to the `backon` crate, but that made
`lock::tests::lock_concurrent` tests fail. The test case spawns 72
threads (on my machine) and lets them all lock a file, and then it
waits 1 millisecond before releasing the file lock. I think the
problem is that their version of jitter is implemented as a random
addition of up to the initial backoff value. In our case, that means
we would add at most a millisecond. The `backoff` crate, on the other
hand does it by adding -50% to +50% of the current backoff value. I
think that leads to a much better distribution of backoffs, while
`backon`'s implementation means that only a few threads can lock the
file for each backoff exponent.
Maybe we can add comparison of ids, commits, etc., but I don't have a practical
use case right now. If we add lt/gt, it might make sense to implement them on
Timestamp type.
I also changed lhs.and_then(..) to (lhs, rhs).map(..) since we don't need
short-circuiting behavior here.
Visible aliases interfere with shell completion, at least in Fish. For
example, `jj des<tab>` stops as `jj desc` without adding a space
afterwards, which make me stop for a second and wonder if I need to
add more letters. They also show up in the auto-complete menu.
I added "[aliases: <name>]" as part of the first line of the help text
so the `jj help` output still looks the same.
The recover commit we create in some cases (when an operation has been
lost) doesn't currently have a description. That makes it easy to miss
that it's special.
When a pull request is pushed multiple times in sequence (e.g. to fix small
errors that are noticed post-submission), a backlog of builds ends up needing
to be cleared out before your new update can be built. If you push N times, N
builds are queued and need to clear out first.
This can cause higher latency for *every* PR since the pool of public runners
is shared, and in general seems uneconomical and inefficient since 99% of the
time you want to build the new version ASAP.
Luckily GHA has a universal solution to this: use the `concurrency` directive
to group all builds for a PR under a name, and when a new build appears in that
group, cancel all builds in the group that are in-progress.
Taken from this useful blog post: "Simple trick to save environment and money
when using GitHub Actions"
https://turso.tech/blog/simple-trick-to-save-environment-and-money-when-using-github-actions
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Both user and programmatic expressions use the same .evaluate() function now.
optimize() is applied globally after symbol resolution. The order shouldn't
matter, but it might be nicer because union of commit refs could be rewritten
to a single Commits(Vec<CommitId>) node.
This helps add library API that takes resolved revset expressions. For example,
"jj absorb" will first compute annotation within a user-specified ancestor range
such as "mutable()". Because the range expression may contain symbols, it should
be resolved by caller.
There are two ideas to check resolution state at compile time:
<https://github.com/martinvonz/jj/pull/4374>
a. add RevsetExpressionWrapper<PhantomState> and guarantee inner tree
consistency at public API boundary
b. parameterize RevsetExpression variant types in a way that invalid variants
can never be constructed
(a) is nice if we want to combine "resolved" and "unresolved" expressions. The
inner expression types are the same, so we can just calculate new state as
Resolved & Unresolved = Unresolved. (b) is stricter as the compiler can
guarantee invariants. This patch implements (b) because there are no existing
callers who need to construct "resolved" expression and convert it to "user"
expression.
.evaluate_programmatic() now requires that the expression is resolved.
This will become safe as I'm going to add static check that the expression does
never contain CommitRef(_)s. We could make Present(_) unconstructible, but the
existence of Present node is harmless.
I'm going to add RevsetExpression<State> type parameter, but the existing tree
transformer can't rewrite nodes to different state because the input and the
output must be of the same type. (If they were of different types, we couldn't
reuse the input subtree by Rc::clone().) The added visitor API will handle
state transitions by mapping RevsetExpression::<St1>::<Kind> to
RevsetExpression::<St2>::<Kind>.
CommitRef and AtOperation nodes are processed by specialized methods because
these nodes will depend on the State type. OTOH, Present node won't be
State-dependent, so it's inspected by the common fold_expression() method.
An input expression is not taken as an &Rc<RevsetExpression> but a &_ because
we can't reuse the allocation behind the Rc.
* Suggested `watch --color` as recommended by @ilyagr
* Answered monitoring `jj log` with watch and TUIs
* Minor wording fix.
* Adjusted watch(1) and hwatch links as suggested by @ilyagr
Signed-off-by: Tim Janik <timj@gnu.org>
The 0.23.0 release actions failed to run. Perhaps it's because I used
"save as draft" (to view the markdown) before I published it. Running
the actions on publish seems more correct.
2024-11-06 21:54:39 -08:00
491 changed files with 87148 additions and 41732 deletions
poetry install # Only really needed once per environment unless there are updates
# TODO: `--alias-type symlink` is the
# TODO: `--alias-type symlink` is the
# default, and may be nicer in some ways. However,
# default, and may be nicer in some ways. However,
# this requires deploying to GH Pages via a "custom GitHub Action", as in
# this requires deploying to GH Pages via a "custom GitHub Action", as in
@ -24,4 +19,4 @@ poetry install # Only really needed once per environment unless there are updat
# Otherwise, you get an error:
# Otherwise, you get an error:
# > Site contained a symlink that should be dereferenced: /main.
# > Site contained a symlink that should be dereferenced: /main.
# > For more information, see https://docs.github.com/github/working-with-github-pages/troubleshooting-jekyll-build-errors-for-github-pages-sites#config-file-error.
# > For more information, see https://docs.github.com/github/working-with-github-pages/troubleshooting-jekyll-build-errors-for-github-pages-sites#config-file-error.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
