coursier: prefer the primary_url over mirrors #743
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dhalperi
commented
Sep 8, 2022
| @@ -1043,6 +1043,10 @@ def _coursier_fetch_impl(repository_ctx): | |||
| primary_artifact_path = infer_artifact_path_from_primary_and_repos(primary_url, repository_urls) | |||
|
|
|||
| mirror_urls = [url + "/" + primary_artifact_path for url in repository_urls] | |||
| if primary_url in mirror_urls: | |||
There was a problem hiding this comment.
It wasn't obvious to me that this is guaranteed to be the case, but I also didn't see how it couldn't be.
There was a problem hiding this comment.
Ahh, auth makes it not true. <-- never mind this was wrong.
dhalperi
force-pushed
the
order-mirrors
branch
2 times, most recently
from
1421102
to
6f96a40
Compare
dhalperi
commented
Sep 8, 2022
| @@ -2175,34 +2175,34 @@ | |||
| "url": "https://repo1.maven.org/maven2/com/microsoft/azure/adal4j/1.6.0/adal4j-1.6.0.jar" | |||
| }, | |||
| { | |||
| "coord": "com.nimbusds:lang-tag:1.6", | |||
| "coord": "com.nimbusds:lang-tag:1.7", | |||
There was a problem hiding this comment.
FYI repinning just did this upgrade to 1.7. I can add another version set to fix if preferred.
This is for bazelbuild#349 and tangentially for bazelbuild#723. Users are often confused by (expected) 404s when using multiple Maven repositories. These 404s can be safely ignored (Bazel's repository fetching logic gracefully tries all mirrors), but they are chatty and there is as of yet no way to silence them. See also: bazelbuild/bazel#13394 While we cannot solve the issue, we can reduce its impact by ordering the URLs: `http_file` is documented to try them in order: https://bazel.build/rules/lib/repo/http#http_file-urls
dhalperi
force-pushed
the
order-mirrors
branch
from
6f96a40
to
9d2d9e8
Compare
jin
approved these changes
Sep 9, 2022
|
Nice! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is for #349 and tangentially for #723.
Users are often confused by (expected) 404s when using multiple Maven repositories. These 404s can be safely ignored (Bazel's repository fetching logic gracefully tries all mirrors), but they are chatty and there is as of yet no way to silence them.
See also: bazelbuild/bazel#13394
While we cannot solve the issue, we can reduce its impact by ordering the URLs:
http_fileis documented to try them in order:https://bazel.build/rules/lib/repo/http#http_file-urls