Skip to content

Commit 6d8f8f9

Browse files
committed
Apply single_version_override patches to module files
Work towards #19301 RELNOTES: Patches to the module file in `single_version_override` are now effective as long as the patch file lies in the root module. Closes #23536. PiperOrigin-RevId: 678347809 Change-Id: I6a3c4664e55cff90c8e42795f95d8ef211348ca9
1 parent e43c933 commit 6d8f8f9

File tree

7 files changed

+309
-12
lines changed

7 files changed

+309
-12
lines changed

BUILD

+8
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,14 @@ genrule(
103103
"MODULE.bazel",
104104
"//third_party/googleapis:MODULE.bazel",
105105
"//third_party/remoteapis:MODULE.bazel",
106+
"//third_party:BUILD",
107+
"//third_party:rules_jvm_external_6.0.patch",
108+
"//third_party:protobuf_21.7.patch",
109+
"//third_party/upb:BUILD",
110+
"//third_party/upb:00_remove_toolchain_transition.patch",
111+
"//third_party/upb:01_remove_werror.patch",
112+
"//third_party/grpc:BUILD",
113+
"//third_party/grpc:00_disable_layering_check.patch",
106114
],
107115
outs = ["MODULE.bazel.lock.dist"],
108116
cmd = "touch BUILD && " +

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

+3
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ java_library(
241241
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
242242
"//src/main/java/com/google/devtools/build/lib/bazel:bazel_version",
243243
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value",
244+
"//src/main/java/com/google/devtools/build/lib/bazel/repository",
244245
"//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
245246
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
246247
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
@@ -260,6 +261,7 @@ java_library(
260261
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
261262
"//src/main/java/com/google/devtools/build/lib/vfs",
262263
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
264+
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
263265
"//src/main/java/com/google/devtools/build/skyframe",
264266
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
265267
"//src/main/java/net/starlark/java/annot",
@@ -270,6 +272,7 @@ java_library(
270272
"//third_party:auto_value",
271273
"//third_party:gson",
272274
"//third_party:guava",
275+
"//third_party:java-diff-utils",
273276
"//third_party:jsr305",
274277
],
275278
)

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java

+120-3
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@
2020
import static com.google.common.collect.ImmutableSet.toImmutableSet;
2121
import static java.nio.charset.StandardCharsets.UTF_8;
2222

23+
import com.github.difflib.patch.PatchFailedException;
2324
import com.google.common.collect.ImmutableList;
2425
import com.google.common.collect.ImmutableMap;
2526
import com.google.common.collect.ImmutableSet;
27+
import com.google.common.collect.Iterables;
2628
import com.google.common.collect.Maps;
2729
import com.google.devtools.build.lib.actions.FileValue;
2830
import com.google.devtools.build.lib.bazel.bzlmod.CompiledModuleFile.IncludeStatement;
2931
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.NonRootModuleFileValue;
3032
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
33+
import com.google.devtools.build.lib.bazel.repository.PatchUtil;
3134
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum.MissingChecksumException;
3235
import com.google.devtools.build.lib.cmdline.Label;
3336
import com.google.devtools.build.lib.cmdline.LabelConstants;
@@ -50,11 +53,13 @@
5053
import com.google.devtools.build.lib.skyframe.PackageLookupValue;
5154
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
5255
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
56+
import com.google.devtools.build.lib.vfs.DigestHashFunction;
5357
import com.google.devtools.build.lib.vfs.FileSystemUtils;
5458
import com.google.devtools.build.lib.vfs.Path;
5559
import com.google.devtools.build.lib.vfs.PathFragment;
5660
import com.google.devtools.build.lib.vfs.Root;
5761
import com.google.devtools.build.lib.vfs.RootedPath;
62+
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
5863
import com.google.devtools.build.skyframe.SkyFunction;
5964
import com.google.devtools.build.skyframe.SkyFunctionException;
6065
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -613,11 +618,15 @@ private GetModuleFileResult getModuleFile(
613618
StoredEventHandler downloadEventHandler = new StoredEventHandler();
614619
for (Registry registry : registryObjects) {
615620
try {
616-
Optional<ModuleFile> moduleFile = registry.getModuleFile(key, downloadEventHandler);
617-
if (moduleFile.isEmpty()) {
621+
Optional<ModuleFile> maybeModuleFile = registry.getModuleFile(key, downloadEventHandler);
622+
if (maybeModuleFile.isEmpty()) {
618623
continue;
619624
}
620-
return new GetModuleFileResult(moduleFile.get(), registry, downloadEventHandler);
625+
ModuleFile moduleFile = maybePatchModuleFile(maybeModuleFile.get(), override, env);
626+
if (moduleFile == null) {
627+
return null;
628+
}
629+
return new GetModuleFileResult(moduleFile, registry, downloadEventHandler);
621630
} catch (MissingChecksumException e) {
622631
throw new ModuleFileFunctionException(
623632
ExternalDepsException.withCause(Code.BAD_LOCKFILE, e));
@@ -630,6 +639,114 @@ private GetModuleFileResult getModuleFile(
630639
throw errorf(Code.MODULE_NOT_FOUND, "module not found in registries: %s", key);
631640
}
632641

642+
/**
643+
* Applies any patches specified in registry overrides.
644+
*
645+
* <p>This allows users to modify MODULE.bazel files and thus influence resolution and visibility
646+
* for modules via patches without having to replace the entire module via a non-registry
647+
* override.
648+
*
649+
* <p>Note: Only patch files from the main repo are applied, all other patches are ignored. This
650+
* is necessary as we can't load other repos during resolution (unless they are subject to a
651+
* non-registry override). Patch commands are also not supported as they cannot be selectively
652+
* applied to the module file only.
653+
*/
654+
@Nullable
655+
private ModuleFile maybePatchModuleFile(
656+
ModuleFile moduleFile, ModuleOverride override, Environment env)
657+
throws InterruptedException, ModuleFileFunctionException {
658+
if (!(override instanceof SingleVersionOverride singleVersionOverride)) {
659+
return moduleFile;
660+
}
661+
var patchesInMainRepo =
662+
singleVersionOverride.getPatches().stream()
663+
.filter(label -> label.getRepository().isMain())
664+
.collect(toImmutableList());
665+
if (patchesInMainRepo.isEmpty()) {
666+
return moduleFile;
667+
}
668+
669+
// Get the patch paths.
670+
var patchPackageLookupKeys =
671+
patchesInMainRepo.stream()
672+
.map(Label::getPackageIdentifier)
673+
.map(pkg -> (SkyKey) PackageLookupValue.key(pkg))
674+
.collect(toImmutableList());
675+
var patchPackageLookupResult = env.getValuesAndExceptions(patchPackageLookupKeys);
676+
if (env.valuesMissing()) {
677+
return null;
678+
}
679+
List<RootedPath> patchPaths = new ArrayList<>();
680+
for (int i = 0; i < patchesInMainRepo.size(); i++) {
681+
var pkgLookupValue =
682+
(PackageLookupValue) patchPackageLookupResult.get(patchPackageLookupKeys.get(i));
683+
if (pkgLookupValue == null) {
684+
return null;
685+
}
686+
if (!pkgLookupValue.packageExists()) {
687+
Label label = patchesInMainRepo.get(i);
688+
throw errorf(
689+
Code.BAD_MODULE,
690+
"unable to load package for single_version_override patch %s: %s",
691+
label,
692+
PackageLookupFunction.explainNoBuildFileValue(label.getPackageIdentifier(), env));
693+
}
694+
patchPaths.add(
695+
RootedPath.toRootedPath(
696+
pkgLookupValue.getRoot(), patchesInMainRepo.get(i).toPathFragment()));
697+
}
698+
699+
// Register dependencies on the patch files and ensure that they exist.
700+
var fileKeys = Iterables.transform(patchPaths, FileValue::key);
701+
var fileValueResult = env.getValuesAndExceptions(fileKeys);
702+
if (env.valuesMissing()) {
703+
return null;
704+
}
705+
for (var key : fileKeys) {
706+
var fileValue = (FileValue) fileValueResult.get(key);
707+
if (fileValue == null) {
708+
return null;
709+
}
710+
if (!fileValue.isFile()) {
711+
throw errorf(
712+
Code.BAD_MODULE,
713+
"error reading single_version_override patch %s: not a regular file",
714+
key.argument());
715+
}
716+
}
717+
718+
// Apply the patches to the module file only.
719+
InMemoryFileSystem patchFs = new InMemoryFileSystem(DigestHashFunction.SHA256);
720+
try {
721+
Path moduleRoot = patchFs.getPath("/module");
722+
moduleRoot.createDirectoryAndParents();
723+
Path moduleFilePath = moduleRoot.getChild("MODULE.bazel");
724+
FileSystemUtils.writeContent(moduleFilePath, moduleFile.getContent());
725+
for (var patchPath : patchPaths) {
726+
try {
727+
PatchUtil.applyToSingleFile(
728+
patchPath.asPath(),
729+
singleVersionOverride.getPatchStrip(),
730+
moduleRoot,
731+
moduleFilePath);
732+
} catch (PatchFailedException e) {
733+
throw errorf(
734+
Code.BAD_MODULE,
735+
"error applying single_version_override patch %s to module file: %s",
736+
patchPath.asPath(),
737+
e.getMessage());
738+
}
739+
}
740+
return ModuleFile.create(
741+
FileSystemUtils.readContent(moduleFilePath), moduleFile.getLocation());
742+
} catch (IOException e) {
743+
throw errorf(
744+
Code.BAD_MODULE,
745+
"error applying single_version_override patches to module file: %s",
746+
e.getMessage());
747+
}
748+
}
749+
633750
private static byte[] readModuleFile(Path path) throws ModuleFileFunctionException {
634751
try {
635752
return FileSystemUtils.readWithKnownFileSize(path, path.getFileSize());

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -767,15 +767,20 @@ public void include(String label, StarlarkThread thread)
767767
doc =
768768
"A list of labels pointing to patch files to apply for this module. The patch files"
769769
+ " must exist in the source tree of the top level project. They are applied in"
770-
+ " the list order.",
770+
+ " the list order."
771+
+ ""
772+
+ "<p>If a patch makes changes to the MODULE.bazel file, these changes will"
773+
+ " only be effective if the patch file is provided by the root module.",
771774
allowedTypes = {@ParamType(type = Iterable.class, generic1 = String.class)},
772775
named = true,
773776
positional = false,
774777
defaultValue = "[]"),
775778
@Param(
776779
name = "patch_cmds",
777780
doc =
778-
"Sequence of Bash commands to be applied on Linux/Macos after patches are applied.",
781+
"Sequence of Bash commands to be applied on Linux/Macos after patches are applied."
782+
+ ""
783+
+ "<p>Changes to the MODULE.bazel file will not be effective.",
779784
allowedTypes = {@ParamType(type = Iterable.class, generic1 = String.class)},
780785
named = true,
781786
positional = false,

src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java

+39-7
Original file line numberDiff line numberDiff line change
@@ -417,10 +417,32 @@ private static void checkFilesStatusForPatching(
417417
*
418418
* @param patchFile the patch file to apply
419419
* @param strip the number of leading components to strip from file path in the patch file
420-
* @param outputDirectory the repository directory to apply the patch file
420+
* @param outputDirectory the directory to apply the patch file to
421421
*/
422422
public static void apply(Path patchFile, int strip, Path outputDirectory)
423423
throws IOException, PatchFailedException {
424+
applyInternal(patchFile, strip, outputDirectory, /* singleFile= */ null);
425+
}
426+
427+
/**
428+
* Apply a patch file under a directory, skipping all parts of the patch file that do not apply to
429+
* the given single file.
430+
*
431+
* @param patchFile the patch file to apply
432+
* @param strip the number of leading components to strip from file path in the patch file
433+
* @param outputDirectory the directory to apply the patch file to
434+
* @param singleFile only apply the parts of the patch file that apply to this file. Renaming the
435+
* file is not supported in this case.
436+
*/
437+
public static void applyToSingleFile(
438+
Path patchFile, int strip, Path outputDirectory, Path singleFile)
439+
throws IOException, PatchFailedException {
440+
applyInternal(patchFile, strip, outputDirectory, singleFile);
441+
}
442+
443+
private static void applyInternal(
444+
Path patchFile, int strip, Path outputDirectory, @Nullable Path singleFile)
445+
throws IOException, PatchFailedException {
424446
if (!patchFile.exists()) {
425447
throw new PatchFailedException("Cannot find patch file: " + patchFile.getPathString());
426448
}
@@ -565,15 +587,25 @@ public static void apply(Path patchFile, int strip, Path outputDirectory)
565587
patchContent, header, oldLineCount, newLineCount, patchStartLocation);
566588

567589
if (isRenaming) {
568-
checkFilesStatusForRenaming(
569-
oldFile, newFile, oldFileStr, newFileStr, patchStartLocation);
590+
if (singleFile != null) {
591+
if (singleFile.equals(newFile) || singleFile.equals(oldFile)) {
592+
throw new PatchFailedException(
593+
"Renaming %s while applying patches to it as a single file is not supported."
594+
.formatted(singleFile));
595+
}
596+
} else {
597+
checkFilesStatusForRenaming(
598+
oldFile, newFile, oldFileStr, newFileStr, patchStartLocation);
599+
}
570600
}
571601

572-
Patch<String> patch = UnifiedDiffUtils.parseUnifiedDiff(patchContent);
573-
checkFilesStatusForPatching(
574-
patch, oldFile, newFile, oldFileStr, newFileStr, patchStartLocation);
602+
if (singleFile == null || (singleFile.equals(newFile) && singleFile.equals(oldFile))) {
603+
Patch<String> patch = UnifiedDiffUtils.parseUnifiedDiff(patchContent);
604+
checkFilesStatusForPatching(
605+
patch, oldFile, newFile, oldFileStr, newFileStr, patchStartLocation);
575606

576-
applyPatchToFile(patch, oldFile, newFile, isRenaming, filePermission);
607+
applyPatchToFile(patch, oldFile, newFile, isRenaming, filePermission);
608+
}
577609
}
578610

579611
patchContent.clear();

0 commit comments

Comments
 (0)