From 90185775ce180b958f340b41f49d8c2c08a6417e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 8 Jan 2025 13:14:16 +0100 Subject: [PATCH 1/2] Explain why vendored repositories are considered out-of-date --- .../bzlmod/SingleExtensionEvalFunction.java | 5 +- .../rules/repository/RepoRecordedInput.java | 192 +++++++++++------- .../RepositoryDelegatorFunction.java | 74 ++++--- .../rules/repository/RepositoryFunction.java | 15 +- src/test/py/bazel/bzlmod/bazel_vendor_test.py | 26 +-- 5 files changed, 176 insertions(+), 136 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 68c45e3dd773f3..6e350f8afe26ae 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -425,11 +425,12 @@ private static boolean didRecordedInputsChange( BlazeDirectories directories, Map recordedInputs) throws InterruptedException, NeedsSkyframeRestartException { - boolean upToDate = RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedInputs); + Optional outdated = + RepoRecordedInput.isAnyValueOutdated(env, directories, recordedInputs); if (env.valuesMissing()) { throw new NeedsSkyframeRestartException(); } - return !upToDate; + return outdated.isPresent(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java index 32c7adef0d3476..84608ca3d91c23 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java @@ -67,7 +67,7 @@ * input is stored as a string, with a prefix denoting its type, followed by a colon, and then the * information identifying that specific input. */ -public abstract class RepoRecordedInput implements Comparable { +public abstract sealed class RepoRecordedInput implements Comparable { /** Represents a parser for a specific type of recorded inputs. */ public abstract static class Parser { /** @@ -96,13 +96,13 @@ public abstract static class Parser { * Parses a recorded input from its string representation. * * @param s the string representation - * @return The parsed recorded input object, or {@link #NEVER_UP_TO_DATE} if the string + * @return The parsed recorded input object, or {@link #PARSE_FAILURE} if the string * representation is invalid */ public static RepoRecordedInput parse(String s) { List parts = Splitter.on(':').limit(2).splitToList(s); if (parts.size() < 2) { - return NEVER_UP_TO_DATE; + return PARSE_FAILURE; } for (Parser parser : new Parser[] { @@ -112,7 +112,7 @@ public static RepoRecordedInput parse(String s) { return parser.parse(parts.get(1)); } } - return NEVER_UP_TO_DATE; + return PARSE_FAILURE; } /** @@ -120,7 +120,7 @@ public static RepoRecordedInput parse(String s) { * missing, the return value should be ignored; callers are responsible for checking {@code * env.valuesMissing()} and triggering a Skyframe restart if needed. */ - public static boolean areAllValuesUpToDate( + public static Optional isAnyValueOutdated( Environment env, BlazeDirectories directories, Map recordedInputValues) @@ -130,17 +130,17 @@ public static boolean areAllValuesUpToDate( .map(rri -> rri.getSkyKey(directories)) .collect(toImmutableSet())); if (env.valuesMissing()) { - return false; + return UNDECIDED; } for (Map.Entry recordedInputValue : recordedInputValues.entrySet()) { - if (!recordedInputValue - .getKey() - .isUpToDate(env, directories, recordedInputValue.getValue())) { - return false; + Optional reason = + recordedInputValue.getKey().isOutdated(env, directories, recordedInputValue.getValue()); + if (reason.isPresent()) { + return reason; } } - return true; + return Optional.empty(); } @Override @@ -168,54 +168,26 @@ public int compareTo(RepoRecordedInput o) { /** Returns the parser object for this type of recorded inputs. */ public abstract Parser getParser(); - /** Returns the {@link SkyKey} that is necessary to determine {@link #isUpToDate}. */ + /** Returns the {@link SkyKey} that is necessary to determine {@link #isOutdated}. */ public abstract SkyKey getSkyKey(BlazeDirectories directories); /** - * Returns whether the given {@code oldValue} is still up-to-date for this recorded input. This - * method can assume that {@link #getSkyKey(BlazeDirectories)} is already evaluated; it can - * request further Skyframe evaluations, and if any values are missing, this method can return any - * value (doesn't matter what) and will be reinvoked after a Skyframe restart. + * Returns a human-readable reason for why the given {@code oldValue} is no longer up-to-date for + * this recorded input, or an empty Optional if it is still up-to-date. This method can assume + * that {@link #getSkyKey(BlazeDirectories)} is already evaluated; it can request further Skyframe + * evaluations, and if any values are missing, this method can return any value (doesn't matter + * what, although {@link #UNDECIDED} is recommended for clarity) and will be reinvoked after a + * Skyframe restart. */ - public abstract boolean isUpToDate( + public abstract Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException; - /** A sentinel "input" that's always out-of-date to signify parse failure. */ - public static final RepoRecordedInput NEVER_UP_TO_DATE = - new RepoRecordedInput() { - @Override - public boolean equals(Object obj) { - return this == obj; - } - - @Override - public int hashCode() { - return 12345678; - } - - @Override - public String toStringInternal() { - throw new UnsupportedOperationException("this sentinel input should never be serialized"); - } + private static final Optional UNDECIDED = Optional.of("values missing"); - @Override - public Parser getParser() { - throw new UnsupportedOperationException("this sentinel input should never be parsed"); - } - - @Override - public SkyKey getSkyKey(BlazeDirectories directories) { - // Return a random SkyKey to satisfy the contract. - return PrecomputedValue.STARLARK_SEMANTICS.getKey(); - } - - @Override - public boolean isUpToDate( - Environment env, BlazeDirectories directories, @Nullable String oldValue) { - return false; - } - }; + /** A sentinel "input" that's always out-of-date to signify parse failure. */ + public static final RepoRecordedInput PARSE_FAILURE = + new NeverUpToDateRepoRecordedInput("malformed marker file entry encountered"); /** * Represents a filesystem path stored in a way that is repo-cache-friendly. That is, if the path @@ -313,7 +285,7 @@ public RepoRecordedInput parse(String s) { return new File(RepoCacheFriendlyPath.parse(s)); } catch (LabelSyntaxException e) { // malformed inputs cause refetch - return NEVER_UP_TO_DATE; + return PARSE_FAILURE; } } }; @@ -373,24 +345,26 @@ public static String fileValueToMarkerValue(RootedPath rootedPath, FileValue fil } @Override - @Nullable public SkyKey getSkyKey(BlazeDirectories directories) { return FileValue.key(path.getRootedPath(directories)); } @Override - public boolean isUpToDate( + public Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { var skyKey = getSkyKey(directories); try { FileValue fileValue = (FileValue) env.getValueOrThrow(skyKey, IOException.class); if (fileValue == null) { - return false; + return UNDECIDED; } - return oldValue.equals(fileValueToMarkerValue((RootedPath) skyKey.argument(), fileValue)); + if (!oldValue.equals(fileValueToMarkerValue((RootedPath) skyKey.argument(), fileValue))) { + return Optional.of("file info or contents of %s changed".formatted(path)); + } + return Optional.empty(); } catch (IOException e) { - return false; + return Optional.of("failed to stat %s: %s".formatted(path, e.getMessage())); } } } @@ -410,7 +384,7 @@ public RepoRecordedInput parse(String s) { return new Dirents(RepoCacheFriendlyPath.parse(s)); } catch (LabelSyntaxException e) { // malformed inputs cause refetch - return NEVER_UP_TO_DATE; + return PARSE_FAILURE; } } }; @@ -453,18 +427,21 @@ public SkyKey getSkyKey(BlazeDirectories directories) { } @Override - public boolean isUpToDate( + public Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { SkyKey skyKey = getSkyKey(directories); if (env.getValue(skyKey) == null) { - return false; + return UNDECIDED; } try { - return oldValue.equals( - getDirentsMarkerValue(((DirectoryListingKey) skyKey).argument().asPath())); + if (!oldValue.equals( + getDirentsMarkerValue(((DirectoryListingKey) skyKey).argument().asPath()))) { + return Optional.of("directory entries of %s changed".formatted(path)); + } + return Optional.empty(); } catch (IOException e) { - return false; + return Optional.of("failed to readdir %s: %s".formatted(path, e.getMessage())); } } @@ -498,7 +475,7 @@ public RepoRecordedInput parse(String s) { return new DirTree(RepoCacheFriendlyPath.parse(s)); } catch (LabelSyntaxException e) { // malformed inputs cause refetch - return NEVER_UP_TO_DATE; + return PARSE_FAILURE; } } }; @@ -541,15 +518,18 @@ public SkyKey getSkyKey(BlazeDirectories directories) { } @Override - public boolean isUpToDate( + public Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { DirectoryTreeDigestValue value = (DirectoryTreeDigestValue) env.getValue(getSkyKey(directories)); if (value == null) { - return false; + return UNDECIDED; } - return oldValue.equals(value.hexDigest()); + if (!oldValue.equals(value.hexDigest())) { + return Optional.of("directory tree at %s changed".formatted(path)); + } + return Optional.empty(); } } @@ -612,7 +592,7 @@ public SkyKey getSkyKey(BlazeDirectories directories) { } @Override - public boolean isUpToDate( + public Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { String v = PrecomputedValue.REPO_ENV.get(env).get(name); @@ -620,7 +600,15 @@ public boolean isUpToDate( v = ((ClientEnvironmentValue) env.getValue(getSkyKey(directories))).getValue(); } // Note that `oldValue` can be null if the env var was not set. - return Objects.equals(oldValue, v); + if (!Objects.equals(oldValue, v)) { + return Optional.of( + "value of %s changed: %s -> %s" + .formatted( + name, + oldValue == null ? "" : "'%s'".formatted(oldValue), + v == null ? "" : "'%s'".formatted(v))); + } + return Optional.empty(); } } @@ -640,7 +628,7 @@ public RepoRecordedInput parse(String s) { return new RecordedRepoMapping(RepositoryName.create(parts.get(0)), parts.get(1)); } catch (LabelSyntaxException | IndexOutOfBoundsException e) { // malformed inputs cause refetch - return NEVER_UP_TO_DATE; + return PARSE_FAILURE; } } }; @@ -691,19 +679,73 @@ public SkyKey getSkyKey(BlazeDirectories directories) { } @Override - public boolean isUpToDate( + public Optional isOutdated( Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { RepositoryMappingValue repoMappingValue = (RepositoryMappingValue) env.getValue(getSkyKey(directories)); + if (repoMappingValue == RepositoryMappingValue.NOT_FOUND_VALUE) { + return Optional.of("source repo %s doesn't exist anymore".formatted(sourceRepo)); + } + RepositoryName oldCanonicalName; try { - return repoMappingValue != RepositoryMappingValue.NOT_FOUND_VALUE - && RepositoryName.create(oldValue) - .equals(repoMappingValue.repositoryMapping().get(apparentName)); + oldCanonicalName = RepositoryName.create(oldValue); } catch (LabelSyntaxException e) { // malformed old value causes refetch - return false; + return Optional.of("invalid recorded repo name: %s".formatted(e.getMessage())); + } + RepositoryName newCanonicalName = repoMappingValue.repositoryMapping().get(apparentName); + if (!oldCanonicalName.equals(newCanonicalName)) { + return Optional.of( + "canonical name for @%s in %s changed: %s -> %s" + .formatted( + apparentName, + sourceRepo, + oldCanonicalName, + newCanonicalName == null ? "" : newCanonicalName)); } + return Optional.empty(); + } + } + + /** A sentinel "input" that's always out-of-date for a given reason. */ + public static final class NeverUpToDateRepoRecordedInput extends RepoRecordedInput { + private final String reason; + + public NeverUpToDateRepoRecordedInput(String reason) { + this.reason = reason; + } + + @Override + public boolean equals(Object obj) { + return this == obj; + } + + @Override + public int hashCode() { + return 12345678; + } + + @Override + public String toStringInternal() { + throw new UnsupportedOperationException("this sentinel input should never be serialized"); + } + + @Override + public Parser getParser() { + throw new UnsupportedOperationException("this sentinel input should never be parsed"); + } + + @Override + public SkyKey getSkyKey(BlazeDirectories directories) { + // Return a random SkyKey to satisfy the contract. + return PrecomputedValue.STARLARK_SEMANTICS.getKey(); + } + + @Override + public Optional isOutdated( + Environment env, BlazeDirectories directories, @Nullable String oldValue) { + return Optional.of(reason); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index f49e78a70da7b6..651f2189326199 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -204,14 +204,14 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (shouldUseCachedRepos(env, handler, repoRoot, rule)) { // Make sure marker file is up-to-date; correctly describes the current repository state - byte[] markerHash = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env); - if (env.valuesMissing()) { + var repoState = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env); + if (repoState == null) { return null; } - if (markerHash != null) { // repo exist & up-to-date + if (repoState instanceof DigestWriter.RepoDirectoryState.UpToDate(byte[] markerDigest)) { return RepositoryDirectoryValue.builder() .setPath(repoRoot) - .setDigest(markerHash) + .setDigest(markerDigest) .setExcludeFromVendoring(excludeRepoFromVendoring) .build(); } @@ -298,24 +298,25 @@ private RepositoryDirectoryValue tryGettingValueUsingVendoredRepo( return setupOverride(vendorRepoPath.asFragment(), env, repoRoot, repositoryName); } - boolean isVendorRepoUpToDate = - digestWriter.areRepositoryAndMarkerFileConsistent(handler, env, vendorMarker) != null; - if (env.valuesMissing()) { + DigestWriter.RepoDirectoryState vendoredRepoState = + digestWriter.areRepositoryAndMarkerFileConsistent(handler, env, vendorMarker); + if (vendoredRepoState == null) { return null; } // If our repo is up-to-date, or this is an offline build (--nofetch), then the vendored repo // is used. - if (isVendorRepoUpToDate || (!IS_VENDOR_COMMAND.get(env).booleanValue() && !isFetch.get())) { - if (!isVendorRepoUpToDate) { // If the repo is out-of-date, show a warning + if (vendoredRepoState instanceof DigestWriter.RepoDirectoryState.UpToDate + || (!IS_VENDOR_COMMAND.get(env).booleanValue() && !isFetch.get())) { + if (vendoredRepoState instanceof DigestWriter.RepoDirectoryState.OutOfDate(String reason)) { env.getListener() .handle( Event.warn( rule.getLocation(), String.format( - "Vendored repository '%s' is out-of-date and fetching is disabled." + "Vendored repository '%s' is out-of-date (%s) and fetching is disabled." + " Run build without the '--nofetch' option or run" + " the bazel vendor command to update it", - rule.getName()))); + rule.getName(), reason))); } return setupOverride(vendorRepoPath.asFragment(), env, repoRoot, repositoryName); } else if (!IS_VENDOR_COMMAND.get(env).booleanValue()) { // build command & fetch enabled @@ -325,10 +326,11 @@ private RepositoryDirectoryValue tryGettingValueUsingVendoredRepo( Event.warn( rule.getLocation(), String.format( - "Vendored repository '%s' is out-of-date. The up-to-date version will" + "Vendored repository '%s' is out-of-date (%s). The up-to-date version will" + " be fetched into the external cache and used. To update the repo" + " in the vendor directory, run the bazel vendor command", - rule.getName()))); + rule.getName(), + ((DigestWriter.RepoDirectoryState.OutOfDate) vendoredRepoState).reason()))); } } else if (vendorFile.pinnedRepos().contains(repositoryName)) { throw new RepositoryFunctionException( @@ -619,13 +621,12 @@ static String unescape(String str) { } private static class DigestWriter { - // Input value map to force repo invalidation. - private static final ImmutableMap NOT_UP_TO_DATE = - ImmutableMap.of(RepoRecordedInput.NEVER_UP_TO_DATE, ""); + // Input value map to force repo invalidation upon an invalid marker file. + private static final ImmutableMap PARSE_FAILURE = + ImmutableMap.of(RepoRecordedInput.PARSE_FAILURE, ""); private final BlazeDirectories directories; private final Path markerPath; - private final Rule rule; private final String ruleKey; DigestWriter( @@ -636,7 +637,6 @@ private static class DigestWriter { this.directories = directories; ruleKey = computeRuleKey(rule, starlarkSemantics); markerPath = getMarkerPath(directories, repositoryName); - this.rule = rule; } byte[] writeMarkerFile(Map recordedInputValues) @@ -658,8 +658,14 @@ byte[] writeMarkerFile(Map recordedInputVal return new Fingerprint().addString(content).digestAndReset(); } - @Nullable - byte[] areRepositoryAndMarkerFileConsistent(RepositoryFunction handler, Environment env) + sealed interface RepoDirectoryState { + record UpToDate(byte[] markerDigest) implements RepoDirectoryState {} + + record OutOfDate(String reason) implements RepoDirectoryState {} + } + + RepoDirectoryState areRepositoryAndMarkerFileConsistent( + RepositoryFunction handler, Environment env) throws InterruptedException, RepositoryFunctionException { return areRepositoryAndMarkerFileConsistent(handler, env, markerPath); } @@ -668,38 +674,39 @@ byte[] areRepositoryAndMarkerFileConsistent(RepositoryFunction handler, Environm * Checks if the state of the repository in the file system is consistent with the rule in the * WORKSPACE file. * - *

Returns null if the file system is not up to date and a hash of the marker file if the - * file system is up to date. + *

Returns null if a Skyframe status is needed. * *

We check the repository root for existence here, but we can't depend on the FileValue, * because it's possible that we eventually create that directory in which case the FileValue * and the state of the file system would be inconsistent. */ @Nullable - byte[] areRepositoryAndMarkerFileConsistent( + RepoDirectoryState areRepositoryAndMarkerFileConsistent( RepositoryFunction handler, Environment env, Path markerPath) throws RepositoryFunctionException, InterruptedException { if (!markerPath.exists()) { - return null; + return new RepoDirectoryState.OutOfDate("repo hasn't been fetched yet"); } try { String content = FileSystemUtils.readContent(markerPath, UTF_8); Map recordedInputValues = readMarkerFile(content, Preconditions.checkNotNull(ruleKey)); - if (!handler.verifyRecordedInputs(rule, directories, recordedInputValues, env)) { - return null; - } + Optional outdatedReason = + handler.isAnyRecordedInputOutdated(directories, recordedInputValues, env); if (env.valuesMissing()) { return null; } - return new Fingerprint().addString(content).digestAndReset(); + if (outdatedReason.isPresent()) { + return new RepoDirectoryState.OutOfDate(outdatedReason.get()); + } + return new RepoDirectoryState.UpToDate( + new Fingerprint().addString(content).digestAndReset()); } catch (IOException e) { throw new RepositoryFunctionException(e, Transience.TRANSIENT); } } - @Nullable private static Map readMarkerFile( String content, String expectedRuleKey) { Iterable lines = Splitter.on('\n').split(content); @@ -714,7 +721,8 @@ private static Map readMarkerFile( if (!line.equals(expectedRuleKey)) { // Break early, need to reload anyway. This also detects marker file version changes // so that unknown formats are not parsed. - return NOT_UP_TO_DATE; + return ImmutableMap.of( + new RepoRecordedInput.NeverUpToDateRepoRecordedInput("repo rule changed"), ""); } firstLineVerified = true; recordedInputValues = new TreeMap<>(); @@ -722,17 +730,17 @@ private static Map readMarkerFile( int sChar = line.indexOf(' '); if (sChar > 0) { RepoRecordedInput input = RepoRecordedInput.parse(unescape(line.substring(0, sChar))); - if (!input.equals(RepoRecordedInput.NEVER_UP_TO_DATE)) { + if (!input.equals(RepoRecordedInput.PARSE_FAILURE)) { recordedInputValues.put(input, unescape(line.substring(sChar + 1))); continue; } } // On parse failure, just forget everything else and mark the whole input out of date. - return NOT_UP_TO_DATE; + return PARSE_FAILURE; } } if (!firstLineVerified) { - return NOT_UP_TO_DATE; + return PARSE_FAILURE; } return Preconditions.checkNotNull(recordedInputValues); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index c9bdd58b2f480f..eb9a29a012c346 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -185,8 +185,8 @@ public abstract FetchResult fetch( * fields set. * @param recordedInputValues Any recorded inputs (and their values) encountered during the fetch * of the repo. Changes to these inputs will result in the repo being refetched in the future. - * The {@link #verifyRecordedInputs} method is responsible for checking the value added to - * that map when checking the content of a marker file. Not an ImmutableMap, because + * The {@link #isAnyRecordedInputOutdated} method is responsible for checking the value added + * to that map when checking the content of a marker file. Not an ImmutableMap, because * regrettably the values can be null sometimes. */ public record FetchResult( @@ -215,17 +215,16 @@ protected static void ensureNativeRepoRuleEnabled(Rule rule, Environment env, St } /** - * Verify the data provided by the marker file to check if a refetch is needed. Returns true if - * the data is up to date and no refetch is needed and false if the data is obsolete and a refetch - * is needed. + * Verify the data provided by the marker file to check if a refetch is needed. Returns an empty + * Optional if the data is up to date and no refetch is needed and an Optional with a + * human-readable reason if the data is obsolete and a refetch is needed. */ - public boolean verifyRecordedInputs( - Rule rule, + public Optional isAnyRecordedInputOutdated( BlazeDirectories directories, Map recordedInputValues, Environment env) throws InterruptedException { - return RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedInputValues); + return RepoRecordedInput.isAnyValueOutdated(env, directories, recordedInputValues); } public static RootedPath getRootedPathFromLabel(Label label, Environment env) diff --git a/src/test/py/bazel/bzlmod/bazel_vendor_test.py b/src/test/py/bazel/bzlmod/bazel_vendor_test.py index f236571b9a8e68..b0903c181b3016 100644 --- a/src/test/py/bazel/bzlmod/bazel_vendor_test.py +++ b/src/test/py/bazel/bzlmod/bazel_vendor_test.py @@ -427,7 +427,7 @@ def testBuildingWithPinnedRepo(self): stderr, ) self.assertIn( - "Vendored repository '+ext+venRepo' is out-of-date.", + "Vendored repository '+ext+venRepo' is out-of-date (repo hasn't been fetched yet).", '\n'.join(stderr), ) @@ -437,7 +437,7 @@ def testBuildingWithPinnedRepo(self): ['build', '@venRepo//:all', '--vendor_dir=vendor'], ) self.assertNotIn( - "Vendored repository '+ext+venRepo' is out-of-date.", + "Vendored repository '+ext+venRepo' is out-of-date", '\n'.join(stderr), ) @@ -470,13 +470,8 @@ def testBuildingOutOfDateVendoredRepo(self): _, _, stderr = self.RunBazel( ['build', '@justRepo//:all', '--vendor_dir=vendor'] ) - self.assertNotIn( - "WARNING: : Vendored repository '+ext+justRepo' is" - ' out-of-date. The up-to-date version will be fetched into the external' - ' cache and used. To update the repo in the vendor directory, run' - ' the bazel vendor command', - stderr, - ) + stderr = '\n'.join(stderr) + self.assertNotIn("out-of-date", stderr) # Make updates in repo definition self.ScratchFile( @@ -502,7 +497,7 @@ def testBuildingOutOfDateVendoredRepo(self): # external and not a symlink self.assertIn( "WARNING: : Vendored repository '+ext+justRepo' is" - ' out-of-date. The up-to-date version will be fetched into the external' + ' out-of-date (repo rule changed). The up-to-date version will be fetched into the external' ' cache and used. To update the repo in the vendor directory, run' ' the bazel vendor command', stderr, @@ -516,13 +511,8 @@ def testBuildingOutOfDateVendoredRepo(self): _, _, stderr = self.RunBazel( ['build', '@justRepo//:all', '--vendor_dir=vendor'] ) - self.assertNotIn( - "WARNING: : Vendored repository '+ext+justRepo' is" - ' out-of-date. The up-to-date version will be fetched into the external' - ' cache and used. To update the repo in the vendor directory, run' - ' the bazel vendor command', - stderr, - ) + stderr = '\n'.join(stderr) + self.assertNotIn("out-of-date", stderr) def testBuildingVendoredRepoWithNoFetch(self): self.ScratchFile( @@ -594,7 +584,7 @@ def testBuildingVendoredRepoWithNoFetch(self): ) self.assertIn( "WARNING: : Vendored repository '+ext+venRepo' is" - ' out-of-date and fetching is disabled. Run build without the' + ' out-of-date (repo rule changed) and fetching is disabled. Run build without the' " '--nofetch' option or run the bazel vendor command to update it", stderr, ) From ad54539d1dea0046612937a7fdd9722c723922a8 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 8 Jan 2025 16:38:23 +0100 Subject: [PATCH 2/2] Change message --- .../lib/rules/repository/RepositoryDelegatorFunction.java | 4 +++- src/test/py/bazel/bzlmod/bazel_vendor_test.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index 651f2189326199..a62793b8559204 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -722,7 +722,9 @@ private static Map readMarkerFile( // Break early, need to reload anyway. This also detects marker file version changes // so that unknown formats are not parsed. return ImmutableMap.of( - new RepoRecordedInput.NeverUpToDateRepoRecordedInput("repo rule changed"), ""); + new RepoRecordedInput.NeverUpToDateRepoRecordedInput( + "Bazel version, flags, repo rule definition or attributes changed"), + ""); } firstLineVerified = true; recordedInputValues = new TreeMap<>(); diff --git a/src/test/py/bazel/bzlmod/bazel_vendor_test.py b/src/test/py/bazel/bzlmod/bazel_vendor_test.py index b0903c181b3016..adeb807659bb2d 100644 --- a/src/test/py/bazel/bzlmod/bazel_vendor_test.py +++ b/src/test/py/bazel/bzlmod/bazel_vendor_test.py @@ -497,7 +497,7 @@ def testBuildingOutOfDateVendoredRepo(self): # external and not a symlink self.assertIn( "WARNING: : Vendored repository '+ext+justRepo' is" - ' out-of-date (repo rule changed). The up-to-date version will be fetched into the external' + ' out-of-date (Bazel version, flags, repo rule definition or attributes changed). The up-to-date version will be fetched into the external' ' cache and used. To update the repo in the vendor directory, run' ' the bazel vendor command', stderr, @@ -584,7 +584,7 @@ def testBuildingVendoredRepoWithNoFetch(self): ) self.assertIn( "WARNING: : Vendored repository '+ext+venRepo' is" - ' out-of-date (repo rule changed) and fetching is disabled. Run build without the' + ' out-of-date (Bazel version, flags, repo rule definition or attributes changed) and fetching is disabled. Run build without the' " '--nofetch' option or run the bazel vendor command to update it", stderr, )