feat: case insensitive diff_test and fix manifest bug

.bazelci/presubmit.yml

@@ -108,7 +108,6 @@ tasks:
       # TODO(laszlocsomor): remove "--test_env=LOCALAPPDATA" after
       # https://github.com/bazelbuild/bazel/issues/7761 is fixed
       ? "--test_env=LOCALAPPDATA"
-      ? "--test_tag_filters=-no_windows"
 
   last_green:
     <<: *reusable_config
@@ -126,6 +125,5 @@ tasks:
       # TODO(laszlocsomor): remove "--test_env=LOCALAPPDATA" after
       # https://github.com/bazelbuild/bazel/issues/7761 is fixed
       ? "--test_env=LOCALAPPDATA"
-      ? "--test_tag_filters=-no_windows"
 
 buildifier: latest

.bazelrc

@@ -0,0 +1,2 @@
+# symlinks required on windows by copy_directory
+startup --windows_enable_symlinks

docs/diff_test_doc.md

@@ -10,7 +10,7 @@ command (fc.exe) on Windows (no Bash is required).
 ## diff_test
 
 <pre>
-diff_test(<a href="#diff_test-name">name</a>, <a href="#diff_test-file1">file1</a>, <a href="#diff_test-file2">file2</a>, <a href="#diff_test-failure_message">failure_message</a>, <a href="#diff_test-kwargs">kwargs</a>)
+diff_test(<a href="#diff_test-name">name</a>, <a href="#diff_test-file1">file1</a>, <a href="#diff_test-file2">file2</a>, <a href="#diff_test-failure_message">failure_message</a>, <a href="#diff_test-ignore_line_endings">ignore_line_endings</a>, <a href="#diff_test-kwargs">kwargs</a>)
 </pre>
 
 A test that compares two files.
@@ -27,6 +27,7 @@ The test succeeds if the files' contents match.
 | <a id="diff_test-file1"></a>file1 |  Label of the file to compare to `file2`.   |  none |
 | <a id="diff_test-file2"></a>file2 |  Label of the file to compare to `file1`.   |  none |
 | <a id="diff_test-failure_message"></a>failure_message |  Additional message to log if the files' contents do not match.   |  `None` |
+| <a id="diff_test-ignore_line_endings"></a>ignore_line_endings |  Ignore differences between CRLF and LF line endings. On windows, this is forced to False if the 'tr' command can't be found in the bash installation on the host.   |  `True` |
 | <a id="diff_test-kwargs"></a>kwargs |  The [common attributes for tests](https://bazel.build/reference/be/common-definitions#common-attributes-tests).   |  none |
 
 

docs/private/stardoc_with_diff_test.bzl

@@ -58,7 +58,6 @@ def stardoc_with_diff_test(
         file1 = out_label,
         # Output from stardoc rule above
         file2 = out_file.replace(".md", "-docgen.md"),
-        tags = ["no_windows"],
     )
 
 def update_docs(

rules/diff_test.bzl

@@ -26,17 +26,39 @@ def _runfiles_path(f):
     else:
         return f.path  # source file
 
+def _ignore_line_endings(ctx):
+    ignore_line_endings = "0"
+    if ctx.attr.ignore_line_endings:
+        ignore_line_endings = "1"
+    return ignore_line_endings
+
 def _diff_test_impl(ctx):
     if ctx.attr.is_windows:
+        bash_bin = ctx.toolchains["@bazel_tools//tools/sh:toolchain_type"].path
         test_bin = ctx.actions.declare_file(ctx.label.name + "-test.bat")
         ctx.actions.write(
             output = test_bin,
             content = """@rem Generated by diff_test.bzl, do not edit.
 @echo off
 SETLOCAL ENABLEEXTENSIONS
 SETLOCAL ENABLEDELAYEDEXPANSION
-set MF=%RUNFILES_MANIFEST_FILE:/=\\%
 set PATH=%SYSTEMROOT%\\system32
+if defined RUNFILES_MANIFEST_FILE (
+    set MF=%RUNFILES_MANIFEST_FILE:/=\\%
+) else (
+    if exist MANIFEST (
+        set MF=MANIFEST
+    ) else (
+        if exist ..\\MANIFEST (
+            set MF=..\\MANIFEST
+        )
+    )
+)
+if not exist %MF% (
+    echo Manifest file %MF% not found
+    exit /b 1
+)
+echo using %MF%
 set F1={file1}
 set F2={file2}
 if "!F1:~0,9!" equ "external/" (set F1=!F1:~9!) else (set F1=!TEST_WORKSPACE!/!F1!)
@@ -79,10 +101,36 @@ if "!RF2!" equ "" (
     exit /b 1
   )
 )
+rem use tr command from msys64 package, msys64 is a bazel recommendation
+rem todo: in future better to pull in diff.exe to align with non-windows path
+if "{ignore_line_endings}"=="1" (
+  if exist {bash_bin} (
+    for %%f in ({bash_bin}) do set "TR=%%~dpf\\tr.exe"
+  ) else (
+    rem match bazel's algorithm to find unix tools
+    set "TR=C:\\msys64\\usr\\bin\\tr.exe"
+  )
+  if not exist !TR! (
+    echo>&2 WARNING: ignore_line_endings set but !TR! not found; line endings will be compared
+  ) else (
+    for %%f in (!RF1!) do set RF1_TEMP=%TEST_TMPDIR%\\%%~nxf_lf1
+    for %%f in (!RF2!) do set RF2_TEMP=%TEST_TMPDIR%\\%%~nxf_lf2
+    type "!RF1!" | !TR! -d "\\r" > "!RF1_TEMP!"
+    type "!RF2!" | !TR! -d "\\r" > "!RF2_TEMP!"
+    set "RF1=!RF1_TEMP!"
+    set "RF2=!RF2_TEMP!"
+    rem echo original file !RF1! replaced by !RF1_TEMP!
+    rem echo original file !RF2! replaced by !RF2_TEMP!
+  )
+)
 fc.exe 2>NUL 1>NUL /B "!RF1!" "!RF2!"
 if %ERRORLEVEL% neq 0 (
   if %ERRORLEVEL% equ 1 (
-    echo>&2 FAIL: files "{file1}" and "{file2}" differ. {fail_msg}
+    set "FAIL_MSG={fail_msg}"
+    if "!FAIL_MSG!"=="" (
+        set "FAIL_MSG=why? diff ^"!RF1!^" ^"!RF2!^" ^| cat -v"
+    )
+    echo>&2 FAIL: files "{file1}" and "{file2}" differ. !FAIL_MSG!
     exit /b 1
   ) else (
     fc.exe /B "!RF1!" "!RF2!"
@@ -94,6 +142,8 @@ if %ERRORLEVEL% neq 0 (
                 fail_msg = ctx.attr.failure_message,
                 file1 = _runfiles_path(ctx.file.file1),
                 file2 = _runfiles_path(ctx.file.file2),
+                ignore_line_endings = _ignore_line_endings(ctx),
+                bash_bin = bash_bin,
             ),
             is_executable = True,
         )
@@ -120,14 +170,19 @@ else
   echo >&2 "ERROR: could not find \"{file1}\" and \"{file2}\""
   exit 1
 fi
-if ! diff "$RF1" "$RF2"; then
-  echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ. "{fail_msg}
+if ! diff {strip_trailing_cr}"$RF1" "$RF2"; then
+  MSG={fail_msg}
+  if [[ "${{MSG}}" == "" ]]; then
+      MSG="why? diff {strip_trailing_cr}"${{RF1}}" "${{RF2}}" | cat -v"
+  fi
+  echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ. ${{MSG}}"
   exit 1
 fi
 """.format(
                 fail_msg = shell.quote(ctx.attr.failure_message),
                 file1 = _runfiles_path(ctx.file.file1),
                 file2 = _runfiles_path(ctx.file.file2),
+                strip_trailing_cr = "--strip-trailing-cr " if ctx.attr.ignore_line_endings else "",
             ),
             is_executable = True,
         )
@@ -148,13 +203,19 @@ _diff_test = rule(
             allow_single_file = True,
             mandatory = True,
         ),
+        "ignore_line_endings": attr.bool(
+            default = True,
+        ),
         "is_windows": attr.bool(mandatory = True),
     },
+    toolchains = [
+        "@bazel_tools//tools/sh:toolchain_type",
+    ],
     test = True,
     implementation = _diff_test_impl,
 )
 
-def diff_test(name, file1, file2, failure_message = None, **kwargs):
+def diff_test(name, file1, file2, failure_message = None, ignore_line_endings = True, **kwargs):
     """A test that compares two files.
 
     The test succeeds if the files' contents match.
@@ -163,13 +224,16 @@ def diff_test(name, file1, file2, failure_message = None, **kwargs):
       name: The name of the test rule.
       file1: Label of the file to compare to `file2`.
       file2: Label of the file to compare to `file1`.
+      ignore_line_endings: Ignore differences between CRLF and LF line endings. On windows, this is
+          forced to False if the 'tr' command can't be found in the bash installation on the host.
       failure_message: Additional message to log if the files' contents do not match.
       **kwargs: The [common attributes for tests](https://bazel.build/reference/be/common-definitions#common-attributes-tests).
     """
     _diff_test(
         name = name,
         file1 = file1,
         file2 = file2,
+        ignore_line_endings = ignore_line_endings,
         failure_message = failure_message,
         is_windows = select({
             "@bazel_tools//src/conditions:host_windows": True,

rules/private/copy_directory_private.bzl

@@ -34,9 +34,9 @@ def _copy_cmd(ctx, src, dst):
     # https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy
     # NB: robocopy return non-zero exit codes on success so we must exit 0 after calling it
     cmd_tmpl = """\
-if not exist \"{src}\\\" (
-  echo Error: \"{src}\" is not a directory
-  @exit 1
+@if not exist \"{src}\\*\" (
+@  echo Error: \"{src}\" is not a directory
+@  exit 1
 )
 @robocopy \"{src}\" \"{dst}\" /E /MIR >NUL & @exit 0
 """

tests/diff_test/diff_test_tests.sh

@@ -80,12 +80,13 @@ eof
   echo bar > "$ws/$subdir/b.txt"
 
   (cd "$ws" && \
-   bazel test ${flags} "//${subdir%/}:same" --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test ${flags} "//${subdir%/}:same" --test_output=errors --noshow_progress 1>>"$TEST_log" 2>&1 \
      || fail "expected success")
 
   (cd "$ws" && \
-   bazel test ${flags} "//${subdir%/}:different" --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test ${flags} "//${subdir%/}:different" --test_output=all --noshow_progress 1>>"$TEST_log" 2>&1 \
      && fail "expected failure" || true)
+
   expect_log "FAIL: files \"${subdir}a.txt\" and \"${subdir}b.txt\" differ"
 }
 
@@ -175,21 +176,21 @@ diff_test(
 eof
 
   (cd "$ws/main" && \
-   bazel test ${flags} //:same --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test ${flags} //:same --test_output=errors --noshow_progress 1>"$TEST_log" 2>&1 \
      || fail "expected success")
 
   (cd "$ws/main" && \
-   bazel test ${flags} //:different1 --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test ${flags} //:different1 --test_output=all --noshow_progress 1>"$TEST_log" 2>&1 \
      && fail "expected failure" || true)
   expect_log 'FAIL: files "external/ext1/foo/foo.txt" and "external/ext2/foo/bar.txt" differ'
 
   (cd "$ws/main" && \
-   bazel test ${flags} //:different2 --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test ${flags} //:different2 --test_output=all --noshow_progress 1>"$TEST_log" 2>&1 \
      && fail "expected failure" || true)
   expect_log 'FAIL: files "external/ext1/foo/foo.txt" and "ext1/foo/foo.txt" differ'
 
   (cd "$ws/main" && \
-   bazel test ${flags} //:different3 --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test ${flags} //:different3 --test_output=all --noshow_progress 1>"$TEST_log" 2>&1 \
      && fail "expected failure" || true)
   expect_log 'FAIL: files "ext2/foo/foo.txt" and "external/ext2/foo/foo.txt" differ'
 }
@@ -257,15 +258,15 @@ eof
   echo bar > "$ws/d.txt"
 
   (cd "$ws" && \
-   bazel test //:different_with_message --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test //:different_with_message --test_output=errors --noshow_progress 1>"$TEST_log" 2>&1 \
      && fail "expected failure" || true)
   # TODO(arostovtsev): also test Windows cmd.exe escapes when https://github.com/bazelbuild/bazel-skylib/pull/363 is merged
   expect_log "FAIL: files \"a.txt\" and \"b.txt\" differ. This is an \`\$error\`"
 
   (cd "$ws" && \
-   bazel test //:different_without_message --test_output=errors 1>"$TEST_log" 2>&1 \
+   bazel test //:different_without_message --test_output=all --noshow_progress 1>"$TEST_log" 2>&1 \
      && fail "expected failure" || true)
-  expect_log "FAIL: files \"c.txt\" and \"d.txt\" differ. $"
+  expect_log "FAIL: files \"c.txt\" and \"d.txt\" differ. why? diff"
 }
 
 cd "$TEST_TMPDIR"