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

commits: use empty strings instead of placeholders for missing name or email

Browse source 
This commit replaces the functions `UserSettings::user_name_placeholder()`` and
`UserSettings::user_email_placeholder()` with `const` `&str`s to emphasize that
the placeholder strings must not be changed to support commits without
names or email addresses made before this change.
This commit is contained in:
Emily Fox 2023-08-16 16:10:54 -05:00 committed by Emily Kyle Fox
parent 2c88da02b4
commit 3f8ac2198d
8 changed files with 41 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
CHANGELOG.md
View file

@ -32,6 +32,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj` will fail attempts to snapshot new files larger than 1MiB by default. This behavior
can be customized with the `snapshot.max-new-file-size` config option.
* Author and committer signatures now use empty strings to represent unset
names and email addresses.
Older binaries may not warn user when attempting to `git push` commits
with such signatures.
### New features

4
cli/src/cli_util.rs
View file

@ -1397,9 +1397,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
self.update_working_copy(ui, maybe_old_commit.as_ref())?;
}
let settings = &self.settings;
if settings.user_name() == UserSettings::user_name_placeholder()
|| settings.user_email() == UserSettings::user_email_placeholder()
{
if settings.user_name().is_empty() || settings.user_email().is_empty() {
writeln!(
ui.warning(),
r#"Name and email not configured. Until configured, your commits will be created with the empty identity, and can't be pushed to remotes. To configure, run:

12
cli/src/commands/git.rs
View file

@ -905,10 +905,14 @@ fn cmd_git_push(
if commit.description().is_empty() {
reasons.push("it has no description");
}
if commit.author().name == UserSettings::user_name_placeholder()
|| commit.author().email == UserSettings::user_email_placeholder()
|| commit.committer().name == UserSettings::user_name_placeholder()
|| commit.committer().email == UserSettings::user_email_placeholder()
if commit.author().name.is_empty()
|| commit.author().name == UserSettings::USER_NAME_PLACEHOLDER
|| commit.author().email.is_empty()
|| commit.author().email == UserSettings::USER_EMAIL_PLACEHOLDER
|| commit.committer().name.is_empty()
|| commit.committer().name == UserSettings::USER_NAME_PLACEHOLDER
|| commit.committer().email.is_empty()
|| commit.committer().email == UserSettings::USER_EMAIL_PLACEHOLDER
{
reasons.push("it has no author and/or committer set");
}

10
cli/tests/test_git_push.rs
View file

@ -483,14 +483,14 @@ fn test_git_push_missing_author() {
&["git", "push", "--branch", "missing-name"],
);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit 574dffd73428 since it has no author and/or committer set
Error: Won't push commit 944313939bbd since it has no author and/or committer set
"###);
run_without_var("JJ_EMAIL", &["checkout", "root", "-m=initial"]);
run_without_var("JJ_EMAIL", &["branch", "create", "missing-email"]);
let stderr =
test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--branch=missing-email"]);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit e6c50f13f197 since it has no author and/or committer set
Error: Won't push commit 59354714f789 since it has no author and/or committer set
"###);
}
@ -509,7 +509,7 @@ fn test_git_push_missing_committer() {
let stderr =
test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--branch=missing-name"]);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit e009726caa4a since it has no author and/or committer set
Error: Won't push commit 4fd190283d1a since it has no author and/or committer set
"###);
test_env.jj_cmd_success(&workspace_root, &["checkout", "root"]);
test_env.jj_cmd_success(&workspace_root, &["branch", "create", "missing-email"]);
@ -517,7 +517,7 @@ fn test_git_push_missing_committer() {
let stderr =
test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--branch=missing-email"]);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit 27ec5f0793e6 since it has no author and/or committer set
Error: Won't push commit eab97428a6ec since it has no author and/or committer set
"###);
// Test message when there are multiple reasons (missing committer and
@ -526,7 +526,7 @@ fn test_git_push_missing_committer() {
let stderr =
test_env.jj_cmd_failure(&workspace_root, &["git", "push", "--branch=missing-email"]);
insta::assert_snapshot!(stderr, @r###"
Error: Won't push commit f73024ee65ec since it has no description and it has no author and/or committer set
Error: Won't push commit 1143ed607f54 since it has no description and it has no author and/or committer set
"###);
}

8
lib/src/commit_builder.rs
View file

@ -70,10 +70,14 @@ impl CommitBuilder<'_> {
commit.committer = settings.signature();
// If the user had not configured a name and email before but now they have,
// update the author fields with the new information.
if commit.author.name == UserSettings::user_name_placeholder() {
if commit.author.name.is_empty()
|| commit.author.name == UserSettings::USER_NAME_PLACEHOLDER
{
commit.author.name = commit.committer.name.clone();
}
if commit.author.email == UserSettings::user_email_placeholder() {
if commit.author.email.is_empty()
|| commit.author.email == UserSettings::USER_EMAIL_PLACEHOLDER
{
commit.author.email = commit.committer.email.clone();
}
CommitBuilder {

19
lib/src/settings.rs
View file

@ -98,19 +98,14 @@ impl UserSettings {
}
pub fn user_name(&self) -> String {
self.config
.get_string("user.name")
.unwrap_or_else(|_| Self::user_name_placeholder().to_string())
self.config.get_string("user.name").unwrap_or_default()
}
pub fn user_name_placeholder() -> &'static str {
"(no name configured)"
}
// Must not be changed to avoid git pushing older commits with no set name
pub const USER_NAME_PLACEHOLDER: &str = "(no name configured)";
pub fn user_email(&self) -> String {
self.config
.get_string("user.email")
.unwrap_or_else(|_| Self::user_email_placeholder().to_string())
self.config.get_string("user.email").unwrap_or_default()
}
pub fn fsmonitor_kind(&self) -> Result<Option<FsmonitorKind>, config::ConfigError> {
@ -121,9 +116,9 @@ impl UserSettings {
}
}
pub fn user_email_placeholder() -> &'static str {
"(no email configured)"
}
// Must not be changed to avoid git pushing older commits with no set email
// address
pub const USER_EMAIL_PLACEHOLDER: &str = "(no email configured)";
pub fn operation_timestamp(&self) -> Option<Timestamp> {
get_timestamp_config(&self.config, "debug.operation-timestamp")

11
lib/tests/test_commit_builder.rs
View file

@ -188,8 +188,7 @@ fn test_rewrite(use_git: bool) {
);
}
// An author field with the placeholder name/email should get filled in on
// rewrite
// An author field with an empty name/email should get filled in on rewrite
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_rewrite_update_missing_user(use_git: bool) {
@ -208,10 +207,10 @@ fn test_rewrite_update_missing_user(use_git: bool) {
)
.write()
.unwrap();
assert_eq!(initial_commit.author().name, "(no name configured)");
assert_eq!(initial_commit.author().email, "(no email configured)");
assert_eq!(initial_commit.committer().name, "(no name configured)");
assert_eq!(initial_commit.committer().email, "(no email configured)");
assert_eq!(initial_commit.author().name, "");
assert_eq!(initial_commit.author().email, "");
assert_eq!(initial_commit.committer().name, "");
assert_eq!(initial_commit.committer().email, "");
let config = config::Config::builder()
.set_override("user.name", "Configured User")

17
lib/tests/test_init.rs
View file

@ -105,19 +105,10 @@ fn test_init_no_config_set(use_git: bool) {
.get_wc_commit_id(&WorkspaceId::default())
.unwrap();
let wc_commit = repo.store().get_commit(wc_commit_id).unwrap();
assert_eq!(wc_commit.author().name, "(no name configured)".to_string());
assert_eq!(
wc_commit.author().email,
"(no email configured)".to_string()
);
assert_eq!(
wc_commit.committer().name,
"(no name configured)".to_string()
);
assert_eq!(
wc_commit.committer().email,
"(no email configured)".to_string()
);
assert_eq!(wc_commit.author().name, "".to_string());
assert_eq!(wc_commit.author().email, "".to_string());
assert_eq!(wc_commit.committer().name, "".to_string());
assert_eq!(wc_commit.committer().email, "".to_string());
}
#[test_case(false ; "local backend")]