ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

cli: implement enough of jj fix to run a single tool on all files

Browse source
This commit is contained in:
Danny Hooper 2024-05-15 17:19:55 -05:00
parent bbd9ba31df
commit 3050685ff3
10 changed files with 811 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
CHANGELOG.md
View file

@ -58,6 +58,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Upgraded `scm-record` from v0.2.0 to v0.3.0. See release notes at https://github.com/arxanas/scm-record/releases/tag/v0.3.0
* New command `jj fix` that can be configured to update commits by running code
formatters (or similar tools) on changed files. The configuration schema and
flags are minimal for now, with a number of improvements planned (for example,
[#3800](https://github.com/martinvonz/jj/issues/3800) and
[#3801](https://github.com/martinvonz/jj/issues/3801)).
### Fixed bugs
* Previously, `jj git push` only made sure that the branch is in the expected

1
Cargo.lock generated
View file

@ -1715,6 +1715,7 @@ dependencies = [
"pest",
"pest_derive",
"pollster",
"rayon",
"regex",
"rpassword",
"scm-record",

1
cli/Cargo.toml
View file

@ -75,6 +75,7 @@ once_cell = { workspace = true }
pest = { workspace = true }
pest_derive = { workspace = true }
pollster = { workspace = true }
rayon = { workspace = true }
regex = { workspace = true }
rpassword = { workspace = true }
scm-record = { workspace = true }

277
cli/src/commands/fix.rs
View file

@ -13,11 +13,13 @@
// limitations under the License.
use std::collections::{HashMap, HashSet};
use std::io::Write;
use std::process::Stdio;
use std::sync::mpsc::channel;
use futures::StreamExt;
use itertools::Itertools;
use jj_lib::backend::{BackendError, BackendResult, CommitId, FileId, TreeValue};
use jj_lib::commit::{Commit, CommitIteratorExt};
use jj_lib::matchers::EverythingMatcher;
use jj_lib::merged_tree::MergedTreeBuilder;
use jj_lib::repo::Repo;
@ -25,19 +27,48 @@ use jj_lib::repo_path::RepoPathBuf;
use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::store::Store;
use pollster::FutureExt;
use rayon::iter::IntoParallelIterator;
use rayon::prelude::ParallelIterator;
use tracing::instrument;
use crate::cli_util::{CommandHelper, RevisionArg};
use crate::command_error::CommandError;
use crate::command_error::{config_error_with_message, CommandError};
use crate::config::CommandNameAndArgs;
use crate::ui::Ui;
/// Format files
/// Update files with formatting fixes or other changes
///
/// The primary use case for this command is to apply the results of automatic
/// code formatting tools to revisions that may not be properly formatted yet.
/// It can also be used to modify files with other tools like `sed` or `sort`.
///
/// The changed files in the given revisions will be updated with any fixes
/// determined by passing their file content through the external tool.
/// Descendants will also be updated by passing their versions of the same files
/// through the same external tool, which will never result in new conflicts.
/// Files with existing conflicts will be updated on all sides of the conflict,
/// which can potentially increase or decrease the number of conflict markers.
///
/// The external tool must accept the current file content on standard input,
/// and return the updated file content on standard output. The output will not
/// be used unless the tool exits with a successful exit code. Output on
/// standard error will be passed through to the terminal.
///
/// The configuration schema is expected to change in the future. For now, it
/// defines a single command that will affect all changed files in the specified
/// revisions. For example, to format some Rust code changed in the working copy
/// revision, you could write this configuration:
///
/// [fix]
/// tool-command = ["rustfmt", "--emit", "stdout"]
///
/// And then run the command `jj fix -s @`.
#[derive(clap::Args, Clone, Debug)]
#[command(verbatim_doc_comment)]
pub(crate) struct FixArgs {
/// Fix these commits and all their descendants
/// Fix files in the specified revision(s) and their descendants
#[arg(long, short)]
sources: Vec<RevisionArg>,
source: Vec<RevisionArg>,
}
#[instrument(skip_all)]
@ -47,107 +78,245 @@ pub(crate) fn cmd_fix(
args: &FixArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let root_commits: Vec<Commit> = workspace_command
.parse_union_revsets(&args.sources)?
.evaluate_to_commits()?
.try_collect()?;
workspace_command.check_rewritable(root_commits.iter().ids())?;
let root_commits: Vec<CommitId> = workspace_command
.parse_union_revsets(if args.source.is_empty() {
&[RevisionArg::AT]
} else {
&args.source
})?
.evaluate_to_commit_ids()?
.collect();
workspace_command.check_rewritable(root_commits.iter())?;
let mut tx = workspace_command.start_transaction();
// Collect all FileIds we're going to format and which commits they appear in
let commits: Vec<_> = RevsetExpression::commits(root_commits.iter().ids().cloned().collect())
// Collect all of the unique `ToolInput`s we're going to use. Tools should be
// deterministic, and should not consider outside information, so it is safe to
// deduplicate inputs that correspond to multiple files or commits. This is
// typically more efficient, but it does prevent certain use cases like
// providing commit IDs as inputs to be inserted into files. We also need to
// record the mapping between tool inputs and paths/commits, to efficiently
// rewrite the commits later.
//
// If a path is being fixed in a particular commit, it must also be fixed in all
// that commit's descendants. We do this as a way of propagating changes,
// under the assumption that it is more useful than performing a rebase and
// risking merge conflicts. In the case of code formatters, rebasing wouldn't
// reliably produce well formatted code anyway. Deduplicating inputs helps
// to prevent quadratic growth in the number of tool executions required for
// doing this in long chains of commits with disjoint sets of modified files.
let commits: Vec<_> = RevsetExpression::commits(root_commits.to_vec())
.descendants()
.evaluate_programmatic(tx.base_repo().as_ref())?
.iter()
.commits(tx.repo().store())
.try_collect()?;
let mut file_ids = HashSet::new();
let mut commit_paths: HashMap<CommitId, Vec<RepoPathBuf>> = HashMap::new();
let mut unique_tool_inputs: HashSet<ToolInput> = HashSet::new();
let mut commit_paths: HashMap<CommitId, HashSet<RepoPathBuf>> = HashMap::new();
for commit in commits.iter().rev() {
let mut paths = vec![];
// Paths modified in parent commits in the set should also be updated in this
// commit
let mut paths: HashSet<RepoPathBuf> = HashSet::new();
// Fix all paths that were fixed in ancestors, so we don't lose those changes.
// We do this instead of rebasing onto those changes, to avoid merge conflicts.
for parent_id in commit.parent_ids() {
if let Some(parent_paths) = commit_paths.get(parent_id) {
paths.extend_from_slice(parent_paths);
paths.extend(parent_paths.iter().cloned());
}
}
let parent_tree = commit.parent_tree(tx.repo())?;
// Also fix any new paths that were changed in this commit.
let tree = commit.tree()?;
let parent_tree = commit.parent_tree(tx.repo())?;
let mut diff_stream = parent_tree.diff_stream(&tree, &EverythingMatcher);
async {
while let Some((repo_path, diff)) = diff_stream.next().await {
let (_before, after) = diff?;
// Deleted files have no file content to fix, and they have no terms in `after`,
// so we don't add any tool inputs for them. Conflicted files produce one tool
// input for each side of the conflict.
for term in after.into_iter().flatten() {
// We currently only support fixing the content of normal files, so we skip
// directories and symlinks, and we ignore the executable bit.
if let TreeValue::File { id, executable: _ } = term {
file_ids.insert((repo_path.clone(), id));
paths.push(repo_path.clone());
// TODO: Consider filename arguments and tool configuration instead of
// passing every changed file into the tool. Otherwise, the tool has to
// be modified to implement that kind of stuff.
let tool_input = ToolInput {
file_id: id.clone(),
repo_path: repo_path.clone(),
};
unique_tool_inputs.insert(tool_input.clone());
paths.insert(repo_path.clone());
}
}
}
Ok::<(), BackendError>(())
}
.block_on()?;
commit_paths.insert(commit.id().clone(), paths);
}
let formatted = format_files(tx.repo().store().as_ref(), &file_ids)?;
// Run the configured tool on all of the chosen inputs.
// TODO: Support configuration of multiple tools and which files they affect.
let tool_command: CommandNameAndArgs = command
.settings()
.config()
.get("fix.tool-command")
.map_err(|err| config_error_with_message("Invalid `fix.tool-command`", err))?;
let fixed_file_ids = fix_file_ids(
tx.repo().store().as_ref(),
&tool_command,
&unique_tool_inputs,
)?;
// Substitute the fixed file IDs into all of the affected commits. Currently,
// fixes cannot delete or rename files, change the executable bit, or modify
// other parts of the commit like the description.
let mut num_checked_commits = 0;
let mut num_fixed_commits = 0;
tx.mut_repo().transform_descendants(
command.settings(),
root_commits.iter().ids().cloned().collect_vec(),
root_commits.iter().cloned().collect_vec(),
|mut rewriter| {
let paths = commit_paths.get(rewriter.old_commit().id()).unwrap();
// TODO: Build the trees in parallel before `transform_descendants()` and only
// keep the tree IDs in memory, so we can pass them to the rewriter.
let repo_paths = commit_paths.get(rewriter.old_commit().id()).unwrap();
let old_tree = rewriter.old_commit().tree()?;
let mut tree_builder = MergedTreeBuilder::new(old_tree.id().clone());
for path in paths {
let old_value = old_tree.path_value(path);
let mut changes = 0;
for repo_path in repo_paths {
let old_value = old_tree.path_value(repo_path)?;
let new_value = old_value.map(|old_term| {
if let Some(TreeValue::File { id, executable }) = old_term {
if let Some(new_id) = formatted.get(&(path, id)) {
Some(TreeValue::File {
let tool_input = ToolInput {
file_id: id.clone(),
repo_path: repo_path.clone(),
};
if let Some(new_id) = fixed_file_ids.get(&tool_input) {
return Some(TreeValue::File {
id: new_id.clone(),
executable: *executable,
})
} else {
old_term.clone()
});
}
} else {
old_term.clone()
}
old_term.clone()
});
if new_value != old_value {
tree_builder.set_or_remove(path.clone(), new_value);
tree_builder.set_or_remove(repo_path.clone(), new_value);
changes += 1;
}
}
let new_tree = tree_builder.write_tree(rewriter.mut_repo().store())?;
let builder = rewriter.reparent(command.settings())?;
builder.set_tree_id(new_tree).write()?;
num_checked_commits += 1;
if changes > 0 {
num_fixed_commits += 1;
let new_tree = tree_builder.write_tree(rewriter.mut_repo().store())?;
let builder = rewriter.reparent(command.settings())?;
builder.set_tree_id(new_tree).write()?;
}
Ok(())
},
)?;
tx.finish(ui, format!("fixed {} commits", root_commits.len()))
writeln!(
ui.status(),
"Fixed {num_fixed_commits} commits of {num_checked_commits} checked."
)?;
tx.finish(ui, format!("fixed {num_fixed_commits} commits"))
}
fn format_files<'a>(
/// Represents the API between `jj fix` and the tools it runs.
// TODO: Add the set of changed line/byte ranges, so those can be passed into code formatters via
// flags. This will help avoid introducing unrelated changes when working on code with out of date
// formatting.
#[derive(PartialEq, Eq, Hash, Clone)]
struct ToolInput {
/// File content is the primary input, provided on the tool's standard
/// input. We use the `FileId` as a placeholder here, so we can hold all
/// the inputs in memory without also holding all the content at once.
file_id: FileId,
/// The path is provided to allow passing it into the tool so it can
/// potentially:
/// - Choose different behaviors for different file names, extensions, etc.
/// - Update parts of the file's content that should be derived from the
/// file's path.
repo_path: RepoPathBuf,
}
/// Applies `run_tool()` to the inputs and stores the resulting file content.
///
/// Returns a map describing the subset of `tool_inputs` that resulted in
/// changed file content. Failures when handling an input will cause it to be
/// omitted from the return value, which is indistinguishable from succeeding
/// with no changes.
/// TODO: Better error handling so we can tell the user what went wrong with
/// each failed input.
fn fix_file_ids<'a>(
store: &Store,
file_ids: &'a HashSet<(RepoPathBuf, FileId)>,
) -> BackendResult<HashMap<(&'a RepoPathBuf, &'a FileId), FileId>> {
tool_command: &CommandNameAndArgs,
tool_inputs: &'a HashSet<ToolInput>,
) -> BackendResult<HashMap<&'a ToolInput, FileId>> {
let (updates_tx, updates_rx) = channel();
// TODO: Switch to futures, or document the decision not to. We don't need
// threads unless the threads will be doing more than waiting for pipes.
tool_inputs.into_par_iter().try_for_each_init(
|| updates_tx.clone(),
|updates_tx, tool_input| -> Result<(), BackendError> {
let mut read = store.read_file(&tool_input.repo_path, &tool_input.file_id)?;
let mut old_content = vec![];
read.read_to_end(&mut old_content).unwrap();
if let Ok(new_content) = run_tool(tool_command, tool_input, &old_content) {
if new_content != *old_content {
let new_file_id =
store.write_file(&tool_input.repo_path, &mut new_content.as_slice())?;
updates_tx.send((tool_input, new_file_id)).unwrap();
}
}
Ok(())
},
)?;
drop(updates_tx);
let mut result = HashMap::new();
for (path, id) in file_ids {
// TODO: read asynchronously
let mut read = store.read_file(path, id)?;
let mut buf = vec![];
read.read_to_end(&mut buf).unwrap();
// TODO: Call a formatter instead of just uppercasing
for b in &mut buf {
b.make_ascii_uppercase();
}
// TODO: Don't write it if it didn't change
let formatted_id = store.write_file(path, &mut buf.as_slice())?;
result.insert((path, id), formatted_id);
while let Ok((tool_input, new_file_id)) = updates_rx.recv() {
result.insert(tool_input, new_file_id);
}
Ok(result)
}
/// Runs the `tool_command` to fix the given file content.
///
/// The `old_content` is assumed to be that of the `tool_input`'s `FileId`, but
/// this is not verified.
///
/// Returns the new file content, whose value will be the same as `old_content`
/// unless the command introduced changes. Returns `None` if there were any
/// failures when starting, stopping, or communicating with the subprocess.
fn run_tool(
tool_command: &CommandNameAndArgs,
tool_input: &ToolInput,
old_content: &[u8],
) -> Result<Vec<u8>, ()> {
// TODO: Pipe stderr so we can tell the user which commit, file, and tool it is
// associated with.
let mut vars: HashMap<&str, &str> = HashMap::new();
vars.insert("path", tool_input.repo_path.as_internal_file_string());
let mut child = tool_command
.to_command_with_variables(&vars)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()
.or(Err(()))?;
let mut stdin = child.stdin.take().unwrap();
let output = std::thread::scope(|s| {
s.spawn(move || {
stdin.write_all(old_content).ok();
});
Some(child.wait_with_output().or(Err(())))
})
.unwrap()?;
if output.status.success() {
Ok(output.stdout)
} else {
Err(())
}
}

1
cli/src/commands/mod.rs
View file

@ -93,7 +93,6 @@ enum Command {
Duplicate(duplicate::DuplicateArgs),
Edit(edit::EditArgs),
Files(files::FilesArgs),
#[command(hide = true)]
Fix(fix::FixArgs),
#[command(subcommand)]
Git(git::GitCommand),

13
cli/src/config-schema.json
View file

@ -482,6 +482,19 @@
"additionalProperties": true
}
}
},
"fix": {
"type": "object",
"description": "Settings for jj fix",
"properties": {
"tool-command": {
"type": "array",
"items": {
"type": "string"
},
"description": "Shell command that takes file content on stdin and returns fixed file content on stdout"
}
}
}
}
}

40
cli/tests/cli-reference@.md.snap
View file

@ -37,6 +37,7 @@ This document contains the help content for the `jj` command-line program.
* [`jj duplicate`↴](#jj-duplicate)
* [`jj edit`↴](#jj-edit)
* [`jj files`↴](#jj-files)
* [`jj fix`↴](#jj-fix)
* [`jj git`↴](#jj-git)
* [`jj git remote`↴](#jj-git-remote)
* [`jj git remote add`↴](#jj-git-remote-add)
@ -117,6 +118,7 @@ To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/d
* `duplicate` — Create a new change with the same content as an existing one
* `edit` — Sets the specified revision as the working-copy revision
* `files` — List files in a revision
* `fix` — Update files with formatting fixes or other changes
* `git` — Commands for working with Git remotes and the underlying Git repo
* `init` — Create a new repo in the given directory
* `interdiff` — Compare the changes of two commits
@ -758,6 +760,44 @@ List files in a revision
## `jj fix`
Update files with formatting fixes or other changes
The primary use case for this command is to apply the results of automatic
code formatting tools to revisions that may not be properly formatted yet.
It can also be used to modify files with other tools like `sed` or `sort`.
The changed files in the given revisions will be updated with any fixes
determined by passing their file content through the external tool.
Descendants will also be updated by passing their versions of the same files
through the same external tool, which will never result in new conflicts.
Files with existing conflicts will be updated on all sides of the conflict,
which can potentially increase or decrease the number of conflict markers.
The external tool must accept the current file content on standard input,
and return the updated file content on standard output. The output will not
be used unless the tool exits with a successful exit code. Output on
standard error will be passed through to the terminal.
The configuration schema is expected to change in the future. For now, it
defines a single command that will affect all changed files in the specified
revisions. For example, to format some Rust code changed in the working copy
revision, you could write this configuration:
[fix]
tool-command = ["rustfmt", "--emit", "stdout"]
And then run the command `jj fix -s @`.
**Usage:** `jj fix [OPTIONS]`
###### **Options:**
* `-s`, `--source <SOURCE>` — Fix files in the specified revision(s) and their descendants
## `jj git`
Commands for working with Git remotes and the underlying Git repo

1
cli/tests/runner.rs
View file

@ -27,6 +27,7 @@ mod test_diff_command;
mod test_diffedit_command;
mod test_duplicate_command;
mod test_edit_command;
mod test_fix_command;
mod test_generate_md_cli_help;
mod test_git_clone;
mod test_git_colocated;

524
cli/tests/test_fix_command.rs Normal file
View file

@ -0,0 +1,524 @@
// Copyright 2024 The Jujutsu Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::path::PathBuf;
use itertools::Itertools;
use jj_lib::file_util::try_symlink;
use crate::common::TestEnvironment;
/// Set up a repo where the `jj fix` command uses the fake editor with the given
/// flags.
fn init_with_fake_formatter(args: &[&str]) -> (TestEnvironment, PathBuf) {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");
let formatter_path = assert_cmd::cargo::cargo_bin("fake-formatter");
assert!(formatter_path.is_file());
let escaped_formatter_path = formatter_path.to_str().unwrap().replace('\\', r"\\");
test_env.add_config(&format!(
r#"fix.tool-command = ["{}"]"#,
[escaped_formatter_path.as_str()]
.iter()
.chain(args)
.join(r#"", ""#)
));
(test_env, repo_path)
}
#[test]
fn test_fix_no_config() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let stderr = test_env.jj_cmd_failure(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stderr, @r###"
Config error: Invalid `fix.tool-command`
Caused by: configuration property "fix.tool-command" not found
For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md.
"###);
}
#[test]
fn test_fix_empty_commit() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
}
#[test]
fn test_fix_leaf_commit() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "unaffected").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "affected").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: rlvkpnrz 8b02703b (no description set)
Parent commit : qpvuntsm fda57e40 (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@-"]);
insta::assert_snapshot!(content, @"unaffected");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"AFFECTED");
}
#[test]
fn test_fix_parent_commit() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
// Using one file name for all commits adds coverage of some possible bugs.
std::fs::write(repo_path.join("file"), "parent").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "parent"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "child1").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "child1"]);
test_env.jj_cmd_ok(&repo_path, &["new", "-r", "parent"]);
std::fs::write(repo_path.join("file"), "child2").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "child2"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "parent"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 3 commits of 3 checked.
Working copy now at: mzvwutvl d6abb1f4 child2 | (no description set)
Parent commit : qpvuntsm 4f4d2103 parent | (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "parent"]);
insta::assert_snapshot!(content, @"PARENT");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "child1"]);
insta::assert_snapshot!(content, @"CHILD1");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "child2"]);
insta::assert_snapshot!(content, @"CHILD2");
}
#[test]
fn test_fix_sibling_commit() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "parent").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "parent"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "child1").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "child1"]);
test_env.jj_cmd_ok(&repo_path, &["new", "-r", "parent"]);
std::fs::write(repo_path.join("file"), "child2").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "child2"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "child1"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "parent"]);
insta::assert_snapshot!(content, @"parent");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "child1"]);
insta::assert_snapshot!(content, @"CHILD1");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "child2"]);
insta::assert_snapshot!(content, @"child2");
}
#[test]
fn test_fix_immutable_commit() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "immutable").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "immutable"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "mutable").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "mutable"]);
test_env.add_config(r#"revset-aliases."immutable_heads()" = "immutable""#);
let stderr = test_env.jj_cmd_failure(&repo_path, &["fix", "-s", "immutable"]);
insta::assert_snapshot!(stderr, @r###"
Error: Commit 83eee3c8dce2 is immutable
Hint: Pass `--ignore-immutable` or configure the set of immutable commits via `revset-aliases.immutable_heads()`.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "immutable"]);
insta::assert_snapshot!(content, @"immutable");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "mutable"]);
insta::assert_snapshot!(content, @"mutable");
}
#[test]
fn test_fix_empty_file() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"");
}
#[test]
fn test_fix_cyclic() {
let (test_env, repo_path) = init_with_fake_formatter(&["--reverse"]);
std::fs::write(repo_path.join("file"), "content\n").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm affcf432 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"tnetnoc\n");
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm 2de05835 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"content\n");
}
#[test]
fn test_deduplication() {
// Append all fixed content to a log file. This assumes we're running the tool
// in the root directory of the repo, which is worth reconsidering if we
// establish a contract regarding cwd.
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase", "--tee", "$path-fixlog"]);
// There are at least two interesting cases: the content is repeated immediately
// in the child commit, or later in another descendant.
std::fs::write(repo_path.join("file"), "foo\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "bar\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "bar\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "c"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "foo\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "d"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 4 commits of 4 checked.
Working copy now at: yqosqzyt 5ac0edc4 d | (no description set)
Parent commit : mzvwutvl 90d9a032 c | (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "a"]);
insta::assert_snapshot!(content, @"FOO\n");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "b"]);
insta::assert_snapshot!(content, @"BAR\n");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "c"]);
insta::assert_snapshot!(content, @"BAR\n");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "d"]);
insta::assert_snapshot!(content, @"FOO\n");
// Each new content string only appears once in the log, because all the other
// inputs (like file name) were identical, and so the results were re-used. We
// sort the log because the order of execution inside `jj fix` is undefined.
insta::assert_snapshot!(sorted_lines(repo_path.join("file-fixlog")), @"BAR\nFOO\n");
}
fn sorted_lines(path: PathBuf) -> String {
let mut log: Vec<_> = std::fs::read_to_string(path.as_os_str())
.unwrap()
.lines()
.map(String::from)
.collect();
log.sort();
log.join("\n")
}
#[test]
fn test_executed_but_nothing_changed() {
// Show that the tool ran by causing a side effect with --tee, and test that we
// do the right thing when the tool's output is exactly equal to its input.
let (test_env, repo_path) = init_with_fake_formatter(&["--tee", "$path-copy"]);
std::fs::write(repo_path.join("file"), "content\n").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"content\n");
let copy_content = std::fs::read_to_string(repo_path.join("file-copy").as_os_str()).unwrap();
insta::assert_snapshot!(copy_content, @"content\n");
}
#[test]
fn test_failure() {
let (test_env, repo_path) = init_with_fake_formatter(&["--fail"]);
std::fs::write(repo_path.join("file"), "content").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"content");
}
#[test]
fn test_stderr_success() {
let (test_env, repo_path) =
init_with_fake_formatter(&["--stderr", "error", "--stdout", "new content"]);
std::fs::write(repo_path.join("file"), "old content").unwrap();
// TODO: Associate the stderr lines with the relevant tool/file/commit instead
// of passing it through directly.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
errorFixed 1 commits of 1 checked.
Working copy now at: qpvuntsm e8c5cda3 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"new content");
}
#[test]
fn test_stderr_failure() {
let (test_env, repo_path) =
init_with_fake_formatter(&["--stderr", "error", "--stdout", "new content", "--fail"]);
std::fs::write(repo_path.join("file"), "old content").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
errorFixed 0 commits of 1 checked.
Nothing changed.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"old content");
}
#[test]
fn test_missing_command() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
test_env.add_config(r#"fix.tool-command = ["this_executable_shouldnt_exist"]"#);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
// TODO: We should display a warning about invalid tool configurations. When we
// support multiple tools, we should also keep going to see if any of the other
// executions succeed.
insta::assert_snapshot!(stderr, @r###"
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
}
#[test]
fn test_fix_file_types() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "content").unwrap();
std::fs::create_dir(repo_path.join("dir")).unwrap();
try_symlink("file", repo_path.join("link")).unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm 72bf7048 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"CONTENT");
}
#[cfg(unix)]
#[test]
fn test_fix_executable() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
let path = repo_path.join("file");
std::fs::write(&path, "content").unwrap();
let mut permissions = std::fs::metadata(&path).unwrap().permissions();
permissions.set_mode(permissions.mode() | 0o111);
std::fs::set_permissions(&path, permissions).unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: qpvuntsm eea49ac9 (no description set)
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @"CONTENT");
let executable = std::fs::metadata(&path).unwrap().permissions().mode() & 0o111;
assert_eq!(executable, 0o111);
}
#[test]
fn test_fix_trivial_merge_commit() {
// All the changes are attributable to a parent, so none are fixed (in the same
// way that none would be shown in `jj diff -r @`).
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file_a"), "content a").unwrap();
std::fs::write(repo_path.join("file_c"), "content c").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]);
test_env.jj_cmd_ok(&repo_path, &["new", "@-"]);
std::fs::write(repo_path.join("file_b"), "content b").unwrap();
std::fs::write(repo_path.join("file_c"), "content c").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]);
test_env.jj_cmd_ok(&repo_path, &["new", "a", "b"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 0 commits of 1 checked.
Nothing changed.
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_a", "-r", "@"]);
insta::assert_snapshot!(content, @"content a");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_b", "-r", "@"]);
insta::assert_snapshot!(content, @"content b");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_c", "-r", "@"]);
insta::assert_snapshot!(content, @"content c");
}
#[test]
fn test_fix_adding_merge_commit() {
// None of the changes are attributable to a parent, so they are all fixed (in
// the same way that they would be shown in `jj diff -r @`).
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file_a"), "content a").unwrap();
std::fs::write(repo_path.join("file_c"), "content c").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]);
test_env.jj_cmd_ok(&repo_path, &["new", "@-"]);
std::fs::write(repo_path.join("file_b"), "content b").unwrap();
std::fs::write(repo_path.join("file_c"), "content c").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]);
test_env.jj_cmd_ok(&repo_path, &["new", "a", "b"]);
std::fs::write(repo_path.join("file_a"), "change a").unwrap();
std::fs::write(repo_path.join("file_b"), "change b").unwrap();
std::fs::write(repo_path.join("file_c"), "change c").unwrap();
std::fs::write(repo_path.join("file_d"), "change d").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: mzvwutvl 899a1398 (no description set)
Parent commit : qpvuntsm 34782c48 a | (no description set)
Parent commit : kkmpptxz 82e9bc6a b | (no description set)
Added 0 files, modified 4 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_a", "-r", "@"]);
insta::assert_snapshot!(content, @"CHANGE A");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_b", "-r", "@"]);
insta::assert_snapshot!(content, @"CHANGE B");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_c", "-r", "@"]);
insta::assert_snapshot!(content, @"CHANGE C");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file_d", "-r", "@"]);
insta::assert_snapshot!(content, @"CHANGE D");
}
#[test]
fn test_fix_both_sides_of_conflict() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "content a\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]);
test_env.jj_cmd_ok(&repo_path, &["new", "@-"]);
std::fs::write(repo_path.join("file"), "content b\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]);
test_env.jj_cmd_ok(&repo_path, &["new", "a", "b"]);
// The conflicts are not different from the merged parent, so they would not be
// fixed if we didn't fix the parents also.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a", "-s", "b"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 3 commits of 3 checked.
Working copy now at: mzvwutvl b7967885 (conflict) (empty) (no description set)
Parent commit : qpvuntsm 06fe435a a | (no description set)
Parent commit : kkmpptxz ce7ee79e b | (no description set)
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "a"]);
insta::assert_snapshot!(content, @r###"
CONTENT A
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "b"]);
insta::assert_snapshot!(content, @r###"
CONTENT B
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @r###"
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
+CONTENT A
+++++++ Contents of side #2
CONTENT B
>>>>>>> Conflict 1 of 1 ends
"###);
}
#[test]
fn test_fix_resolve_conflict() {
// If both sides of the conflict look the same after being fixed, the conflict
// will be resolved.
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "Content\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]);
test_env.jj_cmd_ok(&repo_path, &["new", "@-"]);
std::fs::write(repo_path.join("file"), "cOnTeNt\n").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]);
test_env.jj_cmd_ok(&repo_path, &["new", "a", "b"]);
// The conflicts are not different from the merged parent, so they would not be
// fixed if we didn't fix the parents also.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix", "-s", "a", "-s", "b"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 3 commits of 3 checked.
Working copy now at: mzvwutvl 98dec555 (conflict) (no description set)
Parent commit : qpvuntsm 3c63716f a | (no description set)
Parent commit : kkmpptxz 82703f5e b | (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "@"]);
insta::assert_snapshot!(content, @r###"
CONTENT
"###);
}

2
cli/tests/test_util_command.rs
View file

@ -30,6 +30,8 @@ fn test_util_config_schema() {
"description": "User configuration for Jujutsu VCS. See https://github.com/martinvonz/jj/blob/main/docs/config.md for details",
"properties": {
[...]
"fix": {
[...]
}
}
"###)