Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explain why vendored repositories are considered out-of-date #24857

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Explain why vendored repositories are considered out-of-date #24857

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9018577
Explain why vendored repositories are considered out-of-date
fmeum Jan 8, 2025
ad54539
Change message
fmeum Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,12 @@ private static boolean didRecordedInputsChange(
BlazeDirectories directories,
Map<? extends RepoRecordedInput, String> recordedInputs)
throws InterruptedException, NeedsSkyframeRestartException {
boolean upToDate = RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedInputs);
Optional<String> outdated =
RepoRecordedInput.isAnyValueOutdated(env, directories, recordedInputs);
if (env.valuesMissing()) {
throw new NeedsSkyframeRestartException();
}
return !upToDate;
return outdated.isPresent();
}

/**
Expand Down
192 changes: 117 additions & 75 deletions src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<RepoRecordedInput> {
public abstract sealed class RepoRecordedInput implements Comparable<RepoRecordedInput> {
/** Represents a parser for a specific type of recorded inputs. */
public abstract static class Parser {
/**
Expand Down Expand Up @@ -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<String> parts = Splitter.on(':').limit(2).splitToList(s);
if (parts.size() < 2) {
return NEVER_UP_TO_DATE;
return PARSE_FAILURE;
}
for (Parser parser :
new Parser[] {
Expand All @@ -112,15 +112,15 @@ public static RepoRecordedInput parse(String s) {
return parser.parse(parts.get(1));
}
}
return NEVER_UP_TO_DATE;
return PARSE_FAILURE;
}

/**
* Returns whether all values are still up-to-date for each recorded input. If Skyframe values are
* 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<String> isAnyValueOutdated(
Environment env,
BlazeDirectories directories,
Map<? extends RepoRecordedInput, String> recordedInputValues)
Expand All @@ -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<? extends RepoRecordedInput, String> recordedInputValue :
recordedInputValues.entrySet()) {
if (!recordedInputValue
.getKey()
.isUpToDate(env, directories, recordedInputValue.getValue())) {
return false;
Optional<String> reason =
recordedInputValue.getKey().isOutdated(env, directories, recordedInputValue.getValue());
if (reason.isPresent()) {
return reason;
}
}
return true;
return Optional.empty();
}

@Override
Expand Down Expand Up @@ -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<String> 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<String> 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
Expand Down Expand Up @@ -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;
}
}
};
Expand Down Expand Up @@ -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<String> 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()));
}
}
}
Expand All @@ -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;
}
}
};
Expand Down Expand Up @@ -453,18 +427,21 @@ public SkyKey getSkyKey(BlazeDirectories directories) {
}

@Override
public boolean isUpToDate(
public Optional<String> 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()));
}
}

Expand Down Expand Up @@ -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;
}
}
};
Expand Down Expand Up @@ -541,15 +518,18 @@ public SkyKey getSkyKey(BlazeDirectories directories) {
}

@Override
public boolean isUpToDate(
public Optional<String> 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();
}
}

Expand Down Expand Up @@ -612,15 +592,23 @@ public SkyKey getSkyKey(BlazeDirectories directories) {
}

@Override
public boolean isUpToDate(
public Optional<String> isOutdated(
Environment env, BlazeDirectories directories, @Nullable String oldValue)
throws InterruptedException {
String v = PrecomputedValue.REPO_ENV.get(env).get(name);
if (v == null) {
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();
}
}

Expand All @@ -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;
}
}
};
Expand Down Expand Up @@ -691,19 +679,73 @@ public SkyKey getSkyKey(BlazeDirectories directories) {
}

@Override
public boolean isUpToDate(
public Optional<String> 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 ? "<doesn't exist>" : 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<String> isOutdated(
Environment env, BlazeDirectories directories, @Nullable String oldValue) {
return Optional.of(reason);
}
}
}
Loading