Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --experimental_repository_cache_urls_as_default_canonical_id to help detect broken repository URLs #14268

Closed
wants to merge 2 commits into from

Conversation

kmicklas
Copy link
Contributor

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.

…elp 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 bazelbuild#14128.
@google-cla google-cla bot added the cla: yes label Nov 12, 2021
@davido
Copy link
Contributor

davido commented Jan 8, 2022

I tested this PR and it works as expected, see my comment in the corresponding issue.

Code looks sane. Would it be feasible to add a test?
Looking into the code base:

  $ cd src/test/java && git grep -i CanonicalId
com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java:  public void testCanonicalId() throws Exception {
com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java:              "testCanonicalId",
com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java:              "testCanonicalId",
com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java:              "testCanonicalId",
com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java:            "testCanonicalId",
com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java:                "testCanonicalId",
com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java:            "testCanonicalId",
com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java:    final String canonicalId = "";
com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java:        canonicalId,

I see a number of occurrences of CanonicalId in test code.

@kmicklas
Copy link
Contributor Author

kmicklas commented Jan 8, 2022

I'll take a look at adding a test on Monday, thanks!

@meteorcloudy
Copy link
Member

…s_as_default_canonical_id

We test the current default behavior, which is that broken URLs are not detected, and that this is detected when using the new flag.
@kmicklas
Copy link
Contributor Author

I added an integration test for the new flag and the default behavior.

Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bazel-io bazel-io closed this in f9882e4 Jan 11, 2022
@kmicklas kmicklas deleted the urls-canonical-id-flag branch January 11, 2022 17:09
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Mar 7, 2022
…elp 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 bazelbuild#14128.

Closes bazelbuild#14268.

PiperOrigin-RevId: 420976730
(cherry picked from commit f9882e4)
Wyverald pushed a commit that referenced this pull request Mar 7, 2022
…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 <[email protected]>
sluongng added a commit to sluongng/bazel that referenced this pull request Sep 18, 2023
Introduced in bazelbuild#14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the right move.
sluongng added a commit to sluongng/bazel that referenced this pull request Sep 19, 2023
Introduced in bazelbuild#14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the right move.
copybara-service bot pushed a commit that referenced this pull request Sep 21, 2023
Introduced in #14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the right move.

Closes #19549.

PiperOrigin-RevId: 567356017
Change-Id: I6402e33f569789545e3ce17ebb42c51a8d56126f
copybara-service bot pushed a commit that referenced this pull request Nov 2, 2023
*** Reason for rollback ***

Due to #19749

*** Original change description ***

Stablize and flip repository_cache_urls_as_default_canonical_id

Introduced in #14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the r...

***

RELNOTES: None
PiperOrigin-RevId: 578840189
Change-Id: I390910b5dd838a4a1384c5d24cedde0b7c1b2b98
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Nov 2, 2023
*** Reason for rollback ***

Due to bazelbuild#19749

*** Original change description ***

Stablize and flip repository_cache_urls_as_default_canonical_id

Introduced in bazelbuild#14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the r...

***

RELNOTES: None
PiperOrigin-RevId: 578840189
Change-Id: I390910b5dd838a4a1384c5d24cedde0b7c1b2b98
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Nov 2, 2023
*** Reason for rollback ***

Due to bazelbuild#19749

*** Original change description ***

Stablize and flip repository_cache_urls_as_default_canonical_id

Introduced in bazelbuild#14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the r...

***

RELNOTES: None
PiperOrigin-RevId: 578840189
Change-Id: I390910b5dd838a4a1384c5d24cedde0b7c1b2b98
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Nov 2, 2023
*** Reason for rollback ***

Due to bazelbuild#19749

*** Original change description ***

Stablize and flip repository_cache_urls_as_default_canonical_id

Introduced in bazelbuild#14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the r...

***

RELNOTES: None
PiperOrigin-RevId: 578840189
Change-Id: I390910b5dd838a4a1384c5d24cedde0b7c1b2b98
meteorcloudy added a commit that referenced this pull request Nov 2, 2023
*** Reason for rollback ***

Due to #19749

*** Original change description ***

Stablize and flip repository_cache_urls_as_default_canonical_id

Introduced in #14268, this flag is very useful for bigger enterprise
context where folks often version bumping dependencies without
remembering to update the SHA256 of the downloaded file, leading to
Bazel picking up older download entries from the repository cache.

As we get more questions about this flag in Slack, marking it as
stable and flip the default seems to be the r...

***

RELNOTES: None
Commit
0a1dce2

PiperOrigin-RevId: 578840189
Change-Id: I390910b5dd838a4a1384c5d24cedde0b7c1b2b98

Co-authored-by: Googler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag to include URLs in repository cache key when canonical_id not specified
4 participants