-
Notifications
You must be signed in to change notification settings - Fork 319
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 various `author`/`committer` DSL and Rust 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. This does constitute a breaking change of the external Rust API, but hopefully the new parameter 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
Showing
19 changed files
with
559 additions
and
27 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]> | ||
│ | ||
~ | ||
"###); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.