Skip to content

Commit

Permalink
Add --experimental_repository_cache_urls_as_default_canonical_id to h…
Browse files Browse the repository at this point in the history
…elp detect broken repository URLs (#14989)

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
(cherry picked from commit f9882e4)

Co-authored-by: Ken Micklas <kmicklas@uber.com>
  • Loading branch information
brentleyjones and kmicklas authored Mar 7, 2022
1 parent 0764821 commit f15e0c7
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
if (repoOptions.repositoryDownloaderRetries >= 0) {
downloadManager.setRetries(repoOptions.repositoryDownloaderRetries);
}
downloadManager.setUrlsAsDefaultCanonicalId(repoOptions.urlsAsDefaultCanonicalId);

repositoryCache.setHardlink(repoOptions.useHardlinks);
if (repoOptions.experimentalScaleTimeouts > 0.0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,19 @@ public class RepositoryOptions extends OptionsBase {
+ " to escalate it to a resolution failure.")
public CheckDirectDepsMode checkDirectDependencies;

@Option(
name = "experimental_repository_cache_urls_as_default_canonical_id",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
"If true, use a string derived from the URLs of repository downloads as the canonical_id "
+ "if not specified. This causes a change in the URLs to result in a redownload even "
+ "if the cache contains a download with the same hash. This can be used to verify "
+ "that URL changes don't result in broken repositories being masked by the cache.")
public boolean urlsAsDefaultCanonicalId;

/** An enum for specifying different modes for checking direct dependency accuracy. */
public enum CheckDirectDepsMode {
OFF, // Don't check direct dependency accuracy.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.net.URL;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

/**
Expand All @@ -52,6 +53,7 @@ public class DownloadManager {
private final Downloader downloader;
private boolean disableDownload = false;
private int retries = 0;
private boolean urlsAsDefaultCanonicalId;

public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) {
this.repositoryCache = repositoryCache;
Expand All @@ -75,6 +77,10 @@ public void setRetries(int retries) {
this.retries = retries;
}

public void setUrlsAsDefaultCanonicalId(boolean urlsAsDefaultCanonicalId) {
this.urlsAsDefaultCanonicalId = urlsAsDefaultCanonicalId;
}

/**
* Downloads file to disk and returns path.
*
Expand Down Expand Up @@ -109,6 +115,10 @@ public Path download(
throw new InterruptedException();
}

if (Strings.isNullOrEmpty(canonicalId) && urlsAsDefaultCanonicalId) {
canonicalId = originalUrls.stream().map(URL::toExternalForm).collect(Collectors.joining(" "));
}

ImmutableList<URL> rewrittenUrls = ImmutableList.copyOf(originalUrls);
Map<URI, Map<String, String>> rewrittenAuthHeaders = authHeaders;

Expand Down
64 changes: 64 additions & 0 deletions src/test/shell/bazel/bazel_repository_cache_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -465,4 +465,68 @@ EOF
expect_log "Error downloading"
}

function test_break_url() {
setup_repository

bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"

shutdown_server

bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"

# Break url in WORKSPACE
rm WORKSPACE
cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = 'endangered',
url = 'http://localhost:$nc_port/bleh.broken',
sha256 = '$sha256',
type = 'zip',
)
EOF

# By default, cache entry will still match by sha256, even if url is changed.
bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"
}

function test_experimental_repository_cache_urls_as_default_canonical_id() {
setup_repository

bazel fetch --repository_cache="$repo_cache_dir" \
--experimental_repository_cache_urls_as_default_canonical_id \
//zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"

shutdown_server

bazel fetch --repository_cache="$repo_cache_dir" \
--experimental_repository_cache_urls_as_default_canonical_id \
//zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"

# Break url in WORKSPACE
rm WORKSPACE
cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = 'endangered',
url = 'http://localhost:$nc_port/bleh.broken',
sha256 = '$sha256',
type = 'zip',
)
EOF

# As repository cache key should depend on urls, we expect fetching to fail now.
bazel fetch --repository_cache="$repo_cache_dir" \
--experimental_repository_cache_urls_as_default_canonical_id \
//zoo:breeding-program >& $TEST_log \
&& fail "expected failure" || :
}

run_suite "repository cache tests"

0 comments on commit f15e0c7

Please sign in to comment.