Skip to content

Commit

Permalink
Remove the --experimental_repository_hash_file and --experimental_ver…
Browse files Browse the repository at this point in the history
…ify_repository_rules command line options.

RELNOTES[INC]: The --experimental_repository_hash_file and --experimental_verify_repository_rules command line options are not available anymore.

PiperOrigin-RevId: 569122198
Change-Id: I8bc4007a41927176e981c1901c98bfd40b9169df
  • Loading branch information
lberki authored and Copybara-Service committed Sep 28, 2023
1 parent 8c24877 commit 176b07f
Show file tree
Hide file tree
Showing 26 changed files with 48 additions and 333 deletions.
Expand Up @@ -20,7 +20,6 @@
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
Expand Down Expand Up @@ -119,7 +118,6 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -149,9 +147,7 @@ public class BazelRepositoryModule extends BlazeModule {
new MutableSupplier<>();
private ImmutableMap<RepositoryName, PathFragment> overrides = ImmutableMap.of();
private ImmutableMap<String, ModuleOverride> moduleOverrides = ImmutableMap.of();
private Optional<RootedPath> resolvedFile = Optional.empty();
private Optional<RootedPath> resolvedFileReplacingWorkspace = Optional.empty();
private Set<String> outputVerificationRules = ImmutableSet.of();
private FileSystem filesystem;
private List<String> registries;
private final AtomicBoolean ignoreDevDeps = new AtomicBoolean(false);
Expand Down Expand Up @@ -311,9 +307,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
clientEnvironmentSupplier.set(env.getRepoEnv());
PackageOptions pkgOptions = env.getOptions().getOptions(PackageOptions.class);
isFetch.set(pkgOptions != null && pkgOptions.fetch);
resolvedFile = Optional.empty();
resolvedFileReplacingWorkspace = Optional.empty();
outputVerificationRules = ImmutableSet.of();

ProcessWrapper processWrapper = ProcessWrapper.fromCommandEnvironment(env);
starlarkRepositoryFunction.setProcessWrapper(processWrapper);
Expand Down Expand Up @@ -511,17 +505,6 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
registries = DEFAULT_REGISTRIES;
}

if (!Strings.isNullOrEmpty(repoOptions.repositoryHashFile)) {
Path hashFile;
if (env.getWorkspace() != null) {
hashFile = env.getWorkspace().getRelative(repoOptions.repositoryHashFile);
} else {
hashFile = filesystem.getPath(repoOptions.repositoryHashFile);
}
resolvedFile =
Optional.of(RootedPath.toRootedPath(Root.absoluteRoot(filesystem), hashFile));
}

if (!Strings.isNullOrEmpty(repoOptions.experimentalResolvedFileInsteadOfWorkspace)) {
Path resolvedFile;
if (env.getWorkspace() != null) {
Expand All @@ -535,11 +518,6 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
Optional.of(RootedPath.toRootedPath(Root.absoluteRoot(filesystem), resolvedFile));
}

if (repoOptions.experimentalVerifyRepositoryRules != null) {
outputVerificationRules =
ImmutableSet.copyOf(repoOptions.experimentalVerifyRepositoryRules);
}

RepositoryRemoteExecutorFactory remoteExecutorFactory =
env.getRuntime().getRepositoryRemoteExecutorFactory();
RepositoryRemoteExecutor remoteExecutor = null;
Expand Down Expand Up @@ -574,11 +552,6 @@ public ImmutableList<Injected> getPrecomputedValues() {
return ImmutableList.of(
PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides),
PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, moduleOverrides),
PrecomputedValue.injected(
RepositoryDelegatorFunction.RESOLVED_FILE_FOR_VERIFICATION, resolvedFile),
PrecomputedValue.injected(
RepositoryDelegatorFunction.OUTPUT_VERIFICATION_REPOSITORY_RULES,
outputVerificationRules),
PrecomputedValue.injected(
RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE,
resolvedFileReplacingWorkspace),
Expand Down
Expand Up @@ -48,6 +48,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value",
"//src/main/java/com/google/devtools/build/lib/runtime/commands",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
Expand Down
Expand Up @@ -34,7 +34,7 @@
import com.google.devtools.build.lib.pkgcache.PackageOptions;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction;
import com.google.devtools.build.lib.rules.repository.ResolvedFileValue;
import com.google.devtools.build.lib.runtime.BlazeCommand;
import com.google.devtools.build.lib.runtime.BlazeCommandResult;
import com.google.devtools.build.lib.runtime.Command;
Expand Down Expand Up @@ -270,11 +270,11 @@ public String getName() {
@Override
public Object getResolvedInformation(XattrProvider xattrProvider) {
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, "bind")
.put(ResolvedFileValue.ORIGINAL_RULE_CLASS, "bind")
.put(
ResolvedHashesFunction.ORIGINAL_ATTRIBUTES,
ResolvedFileValue.ORIGINAL_ATTRIBUTES,
ImmutableMap.<String, Object>of("name", name, "actual", actual))
.put(ResolvedHashesFunction.NATIVE, nativeCommand)
.put(ResolvedFileValue.NATIVE, nativeCommand)
.buildOrThrow();
}
};
Expand Down Expand Up @@ -302,9 +302,9 @@ public String getName() {
@Override
public Object getResolvedInformation(XattrProvider xattrProvider) {
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, ruleName)
.put(ResolvedFileValue.ORIGINAL_RULE_CLASS, ruleName)
.put(
ResolvedHashesFunction.ORIGINAL_ATTRIBUTES,
ResolvedFileValue.ORIGINAL_ATTRIBUTES,
// The original attributes are a bit of a problem, as the arguments to
// the rule do not at all look like those of a repository rule:
// they're all positional, and, in particular, there is no keyword argument
Expand All @@ -315,7 +315,7 @@ public Object getResolvedInformation(XattrProvider xattrProvider) {
// that rule. Note that the original arguments are always ignored when bazel uses
// a resolved file instead of a workspace file.
ImmutableMap.<String, Object>of("name", name, "*args", args))
.put(ResolvedHashesFunction.NATIVE, nativeCommand)
.put(ResolvedFileValue.NATIVE, nativeCommand)
.build();
}
};
Expand Down
Expand Up @@ -39,6 +39,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/repository:repository_events",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_base_rule",
"//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_configured_target_factory",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down
Expand Up @@ -22,7 +22,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction;
import com.google.devtools.build.lib.rules.repository.ResolvedFileValue;
import com.google.devtools.build.lib.util.CPU;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -90,11 +90,11 @@ public String getName() {
public Object getResolvedInformation(XattrProvider xattrProvider) {
String repr = String.format("local_config_platform(name = '%s')", name);
return ImmutableMap.<String, Object>builder()
.put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, LocalConfigPlatformRule.NAME)
.put(ResolvedFileValue.ORIGINAL_RULE_CLASS, LocalConfigPlatformRule.NAME)
.put(
ResolvedHashesFunction.ORIGINAL_ATTRIBUTES,
ResolvedFileValue.ORIGINAL_ATTRIBUTES,
ImmutableMap.<String, Object>builder().put("name", name).buildOrThrow())
.put(ResolvedHashesFunction.NATIVE, repr)
.put(ResolvedFileValue.NATIVE, repr)
.buildOrThrow();
}
});
Expand Down
Expand Up @@ -190,30 +190,6 @@ public class RepositoryOptions extends OptionsBase {
+ " source code")
public double experimentalScaleTimeouts;

@Option(
name = "experimental_repository_hash_file",
defaultValue = "",
documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
"If non-empty, specifies a file containing a resolved value, against which"
+ " the repository directory hashes should be verified")
public String repositoryHashFile;

@Option(
name = "experimental_verify_repository_rules",
allowMultiple = true,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
"If list of repository rules for which the hash of the output directory should be"
+ " verified, provided a file is specified by"
+ " --experimental_repository_hash_file.")
public List<String> experimentalVerifyRepositoryRules;

@Option(
name = "experimental_resolved_file_instead_of_workspace",
defaultValue = "",
Expand Down
Expand Up @@ -13,21 +13,14 @@
// limitations under the License.
package com.google.devtools.build.lib.bazel.repository;

import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.ATTRIBUTES;
import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.DEFINITION_INFORMATION;
import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.ORIGINAL_ATTRIBUTES;
import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.ORIGINAL_RULE_CLASS;
import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.OUTPUT_TREE_HASH;
import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.REPOSITORIES;
import static com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction.RULE_CLASS;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.bazel.ResolvedEvent;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.repository.ResolvedFileValue;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.XattrProvider;
Expand All @@ -42,7 +35,6 @@
* Event indicating that a repository rule was executed, together with the return value of the rule.
*/
public class RepositoryResolvedEvent implements ResolvedEvent {

/**
* The entry for WORSPACE.resolved corresponding to that rule invocation.
*
Expand Down Expand Up @@ -81,8 +73,9 @@ public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory

String originalClass =
rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel() + "%" + rule.getRuleClass();
resolvedInformationBuilder.put(ORIGINAL_RULE_CLASS, originalClass);
resolvedInformationBuilder.put(DEFINITION_INFORMATION, getRuleDefinitionInformation(rule));
resolvedInformationBuilder.put(ResolvedFileValue.ORIGINAL_RULE_CLASS, originalClass);
resolvedInformationBuilder.put(
ResolvedFileValue.DEFINITION_INFORMATION, getRuleDefinitionInformation(rule));

ImmutableMap.Builder<String, Object> origAttrBuilder = ImmutableMap.builder();
ImmutableMap.Builder<String, Object> defaults = ImmutableMap.builder();
Expand All @@ -103,19 +96,19 @@ public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory
}
}
ImmutableMap<String, Object> origAttr = origAttrBuilder.buildOrThrow();
resolvedInformationBuilder.put(ORIGINAL_ATTRIBUTES, origAttr);
resolvedInformationBuilder.put(ResolvedFileValue.ORIGINAL_ATTRIBUTES, origAttr);

repositoryBuilder.put(RULE_CLASS, originalClass);
repositoryBuilder.put(ResolvedFileValue.RULE_CLASS, originalClass);

if (result == Starlark.NONE) {
// Rule claims to be already reproducible, so wants to be called as is.
repositoryBuilder.put(ATTRIBUTES, origAttr);
repositoryBuilder.put(ResolvedFileValue.ATTRIBUTES, origAttr);
this.informationReturned = false;
this.message = "Repository rule '" + rule.getName() + "' finished.";
} else if (result instanceof Map) {
// Rule claims that the returned (probably changed) arguments are a reproducible
// version of itself.
repositoryBuilder.put(ATTRIBUTES, result);
repositoryBuilder.put(ResolvedFileValue.ATTRIBUTES, result);
Pair<Map<String, Object>, List<String>> diff =
compare(origAttr, defaults.buildOrThrow(), (Map<?, ?>) result);
if (diff.getFirst().isEmpty() && diff.getSecond().isEmpty()) {
Expand Down Expand Up @@ -151,7 +144,7 @@ public RepositoryResolvedEvent(Rule rule, StructImpl attrs, Path outputDirectory
} else {
// TODO(aehlig): handle strings specially to allow encodings of the former
// values to be accepted as well.
resolvedInformationBuilder.put(REPOSITORIES, result);
resolvedInformationBuilder.put(ResolvedFileValue.REPOSITORIES, result);
repositoryBuilder = null; // We already added the REPOSITORIES entry
this.informationReturned = true;
this.message = "Repository rule '" + rule.getName() + "' returned: " + result;
Expand All @@ -171,15 +164,16 @@ private synchronized void finalizeResolvedInformation(XattrProvider xattrProvide
String digest = "[unavailable]";
try {
digest = outputDirectory.getDirectoryDigest(xattrProvider);
repositoryBuilder.put(OUTPUT_TREE_HASH, digest);
repositoryBuilder.put(ResolvedFileValue.OUTPUT_TREE_HASH, digest);
} catch (IOException e) {
// Digest not available, but we still have to report that a repository rule
// was invoked. So we can do nothing, but ignore the event.
}
this.directoryDigest = digest;
if (repositoryBuilder != null) {
resolvedInformationBuilder.put(
REPOSITORIES, ImmutableList.<Object>of(repositoryBuilder.buildOrThrow()));
ResolvedFileValue.REPOSITORIES,
ImmutableList.<Object>of(repositoryBuilder.buildOrThrow()));
}
this.resolvedInformation = resolvedInformationBuilder.buildOrThrow();
this.resolvedInformationBuilder = null;
Expand Down
Expand Up @@ -37,7 +37,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/repository:request_repository_information_event",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_hashes_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_attribute_mapper",
"//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_file_helper",
"//src/main/java/com/google/devtools/build/lib/shell",
Expand Down
Expand Up @@ -36,10 +36,8 @@
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.rules.repository.NeedsSkyframeRestartException;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.rules.repository.ResolvedHashesValue;
import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper;
import com.google.devtools.build.lib.runtime.ProcessWrapper;
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
Expand All @@ -54,7 +52,6 @@
import com.google.devtools.build.skyframe.SkyKey;
import java.io.IOException;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -225,18 +222,6 @@ private RepositoryDirectoryValue.Builder fetchInternal(
markerData.put(SEMANTICS, describeSemantics(starlarkSemantics));
markerData.put("ARCH:", CPU.getCurrent().getCanonicalName());

Set<String> verificationRules =
RepositoryDelegatorFunction.OUTPUT_VERIFICATION_REPOSITORY_RULES.get(env);
if (env.valuesMissing()) {
return null;
}
ResolvedHashesValue resolvedHashesValue =
(ResolvedHashesValue) env.getValue(ResolvedHashesValue.key());
if (env.valuesMissing()) {
return null;
}
Map<String, String> resolvedHashes = checkNotNull(resolvedHashesValue).getHashes();

PathPackageLocator packageLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env);
if (env.valuesMissing()) {
return null;
Expand Down Expand Up @@ -322,20 +307,6 @@ private RepositoryDirectoryValue.Builder fetchInternal(
markerData.put("FILE:" + entry.getKey(), entry.getValue());
}

String ruleClass =
rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel() + "%" + rule.getRuleClass();
if (verificationRules.contains(ruleClass)) {
String expectedHash = resolvedHashes.get(rule.getName());
if (expectedHash != null) {
String actualHash = resolved.getDirectoryDigest(syscallCache);
if (!expectedHash.equals(actualHash)) {
throw new RepositoryFunctionException(
new IOException(
rule + " failed to create a directory with expected hash " + expectedHash),
Transience.PERSISTENT);
}
}
}
env.getListener().post(resolved);
} catch (NeedsSkyframeRestartException e) {
// A dependency is missing, cleanup and returns null
Expand Down

0 comments on commit 176b07f

Please sign in to comment.