Add --experimental_repository_disable_download to allow users disable download for external repos #12940
Conversation
… download for external repos
|
cc @lberki |
|
Thanks! Maybe add a simple test for this? |
|
@jin , it looks like this is enough, isn't it? |
|
For repo rules, this covers the |
| @@ -194,6 +199,10 @@ public Path download( | |||
| } | |||
| } | |||
|
|
|||
| if (disableDownload) { | |||
There was a problem hiding this comment.
Do I understand correctly that this works because we don't provide any built-in repositories anymore that do downloading and we delegate everything to Starlark code instead?
There was a problem hiding this comment.
This only works for repo rule which uses repository_ctx.download.
| @@ -194,6 +199,10 @@ public Path download( | |||
| } | |||
| } | |||
|
|
|||
| if (disableDownload) { | |||
| throw new IOException(String.format("Failed to download repo %s: download is disabled.", repo)); | |||
There was a problem hiding this comment.
Please add test cases to verify at least the following cases (starlark_repository_test looks like a logical location, but maybe there is something better):
- When the flag is set, downloads don't work
- When the flag is set, repositories in distdir are still accessible
- Maybe: when the flag is unset, downloads work (although that case is probably covered by existing tests?)
I'd love if this flag to also affected git_repository and the like, but I guess that's not possible unless we put repository download actions into a sandbox which is probably too big a change for now :(
There was a problem hiding this comment.
Test cases added.
git_repository uses repository_ctx.execute under the hood so it's impossible for this change to have affect on it.
There was a problem hiding this comment.
Yeah, I don't think there is a lot we can do about that :(
|
@jin , can't disagree with that, but that's not any different from regular actions doing network calls, is it? |
|
Indeed, and that's the third and final area where Bazel can make user-initiated network calls today AFAICT. Let's get this in to cover this case. |
|
Yeah. This change alone is probably good enough and if not, we can think further. It's definitely necessary, but probably also necessary and sufficient. |
| EOF | ||
|
|
||
| bazel build --experimental_repository_disable_download //:it > "${TEST_log}" 2>&1 \ | ||
| && fail "Expected failure" || : |
There was a problem hiding this comment.
Since the first command failed (exit code is not 0), we need to use : (which is a symbol for true) to set the exit code to 0. Just a trick to ignore the errors. (Copied from other test cases in the same file)
| @@ -194,6 +199,10 @@ public Path download( | |||
| } | |||
| } | |||
|
|
|||
| if (disableDownload) { | |||
| throw new IOException(String.format("Failed to download repo %s: download is disabled.", repo)); | |||
There was a problem hiding this comment.
Yeah, I don't think there is a lot we can do about that :(
| echo 'Hello World' > x/file.txt | ||
| tar cvf x.tar x | ||
| sha256=$(sha256sum x.tar | head -c 64) | ||
| serve_file x.tar |
There was a problem hiding this comment.
If this uses distdir, why do you need this serve_file call?
… download for external repos This PR adds a flag `--experimental_repository_disable_download` which when set will prevent Bazel from downloading external repositories. However, `local_repository`, `new_local_repository` and external repositories already downloaded in the repository cache would still work. Closes #12940. PiperOrigin-RevId: 355332927
… download for external repos This PR adds a flag `--experimental_repository_disable_download` which when set will prevent Bazel from downloading external repositories. However, `local_repository`, `new_local_repository` and external repositories already downloaded in the repository cache would still work. Closes #12940. PiperOrigin-RevId: 355332927
This PR adds a flag
--experimental_repository_disable_downloadwhich when set will prevent Bazel from downloading external repositories. However,local_repository,new_local_repositoryand external repositories already downloaded in the repository cache would still work.