Fix non-determinismi in compiler outputs (#330)

* Fix non-determinismi in compiler outputs ## What? The C# and F# compilers embed absolute paths into their outputs and that causes non-determinism in the builds. By using the `--pathmap` flag we can strip those paths out of the outputs and have some of that sweet sweet determinism.
bazel-contrib · Jan 11, 2023 · 8b1f6bf · 8b1f6bf
1 parent 9177149
commit 8b1f6bf
Show file tree

Hide file tree

Showing 17 changed files with 133 additions and 18 deletions.
diff --git a/dotnet/private/BUILD.bazel b/dotnet/private/BUILD.bazel
@@ -4,6 +4,8 @@ exports_files(
     [
         "launcher.sh.tpl",
         "launcher.bat.tpl",
+        "compiler_wrapper.sh",
+        "compiler_wrapper.bat",
     ],
     visibility = ["//visibility:public"],
 )

diff --git a/dotnet/private/compiler_wrapper.bat b/dotnet/private/compiler_wrapper.bat
@@ -0,0 +1,38 @@
+@echo off
+SETLOCAL ENABLEEXTENSIONS
+SETLOCAL ENABLEDELAYEDEXPANSION
+
+::
+: This wrapper script is used because the C#/F# compilers both embed absolute paths
+: into their outputs and those paths are not deterministic. The compilers also
+: allow overriding these paths using pathmaps. Since the paths can not be known
+: at analysis time we need to override them at execution time.
+::
+
+set DOTNET_EXECUTABLE=%1
+set COMPILER=%2
+for %%F in ("%COMPILER%") do set COMPILER_BASENAME=%%~nxF
+
+set PATHMAP_FLAG=-pathmap
+
+:: Needed because unfortunately the F# compiler uses a different flag name
+if %COMPILER_BASENAME% == fsc.dll set PATHMAP_FLAG=--pathmap
+
+set PATHMAP=%PATHMAP_FLAG%:"%cd%=."
+
+shift
+set args=%1
+:loop
+shift
+if [%1]==[] goto afterloop
+set args=%args% %1
+goto loop
+:afterloop
+
+rem Escape \ and * in args before passsing it with double quote
+if defined args (
+  set args=!args:\=\\\\!
+  set args=!args:"=\"!
+)
+
+"%DOTNET_EXECUTABLE%" %args% %PATHMAP%
diff --git a/dotnet/private/compiler_wrapper.sh b/dotnet/private/compiler_wrapper.sh
@@ -0,0 +1,19 @@
+#! /usr/bin/env bash
+set -eou pipefail
+
+# This wrapper script is used because the C#/F# compilers both embed absolute paths
+# into their outputs and those paths are not deterministic. The compilers also
+# allow overriding these paths using pathmaps. Since the paths can not be known
+# at analysis time we need to override them at execution time.
+
+COMPILER="$2"
+PATHMAP_FLAG="-pathmap"
+
+# Needed because unfortunately the F# compiler uses a different flag name
+if [[ $(basename "$COMPILER") == "fsc.dll" ]]; then
+  PATHMAP_FLAG="--pathmap"
+fi
+PATHMAP="$PATHMAP_FLAG:$PWD=."
+
+# shellcheck disable=SC2145
+./"$@" "$PATHMAP"
diff --git a/dotnet/private/rules/common/attrs.bzl b/dotnet/private/rules/common/attrs.bzl
@@ -124,6 +124,18 @@ COMMON_ATTRS = {
     "_allowlist_function_transition": attr.label(
         default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
     ),
+    "_compiler_wrapper_sh": attr.label(
+        default = "@rules_dotnet//dotnet/private:compiler_wrapper.sh",
+        executable = True,
+        cfg = "exec",
+        allow_single_file = True,
+    ),
+    "_compiler_wrapper_bat": attr.label(
+        default = "@rules_dotnet//dotnet/private:compiler_wrapper.bat",
+        executable = True,
+        cfg = "exec",
+        allow_single_file = True,
+    ),
 }
 
 # These are attributes that are common across all libarary rules

diff --git a/dotnet/private/rules/csharp/actions/csharp_assembly.bzl b/dotnet/private/rules/csharp/actions/csharp_assembly.bzl
@@ -52,6 +52,7 @@ def _write_internals_visible_to_csharp(actions, name, others):
 # buildifier: disable=unnamed-macro
 def AssemblyAction(
         actions,
+        compiler_wrapper,
         additionalfiles,
         debug,
         defines,
@@ -83,6 +84,7 @@ def AssemblyAction(
 
     Args:
         actions: Bazel module providing functions to create actions.
+        compiler_wrapper: The wrapper script that invokes the C# compiler.
         additionalfiles: Names additional files that don't directly affect code generation but may be used by analyzers for producing errors or warnings.
         debug: Emits debugging information.
         defines: The list of conditional compilation symbols.
@@ -148,6 +150,7 @@ def AssemblyAction(
     if len(internals_visible_to) == 0:
         _compile(
             actions,
+            compiler_wrapper,
             additionalfiles,
             analyzers,
             private_analyzers,
@@ -189,6 +192,7 @@ def AssemblyAction(
 
         _compile(
             actions,
+            compiler_wrapper,
             additionalfiles,
             analyzers,
             private_analyzers,
@@ -220,6 +224,7 @@ def AssemblyAction(
         # Generate a ref-only DLL without internals
         _compile(
             actions,
+            compiler_wrapper,
             additionalfiles,
             analyzers,
             private_analyzers,
@@ -274,6 +279,7 @@ def AssemblyAction(
 
 def _compile(
         actions,
+        compiler_wrapper,
         additionalfiles,
         analyzer_assemblies,
         private_analyzer_assemblies,
@@ -389,17 +395,14 @@ def _compile(
         mnemonic = "CSharpCompile",
         progress_message = "Compiling " + target_name + (" (internals ref-only dll)" if out_dll == None else ""),
         inputs = depset(
-            direct = direct_inputs,
+            direct = direct_inputs + [compiler_wrapper, toolchain.runtime.files_to_run.executable],
             transitive = [private_refs, refs, analyzer_assemblies, private_analyzer_assemblies, toolchain.runtime.default_runfiles.files, toolchain.csharp_compiler.default_runfiles.files, compile_data],
         ),
         outputs = outputs,
-        executable = toolchain.runtime.files_to_run.executable,
+        executable = compiler_wrapper,
         arguments = [
+            toolchain.runtime.files_to_run.executable.path,
             toolchain.csharp_compiler.files_to_run.executable.path,
-
-            # This can't go in the response file (if it does it won't be seen
-            # until it's too late).
-            "/noconfig",
             args,
         ],
         env = {

diff --git a/dotnet/private/rules/csharp/binary.bzl b/dotnet/private/rules/csharp/binary.bzl
@@ -17,6 +17,7 @@ def _compile_action(ctx, tfm):
     toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"]
     return AssemblyAction(
         ctx.actions,
+        ctx.executable._compiler_wrapper_bat if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else ctx.executable._compiler_wrapper_sh,
         additionalfiles = ctx.files.additionalfiles,
         debug = is_debug(ctx),
         defines = ctx.attr.defines,

diff --git a/dotnet/private/rules/csharp/library.bzl b/dotnet/private/rules/csharp/library.bzl
@@ -16,6 +16,7 @@ def _compile_action(ctx, tfm):
     toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"]
     return AssemblyAction(
         ctx.actions,
+        ctx.executable._compiler_wrapper_bat if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else ctx.executable._compiler_wrapper_sh,
         additionalfiles = ctx.files.additionalfiles,
         debug = is_debug(ctx),
         defines = ctx.attr.defines,

diff --git a/dotnet/private/rules/csharp/test.bzl b/dotnet/private/rules/csharp/test.bzl
@@ -19,6 +19,7 @@ def _compile_action(ctx, tfm):
     toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"]
     return AssemblyAction(
         ctx.actions,
+        ctx.executable._compiler_wrapper_bat if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else ctx.executable._compiler_wrapper_sh,
         additionalfiles = ctx.files.additionalfiles,
         debug = is_debug(ctx),
         defines = ctx.attr.defines,

diff --git a/dotnet/private/rules/fsharp/actions/fsharp_assembly.bzl b/dotnet/private/rules/fsharp/actions/fsharp_assembly.bzl
@@ -63,14 +63,15 @@ do()
 
     return output
 
-# Reference assembly support did not come until .Net 7.0
+# Reference assembly support did not come to F# until .Net 7.0
 # This check should be removed once the .Net 6.0 LTS release is no longer supported
-def _should_output_ref_assembly(toolchain, target_framework):
-    return is_greater_or_equal_framework(target_framework, "net7.0") and is_greater_or_equal_framework(toolchain.dotnetinfo.runtime_tfm, "net7.0")
+def _should_output_ref_assembly(toolchain):
+    return is_greater_or_equal_framework(toolchain.dotnetinfo.runtime_tfm, "net7.0")
 
 # buildifier: disable=unnamed-macro
 def AssemblyAction(
         actions,
+        compiler_wrapper,
         debug,
         defines,
         deps,
@@ -100,6 +101,7 @@ def AssemblyAction(
 
     Args:
         actions: Bazel module providing functions to create actions.
+        compiler_wrapper: The wrapper script that invokes the F# compiler.
         debug: Emits debugging information.
         defines: The list of conditional compilation symbols.
         deps: The list of other libraries to be linked in to the assembly.
@@ -156,12 +158,13 @@ def AssemblyAction(
     out_ext = "dll"
     out_dll = actions.declare_file("%s/%s.%s" % (out_dir, assembly_name, out_ext))
     out_iref = None
-    out_ref = actions.declare_file("%s/ref/%s.%s" % (out_dir, assembly_name, out_ext)) if _should_output_ref_assembly(toolchain, target_framework) else None
+    out_ref = actions.declare_file("%s/ref/%s.%s" % (out_dir, assembly_name, out_ext)) if _should_output_ref_assembly(toolchain) else None
     out_pdb = actions.declare_file("%s/%s.pdb" % (out_dir, assembly_name))
 
     if len(internals_visible_to) == 0:
         _compile(
             actions,
+            compiler_wrapper,
             debug,
             defines,
             keyfile,
@@ -189,7 +192,7 @@ def AssemblyAction(
         # If the user is using internals_visible_to generate an additional
         # reference-only DLL that contains the internals. We want the
         # InternalsVisibleTo in the main DLL too to be less suprising to users.
-        out_iref = actions.declare_file("%s/iref/%s.%s" % (out_dir, assembly_name, out_ext)) if _should_output_ref_assembly(toolchain, target_framework) else None
+        out_iref = actions.declare_file("%s/iref/%s.%s" % (out_dir, assembly_name, out_ext)) if _should_output_ref_assembly(toolchain) else None
 
         internals_visible_to_fs = _write_internals_visible_to_fsharp(
             actions,
@@ -198,6 +201,7 @@ def AssemblyAction(
         )
         _compile(
             actions,
+            compiler_wrapper,
             debug,
             defines,
             keyfile,
@@ -226,6 +230,7 @@ def AssemblyAction(
             # Generate a ref-only DLL without internals
             _compile(
                 actions,
+                compiler_wrapper,
                 debug,
                 defines,
                 keyfile,
@@ -276,6 +281,7 @@ def AssemblyAction(
 
 def _compile(
         actions,
+        compiler_wrapper,
         debug,
         defines,
         keyfile,
@@ -387,12 +393,13 @@ def _compile(
         mnemonic = "FSharpCompile",
         progress_message = "Compiling " + target_name + (" (internals ref-only dll)" if out_dll == None else ""),
         inputs = depset(
-            direct = direct_inputs,
+            direct = direct_inputs + [compiler_wrapper, toolchain.runtime.files_to_run.executable],
             transitive = [refs, private_refs, toolchain.runtime.default_runfiles.files, toolchain.fsharp_compiler.default_runfiles.files, compile_data],
         ),
         outputs = outputs,
-        executable = toolchain.runtime.files_to_run.executable,
+        executable = compiler_wrapper,
         arguments = [
+            toolchain.runtime.files_to_run.executable.path,
             toolchain.fsharp_compiler.files_to_run.executable.path,
             args,
         ],

diff --git a/dotnet/private/rules/fsharp/binary.bzl b/dotnet/private/rules/fsharp/binary.bzl
@@ -16,6 +16,7 @@ def _compile_action(ctx, tfm):
     toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"]
     return AssemblyAction(
         ctx.actions,
+        ctx.executable._compiler_wrapper_bat if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else ctx.executable._compiler_wrapper_sh,
         debug = is_debug(ctx),
         defines = ctx.attr.defines,
         deps = ctx.attr.deps,

diff --git a/dotnet/private/rules/fsharp/library.bzl b/dotnet/private/rules/fsharp/library.bzl
@@ -17,6 +17,7 @@ def _compile_action(ctx, tfm):
 
     return AssemblyAction(
         ctx.actions,
+        ctx.executable._compiler_wrapper_bat if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else ctx.executable._compiler_wrapper_sh,
         debug = is_debug(ctx),
         defines = ctx.attr.defines,
         deps = ctx.attr.deps,

diff --git a/dotnet/private/rules/fsharp/test.bzl b/dotnet/private/rules/fsharp/test.bzl
@@ -19,6 +19,7 @@ def _compile_action(ctx, tfm):
     toolchain = ctx.toolchains["@rules_dotnet//dotnet:toolchain_type"]
     return AssemblyAction(
         ctx.actions,
+        ctx.executable._compiler_wrapper_bat if ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) else ctx.executable._compiler_wrapper_sh,
         debug = is_debug(ctx),
         defines = ctx.attr.defines,
         deps = ctx.attr.deps,

diff --git a/dotnet/private/rules/nuget/nuget_archive.bzl b/dotnet/private/rules/nuget/nuget_archive.bzl
@@ -29,11 +29,16 @@ def _read_dir(repository_ctx, src_dir):
     """
     if _is_windows(repository_ctx):
         src_dir = src_dir.replace("/", "\\")
+        nuget_directory = repository_ctx.execute(["cmd.exe", "/c", "echo|set", "/p=%cd%"])
         find_result = repository_ctx.execute(["cmd.exe", "/c", "dir", src_dir, "/b", "/s", "/a-d"])
 
+        # The output from the find command includes absolute paths so we strip the
+        # current working directory from the paths
+        result = find_result.stdout.replace(nuget_directory.stdout + "\\", "")
+
         # src_files will be used in genrule.outs where the paths must
         # use forward slashes.
-        result = find_result.stdout.replace("\\", "/")
+        result = result.replace("\\", "/")
     else:
         find_result = repository_ctx.execute(["find", src_dir, "-follow", "-type", "f"])
         result = find_result.stdout

diff --git a/dotnet/private/rules/publish_binary/publish_binary.bzl b/dotnet/private/rules/publish_binary/publish_binary.bzl
@@ -96,6 +96,7 @@ publish_binary = rule(
 
 def _copy_file(script_body, src, dst, is_windows):
     if is_windows:
+        script_body.append("if not exist \"{dir}\" @mkdir \"{dir}\" >NUL".format(dir = dst.dirname.replace("/", "\\")))
         script_body.append("@copy /Y \"{src}\" \"{dst}\" >NUL".format(src = src.path.replace("/", "\\"), dst = dst.path.replace("/", "\\")))
     else:
         script_body.append("mkdir -p {dir} && cp -f {src} {dst}".format(dir = shell.quote(dst.dirname), src = shell.quote(src.path), dst = shell.quote(dst.path)))
@@ -107,7 +108,7 @@ def _copy_to_publish(ctx, runtime_identifier, publish_binary_info, binary_info,
         "{}/publish/{}/{}".format(ctx.label.name, runtime_identifier, binary_info.app_host.basename),
     )
     outputs = [app_host_copy]
-    script_body = [] if is_windows else ["#! /usr/bin/env bash"]
+    script_body = ["@echo off"] if is_windows else ["#! /usr/bin/env bash", "set -eou pipefail"]
 
     _copy_file(script_body, binary_info.app_host, app_host_copy, is_windows = is_windows)
 

diff --git a/dotnet/private/tests/publish/framework_dependent/BUILD.bazel b/dotnet/private/tests/publish/framework_dependent/BUILD.bazel
@@ -43,6 +43,11 @@ pkg_tar(
             "@platforms//os:windows": "framework_dependent/publish/win-x64",
         },
     ),
+    # Disabling this for Windows because there are some runfiles issues with Windows and rules_pkg
+    target_compatible_with = [
+        "@platforms//os:linux",
+        "@platforms//os:macos",
+    ],
 )
 
 sh_test(
@@ -55,5 +60,11 @@ sh_test(
         ":tar",
         ":toolchain_provider",
     ],
+
+    # Disabling this for Windows because there are some runfiles issues with Windows and rules_pkg
+    target_compatible_with = [
+        "@platforms//os:linux",
+        "@platforms//os:macos",
+    ],
     toolchains = ["@dotnet_toolchains//:resolved_toolchain"],
 )
diff --git a/dotnet/private/tests/publish/self_contained/BUILD.bazel b/dotnet/private/tests/publish/self_contained/BUILD.bazel
@@ -23,6 +23,7 @@ publish_binary(
 pkg_tar(
     name = "tar",
     srcs = [":self_contained"],
+    include_runfiles = True,
     mode = "0755",
     package_dir = "/",
     strip_prefix = select(
@@ -32,6 +33,11 @@ pkg_tar(
             "@platforms//os:windows": "self_contained/publish/win-x64",
         },
     ),
+    # Disabling this for Windows because there are some runfiles issues with Windows and rules_pkg
+    target_compatible_with = [
+        "@platforms//os:linux",
+        "@platforms//os:macos",
+    ],
 )
 
 sh_test(
@@ -40,4 +46,9 @@ sh_test(
         "test.sh",
     ],
     data = [":tar"],
+    # Disabling this for Windows because there are some runfiles issues with Windows and rules_pkg
+    target_compatible_with = [
+        "@platforms//os:linux",
+        "@platforms//os:macos",
+    ],
 )
diff --git a/internal_deps.bzl b/internal_deps.bzl
@@ -70,8 +70,8 @@ def rules_dotnet_internal_deps():
     http_archive(
         name = "rules_pkg",
         urls = [
-            "https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/0.7.0/rules_pkg-0.7.0.tar.gz",
-            "https://github.com/bazelbuild/rules_pkg/releases/download/0.7.0/rules_pkg-0.7.0.tar.gz",
+            "https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/0.8.0/rules_pkg-0.8.0.tar.gz",
+            "https://github.com/bazelbuild/rules_pkg/releases/download/0.8.0/rules_pkg-0.8.0.tar.gz",
         ],
-        sha256 = "8a298e832762eda1830597d64fe7db58178aa84cd5926d76d5b744d6558941c2",
+        sha256 = "eea0f59c28a9241156a47d7a8e32db9122f3d50b505fae0f33de6ce4d9b61834",
     )