filter: raise fatal notifications for core.safecrlf failures

Don't just raise CRLF notifications when `core.safecrlf=warn`, also
raise notifications when `core.safecrlf=true`. The `core.safecrlf=true`
notifications are _fatal_ errors, however. These exist so that users can
get the filename that failed during a fatal error.

include/git2/notification.h

@@ -83,7 +83,9 @@ typedef enum {
 typedef enum {
 	/**
 	 * A notification provided when `core.safecrlf` is configured and a
-	 * file has line-ending reversability problems.
+	 * file has line-ending reversability problems. The level will be
+	 * `WARN` (when `core.safecrlf=warn`) or `FATAL` (when
+	 * `core.safecrlf=on`).
 	 *
 	 * The data will be:
 	 *

src/libgit2/crlf.c

@@ -151,7 +151,10 @@ static git_configmap_value output_eol(struct crlf_attrs *ca)
 	return ca->core_eol;
 }
 
-static int warn_safecrlf(int direction, const char *filename)
+static int notify_safecrlf(
+	git_notification_level_t level,
+	int direction,
+	const char *filename)
 {
 	git_str message = GIT_STR_INIT;
 	int error;
@@ -179,7 +182,7 @@ static int warn_safecrlf(int direction, const char *filename)
 	if (git_str_oom(&message))
 		error = -1;
 	else
-		error = git_notification(GIT_NOTIFICATION_WARN,
+		error = git_notification(level,
 			GIT_NOTIFICATION_CRLF,
 			message.ptr, filename);
 
@@ -187,38 +190,51 @@ static int warn_safecrlf(int direction, const char *filename)
 	return error;
 }
 
+static int error_safecrlf(
+	int direction,
+	const char *filename)
+{
+	const char *message = (direction == GIT_EOL_LF) ?
+		"CRLF would be replaced by LF" :
+		"LF would be replaced by CRLF";
+
+	if (filename && *filename)
+		git_error_set(GIT_ERROR_FILTER, "%s in '%s'", message,
+			filename);
+	else
+		git_error_set(GIT_ERROR_FILTER, "%s", message);
+
+	return -1;
+}
+
 GIT_INLINE(int) check_safecrlf(
 	struct crlf_attrs *ca,
 	const git_filter_source *src,
 	git_str_text_stats *stats)
 {
 	const char *filename = git_filter_source_path(src);
+	git_notification_level_t level;
 
 	if (!ca->safe_crlf)
 		return 0;
 
+	level = (ca->safe_crlf == GIT_SAFE_CRLF_WARN) ?
+		GIT_NOTIFICATION_WARN :
+		GIT_NOTIFICATION_FATAL;
+
 	if (output_eol(ca) == GIT_EOL_LF) {
 		/*
 		 * CRLFs would not be restored by checkout:
 		 * check if we'd remove CRLFs
 		 */
 		if (stats->crlf) {
-			if (ca->safe_crlf == GIT_SAFE_CRLF_WARN) {
-				int error = warn_safecrlf(GIT_EOL_LF, filename);
+			int error = notify_safecrlf(level,
+					GIT_EOL_LF, filename);
 
-				if (error != 0)
-					return error;
-			} else {
-				if (filename && *filename)
-					git_error_set(
-						GIT_ERROR_FILTER, "CRLF would be replaced by LF in '%s'",
-						filename);
-				else
-					git_error_set(
-						GIT_ERROR_FILTER, "CRLF would be replaced by LF");
-
-				return -1;
-			}
+			if (error != 0)
+				return error;
+			else if (ca->safe_crlf != GIT_SAFE_CRLF_WARN)
+				return error_safecrlf(GIT_EOL_LF, filename);
 		}
 	} else if (output_eol(ca) == GIT_EOL_CRLF) {
 		/*
@@ -226,22 +242,13 @@ GIT_INLINE(int) check_safecrlf(
 		 * check if we have "naked" LFs
 		 */
 		if (stats->crlf != stats->lf) {
-			if (ca->safe_crlf == GIT_SAFE_CRLF_WARN) {
-				int error = warn_safecrlf(GIT_EOL_CRLF, filename);
+			int error = notify_safecrlf(level,
+				GIT_EOL_CRLF, filename);
 
-				if (error != 0)
-					return error;
-			} else {
-				if (filename && *filename)
-					git_error_set(
-						GIT_ERROR_FILTER, "LF would be replaced by CRLF in '%s'",
-						filename);
-				else
-					git_error_set(
-						GIT_ERROR_FILTER, "LF would be replaced by CRLF");
-
-				return -1;
-			}
+			if (error != 0)
+				return error;
+			else if (ca->safe_crlf != GIT_SAFE_CRLF_WARN)
+				return error_safecrlf(GIT_EOL_CRLF, filename);
 		}
 	}
 

tests/libgit2/filter/crlf.c

@@ -73,7 +73,24 @@ void test_filter_crlf__to_odb(void)
 	git_buf_dispose(&out);
 }
 
-static int notification_cb(
+static int fatal_notification_cb(
+	git_notification_level_t notification_level,
+	git_notification_t notification_type,
+	const char *message,
+	void *data,
+	...)
+{
+	GIT_UNUSED(message);
+
+	cl_assert_equal_i(notification_level, GIT_NOTIFICATION_FATAL);
+	cl_assert_equal_i(notification_type, GIT_NOTIFICATION_CRLF);
+
+	(*((int *)data))++;
+
+	return 0;
+}
+
+static int warn_notification_cb(
 	git_notification_level_t notification_level,
 	git_notification_t notification_type,
 	const char *message,
@@ -100,7 +117,7 @@ void test_filter_crlf__with_safecrlf(void)
 	size_t in_len;
 
 	cl_repo_set_bool(g_repo, "core.safecrlf", true);
-	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, notification_cb, &notification_count));
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, fatal_notification_cb, &notification_count));
 
 	cl_git_pass(git_filter_list_new(
 		&fl, g_repo, GIT_FILTER_TO_ODB, 0));
@@ -124,7 +141,7 @@ void test_filter_crlf__with_safecrlf(void)
 
 	cl_invoke(cl_git_fail(git_filter_list_apply_to_buffer(&out, fl, in, in_len)));
 	cl_assert_equal_i(git_error_last()->klass, GIT_ERROR_FILTER);
-	cl_assert_equal_i(0, notification_count);
+	cl_assert_equal_i(1, notification_count);
 
 	/* Normalized \n fails for autocrlf=true when safecrlf=true */
 	in = "Normal\nLF\nonly\nline-endings.\n";
@@ -132,7 +149,7 @@ void test_filter_crlf__with_safecrlf(void)
 
 	cl_invoke(cl_git_fail(git_filter_list_apply_to_buffer(&out, fl, in, in_len)));
 	cl_assert_equal_i(git_error_last()->klass, GIT_ERROR_FILTER);
-	cl_assert_equal_i(0, notification_count);
+	cl_assert_equal_i(2, notification_count);
 
 	/* String with \r but without \r\n does not fail with safecrlf */
 	in = "Normal\nCR only\rand some more\nline-endings.\n";
@@ -140,7 +157,7 @@ void test_filter_crlf__with_safecrlf(void)
 
 	cl_invoke(cl_git_pass(git_filter_list_apply_to_buffer(&out, fl, in, in_len)));
 	cl_assert_equal_s("Normal\nCR only\rand some more\nline-endings.\n", out.ptr);
-	cl_assert_equal_i(0, notification_count);
+	cl_assert_equal_i(2, notification_count);
 
 	git_filter_list_free(fl);
 	git_buf_dispose(&out);
@@ -156,7 +173,7 @@ void test_filter_crlf__with_safecrlf_and_unsafe_allowed(void)
 	size_t in_len;
 
 	cl_repo_set_bool(g_repo, "core.safecrlf", true);
-	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, notification_cb, &notification_count));
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, warn_notification_cb, &notification_count));
 
 	cl_git_pass(git_filter_list_new(
 		&fl, g_repo, GIT_FILTER_TO_ODB, GIT_FILTER_ALLOW_UNSAFE));
@@ -203,7 +220,7 @@ void test_filter_crlf__no_safecrlf(void)
 	const char *in;
 	size_t in_len;
 
-	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, notification_cb, &notification_count));
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, warn_notification_cb, &notification_count));
 
 	cl_git_pass(git_filter_list_new(
 		&fl, g_repo, GIT_FILTER_TO_ODB, 0));
@@ -251,7 +268,7 @@ void test_filter_crlf__safecrlf_warn(void)
 	size_t in_len;
 
 	cl_repo_set_string(g_repo, "core.safecrlf", "warn");
-	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, notification_cb, &notification_count));
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_NOTIFICATION_CALLBACK, warn_notification_cb, &notification_count));
 
 	cl_git_pass(git_filter_list_new(
 		&fl, g_repo, GIT_FILTER_TO_ODB, 0));