Merge branch 'tb/t0027-raciness-fix'

The t0027 test for CRLF conversion was timing dependent and flaky.

* tb/t0027-raciness-fix:
convert: Correct NNO tests and missing `LF will be replaced by CRLF`

diff --git a/convert.c b/convert.c
index 67d69b5c0e22a74e6dc46764beaff9978a78b85a..077f5e601e3283b7b2566bff791290209be4694e 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -189,33 +189,25 @@ static enum eol output_eol(enum crlf_action crlf_action)
 }
 
 static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
-                            struct text_stat *stats, enum safe_crlf checksafe)
+                           struct text_stat *old_stats, struct text_stat *new_stats,
+                           enum safe_crlf checksafe)
 {
-       if (!checksafe)
-               return;
-
-       if (output_eol(crlf_action) == EOL_LF) {
+       if (old_stats->crlf && !new_stats->crlf ) {
                /*
-                * CRLFs would not be restored by checkout:
-                * check if we'd remove CRLFs
+                * CRLFs would not be restored by checkout
                 */
-               if (stats->crlf) {
-                       if (checksafe == SAFE_CRLF_WARN)
-                               warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
-                       else /* i.e. SAFE_CRLF_FAIL */
-                               die("CRLF would be replaced by LF in %s.", path);
-               }
-       } else if (output_eol(crlf_action) == EOL_CRLF) {
+               if (checksafe == SAFE_CRLF_WARN)
+                       warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
+               else /* i.e. SAFE_CRLF_FAIL */
+                       die("CRLF would be replaced by LF in %s.", path);
+       } else if (old_stats->lonelf && !new_stats->lonelf ) {
                /*
-                * CRLFs would be added by checkout:
-                * check if we have "naked" LFs
+                * CRLFs would be added by checkout
                 */
-               if (stats->lonelf) {
-                       if (checksafe == SAFE_CRLF_WARN)
-                               warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
-                       else /* i.e. SAFE_CRLF_FAIL */
-                               die("LF would be replaced by CRLF in %s", path);
-               }
+               if (checksafe == SAFE_CRLF_WARN)
+                       warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
+               else /* i.e. SAFE_CRLF_FAIL */
+                       die("LF would be replaced by CRLF in %s", path);
        }
 }
 
@@ -233,12 +225,35 @@ static int has_cr_in_index(const char *path)
        return has_cr;
 }
 
+static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
+                                  enum crlf_action crlf_action)
+{
+       if (output_eol(crlf_action) != EOL_CRLF)
+               return 0;
+       /* No "naked" LF? Nothing to convert, regardless. */
+       if (!stats->lonelf)
+               return 0;
+
+       if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
+               /* If we have any CR or CRLF line endings, we do not touch it */
+               /* This is the new safer autocrlf-handling */
+               if (stats->lonecr || stats->crlf)
+                       return 0;
+
+               if (convert_is_binary(len, stats))
+                       return 0;
+       }
+       return 1;
+
+}
+
 static int crlf_to_git(const char *path, const char *src, size_t len,
                       struct strbuf *buf,
                       enum crlf_action crlf_action, enum safe_crlf checksafe)
 {
        struct text_stat stats;
        char *dst;
+       int convert_crlf_into_lf;
 
        if (crlf_action == CRLF_BINARY ||
            (src && !len))
@@ -252,6 +267,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
                return 1;
 
        gather_stats(src, len, &stats);
+       /* Optimization: No CRLF? Nothing to convert, regardless. */
+       convert_crlf_into_lf = !!stats.crlf;
 
        if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
                if (convert_is_binary(len, &stats))
@@ -263,12 +280,24 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
                if (checksafe == SAFE_CRLF_RENORMALIZE)
                        checksafe = SAFE_CRLF_FALSE;
                else if (has_cr_in_index(path))
-                       return 0;
+                       convert_crlf_into_lf = 0;
        }
-       check_safe_crlf(path, crlf_action, &stats, checksafe);
-
-       /* Optimization: No CRLF? Nothing to convert, regardless. */
-       if (!stats.crlf)
+       if (checksafe && len) {
+               struct text_stat new_stats;
+               memcpy(&new_stats, &stats, sizeof(new_stats));
+               /* simulate "git add" */
+               if (convert_crlf_into_lf) {
+                       new_stats.lonelf += new_stats.crlf;
+                       new_stats.crlf = 0;
+               }
+               /* simulate "git checkout" */
+               if (will_convert_lf_to_crlf(len, &new_stats, crlf_action)) {
+                       new_stats.crlf += new_stats.lonelf;
+                       new_stats.lonelf = 0;
+               }
+               check_safe_crlf(path, crlf_action, &stats, &new_stats, checksafe);
+       }
+       if (!convert_crlf_into_lf)
                return 0;
 
        /*
@@ -314,21 +343,9 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
                return 0;
 
        gather_stats(src, len, &stats);
-
-       /* No "naked" LF? Nothing to convert, regardless. */
-       if (!stats.lonelf)
+       if (!will_convert_lf_to_crlf(len, &stats, crlf_action))
                return 0;
 
-       if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || crlf_action == CRLF_AUTO_CRLF) {
-               /* If we have any CR or CRLF line endings, we do not touch it */
-               /* This is the new safer autocrlf-handling */
-               if (stats.lonecr || stats.crlf )
-                       return 0;
-
-               if (convert_is_binary(len, &stats))
-                       return 0;
-       }
-
        /* are we "faking" in place editing ? */
        if (src == buf->buf)
                to_free = strbuf_detach(buf, NULL);

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2860d2d08ba08f83dfeacc16616af11d0183dc0e..90db54c9f976c3c7b1d1d1a55bc3be6e557f2aa2 100755 (executable)
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
                fname=${pfx}_$f.txt &&
                cp $f $fname &&
                printf Z >>"$fname" &&
-               git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
-               git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
+               git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
        done
 
        test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
@@ -417,7 +416,8 @@ commit_chk_wrnNNO "text"  ""      false   "$WILC"   "$WICL"   "$WAMIX"    "$WILC
 commit_chk_wrnNNO "text"  ""      true    LF_CRLF   ""        LF_CRLF     LF_CRLF     ""
 commit_chk_wrnNNO "text"  ""      input   ""        CRLF_LF   CRLF_LF     ""          CRLF_LF
 
-test_expect_success 'create files cleanup' '
+test_expect_success 'commit NNO and cleanup' '
+       git commit -m "commit files on top of NNO" &&
        rm -f *.txt &&
        git -c core.autocrlf=false reset --hard
 '