Add GIT_DIFF_LINE_CONTEXT_EOFNL

This adds a new line origin constant for the special line that
is used when both files end without a newline.

In the course of writing the tests for this, I was having problems
with modifying a file but not having diff notice because it was
the same size and modified less than one second from the start of
the test, so I decided to start working on nanosecond timestamp
support.  This commit doesn't contain the nanosecond support, but
it contains the reorganization of maybe_modified and the hooks so
that if the nanosecond data were being read by stat() (or rather
being copied by git_index_entry__init_from_stat), then the nsec
would be taken into account.

This new stuff could probably use some more tests, although there
is some amount of it here.

include/git2/diff.h

@@ -356,8 +356,10 @@ typedef enum {
 	GIT_DIFF_LINE_CONTEXT   = ' ',
 	GIT_DIFF_LINE_ADDITION  = '+',
 	GIT_DIFF_LINE_DELETION  = '-',
-	GIT_DIFF_LINE_ADD_EOFNL = '\n', /**< Removed line w/o LF & added one with */
-	GIT_DIFF_LINE_DEL_EOFNL = '\0', /**< LF was removed at end of file */
+
+	GIT_DIFF_LINE_CONTEXT_EOFNL = '=', /**< Both files have no LF at end */
+	GIT_DIFF_LINE_ADD_EOFNL = '>',     /**< Old has no LF at end, new does */
+	GIT_DIFF_LINE_DEL_EOFNL = '<',     /**< Old has LF at end, new does not */
 
 	/* The following values will only be sent to a `git_diff_data_cb` when
 	 * the content of a diff is being formatted (eg. through

src/diff.c

@@ -389,6 +389,9 @@ static int diff_list_apply_options(
 
 	/* Don't set GIT_DIFFCAPS_USE_DEV - compile time option in core git */
 
+	/* Set GIT_DIFFCAPS_TRUST_NANOSECS on a platform basis */
+	diff->diffcaps = diff->diffcaps | GIT_DIFFCAPS_TRUST_NANOSECS;
+
 	/* If not given explicit `opts`, check `diff.xyz` configs */
 	if (!opts) {
 		diff->opts.context_lines = config_int(cfg, "diff.context", 3);
@@ -529,6 +532,13 @@ cleanup:
 	return result;
 }
 
+static bool diff_time_eq(
+	const git_index_time *a, const git_index_time *b, bool use_nanos)
+{
+	return a->seconds == a->seconds &&
+		(!use_nanos || a->nanoseconds == b->nanoseconds);
+}
+
 typedef struct {
 	git_repository *repo;
 	git_iterator *old_iter;
@@ -540,11 +550,51 @@ typedef struct {
 
 #define MODE_BITS_MASK 0000777
 
+static int maybe_modified_submodule(
+	git_delta_t *status,
+	git_oid *found_oid,
+	git_diff_list *diff,
+	diff_in_progress *info)
+{
+	int error = 0;
+	git_submodule *sub;
+	unsigned int sm_status = 0;
+	const git_oid *sm_oid;
+
+	*status = GIT_DELTA_UNMODIFIED;
+
+	if (!DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES) &&
+		!(error = git_submodule_lookup(
+			  &sub, diff->repo, info->nitem->path)) &&
+		git_submodule_ignore(sub) != GIT_SUBMODULE_IGNORE_ALL &&
+		!(error = git_submodule_status(&sm_status, sub)))
+	{
+		/* check IS_WD_UNMODIFIED because this case is only used
+		 * when the new side of the diff is the working directory
+		 */
+		if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status))
+			*status = GIT_DELTA_MODIFIED;
+
+		/* grab OID while we are here */
+		if (git_oid_iszero(&info->nitem->oid) &&
+			(sm_oid = git_submodule_wd_id(sub)) != NULL)
+			git_oid_cpy(found_oid, sm_oid);
+	}
+
+	/* GIT_EEXISTS means a dir with .git in it was found - ignore it */
+	if (error == GIT_EEXISTS) {
+		giterr_clear();
+		error = 0;
+	}
+
+	return error;
+}
+
 static int maybe_modified(
 	git_diff_list *diff,
 	diff_in_progress *info)
 {
-	git_oid noid, *use_noid = NULL;
+	git_oid noid;
 	git_delta_t status = GIT_DELTA_MODIFIED;
 	const git_index_entry *oitem = info->oitem;
 	const git_index_entry *nitem = info->nitem;
@@ -560,6 +610,8 @@ static int maybe_modified(
 			&matched_pathspec))
 		return 0;
 
+	memset(&noid, 0, sizeof(noid));
+
 	/* on platforms with no symlinks, preserve mode of existing symlinks */
 	if (S_ISLNK(omode) && S_ISREG(nmode) && new_is_workdir &&
 		!(diff->diffcaps & GIT_DIFFCAPS_HAS_SYMLINKS))
@@ -600,55 +652,30 @@ static int maybe_modified(
 	 * circumstances that can accelerate things or need special handling
 	 */
 	else if (git_oid_iszero(&nitem->oid) && new_is_workdir) {
-		/* TODO: add check against index file st_mtime to avoid racy-git */
+		bool use_ctime = ((diff->diffcaps & GIT_DIFFCAPS_TRUST_CTIME) != 0);
+		bool use_nanos = ((diff->diffcaps & GIT_DIFFCAPS_TRUST_NANOSECS) != 0);
 
-		/* if the stat data looks exactly alike, then assume the same */
-		if (omode == nmode &&
-			oitem->file_size == nitem->file_size &&
-			(!(diff->diffcaps & GIT_DIFFCAPS_TRUST_CTIME) ||
-			 (oitem->ctime.seconds == nitem->ctime.seconds)) &&
-			oitem->mtime.seconds == nitem->mtime.seconds &&
-			(!(diff->diffcaps & GIT_DIFFCAPS_USE_DEV) ||
-			 (oitem->dev == nitem->dev)) &&
-			oitem->ino == nitem->ino &&
-			oitem->uid == nitem->uid &&
-			oitem->gid == nitem->gid)
-			status = GIT_DELTA_UNMODIFIED;
+		status = GIT_DELTA_UNMODIFIED;
 
-		else if (S_ISGITLINK(nmode)) {
-			int err;
-			git_submodule *sub;
-
-			if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES))
-				status = GIT_DELTA_UNMODIFIED;
-			else if ((err = git_submodule_lookup(&sub, diff->repo, nitem->path)) < 0) {
-				if (err == GIT_EEXISTS)
-					status = GIT_DELTA_UNMODIFIED;
-				else
-					return err;
-			} else if (git_submodule_ignore(sub) == GIT_SUBMODULE_IGNORE_ALL)
-				status = GIT_DELTA_UNMODIFIED;
-			else {
-				unsigned int sm_status = 0;
-				if (git_submodule_status(&sm_status, sub) < 0)
-					return -1;
-
-				/* check IS_WD_UNMODIFIED because this case is only used
-				 * when the new side of the diff is the working directory
-				 */
-				status = GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status)
-						 ? GIT_DELTA_UNMODIFIED : GIT_DELTA_MODIFIED;
-
-				/* grab OID while we are here */
-				if (git_oid_iszero(&nitem->oid)) {
-					const git_oid *sm_oid = git_submodule_wd_id(sub);
-					if (sm_oid != NULL) {
-						git_oid_cpy(&noid, sm_oid);
-						use_noid = &noid;
-					}
-				}
-			}
+		/* TODO: add check against index file st_mtime to avoid racy-git */
+
+		if (S_ISGITLINK(nmode)) {
+			if (maybe_modified_submodule(&status, &noid, diff, info) < 0)
+				return -1;
 		}
+
+		/* if the stat data looks different, then mark modified - this just
+		 * means that the OID will be recalculated below to confirm change
+		 */
+		else if (omode != nmode ||
+			oitem->file_size != nitem->file_size ||
+			!diff_time_eq(&oitem->mtime, &nitem->mtime, use_nanos) ||
+			(use_ctime &&
+			 !diff_time_eq(&oitem->ctime, &nitem->ctime, use_nanos)) ||
+			oitem->ino != nitem->ino ||
+			oitem->uid != nitem->uid ||
+			oitem->gid != nitem->gid)
+			status = GIT_DELTA_MODIFIED;
 	}
 
 	/* if mode is GITLINK and submodules are ignored, then skip */
@@ -660,11 +687,10 @@ static int maybe_modified(
 	 * haven't calculated the OID of the new item, then calculate it now
 	 */
 	if (status != GIT_DELTA_UNMODIFIED && git_oid_iszero(&nitem->oid)) {
-		if (!use_noid) {
+		if (git_oid_iszero(&noid)) {
 			if (git_diff__oid_for_file(diff->repo,
 					nitem->path, nitem->mode, nitem->file_size, &noid) < 0)
 				return -1;
-			use_noid = &noid;
 		}
 
 		/* if oid matches, then mark unmodified (except submodules, where
@@ -672,12 +698,13 @@ static int maybe_modified(
 		 * matches between the index and the workdir HEAD)
 		 */
 		if (omode == nmode && !S_ISGITLINK(omode) &&
-			git_oid_equal(&oitem->oid, use_noid))
+			git_oid_equal(&oitem->oid, &noid))
 			status = GIT_DELTA_UNMODIFIED;
 	}
 
 	return diff_delta__from_two(
-		diff, status, oitem, omode, nitem, nmode, use_noid, matched_pathspec);
+		diff, status, oitem, omode, nitem, nmode,
+		git_oid_iszero(&noid) ? NULL : &noid, matched_pathspec);
 }
 
 static bool entry_is_prefixed(

src/diff.h

@@ -26,6 +26,7 @@ enum {
 	GIT_DIFFCAPS_TRUST_MODE_BITS  = (1 << 2), /* use st_mode? */
 	GIT_DIFFCAPS_TRUST_CTIME      = (1 << 3), /* use st_ctime? */
 	GIT_DIFFCAPS_USE_DEV          = (1 << 4), /* use st_dev? */
+	GIT_DIFFCAPS_TRUST_NANOSECS   = (1 << 5), /* use stat time nanoseconds */
 };
 
 enum {

src/diff_output.c

@@ -726,7 +726,7 @@ static int diff_patch_cb(void *priv, mmbuffer_t *bufs, int len)
 		char origin =
 			(*bufs[0].ptr == '+') ? GIT_DIFF_LINE_DEL_EOFNL :
 			(*bufs[0].ptr == '-') ? GIT_DIFF_LINE_ADD_EOFNL :
-			GIT_DIFF_LINE_CONTEXT;
+			GIT_DIFF_LINE_CONTEXT_EOFNL;
 
 		if (ctxt->data_cb != NULL &&
 			ctxt->data_cb(patch->delta, &ctxt->range,

tests-clar/diff/diff_helpers.c

@@ -97,20 +97,15 @@ int diff_line_cb(
 	e->lines++;
 	switch (line_origin) {
 	case GIT_DIFF_LINE_CONTEXT:
+	case GIT_DIFF_LINE_CONTEXT_EOFNL: /* techically not a line */
 		e->line_ctxt++;
 		break;
 	case GIT_DIFF_LINE_ADDITION:
-		e->line_adds++;
-		break;
-	case GIT_DIFF_LINE_ADD_EOFNL:
-		/* technically not a line add, but we'll count it as such */
+	case GIT_DIFF_LINE_ADD_EOFNL: /* technically not a line add */
 		e->line_adds++;
 		break;
 	case GIT_DIFF_LINE_DELETION:
-		e->line_dels++;
-		break;
-	case GIT_DIFF_LINE_DEL_EOFNL:
-		/* technically not a line delete, but we'll count it as such */
+	case GIT_DIFF_LINE_DEL_EOFNL: /* technically not a line delete */
 		e->line_dels++;
 		break;
 	default:

tests-clar/diff/patch.c

@@ -323,12 +323,12 @@ void test_diff_patch__hunks_have_correct_line_numbers(void)
 }
 
 static void check_single_patch_stats(
-	git_repository *repo, size_t hunks, size_t adds, size_t dels)
+	git_repository *repo, size_t hunks, size_t adds, size_t dels, size_t ctxt)
 {
 	git_diff_list *diff;
 	git_diff_patch *patch;
 	const git_diff_delta *delta;
-	size_t actual_adds, actual_dels;
+	size_t actual_ctxt, actual_adds, actual_dels;
 
 	cl_git_pass(git_diff_index_to_workdir(&diff, repo, NULL, NULL));
 
@@ -339,9 +339,10 @@ static void check_single_patch_stats(
 
 	cl_assert_equal_i((int)hunks, (int)git_diff_patch_num_hunks(patch));
 
-	cl_git_pass(
-		git_diff_patch_line_stats(NULL, &actual_adds, &actual_dels, patch));
+	cl_git_pass( git_diff_patch_line_stats(
+		&actual_ctxt, &actual_adds, &actual_dels, patch) );
 
+	cl_assert_equal_sz(ctxt, actual_ctxt);
 	cl_assert_equal_sz(adds, actual_adds);
 	cl_assert_equal_sz(dels, actual_dels);
 
@@ -369,14 +370,14 @@ void test_diff_patch__line_counts_with_eofnl(void)
 	git_buf_consume(&content, end);
 	cl_git_rewritefile("renames/songof7cities.txt", content.ptr);
 
-	check_single_patch_stats(g_repo, 1, 0, 1);
+	check_single_patch_stats(g_repo, 1, 0, 1, 3);
 
 	/* remove trailing whitespace */
 
 	git_buf_rtrim(&content);
 	cl_git_rewritefile("renames/songof7cities.txt", content.ptr);
 
-	check_single_patch_stats(g_repo, 2, 1, 2);
+	check_single_patch_stats(g_repo, 2, 1, 2, 6);
 
 	/* add trailing whitespace */
 
@@ -388,7 +389,29 @@ void test_diff_patch__line_counts_with_eofnl(void)
 	cl_git_pass(git_buf_putc(&content, '\n'));
 	cl_git_rewritefile("renames/songof7cities.txt", content.ptr);
 
-	check_single_patch_stats(g_repo, 1, 1, 1);
+	check_single_patch_stats(g_repo, 1, 1, 1, 3);
+
+	/* no trailing whitespace as context line */
+
+	{
+		/* walk back a couple lines, make space and insert char */
+		char *scan = content.ptr + content.size;
+		int i;
+
+		for (i = 0; i < 5; ++i) {
+			for (--scan; scan > content.ptr && *scan != '\n'; --scan)
+				/* seek to prev \n */;
+		}
+		cl_assert(scan > content.ptr);
+
+		/* overwrite trailing \n with right-shifted content */
+		memmove(scan + 1, scan, content.size - (scan - content.ptr) - 1);
+		/* insert '#' char into space we created */
+		scan[1] = '#';
+	}
+	cl_git_rewritefile("renames/songof7cities.txt", content.ptr);
+
+	check_single_patch_stats(g_repo, 1, 1, 1, 6);
 
 	git_buf_free(&content);
 	git_config_free(cfg);