Skip to content

Commit

Permalink
Automated rollback of commit 5ac62a2.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Breaks tests in Blaze nightly

*** Original change description ***

BEGIN_PUBLIC

Switch Cpp `BuildInfo` system to new API.

END_PUBLIC

PiperOrigin-RevId: 571997366
Change-Id: Ic8804ec876da3762584c79146f9945ee634a483c
  • Loading branch information
susinmotion authored and copybara-github committed Oct 9, 2023
1 parent bbe453e commit 66ac39c
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,6 @@ public class BuildConfigurationValue
private static final Interner<ImmutableSortedMap<Class<? extends Fragment>, Fragment>>
fragmentsInterner = BlazeInterners.newWeakInterner();

private static final ImmutableSet<BuiltinRestriction.AllowlistEntry> ANDROID_ALLOWLIST =
ImmutableSet.of(
BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_android"),
BuiltinRestriction.allowlistEntry("build_bazel_rules_android", ""),
BuiltinRestriction.allowlistEntry("rules_android", ""),
BuiltinRestriction.allowlistEntry("", "tools/build_defs/android"));

/** Global state necessary to build a BuildConfiguration. */
public interface GlobalStateProvider {
/** Computes the default shell environment for actions from the command line options. */
Expand Down Expand Up @@ -740,7 +733,7 @@ public boolean isToolConfiguration() {

@Override
public boolean isToolConfigurationForStarlark(StarlarkThread thread) throws EvalException {
BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ANDROID_ALLOWLIST);
BuiltinRestriction.failIfCalledOutsideBuiltins(thread);
return isToolConfiguration();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,13 @@
public final class StarlarkRuleContext
implements StarlarkRuleContextApi<ConstraintValueInfo>, StarlarkActionContext {

public static final ImmutableSet<BuiltinRestriction.AllowlistEntry>
PRIVATE_STARLARKIFICATION_ALLOWLIST =
ImmutableSet.of(
BuiltinRestriction.allowlistEntry("", "test"), // for tests
BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_android"),
BuiltinRestriction.allowlistEntry("build_bazel_rules_android", ""),
BuiltinRestriction.allowlistEntry("rules_android", ""),
BuiltinRestriction.allowlistEntry("", "tools/build_defs/android"));
static final ImmutableSet<BuiltinRestriction.AllowlistEntry> PRIVATE_STARLARKIFICATION_ALLOWLIST =
ImmutableSet.of(
BuiltinRestriction.allowlistEntry("", "test"), // for tests
BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_android"),
BuiltinRestriction.allowlistEntry("build_bazel_rules_android", ""),
BuiltinRestriction.allowlistEntry("rules_android", ""),
BuiltinRestriction.allowlistEntry("", "tools/build_defs/android"));

private static final String EXECUTABLE_OUTPUT_NAME = "executable";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2977,15 +2977,7 @@ public void registerLinkstampCompileAction(
compilationInputs.getSet(Artifact.class),
/* nonCodeInputs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
inputsForValidation.getSet(Artifact.class),
AnalysisUtils.isStampingEnabled(ruleContext, ruleContext.getConfiguration())
? ccToolchain
.getCcBuildInfoTranslator()
.getOutputGroup("non_redacted_build_info_files")
.toList()
: ccToolchain
.getCcBuildInfoTranslator()
.getOutputGroup("redacted_build_info_files")
.toList(),
ruleContext.getBuildInfo(CppBuildInfo.KEY),
/* additionalLinkstampDefines= */ ImmutableList.of(),
ccToolchain,
ruleContext.getConfiguration().isCodeCoverageEnabled(),
Expand All @@ -3001,6 +2993,18 @@ public void registerLinkstampCompileAction(
getSemantics()));
}

@StarlarkMethod(
name = "get_build_info",
documented = false,
parameters = {@Param(name = "ctx")},
useStarlarkThread = true)
public Sequence<Artifact> getBuildInfo(StarlarkRuleContext ruleContext, StarlarkThread thread)
throws EvalException, InterruptedException {
isCalledFromStarlarkCcCommon(thread);
return StarlarkList.immutableCopyOf(
ruleContext.getRuleContext().getBuildInfo(CppBuildInfo.KEY));
}

@StarlarkMethod(
name = "create_extra_link_time_library",
documented = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -556,17 +556,11 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
}

final ImmutableList<Artifact> buildInfoHeaderArtifacts =
linkstamps.isEmpty()
? ImmutableList.of()
: isStampingEnabled
? toolchain
.getCcBuildInfoTranslator()
.getOutputGroup("non_redacted_build_info_files")
.toList()
: toolchain
.getCcBuildInfoTranslator()
.getOutputGroup("redacted_build_info_files")
.toList();
!linkstamps.isEmpty()
? actionConstructionContext
.getAnalysisEnvironment()
.getBuildInfo(isStampingEnabled, CppBuildInfo.KEY, configuration)
: ImmutableList.of();

boolean needWholeArchive =
wholeArchive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.devtools.build.lib.rules.cpp.CcLinkingHelper;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.cpp.CppBuildInfo;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
import com.google.devtools.build.lib.rules.cpp.CppLinkAction;
Expand Down Expand Up @@ -188,15 +189,12 @@ public static NativeDepsRunfiles createNativeDepsAction(
List<Artifact> buildInfoArtifacts =
linkstamps.isEmpty()
? ImmutableList.<Artifact>of()
: AnalysisUtils.isStampingEnabled(ruleContext, configuration)
? toolchain
.getCcBuildInfoTranslator()
.getOutputGroup("non_redacted_build_info_files")
.toList()
: toolchain
.getCcBuildInfoTranslator()
.getOutputGroup("redacted_build_info_files")
.toList();
: ruleContext
.getAnalysisEnvironment()
.getBuildInfo(
AnalysisUtils.isStampingEnabled(ruleContext, configuration),
CppBuildInfo.KEY,
configuration);

ImmutableSortedSet.Builder<String> requestedFeaturesBuilder =
ImmutableSortedSet.<String>naturalOrder()
Expand Down
5 changes: 5 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,10 @@ def _get_tool_requirement_for_action(*, feature_configuration, action_name):
cc_common_internal.check_private_api(allowlist = _PRIVATE_STARLARKIFICATION_ALLOWLIST)
return cc_common_internal.get_tool_requirement_for_action(feature_configuration = feature_configuration, action_name = action_name)

def _get_build_info(ctx):
cc_common_internal.check_private_api(allowlist = _PRIVATE_STARLARKIFICATION_ALLOWLIST)
return cc_common_internal.get_build_info(ctx)

def _create_extra_link_time_library(*, build_library_func, **kwargs):
cc_common_internal.check_private_api(allowlist = _BUILTINS)
return cc_common_internal.create_extra_link_time_library(build_library_func = build_library_func, **kwargs)
Expand Down Expand Up @@ -915,6 +919,7 @@ cc_common = struct(
create_debug_context = _create_debug_context,
merge_debug_context = _merge_debug_context,
get_tool_requirement_for_action = _get_tool_requirement_for_action,
get_build_info = _get_build_info,
create_extra_link_time_library = _create_extra_link_time_library,
register_linkstamp_compile_action = _register_linkstamp_compile_action,
compile = _compile,
Expand Down
30 changes: 11 additions & 19 deletions src/main/starlark/builtins_bzl/common/python/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,7 @@
# limitations under the License.
"""Common functionality between test/binary executables."""

load(":common/cc/cc_common.bzl", _cc_common = "cc_common")
load(":common/cc/cc_helper.bzl", "cc_helper")
load(
":common/python/attributes.bzl",
"AGNOSTIC_EXECUTABLE_ATTRS",
"COMMON_ATTRS",
"PY_SRCS_ATTRS",
"SRCS_VERSION_ALL_VALUES",
"create_srcs_attr",
"create_srcs_version_attr",
)
load(
":common/python/common.bzl",
"TOOLCHAIN_TYPE",
Expand All @@ -37,6 +27,15 @@ load(
"filter_to_py_srcs",
"union_attrs",
)
load(
":common/python/attributes.bzl",
"AGNOSTIC_EXECUTABLE_ATTRS",
"COMMON_ATTRS",
"PY_SRCS_ATTRS",
"SRCS_VERSION_ALL_VALUES",
"create_srcs_attr",
"create_srcs_version_attr",
)
load(
":common/python/providers.bzl",
"PyCcLinkParamsProvider",
Expand All @@ -49,6 +48,7 @@ load(
"IS_BAZEL",
"PY_RUNTIME_ATTR_NAME",
)
load(":common/cc/cc_common.bzl", _cc_common = "cc_common")

_py_builtins = _builtins.internal.py_builtins

Expand Down Expand Up @@ -490,7 +490,6 @@ def _get_native_deps_details(ctx, *, semantics, cc_details, is_test):
if share_native_deps:
linked_lib = _create_shared_native_deps_dso(
ctx,
cc_toolchain = cc_details.cc_toolchain,
cc_info = cc_info,
is_test = is_test,
requested_features = cc_feature_config.requested_features,
Expand Down Expand Up @@ -528,7 +527,6 @@ def _get_native_deps_details(ctx, *, semantics, cc_details, is_test):
def _create_shared_native_deps_dso(
ctx,
*,
cc_toolchain,
cc_info,
is_test,
feature_configuration,
Expand All @@ -544,12 +542,6 @@ def _create_shared_native_deps_dso(
feature_configuration = feature_configuration,
)
)
if not linkstamps:
build_info_artifacts = []
elif cc_helper.is_stamping_enabled(ctx):
build_info_artifacts = cc_toolchain.build_info_files().non_redacted_build_info_files.to_list()
else:
build_info_artifacts = cc_toolchain.build_info_files().redacted_build_info_files.to_list()
dso_hash = _get_shared_native_deps_hash(
linker_inputs = cc_helper.get_static_mode_params_for_dynamic_library_libraries(
depset([
Expand All @@ -564,7 +556,7 @@ def _create_shared_native_deps_dso(
for flag in input.user_link_flags
],
linkstamps = [linkstamp.file() for linkstamp in linkstamps.to_list()],
build_info_artifacts = build_info_artifacts,
build_info_artifacts = _cc_common.get_build_info(ctx) if linkstamps else [],
features = requested_features,
is_test_target_partially_disabled_thin_lto = is_test and partially_disabled_thin_lto,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ public void testIsToolConfigurationIsBlocked() throws Exception {

AssertionError e =
assertThrows(AssertionError.class, () -> getConfiguredTarget("//example:custom"));
assertThat(e).hasMessageThat().contains("file '//example:rule.bzl' cannot use private API");
assertThat(e)
.hasMessageThat()
.contains("file '//example:rule.bzl' cannot use private @_builtins API");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,15 +629,9 @@ public void setupMockToolsRepository(MockToolsConfig config) throws IOException
config.create(
"embedded_tools/tools/build_defs/build_info/bazel_cc_build_info.bzl",
"def _impl(ctx):",
" volatile_file = ctx.actions.declare_file('volatile_file.h')",
" non_volatile_file = ctx.actions.declare_file('non_volatile_file.h')",
" redacted_file = ctx.actions.declare_file('redacted_file.h')",
" ctx.actions.write(output = volatile_file, content = '')",
" ctx.actions.write(output = non_volatile_file, content = '')",
" ctx.actions.write(output = redacted_file, content = '')",
" output_groups = {",
" 'non_redacted_build_info_files': depset([volatile_file, non_volatile_file]),",
" 'redacted_build_info_files': depset([redacted_file]),",
" 'non_redacted_build_info_files': depset([ctx.info_file, ctx.version_file]),",
" 'redacted_build_info_files': depset([ctx.version_file]),",
" }",
" return OutputGroupInfo(**output_groups)",
"bazel_cc_build_info = rule(implementation = _impl)");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames;
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames;
import static com.google.devtools.build.lib.rules.cpp.SolibSymlinkAction.MAX_FILENAME_LENGTH;
import static org.junit.Assert.assertThrows;

Expand All @@ -27,6 +28,7 @@
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
Expand Down Expand Up @@ -84,11 +86,6 @@
@RunWith(JUnit4.class)
public class StarlarkCcCommonTest extends BuildViewTestCase {

private static final String REDACTED_ARTIFACT_PATH =
"tools/build_defs/build_info/redacted_file.h";
private static final String NON_REDACTED_ARTIFACT_PATH =
"tools/build_defs/build_info/volatile_file.h";

@Before
public void setUp() throws Exception {
scratch.file("myinfo/myinfo.bzl", "MyInfo = provider()");
Expand Down Expand Up @@ -6064,23 +6061,16 @@ private CppCompileAction getLinkstampCompileAction(String label)
return linkstampCompileAction;
}

public String getBuildInfoFile(String commonPath) {
if (AnalysisMock.get().isThisBazel()) {
return getRelativeOutputPath() + "/k8-fastbuild/bin/external/bazel_tools/" + commonPath;
} else {
return getRelativeOutputPath() + "/k8-fastbuild/bin/" + commonPath;
}
}

private void assertStampEnabled(CppCompileAction linkstampAction)
throws CommandLineExpansionException {
assertThat(linkstampAction.getArguments())
.contains(getBuildInfoFile(NON_REDACTED_ARTIFACT_PATH));
.contains(getRelativeOutputPath() + "/k8-fastbuild/include/build-info-volatile.h");
}

private void assertStampDisabled(CppCompileAction linkstampAction)
throws CommandLineExpansionException {
assertThat(linkstampAction.getArguments()).contains(getBuildInfoFile(REDACTED_ARTIFACT_PATH));
assertThat(linkstampAction.getArguments())
.contains(getRelativeOutputPath() + "/k8-fastbuild/include/build-info-redacted.h");
}

@Test
Expand Down Expand Up @@ -8073,6 +8063,56 @@ public void testCcToolchainCompileFilesNotAccessibleFromOutsideBuiltins() throws
assertThat(e).hasMessageThat().contains("cannot use private API");
}

@Test
public void testGetBuildInfoArtifactsIsPrivateApi() throws Exception {
scratch.file(
"foo/BUILD", "load(':custom_rule.bzl', 'custom_rule')", "custom_rule(name = 'custom')");
scratch.file(
"foo/custom_rule.bzl",
"def _impl(ctx):",
" cc_common.get_build_info(ctx)",
" return []",
"custom_rule = rule(",
" implementation = _impl,",
")");
invalidatePackages();

AssertionError e =
assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:custom"));

assertThat(e).hasMessageThat().contains("cannot use private API");
}

@Test
public void testBuildInfoArtifacts() throws Exception {
scratch.file(
"bazel_internal/test_rules/cc/rule.bzl",
"def _impl(ctx):",
" artifacts = cc_common.get_build_info(ctx)",
" return [DefaultInfo(files = depset(artifacts))]",
"build_info_rule = rule(",
" implementation = _impl,",
" attrs = {'stamp': attr.int()},",
")");
scratch.file(
"bazel_internal/test_rules/cc/BUILD",
"load(':rule.bzl', 'build_info_rule')",
"build_info_rule(name = 'stamped', stamp = 1,)",
"build_info_rule(name = 'unstamped', stamp = 0,)");
assertThat(
prettyArtifactNames(
getConfiguredTarget("//bazel_internal/test_rules/cc:stamped")
.getProvider(FileProvider.class)
.getFilesToBuild()))
.containsExactly("build-info-nonvolatile.h", "build-info-volatile.h");
assertThat(
prettyArtifactNames(
getConfiguredTarget("//bazel_internal/test_rules/cc:unstamped")
.getProvider(FileProvider.class)
.getFilesToBuild()))
.containsExactly("build-info-redacted.h");
}

@Test
public void testCheckPrivateApiCanOnlyBeCalledFromCcCommonBzl() throws Exception {
scratch.file(
Expand Down

0 comments on commit 66ac39c

Please sign in to comment.