[6.4.0] MODULE.bazel.lock file contains user specific paths (#19698)

- Store the unresolved registry string "containing %workspace%
placeholder" in IndexRegistry and use it to create the remote_patches
- Later when creating the repo rule, resolve the registry path in
remote_patches

fixes #19621

Commit
736a068

PiperOrigin-RevId: 570028910
Change-Id: I984c6b7494fb377bcf8b3568fa98c740c8618c33

Co-authored-by: salma-samy <salmasamy@google.com>

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

@@ -226,7 +226,8 @@ public void workspaceInit(
             directories,
             BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER);
     RegistryFactory registryFactory =
-        new RegistryFactoryImpl(downloadManager, clientEnvironmentSupplier);
+        new RegistryFactoryImpl(
+            directories.getWorkspace(), downloadManager, clientEnvironmentSupplier);
     singleExtensionEvalFunction =
         new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier, downloadManager);
 

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

@@ -26,6 +26,9 @@
  * an {@code http_archive} repo rule call.
  */
 public class ArchiveRepoSpecBuilder {
+
+  public static final String HTTP_ARCHIVE_PATH = "@bazel_tools//tools/build_defs/repo:http.bzl";
+
   private final ImmutableMap.Builder<String, Object> attrBuilder;
 
   public ArchiveRepoSpecBuilder() {
@@ -96,7 +99,7 @@ public ArchiveRepoSpecBuilder setArchiveType(String archiveType) {
 
   public RepoSpec build() {
     return RepoSpec.builder()
-        .setBzlFile("@bazel_tools//tools/build_defs/repo:http.bzl")
+        .setBzlFile(HTTP_ARCHIVE_PATH)
         .setRuleClassName("http_archive")
         .setAttributes(AttributeValues.create(attrBuilder.buildOrThrow()))
         .build();

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

@@ -83,6 +83,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/util:os",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//third_party:caffeine",
         "//third_party:gson",

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

@@ -48,14 +48,23 @@
  */
 // TODO(wyv): Insert "For details, see ..." when we have public documentation.
 public class IndexRegistry implements Registry {
+
+  /** The unresolved version of the url. Ex: has %workspace% placeholder */
+  private final String unresolvedUri;
+
   private final URI uri;
   private final DownloadManager downloadManager;
   private final Map<String, String> clientEnv;
   private final Gson gson;
   private volatile Optional<BazelRegistryJson> bazelRegistryJson;
 
-  public IndexRegistry(URI uri, DownloadManager downloadManager, Map<String, String> clientEnv) {
+  public IndexRegistry(
+      URI uri,
+      String unresolvedUri,
+      DownloadManager downloadManager,
+      Map<String, String> clientEnv) {
     this.uri = uri;
+    this.unresolvedUri = unresolvedUri;
     this.downloadManager = downloadManager;
     this.clientEnv = clientEnv;
     this.gson =
@@ -66,7 +75,7 @@ public IndexRegistry(URI uri, DownloadManager downloadManager, Map<String, Strin
 
   @Override
   public String getUrl() {
-    return uri.toString();
+    return unresolvedUri;
   }
 
   private String constructUrl(String base, String... segments) {
@@ -97,7 +106,7 @@ public Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler ev
       throws IOException, InterruptedException {
     String url =
         constructUrl(
-            getUrl(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel");
+            uri.toString(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel");
     return grabFile(url, eventHandler).map(content -> ModuleFile.create(content, url));
   }
 
@@ -148,12 +157,16 @@ public RepoSpec getRepoSpec(
     Optional<SourceJson> sourceJson =
         grabJson(
             constructUrl(
-                getUrl(), "modules", key.getName(), key.getVersion().toString(), "source.json"),
+                uri.toString(),
+                "modules",
+                key.getName(),
+                key.getVersion().toString(),
+                "source.json"),
             SourceJson.class,
             eventHandler);
     if (sourceJson.isEmpty()) {
       throw new FileNotFoundException(
-          String.format("Module %s's source information not found in registry %s", key, getUrl()));
+          String.format("Module %s's source information not found in registry %s", key, uri));
     }
 
     String type = sourceJson.get().type;
@@ -176,7 +189,7 @@ private Optional<BazelRegistryJson> getBazelRegistryJson(ExtendedEventHandler ev
         if (bazelRegistryJson == null) {
           bazelRegistryJson =
               grabJson(
-                  constructUrl(getUrl(), "bazel_registry.json"),
+                  constructUrl(uri.toString(), "bazel_registry.json"),
                   BazelRegistryJson.class,
                   eventHandler);
         }
@@ -258,7 +271,7 @@ private RepoSpec createArchiveRepoSpec(
       for (Map.Entry<String, String> entry : sourceJson.get().patches.entrySet()) {
         remotePatches.put(
             constructUrl(
-                getUrl(),
+                unresolvedUri,
                 "modules",
                 key.getName(),
                 key.getVersion().toString(),
@@ -285,7 +298,7 @@ public Optional<ImmutableMap<Version, String>> getYankedVersions(
       throws IOException, InterruptedException {
     Optional<MetadataJson> metadataJson =
         grabJson(
-            constructUrl(getUrl(), "modules", moduleName, "metadata.json"),
+            constructUrl(uri.toString(), "modules", moduleName, "metadata.json"),
             MetadataJson.class,
             eventHandler);
     if (metadataJson.isEmpty()) {

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

@@ -389,9 +389,7 @@ private GetModuleFileResult getModuleFile(
     List<Registry> registryObjects = new ArrayList<>(registries.size());
     for (String registryUrl : registries) {
       try {
-        registryObjects.add(
-            registryFactory.getRegistryWithUrl(
-                registryUrl.replace("%workspace%", workspaceRoot.getPathString())));
+        registryObjects.add(registryFactory.getRegistryWithUrl(registryUrl));
       } catch (URISyntaxException e) {
         throw errorf(Code.INVALID_REGISTRY_URL, e, "Invalid registry URL");
       }

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

@@ -18,26 +18,31 @@
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
+import com.google.devtools.build.lib.vfs.Path;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Map;
 import java.util.function.Supplier;
 
 /** Prod implementation of {@link RegistryFactory}. */
 public class RegistryFactoryImpl implements RegistryFactory {
+  private final Path workspacePath;
   private final DownloadManager downloadManager;
   private final Supplier<Map<String, String>> clientEnvironmentSupplier;
   private final Cache<String, Registry> registries = Caffeine.newBuilder().build();
 
   public RegistryFactoryImpl(
-      DownloadManager downloadManager, Supplier<Map<String, String>> clientEnvironmentSupplier) {
+      Path workspacePath,
+      DownloadManager downloadManager,
+      Supplier<Map<String, String>> clientEnvironmentSupplier) {
+    this.workspacePath = workspacePath;
     this.downloadManager = downloadManager;
     this.clientEnvironmentSupplier = clientEnvironmentSupplier;
   }
 
   @Override
-  public Registry getRegistryWithUrl(String url) throws URISyntaxException {
-    URI uri = new URI(url);
+  public Registry getRegistryWithUrl(String unresolvedUrl) throws URISyntaxException {
+    URI uri = new URI(unresolvedUrl.replace("%workspace%", workspacePath.getPathString()));
     if (uri.getScheme() == null) {
       throw new URISyntaxException(
           uri.toString(), "Registry URL has no scheme -- did you mean to use file://?");
@@ -47,8 +52,10 @@ public Registry getRegistryWithUrl(String url) throws URISyntaxException {
       case "https":
       case "file":
         return registries.get(
-            url,
-            unused -> new IndexRegistry(uri, downloadManager, clientEnvironmentSupplier.get()));
+            unresolvedUrl,
+            unused ->
+                new IndexRegistry(
+                    uri, unresolvedUrl, downloadManager, clientEnvironmentSupplier.get()));
       default:
         throw new URISyntaxException(uri.toString(), "Unrecognized registry URL protocol");
     }

src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java

@@ -14,11 +14,14 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.bazel.bzlmod.ArchiveRepoSpecBuilder;
+import com.google.devtools.build.lib.bazel.bzlmod.AttributeValues;
 import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
 import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleCreator;
 import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue;
@@ -47,6 +50,7 @@
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
+import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -198,7 +202,7 @@ private BzlmodRepoRuleValue createRuleFromSpec(
               env.getListener(),
               "BzlmodRepoRuleFunction.createRule",
               ruleClass,
-              repoSpec.attributes().attributes());
+              resolveRemotePatchesUrl(repoSpec).attributes());
       return new BzlmodRepoRuleValue(rule.getPackage(), rule.getName());
     } catch (InvalidRuleException e) {
       throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
@@ -209,6 +213,35 @@ private BzlmodRepoRuleValue createRuleFromSpec(
     }
   }
 
+  /* Resolves repo specs containing remote patches that are stored with %workspace% place holder*/
+  @SuppressWarnings("unchecked")
+  private AttributeValues resolveRemotePatchesUrl(RepoSpec repoSpec) {
+    if (repoSpec
+        .getRuleClass()
+        .equals(ArchiveRepoSpecBuilder.HTTP_ARCHIVE_PATH + "%http_archive")) {
+      return AttributeValues.create(
+          repoSpec.attributes().attributes().entrySet().stream()
+              .collect(
+                  toImmutableMap(
+                      Map.Entry::getKey,
+                      e -> {
+                        if (e.getKey().equals("remote_patches")) {
+                          Map<String, String> remotePatches = (Map<String, String>) e.getValue();
+                          return remotePatches.keySet().stream()
+                              .collect(
+                                  toImmutableMap(
+                                      key ->
+                                          key.replace(
+                                              "%workspace%",
+                                              directories.getWorkspace().getPathString()),
+                                      remotePatches::get));
+                        }
+                        return e.getValue();
+                      })));
+    }
+    return repoSpec.attributes();
+  }
+
   /** Loads modules from the given bzl file. */
   private ImmutableMap<String, Module> loadBzlModules(
       Environment env, String bzlFile, StarlarkSemantics starlarkSemantics)

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java

@@ -34,6 +34,7 @@
 import com.google.devtools.build.lib.bazel.repository.downloader.UnrecoverableHttpException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
+import com.google.devtools.build.lib.vfs.Path;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
@@ -60,9 +61,11 @@ public class IndexRegistryTest extends FoundationTestCase {
 
   @Before
   public void setUp() throws Exception {
+    Path workspaceRoot = scratch.dir("/ws");
     downloadManager = new DownloadManager(new RepositoryCache(), new HttpDownloader());
     registryFactory =
-        new RegistryFactoryImpl(downloadManager, Suppliers.ofInstance(ImmutableMap.of()));
+        new RegistryFactoryImpl(
+            workspaceRoot, downloadManager, Suppliers.ofInstance(ImmutableMap.of()));
   }
 
   @Test

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java

@@ -22,19 +22,23 @@
 import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
+import com.google.devtools.build.lib.testutil.FoundationTestCase;
+import com.google.devtools.build.lib.vfs.Path;
 import java.net.URISyntaxException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 /** Tests for {@link RegistryFactory}. */
 @RunWith(JUnit4.class)
-public class RegistryFactoryTest {
+public class RegistryFactoryTest extends FoundationTestCase {
 
   @Test
   public void badSchemes() throws Exception {
+    Path workspaceRoot = scratch.dir("/ws");
     RegistryFactory registryFactory =
         new RegistryFactoryImpl(
+            workspaceRoot,
             new DownloadManager(new RepositoryCache(), new HttpDownloader()),
             Suppliers.ofInstance(ImmutableMap.of()));
     assertThrows(URISyntaxException.class, () -> registryFactory.getRegistryWithUrl("/home/www"));

src/test/py/bazel/bzlmod/bazel_lockfile_test.py

@@ -1078,10 +1078,9 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
             '',
             'def _ext_1_impl(ctx):',
             '    print("Ext 1 is being evaluated")',
-            (
-                '    num_tags = len([tag for mod in ctx.modules for tag in'
-                ' mod.tags.tag])'
-            ),
+            '    num_tags = len([',
+            '        tag for mod in ctx.modules for tag in mod.tags.tag',
+            '    ])',
             '    repo_rule(name="dep", value="Ext 1 saw %s tags" % num_tags)',
             '',
             'ext_1 = module_extension(',
@@ -1091,10 +1090,9 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
             '',
             'def _ext_2_impl(ctx):',
             '    print("Ext 2 is being evaluated")',
-            (
-                '    num_tags = len([tag for mod in ctx.modules for tag in'
-                ' mod.tags.tag])'
-            ),
+            '    num_tags = len([',
+            '        tag for mod in ctx.modules for tag in mod.tags.tag',
+            '    ])',
             '    repo_rule(name="dep", value="Ext 2 saw %s tags" % num_tags)',
             '',
             'ext_2 = module_extension(',
@@ -1104,10 +1102,9 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
             '',
             'def _ext_3_impl(ctx):',
             '    print("Ext 3 is being evaluated")',
-            (
-                '    num_tags = len([tag for mod in ctx.modules for tag in'
-                ' mod.tags.tag])'
-            ),
+            '    num_tags = len([',
+            '        tag for mod in ctx.modules for tag in mod.tags.tag',
+            '    ])',
             '    repo_rule(name="dep", value="Ext 3 saw %s tags" % num_tags)',
             '',
             'ext_3 = module_extension(',
@@ -1232,6 +1229,46 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
     )
     self.assertNotIn(ext_3_key, lockfile['moduleExtensions'])
 
+  def testLockfileWithNoUserSpecificPath(self):
+    self.my_registry = BazelRegistry(os.path.join(self._test_cwd, 'registry'))
+    patch_file = self.ScratchFile(
+        'ss.patch',
+        [
+            '--- a/aaa.cc',
+            '+++ b/aaa.cc',
+            '@@ -1,6 +1,6 @@',
+            ' #include <stdio.h>',
+            ' #include "aaa.h"',
+            ' void hello_aaa(const std::string& caller) {',
+            '-    std::string lib_name = "aaa@1.1-1";',
+            '+    std::string lib_name = "aaa@1.1-1 (remotely patched)";',
+            '     printf("%s => %s\\n", caller.c_str(), lib_name.c_str());',
+            ' }',
+        ],
+    )
+    self.my_registry.createCcModule(
+        'ss', '1.3-1', patches=[patch_file], patch_strip=1
+    )
+
+    self.ScratchFile(
+        'MODULE.bazel',
+        [
+            'bazel_dep(name = "ss", version = "1.3-1")',
+        ],
+    )
+    self.ScratchFile('BUILD.bazel', ['filegroup(name = "lala")'])
+    self.RunBazel(
+        ['build', '--registry=file:///%workspace%/registry', '//:lala']
+    )
+
+    with open('MODULE.bazel.lock', 'r') as json_file:
+      lockfile = json.load(json_file)
+    remote_patches = lockfile['moduleDepGraph']['ss@1.3-1']['repoSpec'][
+        'attributes'
+    ]['remote_patches']
+    for key in remote_patches.keys():
+      self.assertIn('%workspace%', key)
+
 
 if __name__ == '__main__':
   unittest.main()