Automated rollback of commit cb9bd8615210dda2104f79d281938e47187dc2de. *** Reason for rollback *** Causes a Blaze performance regression . *** Original change description *** Add support for multiple module interfaces per `cc_library` When multiple `module_interfaces` are specified on a single `cc_library`, the individual compilation actions form a DAG based on `import`s between these modules. Consider the following situation: * `a.cppm` imports `b.cppm`, both of which are in the `module_interfaces` of a single `cc_library`. * Building the target populates the action cache with an entry for `a.pcm` that store... *** PiperOrigin-RevId: 846394946 Change-Id: Ia2d5d8a6dd28186db7f1b61f950fbe0565810ea7
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index 21889d8..65b26f8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -75,6 +75,7 @@ @Nullable private final ActionCache actionCache; // Null when not enabled. + /** Cache config parameters for ActionCacheChecker. */ @AutoValue public abstract static class CacheConfig { @@ -178,7 +179,6 @@ * @param actionInputs the action inputs; usually action.getInputs(), but might be a previously * cached set of discovered inputs for actions that discover them. * @param outputMetadataStore metadata provider for action outputs. - * @param mandatoryInputsDigest the digest of mandatory inputs, or null if not discovering inputs. * @param cachedOutputMetadata cached metadata that should be used instead of {@code * outputMetadataStore}. * @param outputChecker used to check whether remote metadata should be trusted. @@ -195,7 +195,6 @@ NestedSet<Artifact> actionInputs, InputMetadataProvider inputMetadataProvider, OutputMetadataStore outputMetadataStore, - @Nullable byte[] mandatoryInputsDigest, @Nullable CachedOutputMetadata cachedOutputMetadata, @Nullable OutputChecker outputChecker, ImmutableMap<String, String> effectiveEnvironment, @@ -210,8 +209,7 @@ effectiveEnvironment, actionExecutionSalt, outputPermissions, - useArchivedTreeArtifacts, - mandatoryInputsDigest); + useArchivedTreeArtifacts); for (Artifact artifact : action.getOutputs()) { if (artifact.isTreeArtifact()) { @@ -454,7 +452,6 @@ public Token getTokenIfNeedToExecute( Action action, List<Artifact> resolvedCacheArtifacts, - @Nullable byte[] mandatoryInputsDigest, Map<String, String> clientEnv, OutputPermissions outputPermissions, EventHandler handler, @@ -511,7 +508,6 @@ clientEnv, outputPermissions, actionExecutionSalt, - mandatoryInputsDigest, cachedOutputMetadata, outputChecker, useArchivedTreeArtifacts)) { @@ -545,7 +541,6 @@ Map<String, String> clientEnv, OutputPermissions outputPermissions, String actionExecutionSalt, - @Nullable byte[] mandatoryInputsDigest, @Nullable CachedOutputMetadata cachedOutputMetadata, @Nullable OutputChecker outputChecker, boolean useArchivedTreeArtifacts) @@ -583,7 +578,6 @@ actionInputs, inputMetadataProvider, outputMetadataStore, - mandatoryInputsDigest, cachedOutputMetadata, outputChecker, effectiveEnvironment, @@ -620,7 +614,7 @@ // to trigger a re-execution, so we should catch the IOException explicitly there. In others, we // should propagate the exception, because it is unexpected (e.g., bad file system state). @Nullable - public static FileArtifactValue getInputMetadataMaybe( + private static FileArtifactValue getInputMetadataMaybe( InputMetadataProvider inputMetadataProvider, Artifact artifact) { try { return getInputMetadataOrConstant(inputMetadataProvider, artifact); @@ -662,8 +656,7 @@ Map<String, String> clientEnv, OutputPermissions outputPermissions, String actionExecutionSalt, - boolean useArchivedTreeArtifacts, - @Nullable byte[] mandatoryInputsDigest) + boolean useArchivedTreeArtifacts) throws IOException, InterruptedException { checkState(cacheConfig.enabled(), "cache unexpectedly disabled, action: %s", action); Preconditions.checkArgument(token != null, "token unexpectedly null, action: %s", action); @@ -690,8 +683,7 @@ effectiveEnvironment, actionExecutionSalt, outputPermissions, - useArchivedTreeArtifacts, - mandatoryInputsDigest); + useArchivedTreeArtifacts); for (Artifact output : action.getOutputs()) { // Remove old records from the cache if they used different key. @@ -734,14 +726,6 @@ actionCache.put(key, builder.build()); } - public boolean mandatoryInputsMatch(Action action, byte[] mandatoryInputsDigest) { - checkArgument(action.discoversInputs()); - ActionCache.Entry entry = getCacheEntry(action); - return entry != null - && !entry.isCorrupted() - && Arrays.equals(entry.getMandatoryInputsDigest(), mandatoryInputsDigest); - } - @Nullable public List<Artifact> getCachedInputs(Action action, PackageRootResolver resolver) throws PackageRootResolver.PackageRootException, InterruptedException { @@ -815,7 +799,6 @@ public Token getTokenUnconditionallyAfterFailureToRecordActionCacheHit( Action action, List<Artifact> resolvedCacheArtifacts, - @Nullable byte[] mandatoryInputsDigest, Map<String, String> clientEnv, OutputPermissions outputPermissions, EventHandler handler, @@ -831,7 +814,6 @@ return getTokenIfNeedToExecute( action, resolvedCacheArtifacts, - mandatoryInputsDigest, clientEnv, outputPermissions, handler, @@ -894,4 +876,5 @@ this.cacheKey = action.getPrimaryOutput().getExecPathString(); } } + }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java index 3d32588..13fcd0d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
@@ -82,13 +82,12 @@ final class Entry { /** Unique instance standing for a corrupted cache entry. */ public static final ActionCache.Entry CORRUPTED = - new Entry(null, null, null, ImmutableMap.of(), ImmutableMap.of(), ImmutableList.of()); + new Entry(null, null, ImmutableMap.of(), ImmutableMap.of(), ImmutableList.of()); // Digest of all relevant properties of the action for cache invalidation purposes. // Null if the entry is corrupted. @Nullable private final byte[] digest; - @Nullable private final byte[] mandatoryInputsDigest; // List of input paths discovered by the action. // Null if the action does not discover inputs. @Nullable private final ImmutableList<String> discoveredInputPaths; @@ -101,13 +100,11 @@ Entry( @Nullable byte[] digest, - @Nullable byte[] mandatoryInputsDigest, @Nullable ImmutableList<String> discoveredInputPaths, ImmutableMap<String, FileArtifactValue> outputFileMetadata, ImmutableMap<String, SerializableTreeArtifactValue> outputTreeMetadata, ImmutableList<String> proxyOutputs) { this.digest = digest; - this.mandatoryInputsDigest = mandatoryInputsDigest; this.discoveredInputPaths = discoveredInputPaths; this.outputFileMetadata = outputFileMetadata; this.outputTreeMetadata = outputTreeMetadata; @@ -131,11 +128,7 @@ /** Returns whether the action discovers inputs. */ public boolean discoversInputs() { checkState(!isCorrupted()); - if (discoveredInputPaths == null) { - return false; - } - checkState(mandatoryInputsDigest != null); - return true; + return discoveredInputPaths != null; } /** @@ -147,12 +140,6 @@ return discoveredInputPaths; } - @Nullable - public byte[] getMandatoryInputsDigest() { - checkState(discoversInputs()); - return mandatoryInputsDigest; - } - /** Gets the metadata of an output file. */ @Nullable public FileArtifactValue getOutputFile(Artifact output) { @@ -200,7 +187,6 @@ return MoreObjects.toStringHelper(this) .add("digest", digest) .add("discoveredInputPaths", discoveredInputPaths) - .add("mandatoryInputsDigest", mandatoryInputsDigest) .add("outputFileMetadata", outputFileMetadata) .add("outputTreeMetadata", outputTreeMetadata) .add("proxyOutputs", proxyOutputs) @@ -219,9 +205,6 @@ out.format(" %s\n", path); } } - if (mandatoryInputsDigest != null) { - out.format(" mandatoryInputsDigest = %s\n", formatDigest(mandatoryInputsDigest)); - } if (!outputFileMetadata.isEmpty()) { out.println(" outputFileMetadata ="); @@ -293,7 +276,6 @@ // Discovered inputs. // Null if the action does not discover inputs. @Nullable private final ImmutableList.Builder<String> discoveredInputPaths; - @Nullable private final byte[] mandatoryInputsDigest; private final ImmutableMap.Builder<String, FileArtifactValue> outputFileMetadata = ImmutableMap.builder(); @@ -312,8 +294,6 @@ * @param discoversInputs whether the action discovers inputs. * @param outputPermissions the requested output permissions. * @param useArchivedTreeArtifacts whether archived tree artifacts are enabled. - * @param mandatoryInputsDigest the digest of the mandatory inputs, or null if the action - * doesn't discover inputs. */ public Builder( String actionKey, @@ -321,18 +301,13 @@ ImmutableMap<String, String> clientEnv, String actionExecutionSalt, OutputPermissions outputPermissions, - boolean useArchivedTreeArtifacts, - @Nullable byte[] mandatoryInputsDigest) { + boolean useArchivedTreeArtifacts) { this.actionKey = actionKey; this.clientEnv = clientEnv; this.actionExecutionSalt = actionExecutionSalt; this.discoveredInputPaths = discoversInputs ? ImmutableList.builder() : null; this.outputPermissions = outputPermissions; this.useArchivedTreeArtifacts = useArchivedTreeArtifacts; - checkArgument( - (mandatoryInputsDigest != null) == discoversInputs, - "mandatoryInputsDigest must be set iff the action discovers inputs"); - this.mandatoryInputsDigest = mandatoryInputsDigest; } /** Adds metadata of an input file. */ @@ -416,7 +391,6 @@ actionExecutionSalt, outputPermissions, useArchivedTreeArtifacts), - mandatoryInputsDigest, discoveredInputPaths != null ? discoveredInputPaths.build() : null, outputFileMetadata.buildOrThrow(), outputTreeMetadata.buildOrThrow(),
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index fe51b7a..467f991 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
@@ -75,7 +75,7 @@ // cache records. private static final int VALIDATION_KEY = -10; - private static final int VERSION = 24; + private static final int VERSION = 23; /** * A timestamp, represented as the number of minutes since the Unix epoch. @@ -848,8 +848,7 @@ int maxDiscoveredInputsSize = 1; // presence marker if (entry.discoversInputs()) { maxDiscoveredInputsSize += - (1 + DigestUtils.ESTIMATED_SIZE) // mandatoryInputsDigest - + VarInt.MAX_VARINT_SIZE // length + VarInt.MAX_VARINT_SIZE // length + (VarInt.MAX_VARINT_SIZE // execPath * entry.getDiscoveredInputPaths().size()); } @@ -900,7 +899,6 @@ VarInt.putVarInt(entry.discoversInputs() ? 1 : 0, sink); if (entry.discoversInputs()) { - MetadataDigestUtils.write(entry.getMandatoryInputsDigest(), sink); ImmutableList<String> discoveredInputPaths = entry.getDiscoveredInputPaths(); VarInt.putVarInt(discoveredInputPaths.size(), sink); for (String discoveredInputPath : discoveredInputPaths) { @@ -976,7 +974,6 @@ byte[] digest = MetadataDigestUtils.read(source); - byte[] mandatoryInputsDigest = null; ImmutableList<String> discoveredInputPaths = null; int discoveredInputsPresenceMarker = VarInt.getVarInt(source); if (discoveredInputsPresenceMarker != 0) { @@ -984,10 +981,6 @@ throw new IOException( "Invalid presence marker for discovered inputs: " + discoveredInputsPresenceMarker); } - mandatoryInputsDigest = MetadataDigestUtils.read(source); - if (mandatoryInputsDigest.length != digest.length) { - throw new IOException("Corrupted mandatory inputs digest"); - } int numDiscoveredInputs = VarInt.getVarInt(source); if (numDiscoveredInputs < 0) { throw new IOException("Invalid discovered input count: " + numDiscoveredInputs); @@ -1009,7 +1002,6 @@ } return new ActionCache.Entry( digest, - mandatoryInputsDigest, discoveredInputPaths, /* outputFileMetadata= */ ImmutableMap.of(), /* outputTreeMetadata= */ ImmutableMap.of(), @@ -1090,7 +1082,6 @@ } return new ActionCache.Entry( digest, - mandatoryInputsDigest, discoveredInputPaths, outputFiles.buildOrThrow(), outputTrees.buildOrThrow(),
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataDigestUtils.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataDigestUtils.java index d41c141..3c55af9 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataDigestUtils.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataDigestUtils.java
@@ -28,8 +28,6 @@ public final class MetadataDigestUtils { private MetadataDigestUtils() {} - private static final byte[] emptyDigest = new Fingerprint().digestAndReset(); - /** * @param source the byte buffer source. * @return the digest from the given buffer. @@ -56,7 +54,7 @@ * @param mdMap A collection of (execPath, FileArtifactValue) pairs. Values may be null. */ public static byte[] fromMetadata(Map<String, FileArtifactValue> mdMap) { - byte[] result = emptyDigest.clone(); + byte[] result = new byte[1]; // reserve the empty string // Profiling showed that MessageDigest engine instantiation was a hotspot, so create one // instance for this computation to amortize its cost. Fingerprint fp = new Fingerprint();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java index bd76f40..7fb073b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java
@@ -345,15 +345,6 @@ return getInputs(); } - @Override - public NestedSet<Artifact> getMandatoryInputs() { - if (unusedInputsList.isPresent()) { - return NestedSetBuilder.emptySet(Order.STABLE_ORDER); - } else { - return getInputs(); - } - } - @Nullable @Override public NestedSet<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext) @@ -367,11 +358,7 @@ NestedSet<Artifact> inputFilesForExtraAction = shadowedActionObj.getInputFilesForExtraAction(actionExecutionContext); if (inputFilesForExtraAction == null) { - if (unusedInputsList.isPresent()) { - return allStarlarkActionInputs; - } else { - return null; - } + return null; } updateInputs( createInputs(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 657aeef..9003d21 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -33,7 +33,6 @@ import com.google.common.collect.SetMultimap; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.ActionCacheChecker; import com.google.devtools.build.lib.actions.ActionCacheChecker.Token; import com.google.devtools.build.lib.actions.ActionCompletionEvent; import com.google.devtools.build.lib.actions.ActionExecutedEvent.ErrorTiming; @@ -53,7 +52,6 @@ import com.google.devtools.build.lib.actions.RichArtifactData; import com.google.devtools.build.lib.actions.RichDataProducingAction; import com.google.devtools.build.lib.actions.SpawnMetrics; -import com.google.devtools.build.lib.actions.cache.MetadataDigestUtils; import com.google.devtools.build.lib.actions.cache.OutputMetadataStore; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.bugreport.BugReport; @@ -273,7 +271,7 @@ if (!state.hasCollectedInputs()) { try { state.allInputs = collectInputs(action, env); - } catch (ActionExecutionException e) { + } catch (AlreadyReportedActionExecutionException e) { throw new ActionExecutionFunctionException(e); } if (state.allInputs == null) { @@ -537,64 +535,6 @@ } /** - * Computes the digest of the action's mandatory inputs. Callers should check for missing values - * before using the result, which may be null in error cases. - */ - @Nullable - private byte[] computeMandatoryInputsDigest(Action action, Environment env) - throws InterruptedException, ActionExecutionException { - NestedSet<Artifact> mandatoryInputsSet = action.getMandatoryInputs(); - var mandatoryInputsKeysBuilder = ImmutableSet.<SkyKey>builder(); - for (Artifact leaf : mandatoryInputsSet.getLeaves()) { - mandatoryInputsKeysBuilder.add(Artifact.key(leaf)); - } - for (NestedSet<Artifact> nonLeaf : mandatoryInputsSet.getNonLeaves()) { - mandatoryInputsKeysBuilder.add(ArtifactNestedSetKey.create(nonLeaf)); - } - var mandatoryInputsKeys = mandatoryInputsKeysBuilder.build(); - var lookupResult = env.getValuesAndExceptions(mandatoryInputsKeys); - if (env.valuesMissing() && !env.inErrorBubbling()) { - return null; - } - var mandatoryInputs = mandatoryInputsSet.toList(); - var inputArtifactData = new ActionInputMap(mandatoryInputs.size()); - var inputMap = new HashMap<String, FileArtifactValue>(mandatoryInputs.size()); - var actionExecutionFunctionExceptionHandler = - createActionExecutionFunctionExceptionHandler( - action, - lookupResult, - mandatoryInputsKeys, - () -> mandatoryInputs, - /* isMandatoryInput= */ Predicates.alwaysTrue()); - var unused = actionExecutionFunctionExceptionHandler.accumulateAndMaybeThrowExceptions(); - if (env.valuesMissing() && !env.inErrorBubbling()) { - return null; - } - for (var artifact : mandatoryInputs) { - SkyValue value = - getAndCheckInputSkyValue( - env, - action, - artifact, - mandatoryInputsKeys, - /* isMandatoryInput= */ Predicates.alwaysTrue(), - actionExecutionFunctionExceptionHandler); - if (value == null || value instanceof MissingArtifactValue) { - // This can happen with rewinding or in keep-going builds and is always an indication to - // halt the current action execution attempt - we do not have to compute a digest. - return null; - } - ActionInputMapHelper.addToMap( - inputArtifactData, artifact, value, MetadataConsumerForMetrics.NO_OP); - inputMap.put( - artifact.getExecPathString(), - ActionCacheChecker.getInputMetadataMaybe(inputArtifactData, artifact)); - } - actionExecutionFunctionExceptionHandler.maybeThrowException(); - return MetadataDigestUtils.fromMetadata(inputMap); - } - - /** * An action's inputs needed for execution. May not just be the result of Action#getInputs(). If * the action cache's view of this action contains additional inputs, it will request metadata for * them, so we consider those inputs as dependencies of this action as well. Returns null if some @@ -602,30 +542,10 @@ */ @Nullable private AllInputs collectInputs(Action action, Environment env) - throws InterruptedException, ActionExecutionException { - byte[] mandatoryInputsDigest = null; - if (action.discoversInputs()) { - // When the mandatory inputs of an action have changed, it will certainly have to be - // reexecuted. It is important that we detect this before requesting the previously discovered - // inputs from Skyframe as that could result in cycles that otherwise would not occur. - // Note that this approach does not guarantee the absence of such "phantom" cycles in general, - // it just happens to work for all current use cases in Bazel. A theoretically sound solution - // would require checking the discovered inputs for changes one by one, in the order in which - // they were originally discovered. - mandatoryInputsDigest = computeMandatoryInputsDigest(action, env); - if (env.valuesMissing()) { - return null; - } - if (mandatoryInputsDigest == null - || !skyframeActionExecutor.mandatoryInputsMatch(action, mandatoryInputsDigest)) { - action.resetDiscoveredInputs(); - return new AllInputs(action.getInputs(), mandatoryInputsDigest); - } - } - + throws InterruptedException, AlreadyReportedActionExecutionException { NestedSet<Artifact> allKnownInputs = action.getInputs(); if (action.inputsKnown()) { - return new AllInputs(allKnownInputs, mandatoryInputsDigest); + return new AllInputs(allKnownInputs); } checkState(action.discoversInputs(), action); @@ -636,27 +556,21 @@ checkState(env.valuesMissing(), action); return null; } - return new AllInputs(allKnownInputs, actionCacheInputs, mandatoryInputsDigest); + return new AllInputs(allKnownInputs, actionCacheInputs); } static class AllInputs { final NestedSet<Artifact> defaultInputs; @Nullable final List<Artifact> actionCacheInputs; - @Nullable public final byte[] mandatoryInputsDigest; - AllInputs(NestedSet<Artifact> defaultInputs, @Nullable byte[] mandatoryInputsDigest) { + AllInputs(NestedSet<Artifact> defaultInputs) { this.defaultInputs = checkNotNull(defaultInputs); this.actionCacheInputs = null; - this.mandatoryInputsDigest = mandatoryInputsDigest; } - AllInputs( - NestedSet<Artifact> defaultInputs, - List<Artifact> actionCacheInputs, - @Nullable byte[] mandatoryInputsDigest) { + AllInputs(NestedSet<Artifact> defaultInputs, List<Artifact> actionCacheInputs) { this.defaultInputs = checkNotNull(defaultInputs); this.actionCacheInputs = checkNotNull(actionCacheInputs); - this.mandatoryInputsDigest = mandatoryInputsDigest; } /** Compute the inputs to request from Skyframe. */ @@ -798,7 +712,6 @@ pathResolver, actionStartTime, state.allInputs.actionCacheInputs, - state.allInputs.mandatoryInputsDigest, clientEnv); } @@ -902,12 +815,7 @@ } checkState(!env.valuesMissing(), action); skyframeActionExecutor.updateActionCache( - action, - inputMetadataProvider, - outputMetadataStore, - state.token, - state.allInputs.mandatoryInputsDigest, - clientEnv); + action, inputMetadataProvider, outputMetadataStore, state.token, clientEnv); } } @@ -1050,11 +958,30 @@ throws ActionExecutionException, InterruptedException, UndoneInputsException { Predicate<Artifact> isMandatoryInput = makeMandatoryInputPredicate(action); - Supplier<Iterable<Artifact>> allDeps = - () -> Iterables.concat(allInputs.toList(), action.getSchedulingDependencies().toList()); - var actionExecutionFunctionExceptionHandler = - createActionExecutionFunctionExceptionHandler( - action, inputDepsResult, inputDepKeys, allDeps, isMandatoryInput); + ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler = + new ActionExecutionFunctionExceptionHandler( + Suppliers.memoize( + () -> { + ImmutableSet<Artifact> allInputsSet = + ImmutableSet.<Artifact>builder() + .addAll(allInputs.toList()) + .addAll(action.getSchedulingDependencies().toList()) + .build(); + SetMultimap<SkyKey, Artifact> skyKeyToArtifactSet = + MultimapBuilder.hashKeys().hashSetValues().build(); + allInputsSet.forEach( + input -> { + SkyKey key = Artifact.key(input); + if (key != input) { + skyKeyToArtifactSet.put(key, input); + } + }); + return skyKeyToArtifactSet; + }), + inputDepsResult, + action, + isMandatoryInput, + inputDepKeys); boolean hasMissingInputs = actionExecutionFunctionExceptionHandler.accumulateAndMaybeThrowExceptions(); @@ -1136,31 +1063,6 @@ return new CheckInputResults(inputArtifactData); } - private ActionExecutionFunctionExceptionHandler createActionExecutionFunctionExceptionHandler( - Action action, - SkyframeLookupResult inputDepsResult, - ImmutableSet<SkyKey> inputDepKeys, - Supplier<Iterable<Artifact>> allDeps, - Predicate<Artifact> isMandatoryInput) { - return new ActionExecutionFunctionExceptionHandler( - Suppliers.memoize( - () -> { - SetMultimap<SkyKey, Artifact> skyKeyToArtifactSet = - MultimapBuilder.hashKeys().hashSetValues().build(); - for (Artifact input : allDeps.get()) { - SkyKey key = Artifact.key(input); - if (key != input) { - skyKeyToArtifactSet.put(key, input); - } - } - return skyKeyToArtifactSet; - }), - inputDepsResult, - action, - isMandatoryInput, - inputDepKeys); - } - @CanIgnoreReturnValue @Nullable private SkyValue getAndCheckInputSkyValue(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 6975ed4..e44616f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -734,7 +734,6 @@ ArtifactPathResolver artifactPathResolver, long actionStartTime, List<Artifact> resolvedCacheArtifacts, - @Nullable byte[] mandatoryInputsDigest, Map<String, String> clientEnv) throws ActionExecutionException, InterruptedException { Token token; @@ -759,7 +758,6 @@ actionCacheChecker.getTokenIfNeedToExecute( action, resolvedCacheArtifacts, - mandatoryInputsDigest, clientEnv, getOutputPermissions(), handler, @@ -802,7 +800,6 @@ actionCacheChecker.getTokenUnconditionallyAfterFailureToRecordActionCacheHit( action, resolvedCacheArtifacts, - mandatoryInputsDigest, clientEnv, getOutputPermissions(), handler, @@ -841,7 +838,6 @@ InputMetadataProvider inputMetadataProvider, OutputMetadataStore outputMetadataStore, Token token, - @Nullable byte[] mandatoryInputsDigest, Map<String, String> clientEnv) throws ActionExecutionException, InterruptedException { if (!actionCacheChecker.enabled()) { @@ -857,8 +853,7 @@ clientEnv, getOutputPermissions(), actionExecutionSalt, - useArchivedTreeArtifacts(action), - mandatoryInputsDigest); + useArchivedTreeArtifacts(action)); } catch (IOException e) { // Skyframe has already done all the filesystem access needed for outputs and swallows // IOExceptions for inputs. So an IOException is impossible here. @@ -870,10 +865,6 @@ } } - boolean mandatoryInputsMatch(Action action, byte[] mandatoryInputsDigest) { - return actionCacheChecker.mandatoryInputsMatch(action, mandatoryInputsDigest); - } - @Nullable List<Artifact> getActionCachedInputs(Action action, PackageRootResolver resolver) throws AlreadyReportedActionExecutionException, InterruptedException {
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java index c530143..ea78744 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java
@@ -107,7 +107,7 @@ execRoot = scratch.resolve("/output"); cache = new CorruptibleActionCache(cacheRoot, corruptedCacheRoot, tmpDir, clock); - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ false); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ false); digestHashFunction = DigestHashFunction.SHA256; fileSystem = new InMemoryFileSystem(digestHashFunction); artifactRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.OUTPUT, "bin"); @@ -221,7 +221,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, clientEnv, OutputPermissions.READONLY, /* handler= */ null, @@ -296,8 +295,7 @@ clientEnv, OutputPermissions.READONLY, actionExecutionSalt, - useArchivedTreeArtifacts, - /* mandatoryInputsDigest= */ null); + useArchivedTreeArtifacts); } } @@ -330,7 +328,9 @@ cache.corruptAllEntries(); runAction(action); - assertStatistics(0, new MissDetailsBuilder().set(MissReason.CORRUPTED_CACHE_ENTRY, 1).build()); + assertStatistics( + 0, + new MissDetailsBuilder().set(MissReason.CORRUPTED_CACHE_ENTRY, 1).build()); } @Test @@ -489,7 +489,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -559,7 +558,7 @@ @Test public void saveOutputMetadata_remoteFileMetadataSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); Artifact output = createArtifact(artifactRoot, "bin/dummy"); String content = "content"; Action action = new InjectOutputFileMetadataAction(output, createRemoteMetadata(content)); @@ -576,7 +575,7 @@ @Test public void saveOutputMetadata_localFileMetadataNotSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); Artifact output = createArtifact(artifactRoot, "bin/dummy"); Action action = new WriteEmptyOutputAction(output); output.getPath().delete(); @@ -592,7 +591,7 @@ @Test public void saveOutputMetadata_remoteMetadataInjectedAndLocalFilesStored() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); Artifact output = createArtifact(artifactRoot, "bin/dummy"); Action action = new WriteEmptyOutputAction(output) { @@ -632,7 +631,7 @@ @Test public void saveOutputMetadata_remoteFileMetadataLoaded() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); Artifact output = createArtifact(artifactRoot, "bin/dummy"); String content = "content"; Action action = new InjectOutputFileMetadataAction(output, createRemoteMetadata(content)); @@ -643,7 +642,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -678,7 +676,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -708,7 +705,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -727,7 +723,7 @@ @Test public void saveOutputMetadata_localMetadataIsSameAsRemoteMetadata_cached( @TestParameter boolean hasResolvedPath) throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); Artifact output = createArtifact(artifactRoot, "bin/dummy"); String content = "content"; PathFragment resolvedPath = @@ -750,7 +746,7 @@ @Test public void saveOutputMetadata_localMetadataIsDifferentFromRemoteMetadata_notCached() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); Artifact output = createArtifact(artifactRoot, "bin/dummy"); String content1 = "content1"; String content2 = "content2"; @@ -770,7 +766,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -862,7 +857,7 @@ @Test public void saveOutputMetadata_treeMetadata_remoteFileMetadataSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); ImmutableMap<String, FileArtifactValue> children = @@ -894,7 +889,7 @@ @Test public void saveOutputMetadata_treeMetadata_remoteArchivedArtifactSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); Action action = @@ -922,7 +917,7 @@ @Test public void saveOutputMetadata_treeMetadata_resolvedPathSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); Action action = @@ -967,7 +962,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -992,7 +986,7 @@ @Test public void saveOutputMetadata_treeMetadata_localFileMetadataNotSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); writeIsoLatin1(fileSystem.getPath("/file2"), ""); @@ -1026,7 +1020,7 @@ @Test public void saveOutputMetadata_treeMetadata_localArchivedArtifactNotSaved() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); writeIsoLatin1(fileSystem.getPath("/archive"), ""); @@ -1051,7 +1045,7 @@ @Test public void saveOutputMetadata_treeMetadata_remoteFileMetadataLoaded() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); ImmutableMap<String, FileArtifactValue> children = @@ -1073,7 +1067,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1100,7 +1093,7 @@ @Test public void saveOutputMetadata_treeMetadata_localFileMetadataLoaded() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); ImmutableMap<String, FileArtifactValue> children1 = @@ -1134,7 +1127,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1180,7 +1172,7 @@ @Test public void saveOutputMetadata_treeMetadata_localArchivedArtifactLoaded() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); Action action = @@ -1208,7 +1200,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1274,7 +1265,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1319,7 +1309,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1372,7 +1361,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1396,7 +1384,7 @@ @Test public void saveOutputMetadata_treeMetadataWithSameLocalFileMetadata_cached() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); ImmutableMap<String, FileArtifactValue> children = @@ -1417,7 +1405,6 @@ cacheChecker.getTokenIfNeedToExecute( action, /* resolvedCacheArtifacts= */ null, - /* mandatoryInputsDigest= */ null, /* clientEnv= */ ImmutableMap.of(), OutputPermissions.READONLY, /* handler= */ null, @@ -1453,7 +1440,7 @@ @Test public void saveOutputMetadata_treeMetadataWithSameLocalArchivedArtifact_cached() throws Exception { - cacheChecker = createActionCacheChecker(/* storeOutputMetadata= */ true); + cacheChecker = createActionCacheChecker(/*storeOutputMetadata=*/ true); SpecialArtifact output = createTreeArtifactWithGeneratingAction(artifactRoot, PathFragment.create("bin/dummy")); Action action =
diff --git a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java index edf4677..8d2a82d 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java
@@ -37,7 +37,6 @@ import com.google.devtools.build.lib.skyframe.TreeArtifactValue; import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.testutil.Scratch; -import com.google.devtools.build.lib.vfs.DigestUtils; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.OutputPermissions; import com.google.devtools.build.lib.vfs.Path; @@ -79,7 +78,7 @@ private final EventHandler eventHandler = spy(EventHandler.class); @Before - public final void createFiles() throws Exception { + public final void createFiles() throws Exception { execRoot = scratch.resolve("/output"); cacheRoot = scratch.resolve("/cache_root"); corruptedCacheRoot = scratch.resolve("/corrupted_cache_root"); @@ -232,7 +231,7 @@ // Remove entries that discover inputs and flush the journal. cache.removeIf(Entry::discoversInputs); - assertIncrementalSave(cache); + assertFullSave(); // Check that the entries that discover inputs are gone, and the rest are still there. for (int i = 0; i < 100; i++) { @@ -771,7 +770,6 @@ /* clientEnv= */ ImmutableMap.of(), /* actionExecutionSalt= */ "", OutputPermissions.READONLY, - /* useArchivedTreeArtifacts= */ false, - discoversInputs ? new byte[DigestUtils.ESTIMATED_SIZE] : null); + /* useArchivedTreeArtifacts= */ false); } }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index 63e2db3..280d9ea 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -1633,7 +1633,6 @@ "//src/main/java/com/google/devtools/build/lib/skyframe:action_output_metadata_store", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value", - "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/skyframe",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java index 8807233..0e9a918 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java
@@ -41,7 +41,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.NullEventHandler; -import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileStatusWithDigestAdapter; import com.google.devtools.build.lib.vfs.Path; @@ -112,10 +111,10 @@ public void testEmptyTreeArtifacts() throws Exception { TreeArtifactValue value = doTestTreeArtifacts(ImmutableList.of()); // Additional test, only for this test method: we expect the FileArtifactValue is equal to - // the hash of the empty byte array. + // the digest [0] assertThat(value.getMetadata().getDigest()).isEqualTo(value.getDigest()); // Java zero-fills arrays. - assertThat(value.getDigest()).isEqualTo(new Fingerprint().digestAndReset()); + assertThat(value.getDigest()).isEqualTo(new byte[1]); } @Test
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 3a067c7..d1a7723 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD
@@ -1030,7 +1030,6 @@ shard_count = 10, tags = [ "no_windows", - "requires-network", ], )
diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh index 4d1fa2f..00a722a 100755 --- a/src/test/shell/bazel/cc_integration_test.sh +++ b/src/test/shell/bazel/cc_integration_test.sh
@@ -2210,14 +2210,14 @@ function test_cpp20_modules_with_clang() { type -P clang || return 0 # Check if clang version is less than 17 - local -r clang_version=$(clang --version | head -n1 | grep -oE '[0-9]+\.[0-9]+' | head -n1) + clang_version=$(clang --version | head -n1 | grep -oE '[0-9]+\.[0-9]+' | head -n1) if [[ -n "$clang_version" ]]; then - local -r major_version=$(echo "$clang_version" | cut -d. -f1) + major_version=$(echo "$clang_version" | cut -d. -f1) if [[ "$major_version" -lt 17 ]]; then return 0 fi fi - if is_darwin; then + if [[ "$(uname -s)" == "Darwin" ]]; then return 0 fi @@ -2285,7 +2285,6 @@ } EOF - # TODO: Make it so that --cxxopt applies to module_interfaces as well. bazel build //:main --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 --disk_cache=disk &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" # Verify that the build can hit the cache without action cycles. @@ -2294,152 +2293,6 @@ expect_log "17 disk cache hit" } -function test_cpp20_modules_change_ab_to_ba_no_cycle() { - type -P clang || return 0 - # Check if clang version is less than 17 - local -r clang_version=$(clang --version | head -n1 | grep -oE '[0-9]+\.[0-9]+' | head -n1) - if [[ -n "$clang_version" ]]; then - local -r major_version=$(echo "$clang_version" | cut -d. -f1) - if [[ "$major_version" -lt 17 ]]; then - return 0 - fi - fi - - if is_darwin; then - return 0 - fi - - add_rules_cc "MODULE.bazel" - # TODO: Drop this after the next rules_cc release. - cat >> MODULE.bazel <<'EOF' -single_version_override( - module_name = "rules_cc", - patches = ["//:rules_cc.patch"], -) -EOF - cat > rules_cc.patch <<'EOF' ---- cc/private/compile/compile.bzl -+++ cc/private/compile/compile.bzl -@@ -244,9 +244,6 @@ def compile( - - if module_interfaces and not feature_configuration.is_enabled("cpp_modules"): - fail("to use C++20 Modules, the feature cpp_modules must be enabled") -- if module_interfaces and len(module_interfaces) > 1: -- fail("module_interfaces must be a list of files with exactly one file " + -- "due to implementation limitation. see https://github.com/bazelbuild/bazel/pull/22553") - - language_normalized = "c++" if language == None else language - language_normalized = language_normalized.replace("+", "p").upper() -EOF - - cat > BUILD.bazel <<'EOF' -load("@rules_cc//cc:defs.bzl", "cc_library", "cc_binary") - -package(features = ["cpp_modules"]) - -cc_library( - name = "lib", - module_interfaces = ["a.cppm", "b.cppm"], -) -EOF - cat > a.cppm <<'EOF' -export module a; -import b; -EOF - cat > b.cppm <<'EOF' -export module b; -EOF - - bazel build //:lib --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" - - cat > a.cppm <<'EOF' -export module a; -EOF - cat > b.cppm <<'EOF' -export module b; -import a; -EOF - - bazel build //:lib --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" -} - -function test_cpp20_modules_change_abc_to_acb_no_cycle() { - type -P clang || return 0 - # Check if clang version is less than 17 - local -r clang_version=$(clang --version | head -n1 | grep -oE '[0-9]+\.[0-9]+' | head -n1) - if [[ -n "$clang_version" ]]; then - local -r major_version=$(echo "$clang_version" | cut -d. -f1) - if [[ "$major_version" -lt 17 ]]; then - return 0 - fi - fi - - if is_darwin; then - return 0 - fi - - add_rules_cc "MODULE.bazel" - # TODO: Drop this after the next rules_cc release. - cat >> MODULE.bazel <<'EOF' -single_version_override( - module_name = "rules_cc", - patches = ["//:rules_cc.patch"], -) -EOF - cat > rules_cc.patch <<'EOF' ---- cc/private/compile/compile.bzl -+++ cc/private/compile/compile.bzl -@@ -244,9 +244,6 @@ def compile( - - if module_interfaces and not feature_configuration.is_enabled("cpp_modules"): - fail("to use C++20 Modules, the feature cpp_modules must be enabled") -- if module_interfaces and len(module_interfaces) > 1: -- fail("module_interfaces must be a list of files with exactly one file " + -- "due to implementation limitation. see https://github.com/bazelbuild/bazel/pull/22553") - - language_normalized = "c++" if language == None else language - language_normalized = language_normalized.replace("+", "p").upper() -EOF - - cat > BUILD.bazel <<'EOF' -load("@rules_cc//cc:defs.bzl", "cc_library", "cc_binary") - -package(features = ["cpp_modules"]) - -cc_library( - name = "lib", - module_interfaces = ["a.cppm", "b.cppm", "c.cppm"], -) -EOF - cat > a.cppm <<'EOF' -export module a; -import b; -EOF - cat > b.cppm <<'EOF' -export module b; -import c; -EOF - cat > c.cppm <<'EOF' -export module c; -EOF - - bazel build //:lib --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" - - cat > a.cppm <<'EOF' -export module a; -import c; -EOF - cat > b.cppm <<'EOF' -export module b; -EOF - cat > c.cppm <<'EOF' -export module c; -import b; -EOF - - bazel build //:lib --experimental_cpp_modules --repo_env=CC=clang --copt=-std=c++20 &> $TEST_log || fail "Expected build C++20 Modules success with compiler 'clang'" -} - function test_external_repo_lto() { add_rules_cc "MODULE.bazel" REPO_PATH=$TEST_TMPDIR/repo