Skip to content

Commit f9882e4

Browse files
kmicklascopybara-github
kmicklas
authored and
copybara-github
committed
Add --experimental_repository_cache_urls_as_default_canonical_id to help detect broken repository URLs
This new flag can be used to force redownloading when repository URLs are changed. Otherwise, it's possible broken URLs can be masked by the presence of a repository cache entry with the same hash. Specifying a `canonical_id` works as before, overriding this behavior. Closes #14128. Closes #14268. PiperOrigin-RevId: 420976730
1 parent 15ade85 commit f9882e4

4 files changed

Lines changed: 88 additions & 0 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
279279
if (repoOptions.repositoryDownloaderRetries >= 0) {
280280
downloadManager.setRetries(repoOptions.repositoryDownloaderRetries);
281281
}
282+
downloadManager.setUrlsAsDefaultCanonicalId(repoOptions.urlsAsDefaultCanonicalId);
282283

283284
repositoryCache.setHardlink(repoOptions.useHardlinks);
284285
if (repoOptions.experimentalScaleTimeouts > 0.0) {

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,19 @@ public class RepositoryOptions extends OptionsBase {
215215
+ " to escalate it to a resolution failure.")
216216
public CheckDirectDepsMode checkDirectDependencies;
217217

218+
@Option(
219+
name = "experimental_repository_cache_urls_as_default_canonical_id",
220+
defaultValue = "false",
221+
documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
222+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
223+
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
224+
help =
225+
"If true, use a string derived from the URLs of repository downloads as the canonical_id "
226+
+ "if not specified. This causes a change in the URLs to result in a redownload even "
227+
+ "if the cache contains a download with the same hash. This can be used to verify "
228+
+ "that URL changes don't result in broken repositories being masked by the cache.")
229+
public boolean urlsAsDefaultCanonicalId;
230+
218231
/** An enum for specifying different modes for checking direct dependency accuracy. */
219232
public enum CheckDirectDepsMode {
220233
OFF, // Don't check direct dependency accuracy.

‎src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.net.URL;
3636
import java.util.List;
3737
import java.util.Map;
38+
import java.util.stream.Collectors;
3839
import javax.annotation.Nullable;
3940

4041
/**
@@ -51,6 +52,7 @@ public class DownloadManager {
5152
private final Downloader downloader;
5253
private boolean disableDownload = false;
5354
private int retries = 0;
55+
private boolean urlsAsDefaultCanonicalId;
5456

5557
public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) {
5658
this.repositoryCache = repositoryCache;
@@ -74,6 +76,10 @@ public void setRetries(int retries) {
7476
this.retries = retries;
7577
}
7678

79+
public void setUrlsAsDefaultCanonicalId(boolean urlsAsDefaultCanonicalId) {
80+
this.urlsAsDefaultCanonicalId = urlsAsDefaultCanonicalId;
81+
}
82+
7783
/**
7884
* Downloads file to disk and returns path.
7985
*
@@ -108,6 +114,10 @@ public Path download(
108114
throw new InterruptedException();
109115
}
110116

117+
if (Strings.isNullOrEmpty(canonicalId) && urlsAsDefaultCanonicalId) {
118+
canonicalId = originalUrls.stream().map(URL::toExternalForm).collect(Collectors.joining(" "));
119+
}
120+
111121
List<URL> rewrittenUrls = originalUrls;
112122
Map<URI, Map<String, String>> rewrittenAuthHeaders = authHeaders;
113123

‎src/test/shell/bazel/bazel_repository_cache_test.sh‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,4 +465,68 @@ EOF
465465
expect_log "Error downloading"
466466
}
467467

468+
function test_break_url() {
469+
setup_repository
470+
471+
bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
472+
|| echo "Expected fetch to succeed"
473+
474+
shutdown_server
475+
476+
bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
477+
|| echo "Expected fetch to succeed"
478+
479+
# Break url in WORKSPACE
480+
rm WORKSPACE
481+
cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
482+
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
483+
484+
http_archive(
485+
name = 'endangered',
486+
url = 'http://localhost:$nc_port/bleh.broken',
487+
sha256 = '$sha256',
488+
type = 'zip',
489+
)
490+
EOF
491+
492+
# By default, cache entry will still match by sha256, even if url is changed.
493+
bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
494+
|| echo "Expected fetch to succeed"
495+
}
496+
497+
function test_experimental_repository_cache_urls_as_default_canonical_id() {
498+
setup_repository
499+
500+
bazel fetch --repository_cache="$repo_cache_dir" \
501+
--experimental_repository_cache_urls_as_default_canonical_id \
502+
//zoo:breeding-program >& $TEST_log \
503+
|| echo "Expected fetch to succeed"
504+
505+
shutdown_server
506+
507+
bazel fetch --repository_cache="$repo_cache_dir" \
508+
--experimental_repository_cache_urls_as_default_canonical_id \
509+
//zoo:breeding-program >& $TEST_log \
510+
|| echo "Expected fetch to succeed"
511+
512+
# Break url in WORKSPACE
513+
rm WORKSPACE
514+
cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
515+
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
516+
517+
http_archive(
518+
name = 'endangered',
519+
url = 'http://localhost:$nc_port/bleh.broken',
520+
sha256 = '$sha256',
521+
type = 'zip',
522+
)
523+
EOF
524+
525+
# As repository cache key should depend on urls, we expect fetching to fail now.
526+
bazel fetch --repository_cache="$repo_cache_dir" \
527+
--experimental_repository_cache_urls_as_default_canonical_id \
528+
//zoo:breeding-program >& $TEST_log \
529+
&& fail "expected failure" || :
530+
}
531+
468532
run_suite "repository cache tests"

0 commit comments

Comments
 (0)