Skip to content

Commit

Permalink
Address comments and fix renaming check
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Sep 11, 2024
1 parent ba8cd38 commit 01795e3
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>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.
*
* <p>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())
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.");
}
}

0 comments on commit 01795e3

Please sign in to comment.