Skip to content

Commit ad4bf5b

Browse files
peakschrispeakschris
authored
feat: support diff_test/write_source_files on Windows and enable most tests (#1159)
Enable write_source_files with diff_test for windows; allowing documentation checks and updates to run on Windows for repos that use these macros. Enable most disabled windows tests. **Features:** * write_source_files fixed for windows * diff_test enabled for windows; non-binary comparison so allows different line endings * improved runfiles lookup for both rules: * windows: runfiles are looked up via runfiles manifest so --enable_runfiles not required * other platforms: left runfiles handling untouched to avoid regressing working code * assert_contains now uses runfiles manifest lookup * fixed a bug in copy_directory and copy_to_directory that would result in 'failed to get realpath of dangling symlink' error * most tests that were disabled for windows / enable_runfiles only have been enabled for windows without runfiles * enable docs folder tests on windows CI (for extra test coverage of the diff_test macro) **Fixes:** * #1142
1 parent 05f1448 commit ad4bf5b

File tree

25 files changed

+293
-277
lines changed

25 files changed

+293
-277
lines changed

‎CONTRIBUTING.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,16 @@ This means that any usage of `@bazel_lib` on your system will point to this fold
4545
1. `git push --tags`
4646
1. Watch the automation run on GitHub actions
4747
1. Update the release page with auto-generated release notes
48+
49+
## Windows Specifics
50+
51+
Rules and tests in this repo should support Bazel's default on Windows (runfiles directories not enabled).
52+
So you should not need to add the --enable_runfiles option when using the rules or testing them. New
53+
rules must be written using runfiles libraries to maintain this.
54+
55+
This repository contains symlinks in the test data; to test correctly you need to clone with symlinks
56+
enabled:
57+
58+
1. enable Developer mode (to switch on Windows symlink support)
59+
2. clone with `git clone -c core.symlinks=true <repository_url>`
60+
3. or `git config --global core.symlinks true`

‎e2e/coreutils/BUILD.bazel

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
load("@bazel_lib//lib:write_source_files.bzl", "write_source_files")
22
load("@bazel_lib//tools/release:hashes.bzl", "hashes")
33

4+
NOT_WINDOWS = select({
5+
"@platforms//os:windows": ["@platforms//:incompatible"],
6+
"//conditions:default": [],
7+
})
8+
49
genrule(
510
name = "ls",
611
outs = ["ls.txt"],
@@ -35,4 +40,16 @@ write_source_files(
3540
"sha1.txt": ":sha1",
3641
"md5.txt": ":md5",
3742
},
43+
target_compatible_with = NOT_WINDOWS,
44+
)
45+
46+
write_source_files(
47+
name = "write_windows",
48+
files = {
49+
# coreutils ls does not give expected results on windows
50+
"sha256.txt": ":hash",
51+
"sha1.txt": ":sha1",
52+
"md5.txt": ":md5",
53+
},
54+
target_compatible_with = ["@platforms//os:windows"],
3855
)

‎e2e/coreutils/MODULE.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,5 @@ local_path_override(
66

77
bazel_lib_toolchains = use_extension("@bazel_lib//lib:extensions.bzl", "toolchains", dev_dependency = True)
88
use_repo(bazel_lib_toolchains, "coreutils_toolchains")
9+
10+
bazel_dep(name = "platforms", version = "0.0.10")

‎lib/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ bzl_library(
155155
":utils",
156156
# "//lib/private:fail_with_message_test",
157157
"//lib/private:write_source_file",
158+
"//lib:windows_utils",
158159
],
159160
)
160161

‎lib/private/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ bzl_library(
100100
visibility = ["//lib:__subpackages__"],
101101
deps = [
102102
":directory_path",
103+
"//lib:paths",
103104
"//lib:utils",
105+
"//lib:windows_utils",
104106
"@bazel_skylib//lib:shell",
105107
"@bazel_skylib//lib:types",
106108
"@bazel_skylib//rules:write_file",

‎lib/private/diff_test.bzl

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ command (fc.exe) on Windows (no Bash is required).
2424
load("@bazel_skylib//lib:shell.bzl", "shell")
2525
load("@bazel_skylib//lib:types.bzl", "types")
2626
load("@bazel_skylib//rules:write_file.bzl", "write_file")
27+
load("//lib:paths.bzl", "to_rlocation_path")
28+
load("//lib:windows_utils.bzl", "BATCH_RLOCATION_FUNCTION")
2729
load(":directory_path.bzl", "DirectoryPathInfo")
2830

2931
def _runfiles_path(f):
@@ -35,18 +37,22 @@ def _runfiles_path(f):
3537
def _diff_test_impl(ctx):
3638
is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo])
3739

40+
file1_sub_path = ""
3841
if DirectoryPathInfo in ctx.attr.file1:
3942
file1 = ctx.attr.file1[DirectoryPathInfo].directory
4043
file1_path = "/".join([_runfiles_path(file1), ctx.attr.file1[DirectoryPathInfo].path])
44+
file1_sub_path = ctx.attr.file1[DirectoryPathInfo].path
4145
else:
4246
if len(ctx.files.file1) != 1:
4347
fail("file1 must be a single file or a target that provides a DirectoryPathInfo")
4448
file1 = ctx.files.file1[0]
4549
file1_path = _runfiles_path(file1)
4650

51+
file2_sub_path = ""
4752
if DirectoryPathInfo in ctx.attr.file2:
4853
file2 = ctx.attr.file2[DirectoryPathInfo].directory
4954
file2_path = "/".join([_runfiles_path(file2), ctx.attr.file2[DirectoryPathInfo].path])
55+
file2_sub_path = ctx.attr.file2[DirectoryPathInfo].path
5056
else:
5157
if len(ctx.files.file2) != 1:
5258
fail("file2 must be a single file or a target that provides a DirectoryPathInfo")
@@ -60,19 +66,26 @@ def _diff_test_impl(ctx):
6066
if is_windows:
6167
test_suffix = "-test.bat"
6268
template = ctx.file._diff_test_tmpl_bat
69+
file1_path = to_rlocation_path(ctx, file1)
70+
file2_path = to_rlocation_path(ctx, file2)
71+
fail_msg = ["@echo" + (" " + line if line.strip() != "" else ".") for line in ctx.attr.failure_message[:-1].split("\n")]
6372
else:
6473
test_suffix = "-test.sh"
6574
template = ctx.file._diff_test_tmpl_sh
75+
fail_msg = ctx.attr.failure_message.split("\n")
6676

6777
test_bin = ctx.actions.declare_file(ctx.label.name + test_suffix)
6878
ctx.actions.expand_template(
6979
template = template,
7080
output = test_bin,
7181
substitutions = {
82+
"{BATCH_RLOCATION_FUNCTION}": BATCH_RLOCATION_FUNCTION,
7283
"{name}": ctx.attr.name,
73-
"{fail_msg}": ctx.attr.failure_message,
84+
"{fail_msg}": "\n".join(fail_msg),
7485
"{file1}": file1_path,
7586
"{file2}": file2_path,
87+
"{file1_sub_path}": file1_sub_path,
88+
"{file2_sub_path}": file2_sub_path,
7689
"{build_file_path}": ctx.build_file_path,
7790
"{diff_args}": " ".join([
7891
shell.quote(arg)

‎lib/private/diff_test_tmpl.bat

Lines changed: 62 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -3,47 +3,19 @@
33
:: TODO: Add support for XML_OUTPUT_FILE like in diff_test_tmpl.sh
44
SETLOCAL ENABLEEXTENSIONS
55
SETLOCAL ENABLEDELAYEDEXPANSION
6+
set RUNFILES_MANIFEST_ONLY=1
7+
{BATCH_RLOCATION_FUNCTION}
68
set MF=%RUNFILES_MANIFEST_FILE:/=\\%
79
set PATH=%SYSTEMROOT%\\system32
8-
set F1={file1}
9-
set F2={file2}
10-
if "!F1:~0,9!" equ "external/" (set F1=!F1:~9!) else (set F1=!TEST_WORKSPACE!/!F1!)
11-
if "!F2:~0,9!" equ "external/" (set F2=!F2:~9!) else (set F2=!TEST_WORKSPACE!/!F2!)
12-
for /F "tokens=2* usebackq" %%i in (`findstr.exe /l /c:"!F1! " "%MF%"`) do (
13-
set RF1=%%i
14-
set RF1=!RF1:/=\\!
10+
call :rlocation {file1} RF1
11+
call :rlocation {file2} RF2
12+
set RF1=!RF1:/=\!
13+
set RF2=!RF2:/=\!
14+
if "{file1_sub_path}" neq "" (
15+
set RF1=!RF1!\\{file1_sub_path}
1516
)
16-
if "!RF1!" equ "" (
17-
if "%RUNFILES_MANIFEST_ONLY%" neq "1" if exist "%RUNFILES_DIR%\\%F1%" (
18-
set RF1="%RUNFILES_DIR%\\%F1%"
19-
) else (
20-
if exist "{file1}" (
21-
set RF1="{file1}"
22-
)
23-
)
24-
if "!RF1!" neq "" ( set RF1=!RF1:/=\\!
25-
) else (
26-
echo>&2 ERROR: !F1! not found
27-
exit /b 1
28-
)
29-
)
30-
for /F "tokens=2* usebackq" %%i in (`findstr.exe /l /c:"!F2! " "%MF%"`) do (
31-
set RF2=%%i
32-
set RF2=!RF2:/=\\!
33-
)
34-
if "!RF2!" equ "" (
35-
if "%RUNFILES_MANIFEST_ONLY%" neq "1" if exist "%RUNFILES_DIR%\\%F2%" (
36-
set RF2="%RUNFILES_DIR%\\%F2%"
37-
) else (
38-
if exist "{file2}" (
39-
set RF2="{file2}"
40-
)
41-
)
42-
if "!RF2!" neq "" ( set RF2=!RF2:/=\\!
43-
) else (
44-
echo>&2 ERROR: !F2! not found
45-
exit /b 1
46-
)
17+
if "{file2_sub_path}" neq "" (
18+
set RF2=!RF2!\\{file2_sub_path}
4719
)
4820
set DF1=0
4921
set DF2=0
@@ -71,48 +43,65 @@ if %DF1% equ 1 (
7143
set DFX=1
7244
)
7345
)
74-
if %DFX% equ 1 (
75-
for /f "delims=" %%F in (
76-
'echo "."^&forfiles /s /p "!RF1!" /m "*" /c "cmd /c echo @relpath"'
77-
) do (
78-
if not exist "!RF2!\\%%~F" (
79-
echo>&2 FAIL: file "%%~F" exists in "{file1}" and not in "{file2}". {fail_msg}
80-
GOTO fail
81-
)
82-
if not exist "!RF1!\\%%~F\\*" (
83-
fc.exe /B "!RF1!\\%%~F" "!RF2!\\%%~F" 2>NUL 1>NUL
84-
if !ERRORLEVEL! neq 0 (
85-
if !ERRORLEVEL! equ 1 (
86-
echo>&2 FAIL: files "{file1}\\%%~F" and "{file2}\\%%~F" differ. {fail_msg}
87-
GOTO fail
88-
) else (
89-
fc.exe /B "!RF1!\\%%~F" "!RF2!\\%%~F"
90-
GOTO fail
91-
)
46+
47+
if %DFX% equ 0 goto :compare_files
48+
:compare_directories
49+
for /f "delims=" %%F in (
50+
'echo "."^&forfiles /s /p "!RF1!" /m "*" /c "cmd /c echo @relpath"'
51+
) do (
52+
if not exist "!RF2!\\%%~F" (
53+
echo>&2 FAIL: file "%%~F" exists in "{file1}" and not in "{file2}".
54+
GOTO fail
55+
)
56+
if not exist "!RF1!\\%%~F\\*" (
57+
fc.exe "!RF1!\\%%~F" "!RF2!\\%%~F" 2>NUL 1>NUL
58+
if !ERRORLEVEL! neq 0 (
59+
if !ERRORLEVEL! equ 1 (
60+
echo>&2 FAIL: files "!RF1!\\%%~F" and "!RF2!\\%%~F" differ.
61+
set RF1=!RF1!\\%%~F
62+
set RF2=!RF2!\\%%~F
63+
GOTO fail
64+
) else (
65+
fc.exe "!RF1!\\%%~F" "!RF2!\\%%~F"
66+
GOTO fail
9267
)
9368
)
9469
)
95-
for /f "delims=" %%F in (
96-
'echo "."^&forfiles /s /p "!RF2!" /m "*" /c "cmd /c echo @relpath"'
97-
) do (
98-
if not exist "!RF1!\\%%~F" (
99-
echo>&2 FAIL: file "%%~F" exists in "{file2}" and not in "{file1}". {fail_msg}
100-
GOTO fail
101-
)
70+
)
71+
for /f "delims=" %%F in (
72+
'echo "."^&forfiles /s /p "!RF2!" /m "*" /c "cmd /c echo @relpath"'
73+
) do (
74+
if not exist "!RF1!\\%%~F" (
75+
echo>&2 FAIL: file "%%~F" exists in "{file2}" and not in "{file1}".
76+
GOTO fail
10277
)
103-
) else (
104-
fc.exe /B "!RF1!" "!RF2!" 2>NUL 1>NUL
105-
if %ERRORLEVEL% neq 0 (
106-
if %ERRORLEVEL% equ 1 (
107-
echo>&2 FAIL: files "{file1}" and "{file2}" differ. {fail_msg}
108-
exit /b 1
109-
) else (
110-
fc.exe /B "!RF1!" "!RF2!"
111-
exit /b %errorlevel%
112-
)
78+
)
79+
goto :success
80+
81+
:compare_files
82+
echo compare_files
83+
fc.exe "!RF1!" "!RF2!" 2>NUL 1>NUL
84+
set result=%ERRORLEVEL%
85+
if !result! neq 0 (
86+
if !result! equ 1 (
87+
echo>&2 FAIL: files "!RF1!" and "!RF2!" differ.
88+
goto :fail
89+
) else (
90+
echo fc.exe "!RF1!" "!RF2!"
91+
fc.exe "!RF1!" "!RF2!"
92+
set result=%ERRORLEVEL%
93+
exit /b !result!
11394
)
95+
) else (
96+
echo fc returned 0
11497
)
98+
:success
11599
exit /b 0
116100

117101
:fail
102+
{fail_msg}
103+
echo To see differences run:
104+
echo.
105+
echo diff "!RF1!" "!RF2!"
106+
echo.
118107
exit /b 1

‎lib/private/docs.bzl

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,13 @@ def stardoc_with_diff_test(
1717
**kwargs: additional attributes passed to the stardoc() rule, such as for overriding the templates
1818
"""
1919

20-
target_compatible_with = kwargs.pop("target_compatible_with", select({
21-
# stardoc produces different line endings on Windows
22-
# which makes the diff_test fail
23-
Label("@platforms//os:windows"): [Label("@platforms//:incompatible")],
24-
"//conditions:default": [],
25-
}))
26-
2720
# Generate MD from .bzl
2821
_stardoc(
2922
name = name,
3023
out = name + "-docgen.md",
3124
input = bzl_library_target + ".bzl",
3225
deps = [bzl_library_target],
3326
tags = kwargs.pop("tags", []) + ["package:" + native.package_name()], # Tag the package name which will help us reconstruct the write_source_files label in update_docs
34-
target_compatible_with = target_compatible_with,
3527
**kwargs
3628
)
3729

0 commit comments

Comments
 (0)