Bootstrap make reproducibly #817
Conversation
fmeum
force-pushed
the
reproducible-make
branch
from
cf1b930 to
f6f69d6
Compare
fmeum
force-pushed
the
reproducible-make
branch
7 times, most recently
from
93042d1 to
c39ead5
Compare
fmeum
force-pushed
the
reproducible-make
branch
from
c39ead5 to
3ce148f
Compare
There was a problem hiding this comment.
@jheaff1 Thanks for the initial review comments on this.
@fmeum Thanks for this work; making the make build reproducible is something that wasn't even on my radar of tasks to do!
Can I request that the toolchain configuration reaches out to the functions already defined for this purpose? That way we aren't maintaining 2 copies of essentially the same logic. Also we need to propagate across any environment variables from the toolchain configuration as sometimes compiler wrappers may set these values in the toolchain configuration.
| feature_configuration = cc_common.configure_features( | ||
| ctx = ctx, | ||
| cc_toolchain = cc_toolchain, | ||
| requested_features = ctx.features, | ||
| unsupported_features = ctx.disabled_features, | ||
| ) | ||
|
|
||
| ar_path = cc_common.get_tool_for_action( | ||
| feature_configuration = feature_configuration, | ||
| action_name = CPP_LINK_STATIC_LIBRARY_ACTION_NAME, | ||
| ) | ||
| cc_path = cc_common.get_tool_for_action( | ||
| feature_configuration = feature_configuration, | ||
| action_name = C_COMPILE_ACTION_NAME, | ||
| ) | ||
| ld_path = cc_common.get_tool_for_action( | ||
| feature_configuration = feature_configuration, | ||
| action_name = CPP_LINK_EXECUTABLE_ACTION_NAME, | ||
| ) | ||
|
|
||
| c_link_static_library_variables = cc_common.create_link_variables( | ||
| feature_configuration = feature_configuration, | ||
| cc_toolchain = cc_toolchain, | ||
| ) | ||
| frozen_arflags = cc_common.get_memory_inefficient_command_line( | ||
| feature_configuration = feature_configuration, | ||
| action_name = CPP_LINK_STATIC_LIBRARY_ACTION_NAME, | ||
| variables = c_link_static_library_variables, | ||
| ) | ||
| c_compile_variables = cc_common.create_compile_variables( | ||
| feature_configuration = feature_configuration, | ||
| cc_toolchain = cc_toolchain, | ||
| ) | ||
| cflags = cc_common.get_memory_inefficient_command_line( | ||
| feature_configuration = feature_configuration, | ||
| action_name = C_COMPILE_ACTION_NAME, | ||
| variables = c_compile_variables, | ||
| ) | ||
| c_link_executable_variables = cc_common.create_link_variables( | ||
| feature_configuration = feature_configuration, | ||
| cc_toolchain = cc_toolchain, | ||
| ) | ||
| ldflags = cc_common.get_memory_inefficient_command_line( | ||
| feature_configuration = feature_configuration, | ||
| action_name = CPP_LINK_EXECUTABLE_ACTION_NAME, | ||
| variables = c_link_executable_variables, | ||
| ) |
There was a problem hiding this comment.
Can this be replaced by a call to get_flags_info from foreign_cc/private/cc_toolchain_util.bzl; there are a few standard features that need to be disabled in the feature configuration that this code already does.
Also we should probably also be grabbing any environment variables from the toolchain too - again get_env_vars should provide this list.
There was a problem hiding this comment.
Done. This required small changes to cc_toolchain_utils.bzl which I believe to be generally useful. I added them as a separate commit.
| - -DLIBDIR=\"$(libdir)\" -DINCLUDEDIR=\"$(includedir)\" \ | ||
| - -DLOCALEDIR=\"$(localedir)\" $(am__append_2) | ||
| + -DLIBDIR=\".\" -DINCLUDEDIR=\"external/rules_foreign_cc/toolchains/make/include\" \ | ||
| + -DLOCALEDIR=\"external/rules_foreign_cc/toolchains/make/share/locale\" $(am__append_2) |
There was a problem hiding this comment.
I appreciate this actually makes this reproducible but does this actually work to allow make to correctly find its locale directory? The configure_make rules run in $BUILDTMP which is below the bazel execroot and so this relative path is not going to be correct from that directory.
I don't have a good solution for this though that is any better than what you've already done here; I suspect we don't really care that the make tool can't find its translations but we should at least try to ensure that the tool does work (Which I appreciate even in the current configuration isn't the case due to sandbox paths, but it would currently at least currently work in a non-sandboxed local execution environment).
There was a problem hiding this comment.
From what I can tell, these directories default to /lib, /include and /share/locale, so they are not make-specific. Based on a cursory look into the code, these variables are used for the following purposes:
LIBDIRis only used inremake.c, apparently to find out whether a certain system library has changed since the lastmakeinvocation. This should not be needed in the context ofrules_foreign_cc.INCLUDEDIRis the default search path forMakefiles to include, which can also be specified with the-Ioption tomake. Picking upMakefiles from the host in unsandboxed builds sounds really risky, so maybe this should always be set to something like/does/not/exist.LOCALEDIRis used to makemaketranslatable.
While I do favor including this patch as it helps reproducibility and these directories are already non-existent in current sandboxed builds, I can drop it if you prefer.
fmeum
force-pushed
the
reproducible-make
branch
from
3ce148f to
8ba8b6f
Compare
By using getattr, the helper functions in this file can be reused in rules that do not define all of the framework attributes, e.g. bootstrap rules.
fmeum
force-pushed
the
reproducible-make
branch
from
8ba8b6f to
5e8db6f
Compare
Uses the Bazel C/C++ toolchain to bootstrap make and ensure that the resulting binary contains no absolute and thus non-hermetic paths. Building make reproducibly helps with remote caching and removes the dependency on a C compiler installed on the host.
fmeum
force-pushed
the
reproducible-make
branch
from
5e8db6f to
3939b7e
Compare
Uses the Bazel C/C++ toolchain to bootstrap make and ensure that the
resulting binary contains no absolute and thus non-hermetic paths.
Building make reproducibly helps with remote caching and removes the
dependency on a C compiler installed on the host.