From 01795e364c5e830d6fe149fb143baa7e2902f82d Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 11 Sep 2024 14:13:45 +0200 Subject: [PATCH] Address comments and fix renaming check --- .../lib/bazel/bzlmod/ModuleFileFunction.java | 16 ++++++++-- .../build/lib/bazel/repository/PatchUtil.java | 30 ++++++++++++------- .../bazel/bzlmod/ModuleFileFunctionTest.java | 21 ++++++++----- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index e1fef4448663e7..1a63266a48a287 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -642,14 +642,24 @@ private GetModuleFileResult getModuleFile( throw errorf(Code.MODULE_NOT_FOUND, "module not found in registries: %s", key); } + /** + * Applies any patches specified in registry overrides. + * + *

This allows users to modify MODULE.bazel files and thus influence resolution and visibility + * for modules via patches without having to replace the entire module via a non-registry + * override. + * + *

Note: Only patch files from the main repo are applied, all other patches are ignored. This + * is necessary as we can't load other repos during resolution (unless they are subject to a + * non-registry override). Patch commands are also not supported as they cannot be selectively + * applied to the module file only. + */ private ModuleFile maybePatchModuleFile( ModuleFile moduleFile, ModuleOverride override, Environment env) throws InterruptedException, ModuleFileFunctionException { if (!(override instanceof SingleVersionOverride singleVersionOverride)) { return moduleFile; } - // Apply any patches specified in the override that come from the main repo. Non-main repo - // patches as well as patch commands are not supported. var patchesInMainRepo = singleVersionOverride.getPatches().stream() .filter(label -> label.getRepository().isMain()) @@ -716,7 +726,7 @@ private ModuleFile maybePatchModuleFile( FileSystemUtils.writeContent(moduleFilePath, moduleFile.getContent()); for (var patchPath : patchPaths) { try { - PatchUtil.apply( + PatchUtil.applyToSingleFile( patchPath.asPath(), singleVersionOverride.getPatchStrip(), moduleRoot, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java index d59cd76f07908e..ef3dc760175c34 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/PatchUtil.java @@ -421,19 +421,26 @@ private static void checkFilesStatusForPatching( */ public static void apply(Path patchFile, int strip, Path outputDirectory) throws IOException, PatchFailedException { - apply(patchFile, strip, outputDirectory, /* singleFile= */ null); + applyInternal(patchFile, strip, outputDirectory, /* singleFile= */ null); } /** - * Apply a patch file under a directory + * Apply a patch file under a directory, skipping all parts of the patch file that do not apply to + * the given single file. * * @param patchFile the patch file to apply * @param strip the number of leading components to strip from file path in the patch file * @param outputDirectory the directory to apply the patch file to - * @param singleFile if not null, only apply the parts of the patch file that apply to this file. - * Renaming the file is not supported in this case. + * @param singleFile only apply the parts of the patch file that apply to this file. Renaming the + * file is not supported in this case. */ - public static void apply( + public static void applyToSingleFile( + Path patchFile, int strip, Path outputDirectory, Path singleFile) + throws IOException, PatchFailedException { + applyInternal(patchFile, strip, outputDirectory, singleFile); + } + + private static void applyInternal( Path patchFile, int strip, Path outputDirectory, @Nullable Path singleFile) throws IOException, PatchFailedException { if (!patchFile.exists()) { @@ -577,12 +584,15 @@ public static void apply( if (isRenaming) { if (singleFile != null) { - throw new PatchFailedException( - "Renaming is not supported when applying a patch to the single file %s." - .formatted(singleFile)); + if (singleFile.equals(newFile) || singleFile.equals(oldFile)) { + throw new PatchFailedException( + "Renaming %s while applying patches to it as a single file is not supported." + .formatted(singleFile)); + } + } else { + checkFilesStatusForRenaming( + oldFile, newFile, oldFileStr, newFileStr, patchStartLocation); } - checkFilesStatusForRenaming( - oldFile, newFile, oldFileStr, newFileStr, patchStartLocation); } if (singleFile == null || (singleFile.equals(newFile) && singleFile.equals(oldFile))) { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 775d8027b36034..92169491221c15 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -1734,18 +1734,23 @@ public void testSingleVersionOverridePatches() throws Exception { @@ -1,1 +1,2 @@ module(name='bbb',version='1.0',bazel_compatibility=[">=7.0.0"]) +bazel_dep(name='ccc',version='3.0') + diff --git a/also/not/MODULE.bazel b/also/not/MODULE.bazel.bak + similarity index 55% + rename from also/not/MODULE.bazel + rename to also/not/MODULE.bazel.bak + index 3f855b5..949dd15 100644 """); scratch.file("other/pkg/BUILD"); Path otherPatch = scratch.file( "other/pkg/other_patch.diff", """ - --- a/MODULE.bazel - +++ b/MODULE.bazel - @@ -1,1 +1,2 @@ - module(name='bbb',version='1.0',bazel_compatibility=[">=7.0.0"]) - +bazel_dep(name='ccc',version='3.0') - """); + --- a/MODULE.bazel + +++ b/MODULE.bazel + @@ -1,1 +1,2 @@ + module(name='bbb',version='1.0',bazel_compatibility=[">=7.0.0"]) + +bazel_dep(name='ccc',version='3.0') + """); var moduleFileKey = ModuleFileValue.key( @@ -1829,6 +1834,8 @@ public void testSingleVersionOverridePatches_failsOnRename() throws Exception { assertThat(result.getError().getException()) .hasMessageThat() .isEqualTo( - "error applying single_version_override patch /workspace/patch.diff to module file: Renaming is not supported when applying a patch to the single file /module/MODULE.bazel."); + "error applying single_version_override patch /workspace/patch.diff to module file:" + + " Renaming /module/MODULE.bazel while applying patches to it as a single file is" + + " not supported."); } }