Skip to content

Commit 736a068

Browse files
SalmaSamycopybara-github
SalmaSamy
authored and
copybara-github
committed
Remove user specific absolute path from lockfile
- 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 PiperOrigin-RevId: 570028910 Change-Id: I984c6b7494fb377bcf8b3568fa98c740c8618c33
1 parent 3b18d3f commit 736a068

File tree

10 files changed

+133
-33
lines changed

10 files changed

+133
-33
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ public void workspaceInit(
229229
directories,
230230
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER);
231231
RegistryFactory registryFactory =
232-
new RegistryFactoryImpl(downloadManager, clientEnvironmentSupplier);
232+
new RegistryFactoryImpl(
233+
directories.getWorkspace(), downloadManager, clientEnvironmentSupplier);
233234
singleExtensionEvalFunction =
234235
new SingleExtensionEvalFunction(directories, clientEnvironmentSupplier, downloadManager);
235236

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
* an {@code http_archive} repo rule call.
2727
*/
2828
public class ArchiveRepoSpecBuilder {
29+
30+
public static final String HTTP_ARCHIVE_PATH = "@bazel_tools//tools/build_defs/repo:http.bzl";
31+
2932
private final ImmutableMap.Builder<String, Object> attrBuilder;
3033

3134
public ArchiveRepoSpecBuilder() {
@@ -96,7 +99,7 @@ public ArchiveRepoSpecBuilder setArchiveType(String archiveType) {
9699

97100
public RepoSpec build() {
98101
return RepoSpec.builder()
99-
.setBzlFile("@bazel_tools//tools/build_defs/repo:http.bzl")
102+
.setBzlFile(HTTP_ARCHIVE_PATH)
100103
.setRuleClassName("http_archive")
101104
.setAttributes(AttributeValues.create(attrBuilder.buildOrThrow()))
102105
.build();

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ java_library(
8383
"//src/main/java/com/google/devtools/build/lib/events",
8484
"//src/main/java/com/google/devtools/build/lib/profiler",
8585
"//src/main/java/com/google/devtools/build/lib/util:os",
86+
"//src/main/java/com/google/devtools/build/lib/vfs",
8687
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
8788
"//third_party:caffeine",
8889
"//third_party:gson",

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

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,23 @@
4949
* <p>For details, see <a href="https://bazel.build/external/registry">the docs</a>
5050
*/
5151
public class IndexRegistry implements Registry {
52+
53+
/** The unresolved version of the url. Ex: has %workspace% placeholder */
54+
private final String unresolvedUri;
55+
5256
private final URI uri;
5357
private final DownloadManager downloadManager;
5458
private final Map<String, String> clientEnv;
5559
private final Gson gson;
5660
private volatile Optional<BazelRegistryJson> bazelRegistryJson;
5761

58-
public IndexRegistry(URI uri, DownloadManager downloadManager, Map<String, String> clientEnv) {
62+
public IndexRegistry(
63+
URI uri,
64+
String unresolvedUri,
65+
DownloadManager downloadManager,
66+
Map<String, String> clientEnv) {
5967
this.uri = uri;
68+
this.unresolvedUri = unresolvedUri;
6069
this.downloadManager = downloadManager;
6170
this.clientEnv = clientEnv;
6271
this.gson =
@@ -67,7 +76,7 @@ public IndexRegistry(URI uri, DownloadManager downloadManager, Map<String, Strin
6776

6877
@Override
6978
public String getUrl() {
70-
return uri.toString();
79+
return unresolvedUri;
7180
}
7281

7382
private String constructUrl(String base, String... segments) {
@@ -98,7 +107,7 @@ public Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler ev
98107
throws IOException, InterruptedException {
99108
String url =
100109
constructUrl(
101-
getUrl(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel");
110+
uri.toString(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel");
102111
return grabFile(url, eventHandler).map(content -> ModuleFile.create(content, url));
103112
}
104113

@@ -149,12 +158,16 @@ public RepoSpec getRepoSpec(
149158
Optional<SourceJson> sourceJson =
150159
grabJson(
151160
constructUrl(
152-
getUrl(), "modules", key.getName(), key.getVersion().toString(), "source.json"),
161+
uri.toString(),
162+
"modules",
163+
key.getName(),
164+
key.getVersion().toString(),
165+
"source.json"),
153166
SourceJson.class,
154167
eventHandler);
155168
if (sourceJson.isEmpty()) {
156169
throw new FileNotFoundException(
157-
String.format("Module %s's source information not found in registry %s", key, getUrl()));
170+
String.format("Module %s's source information not found in registry %s", key, uri));
158171
}
159172

160173
String type = sourceJson.get().type;
@@ -177,7 +190,7 @@ private Optional<BazelRegistryJson> getBazelRegistryJson(ExtendedEventHandler ev
177190
if (bazelRegistryJson == null) {
178191
bazelRegistryJson =
179192
grabJson(
180-
constructUrl(getUrl(), "bazel_registry.json"),
193+
constructUrl(uri.toString(), "bazel_registry.json"),
181194
BazelRegistryJson.class,
182195
eventHandler);
183196
}
@@ -259,7 +272,7 @@ private RepoSpec createArchiveRepoSpec(
259272
for (Map.Entry<String, String> entry : sourceJson.get().patches.entrySet()) {
260273
remotePatches.put(
261274
constructUrl(
262-
getUrl(),
275+
unresolvedUri,
263276
"modules",
264277
key.getName(),
265278
key.getVersion().toString(),
@@ -286,7 +299,7 @@ public Optional<ImmutableMap<Version, String>> getYankedVersions(
286299
throws IOException, InterruptedException {
287300
Optional<MetadataJson> metadataJson =
288301
grabJson(
289-
constructUrl(getUrl(), "modules", moduleName, "metadata.json"),
302+
constructUrl(uri.toString(), "modules", moduleName, "metadata.json"),
290303
MetadataJson.class,
291304
eventHandler);
292305
if (metadataJson.isEmpty()) {

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,9 +391,7 @@ private GetModuleFileResult getModuleFile(
391391
List<Registry> registryObjects = new ArrayList<>(registries.size());
392392
for (String registryUrl : registries) {
393393
try {
394-
registryObjects.add(
395-
registryFactory.getRegistryWithUrl(
396-
registryUrl.replace("%workspace%", workspaceRoot.getPathString())));
394+
registryObjects.add(registryFactory.getRegistryWithUrl(registryUrl));
397395
} catch (URISyntaxException e) {
398396
throw errorf(Code.INVALID_REGISTRY_URL, e, "Invalid registry URL");
399397
}

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,31 @@
1818
import com.github.benmanes.caffeine.cache.Cache;
1919
import com.github.benmanes.caffeine.cache.Caffeine;
2020
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
21+
import com.google.devtools.build.lib.vfs.Path;
2122
import java.net.URI;
2223
import java.net.URISyntaxException;
2324
import java.util.Map;
2425
import java.util.function.Supplier;
2526

2627
/** Prod implementation of {@link RegistryFactory}. */
2728
public class RegistryFactoryImpl implements RegistryFactory {
29+
private final Path workspacePath;
2830
private final DownloadManager downloadManager;
2931
private final Supplier<Map<String, String>> clientEnvironmentSupplier;
3032
private final Cache<String, Registry> registries = Caffeine.newBuilder().build();
3133

3234
public RegistryFactoryImpl(
33-
DownloadManager downloadManager, Supplier<Map<String, String>> clientEnvironmentSupplier) {
35+
Path workspacePath,
36+
DownloadManager downloadManager,
37+
Supplier<Map<String, String>> clientEnvironmentSupplier) {
38+
this.workspacePath = workspacePath;
3439
this.downloadManager = downloadManager;
3540
this.clientEnvironmentSupplier = clientEnvironmentSupplier;
3641
}
3742

3843
@Override
39-
public Registry getRegistryWithUrl(String url) throws URISyntaxException {
40-
URI uri = new URI(url);
44+
public Registry getRegistryWithUrl(String unresolvedUrl) throws URISyntaxException {
45+
URI uri = new URI(unresolvedUrl.replace("%workspace%", workspacePath.getPathString()));
4146
if (uri.getScheme() == null) {
4247
throw new URISyntaxException(
4348
uri.toString(), "Registry URL has no scheme -- did you mean to use file://?");
@@ -47,8 +52,10 @@ public Registry getRegistryWithUrl(String url) throws URISyntaxException {
4752
case "https":
4853
case "file":
4954
return registries.get(
50-
url,
51-
unused -> new IndexRegistry(uri, downloadManager, clientEnvironmentSupplier.get()));
55+
unresolvedUrl,
56+
unused ->
57+
new IndexRegistry(
58+
uri, unresolvedUrl, downloadManager, clientEnvironmentSupplier.get()));
5259
default:
5360
throw new URISyntaxException(uri.toString(), "Unrecognized registry URL protocol");
5461
}

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414

1515
package com.google.devtools.build.lib.skyframe;
1616

17+
import static com.google.common.collect.ImmutableMap.toImmutableMap;
1718

1819
import com.google.common.base.Preconditions;
1920
import com.google.common.collect.ImmutableList;
2021
import com.google.common.collect.ImmutableMap;
2122
import com.google.devtools.build.lib.analysis.BlazeDirectories;
23+
import com.google.devtools.build.lib.bazel.bzlmod.ArchiveRepoSpecBuilder;
24+
import com.google.devtools.build.lib.bazel.bzlmod.AttributeValues;
2225
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
2326
import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleCreator;
2427
import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue;
@@ -47,6 +50,7 @@
4750
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
4851
import com.google.devtools.build.skyframe.SkyKey;
4952
import com.google.devtools.build.skyframe.SkyValue;
53+
import java.util.Map;
5054
import java.util.Map.Entry;
5155
import java.util.Optional;
5256
import javax.annotation.Nullable;
@@ -198,7 +202,7 @@ private BzlmodRepoRuleValue createRuleFromSpec(
198202
env.getListener(),
199203
"BzlmodRepoRuleFunction.createRule",
200204
ruleClass,
201-
repoSpec.attributes().attributes());
205+
resolveRemotePatchesUrl(repoSpec).attributes());
202206
return new BzlmodRepoRuleValue(rule.getPackage(), rule.getName());
203207
} catch (InvalidRuleException e) {
204208
throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
@@ -209,6 +213,35 @@ private BzlmodRepoRuleValue createRuleFromSpec(
209213
}
210214
}
211215

216+
/* Resolves repo specs containing remote patches that are stored with %workspace% place holder*/
217+
@SuppressWarnings("unchecked")
218+
private AttributeValues resolveRemotePatchesUrl(RepoSpec repoSpec) {
219+
if (repoSpec
220+
.getRuleClass()
221+
.equals(ArchiveRepoSpecBuilder.HTTP_ARCHIVE_PATH + "%http_archive")) {
222+
return AttributeValues.create(
223+
repoSpec.attributes().attributes().entrySet().stream()
224+
.collect(
225+
toImmutableMap(
226+
Map.Entry::getKey,
227+
e -> {
228+
if (e.getKey().equals("remote_patches")) {
229+
Map<String, String> remotePatches = (Map<String, String>) e.getValue();
230+
return remotePatches.keySet().stream()
231+
.collect(
232+
toImmutableMap(
233+
key ->
234+
key.replace(
235+
"%workspace%",
236+
directories.getWorkspace().getPathString()),
237+
remotePatches::get));
238+
}
239+
return e.getValue();
240+
})));
241+
}
242+
return repoSpec.attributes();
243+
}
244+
212245
/** Loads modules from the given bzl file. */
213246
private ImmutableMap<String, Module> loadBzlModules(
214247
Environment env, String bzlFile, StarlarkSemantics starlarkSemantics)

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.devtools.build.lib.bazel.repository.downloader.UnrecoverableHttpException;
3535
import com.google.devtools.build.lib.cmdline.RepositoryName;
3636
import com.google.devtools.build.lib.testutil.FoundationTestCase;
37+
import com.google.devtools.build.lib.vfs.Path;
3738
import java.io.ByteArrayInputStream;
3839
import java.io.File;
3940
import java.io.IOException;
@@ -60,9 +61,11 @@ public class IndexRegistryTest extends FoundationTestCase {
6061

6162
@Before
6263
public void setUp() throws Exception {
64+
Path workspaceRoot = scratch.dir("/ws");
6365
downloadManager = new DownloadManager(new RepositoryCache(), new HttpDownloader());
6466
registryFactory =
65-
new RegistryFactoryImpl(downloadManager, Suppliers.ofInstance(ImmutableMap.of()));
67+
new RegistryFactoryImpl(
68+
workspaceRoot, downloadManager, Suppliers.ofInstance(ImmutableMap.of()));
6669
}
6770

6871
@Test

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,23 @@
2222
import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
2323
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
2424
import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
25+
import com.google.devtools.build.lib.testutil.FoundationTestCase;
26+
import com.google.devtools.build.lib.vfs.Path;
2527
import java.net.URISyntaxException;
2628
import org.junit.Test;
2729
import org.junit.runner.RunWith;
2830
import org.junit.runners.JUnit4;
2931

3032
/** Tests for {@link RegistryFactory}. */
3133
@RunWith(JUnit4.class)
32-
public class RegistryFactoryTest {
34+
public class RegistryFactoryTest extends FoundationTestCase {
3335

3436
@Test
3537
public void badSchemes() throws Exception {
38+
Path workspaceRoot = scratch.dir("/ws");
3639
RegistryFactory registryFactory =
3740
new RegistryFactoryImpl(
41+
workspaceRoot,
3842
new DownloadManager(new RepositoryCache(), new HttpDownloader()),
3943
Suppliers.ofInstance(ImmutableMap.of()));
4044
assertThrows(URISyntaxException.class, () -> registryFactory.getRegistryWithUrl("/home/www"));

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

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,10 +1153,9 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
11531153
'',
11541154
'def _ext_1_impl(ctx):',
11551155
' print("Ext 1 is being evaluated")',
1156-
(
1157-
' num_tags = len([tag for mod in ctx.modules for tag in'
1158-
' mod.tags.tag])'
1159-
),
1156+
' num_tags = len([',
1157+
' tag for mod in ctx.modules for tag in mod.tags.tag',
1158+
' ])',
11601159
' repo_rule(name="dep", value="Ext 1 saw %s tags" % num_tags)',
11611160
'',
11621161
'ext_1 = module_extension(',
@@ -1166,10 +1165,9 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
11661165
'',
11671166
'def _ext_2_impl(ctx):',
11681167
' print("Ext 2 is being evaluated")',
1169-
(
1170-
' num_tags = len([tag for mod in ctx.modules for tag in'
1171-
' mod.tags.tag])'
1172-
),
1168+
' num_tags = len([',
1169+
' tag for mod in ctx.modules for tag in mod.tags.tag',
1170+
' ])',
11731171
' repo_rule(name="dep", value="Ext 2 saw %s tags" % num_tags)',
11741172
'',
11751173
'ext_2 = module_extension(',
@@ -1179,10 +1177,9 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
11791177
'',
11801178
'def _ext_3_impl(ctx):',
11811179
' print("Ext 3 is being evaluated")',
1182-
(
1183-
' num_tags = len([tag for mod in ctx.modules for tag in'
1184-
' mod.tags.tag])'
1185-
),
1180+
' num_tags = len([',
1181+
' tag for mod in ctx.modules for tag in mod.tags.tag',
1182+
' ])',
11861183
' repo_rule(name="dep", value="Ext 3 saw %s tags" % num_tags)',
11871184
'',
11881185
'ext_3 = module_extension(',
@@ -1307,6 +1304,46 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self):
13071304
)
13081305
self.assertNotIn(ext_3_key, lockfile['moduleExtensions'])
13091306

1307+
def testLockfileWithNoUserSpecificPath(self):
1308+
self.my_registry = BazelRegistry(os.path.join(self._test_cwd, 'registry'))
1309+
patch_file = self.ScratchFile(
1310+
'ss.patch',
1311+
[
1312+
'--- a/aaa.cc',
1313+
'+++ b/aaa.cc',
1314+
'@@ -1,6 +1,6 @@',
1315+
' #include <stdio.h>',
1316+
' #include "aaa.h"',
1317+
' void hello_aaa(const std::string& caller) {',
1318+
'- std::string lib_name = "[email protected]";',
1319+
'+ std::string lib_name = "[email protected] (remotely patched)";',
1320+
' printf("%s => %s\\n", caller.c_str(), lib_name.c_str());',
1321+
' }',
1322+
],
1323+
)
1324+
self.my_registry.createCcModule(
1325+
'ss', '1.3-1', patches=[patch_file], patch_strip=1
1326+
)
1327+
1328+
self.ScratchFile(
1329+
'MODULE.bazel',
1330+
[
1331+
'bazel_dep(name = "ss", version = "1.3-1")',
1332+
],
1333+
)
1334+
self.ScratchFile('BUILD.bazel', ['filegroup(name = "lala")'])
1335+
self.RunBazel(
1336+
['build', '--registry=file:///%workspace%/registry', '//:lala']
1337+
)
1338+
1339+
with open('MODULE.bazel.lock', 'r') as json_file:
1340+
lockfile = json.load(json_file)
1341+
remote_patches = lockfile['moduleDepGraph']['[email protected]']['repoSpec'][
1342+
'attributes'
1343+
]['remote_patches']
1344+
for key in remote_patches.keys():
1345+
self.assertIn('%workspace%', key)
1346+
13101347

13111348
if __name__ == '__main__':
13121349
absltest.main()

0 commit comments

Comments
 (0)