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.

Sign up for GitHub

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

Jump to bottom

http.bzl : all_urls is given the remote_patch URL which is surely a mistake. #22201

Closed
fzakaria opened this issue Apr 30, 2024 · 2 comments
Closed

http.bzl : all_urls is given the remote_patch URL which is surely a mistake. #22201

fzakaria opened this issue Apr 30, 2024 · 2 comments
Assignees
@meteorcloudy
Labels
P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug

Comments

@fzakaria
Copy link
Contributor

fzakaria commented Apr 30, 2024 •
edited

Description of the bug:

all_urls includes the remote_patch URL key for http_archive.
This value is passed to downlolad_and_extract which is a bug.

This means that if the original URLs for the http_archive fail, it will treat the remote_patch as the URL for the archive itself.

Please see:

bazel/tools/build_defs/repo/http.bzl

Line 141 in 0e7c0fb

download_info = ctx.download_and_extract(

This came up during my writing of PR #22155

CC @Wyverald
@fzakaria fzakaria mentioned this issue Apr 30, 2024
Closed
@Wyverald
Copy link
Member

ah, indeed -- looks like this was accidentally introduced in 445dfab. We should take the remote patch URLs into account for auth, but not actually download the patches together with the main archive.

@Wyverald Wyverald added P2 We'll consider working on this in future. (Assignee optional) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. and removed untriaged labels Apr 30, 2024
@meteorcloudy meteorcloudy self-assigned this May 23, 2024
@meteorcloudy meteorcloudy added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels May 23, 2024
@meteorcloudy
Copy link
Member

Sorry for the oversight! I'll work on a fix for this..

This was referenced May 23, 2024
Closed
Closed
bazel-io pushed a commit to bazel-io/bazel that referenced this issue May 29, 2024
@meteorcloudy @bazel-io
Fix getting authentication for URLs in http repo rules
38edbde 
- Fixed the leak of `remote_patches` URLs for downloaded the source archive.
- Compute auth for required URLs only

Fixes bazelbuild#22201

Closes bazelbuild#22517.

PiperOrigin-RevId: 638300996
Change-Id: Ib76e3284f209d2314844cfd662ac8eadba785fae
@bazel-io bazel-io mentioned this issue May 29, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue May 29, 2024
@bazel-io @meteorcloudy
[7.2.0] Fix getting authentication for URLs in http repo rules (#22583)
ca2e8fc 
- Fixed the leak of `remote_patches` URLs for downloaded the source
archive.
- Compute auth for required URLs only

Fixes #22201

Closes #22517.

PiperOrigin-RevId: 638300996
Change-Id: Ib76e3284f209d2314844cfd662ac8eadba785fae

Commit
5986420

Co-authored-by: Yun Peng <pcloudy@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@meteorcloudy meteorcloudy

Labels
P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix getting authentication for URLs in http repo rules meteorcloudy/bazel
6 participants
@Wyverald @fzakaria @meteorcloudy @sgowroji @iancha1992 @satyanandak