Skip to content

Commit

Permalink
lib: add support for .mailmap files
Browse files Browse the repository at this point in the history
These live in the repository and map email addresses and names to
canonical ones, as described in [`gitmailmap(5)`].

[`gitmailmap(5)`]: https://git-scm.com/docs/gitmailmap

This is not meant to be a comprehensive solution to identity management
or obsolete the discussion in #2957. There are many possible designs of
forward‐thinking author and committer identity systems that would
be a lot better than `.mailmap` files, but I don’t really want
to get lost in the weeds trying to solve an open research problem
here. Instead, this is just an acknowledgement that any system that
treats user names and emails as immutable (as Jujutsu currently does)
is going to need a mapping layer to keep them updated, and both Git
and Mercurial adopted `.mailmap` files, meaning they are already in
wide use to address this problem. All sufficiently large open source
repositories tend to grow a substantial `.mailmap` file, e.g. [Linux],
[Rust], [curl], [Mesa], [Node.js], and [Git] itself. Currently,
people working on these repositories with Jujutsu see and search
outdated and inconsistent authorship information that contradicts
what Git queries and outputs, which is at the very least somewhere
between confusing and unhelpful. Even if we had a perfect orthogonal
solution in the native backend, as long as we support working on Git
repositories it’s a compatibility‐relevant feature.

[Linux]: https://github.com/torvalds/linux/blob/f2661062f16b2de5d7b6a5c42a9a5c96326b8454/.mailmap
[Rust]: https://github.com/rust-lang/rust/blob/2c243d957008f5909f7a4af19e486ea8a3814be7/.mailmap
[curl]: https://github.com/curl/curl/blob/a7ec6a76abf5e29fb3f951a09d429ce5fbff250f/.mailmap
[Mesa]: https://gitlab.freedesktop.org/mesa/mesa/-/blob/cdf3228f88361410175c338704908ea74dc7b8ae/.mailmap
[Node.js]: https://github.com/nodejs/node/blob/4c730aed7f825af1691740663d599e9de5958f89/.mailmap
[Git]: https://github.com/git/git/blob/9005149a4a77e2d3409c6127bf4fd1a0893c3495/.mailmap

That said, this is not exclusive to the Git backend. The `.mailmap`
name and format is perfectly generic, already shared between Git and
Mercurial, and applies to all systems that bake names and emails into
commits, including the current local backend. The code uses Gitoxide,
but only as a convenient implementation of the file format; in a
hypothetical world where the Git backend was removed without Jujutsu
changing its notion of commit signatures, `gix-mailmap` could be used
standalone, or replaced with a bespoke implementation.

I discussed this on the Discord server and we seemed to arrive
at a consensus that this would be a good feature to have for Git
compatibility and as a pragmatic stop‐gap measure for the larger
identity management problem, and that I should have a crack at
implementing it to see how complex it would be. Happily, it turned
out to be pretty simple! No major plumbing of state is required as
the users of the template and revset engines already have the working
copy commit close to hand to support displaying and matching `@`; I
think this should be more lightweight (but admittedly less powerful)
than the commit rewriting approach @arxanas floated on Discord.

## Notes on various design decisions

* The `.mailmap` file is read from the working copy commit of the
  current workspace.

  This is roughly equivalent to Git reading from
  `$GIT_WORK_TREE/.mailmap`, or `HEAD:.mailmap` in bare repositories,
  and seems like the best fit for Jujutsu’s model. I briefly looked
  into reading it from the actual on‐disk working copy, but it seemed
  a lot more complicated and I’m not sure if there’s any point.

  I didn’t add support for Git’s `mailmap.file` and `mailmap.blob`
  configuration options; unlike ignores, I don’t think I’ve
  ever seen this feature used other than directly in a repository,
  and `mailmap.blob` seems to mostly be there to keep it working in
  bare repositories. I can imagine something like a managed corporate
  multi‐repo environment with a globally‐shared `mailmap.file`
  so if people feel like this is important to keep consistency with I
  can look into implementing it. But genuinely I’ve never personally
  seen anybody use this.

* The `author`/`committer` DSL functions respect the `.mailmap`, with
  `*_raw` variants to ignore it.

  If there’s a `.mailmap` available, signatures should be mapped
  through it unless there’s a specific reason not to; this matches
  Git’s behaviour and is the main thing that makes this feature
  worthwhile. There is a corresponding breaking change of the external
  Rust API, but hopefully the new method name and documentation will
  nudge people towards doing the right thing.

  I was initially considering a keyword argument to the template
  and revset functions to specify whether to map or not (and
  even implemented keyword arguments for template functions), but
  I decided it was probably overkill and settled on the current
  separate functions. A suggestion from Discord was to add a method
  on signatures to the template language, e.g. `.canonical()` or
  `.mailmap()`. While this seems elegant to me, I still wanted
  the short, simple construction to be right by default, and I
  couldn’t think of any immediate uses outside of `.author()`
  and `.committer()`. If this is added later, we will still get the
  elegant property that `commit.{author,committer}()` is short for
  `commit.{author,committer}_raw().canonical()`.

* The mapping to canonical signatures is one‐way, and queries
  only match on the canonical form.

  This is the same behaviour as Git. The alternative would be to
  consider the mapped signatures as an equivalence set and allow
  a query for any member to match all of them, but this would
  contradict what is actually displayed for the commits, require
  a more complicated implementation (to support patterns, we would
  basically have to map signatures back to every possible form and
  run the pattern against each of them), and the `*_raw` functions
  may be more useful in such a case anyway.

* There’s currently no real caching or optimization here.

  The `.mailmap` file is materialized and parsed whenever a template
  or revset context is initialized (although it’s still O(1), not
  parsing it for every processed commit), and `gix-mailmap` does a
  binary search to resolve signatures. I couldn’t measure any kind
  of substantial performance hit here, maybe 1‐3% percent on some
  `jj log` microbenchmarks, but it could just be noise; a couple
  times it was actually faster.
  • Loading branch information
emilazy committed Jun 25, 2024
1 parent b24e73f commit 04df057
Show file tree
Hide file tree
Showing 18 changed files with 582 additions and 18 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* New command `jj git remote set-url` that sets the url of a git remote.

* Support for [`.mailmap`](https://git-scm.com/docs/gitmailmap) files has
been added.

### Fixed bugs

* `jj git push` now ignores immutable commits when checking whether a
Expand Down
19 changes: 17 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ gix = { version = "0.63.0", default-features = false, features = [
"index",
"max-performance-safe",
] }
# We list `gix-{actor,date,mailmap}` separately, as they are required
# even when the Git backend is disabled.
gix-actor = { version = "0.31.2" }
gix-date = { version = "0.8.7" }
gix-mailmap = { version = "0.23.2" }
glob = "0.3.1"
hex = "0.4.3"
ignore = "0.4.20"
Expand Down
6 changes: 6 additions & 0 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use jj_lib::git_backend::GitBackend;
use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile};
use jj_lib::hex_util::to_reverse_hex;
use jj_lib::id_prefix::IdPrefixContext;
use jj_lib::mailmap::get_current_mailmap;
use jj_lib::matchers::Matcher;
use jj_lib::merge::MergedTreeValue;
use jj_lib::merged_tree::MergedTree;
Expand Down Expand Up @@ -991,11 +992,16 @@ impl WorkspaceCommandHelper {
path_converter: &self.path_converter,
workspace_id: self.workspace_id(),
};
let mailmap = Rc::new(get_current_mailmap(
self.repo().as_ref(),
self.workspace_id(),
));
RevsetParseContext::new(
&self.revset_aliases_map,
self.settings.user_email(),
&self.revset_extensions,
Some(workspace_context),
mailmap,
)
}

Expand Down
26 changes: 24 additions & 2 deletions cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use jj_lib::extensions_map::ExtensionsMap;
use jj_lib::git;
use jj_lib::hex_util::to_reverse_hex;
use jj_lib::id_prefix::IdPrefixContext;
use jj_lib::mailmap::{get_current_mailmap, Mailmap};
use jj_lib::object_id::ObjectId as _;
use jj_lib::op_store::{RefTarget, RemoteRef, WorkspaceId};
use jj_lib::repo::Repo;
Expand Down Expand Up @@ -61,6 +62,7 @@ pub struct CommitTemplateLanguage<'repo> {
build_fn_table: CommitTemplateBuildFnTable<'repo>,
keyword_cache: CommitKeywordCache<'repo>,
cache_extensions: ExtensionsMap,
mailmap: Rc<Mailmap>,
}

impl<'repo> CommitTemplateLanguage<'repo> {
Expand All @@ -83,6 +85,8 @@ impl<'repo> CommitTemplateLanguage<'repo> {
.build_cache_extensions(&mut cache_extensions);
}

let mailmap = Rc::new(get_current_mailmap(repo, workspace_id));

CommitTemplateLanguage {
repo,
workspace_id: workspace_id.clone(),
Expand All @@ -91,6 +95,7 @@ impl<'repo> CommitTemplateLanguage<'repo> {
build_fn_table,
keyword_cache: CommitKeywordCache::default(),
cache_extensions,
mailmap,
}
}
}
Expand Down Expand Up @@ -468,8 +473,14 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
Ok(L::wrap_commit_list(out_property))
},
);
map.insert("author", |language, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let mailmap = language.mailmap.clone();
let out_property = self_property.map(move |commit| mailmap.author(&commit));
Ok(L::wrap_signature(out_property))
});
map.insert(
"author",
"author_raw",
|_language, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let out_property = self_property.map(|commit| commit.author_raw().clone());
Expand All @@ -478,6 +489,15 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
);
map.insert(
"committer",
|language, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let mailmap = language.mailmap.clone();
let out_property = self_property.map(move |commit| mailmap.committer(&commit));
Ok(L::wrap_signature(out_property))
},
);
map.insert(
"committer_raw",
|_language, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let out_property = self_property.map(|commit| commit.committer_raw().clone());
Expand All @@ -486,8 +506,10 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
);
map.insert("mine", |language, _build_ctx, self_property, function| {
function.expect_no_arguments()?;
let mailmap = language.mailmap.clone();
let user_email = language.revset_parse_context.user_email().to_owned();
let out_property = self_property.map(move |commit| commit.author_raw().email == user_email);
let out_property =
self_property.map(move |commit| mailmap.author(&commit).email == user_email);
Ok(L::wrap_boolean(out_property))
});
map.insert(
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl TestEnvironment {
cmd
}

fn get_ok(&self, mut cmd: assert_cmd::Command) -> (String, String) {
pub(crate) fn get_ok(&self, mut cmd: assert_cmd::Command) -> (String, String) {
let assert = cmd.assert().success();
let stdout = self.normalize_output(&get_stdout_string(&assert));
let stderr = self.normalize_output(&get_stderr_string(&assert));
Expand Down
1 change: 1 addition & 0 deletions cli/tests/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ mod test_immutable_commits;
mod test_init_command;
mod test_interdiff_command;
mod test_log_command;
mod test_mailmap;
mod test_move_command;
mod test_new_command;
mod test_next_prev_commands;
Expand Down
162 changes: 162 additions & 0 deletions cli/tests/test_mailmap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
// 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.

use crate::common::TestEnvironment;

#[test]
fn test_mailmap() {
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 mut mailmap = String::new();
let mailmap_path = repo_path.join(".mailmap");
let mut append_mailmap = move |extra| {
mailmap.push_str(extra);
std::fs::write(&mailmap_path, &mailmap).unwrap()
};

let jj_cmd_ok_as = |name: &str, email: &str, args: &[&str]| {
let mut cmd = test_env.jj_cmd(&repo_path, args);
cmd.env("JJ_USER", name);
cmd.env("JJ_EMAIL", email);
test_env.get_ok(cmd)
};

append_mailmap("# test comment\n");

let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author"]);
insta::assert_snapshot!(stdout, @r###"
@ Test User <[email protected]>
"###);

// Map an email address without any name change.
jj_cmd_ok_as("Test Üser", "[email protected]", &["new"]);
append_mailmap("<[email protected]> <[email protected]>\n");

let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author"]);
insta::assert_snapshot!(stdout, @r###"
@ Test Üser <[email protected]>
◉ Test User <[email protected]>
"###);

// Map an email address to a new name.
jj_cmd_ok_as("West User", "[email protected]", &["new"]);
append_mailmap("Fest User <[email protected]>\n");

let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author"]);
insta::assert_snapshot!(stdout, @r###"
@ Fest User <[email protected]>
◉ Test Üser <[email protected]>
◉ Test User <[email protected]>
"###);

// Map an email address to a new name and email address.
jj_cmd_ok_as("Pest User", "[email protected]", &["new"]);
append_mailmap("Best User <[email protected]> <[email protected]>\n");

let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author"]);
insta::assert_snapshot!(stdout, @r###"
@ Best User <[email protected]>
◉ Fest User <[email protected]>
◉ Test Üser <[email protected]>
◉ Test User <[email protected]>
"###);

// Map an ambiguous email address using names for disambiguation.
jj_cmd_ok_as("Rest User", "user@test", &["new"]);
jj_cmd_ok_as("Vest User", "user@test", &["new"]);
append_mailmap(
&[
"Jest User <[email protected]> Rest User <user@test>\n",
"Zest User <[email protected]> Vest User <user@test>\n",
]
.concat(),
);

let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author"]);
insta::assert_snapshot!(stdout, @r###"
@ Zest User <[email protected]>
◉ Jest User <[email protected]>
◉ Best User <[email protected]>
◉ Fest User <[email protected]>
◉ Test Üser <[email protected]>
◉ Test User <[email protected]>
"###);

// The `.mailmap` file in the current workspace’s @ commit should be used.
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author", "--at-operation=@-"]);
insta::assert_snapshot!(stdout, @r###"
@ Vest User <user@test>
◉ Rest User <user@test>
◉ Best User <[email protected]>
◉ Fest User <[email protected]>
◉ Test Üser <[email protected]>
◉ Test User <[email protected]>
"###);

// The `author(pattern)` revset function should find mapped committers.
let stdout =
test_env.jj_cmd_success(&repo_path, &["log", "-T", "author", "-r", "author(best)"]);
insta::assert_snapshot!(stdout, @r###"
◉ Best User <[email protected]>
~
"###);

// The `author(pattern)` revset function should only search the mapped form.
// This matches Git’s behaviour and avoids some tricky implementation questions.
let stdout =
test_env.jj_cmd_success(&repo_path, &["log", "-T", "author", "-r", "author(pest)"]);
insta::assert_snapshot!(stdout, @r###"
"###);

// The `author_raw(pattern)` revset function should search the unmapped
// commit data.
let stdout = test_env.jj_cmd_success(
&repo_path,
&["log", "-T", "author", "-r", "author_raw(\"user@test\")"],
);
insta::assert_snapshot!(stdout, @r###"
@ Zest User <[email protected]>
◉ Jest User <[email protected]>
~
"###);

// `mine()` should only search the mapped author; this may be confusing in this
// case, but matches the semantics of it expanding to `author(‹user.email›)`.
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "author", "-r", "mine()"]);
insta::assert_snapshot!(stdout, @r###"
"###);

// `mine()` should find commits that map to the current `user.email`.
let (stdout, _stderr) = jj_cmd_ok_as(
"Tëst Üser",
"[email protected]",
&["log", "-T", "author", "-r", "mine()"],
);
insta::assert_snapshot!(stdout, @r###"
◉ Test Üser <[email protected]>
◉ Test User <[email protected]>
~
"###);
}
2 changes: 1 addition & 1 deletion cli/tests/test_revset_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ fn test_function_name_hint() {
| ^-----^
|
= Function "author_" doesn't exist
Hint: Did you mean "author", "my_author"?
Hint: Did you mean "author", "author_raw", "my_author"?
"###);

insta::assert_snapshot!(evaluate_err("my_branches"), @r###"
Expand Down
10 changes: 8 additions & 2 deletions docs/revsets.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,17 @@ revsets (expressions) as arguments.
* `author(pattern)`: Commits with the author's name or email matching the given
[string pattern](#string-patterns).

* `author_raw(pattern)`: Like `author(pattern)`, but ignoring any mappings in
the [`.mailmap` file](https://git-scm.com/docs/gitmailmap).

* `mine()`: Commits where the author's email matches the email of the current
user.

* `committer(pattern)`: Commits with the committer's name or email matching the
given [string pattern](#string-patterns).
* `committer(pattern)`: Commits with the committer's name or email matching the
given [string pattern](#string-patterns).

* `committer_raw(pattern)`: Like `committer(pattern)`, but ignoring any
mappings in the [`.mailmap` file](https://git-scm.com/docs/gitmailmap).

* `empty()`: Commits modifying no files. This also includes `merges()` without
user modifications and `root()`.
Expand Down
4 changes: 4 additions & 0 deletions docs/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ This type cannot be printed. The following methods are defined.
* `commit_id() -> CommitId`
* `parents() -> List<Commit>`
* `author() -> Signature`
* `author_raw() -> Signature`: Like `author()`, but ignoring any mappings in
the [`.mailmap` file](https://git-scm.com/docs/gitmailmap).
* `committer() -> Signature`
* `committer_raw() -> Signature`: Like `committer()`, but ignoring any mappings
in the [`.mailmap` file](https://git-scm.com/docs/gitmailmap).
* `mine() -> Boolean`: Commits where the author's email matches the email of the current
user.
* `working_copies() -> String`: For multi-workspace repository, indicate
Expand Down
3 changes: 3 additions & 0 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ either = { workspace = true }
futures = { workspace = true }
git2 = { workspace = true, optional = true }
gix = { workspace = true, optional = true }
gix-actor = { workspace = true }
gix-date = { workspace = true }
gix-mailmap = { workspace = true }
glob = { workspace = true }
hex = { workspace = true }
ignore = { workspace = true }
Expand Down
Loading

0 comments on commit 04df057

Please sign in to comment.