-
Notifications
You must be signed in to change notification settings - Fork 367
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
feat: go_repository supports netrc #848
Conversation
I'm a bit surprised that this use case has not been covered yet. I ran into this problem because I had a personal git server that requires http login-password credentials for downloading archives. The
I used this netrc file with |
load( | ||
"@bazel_tools//tools/build_defs/repo:utils.bzl", | ||
"read_netrc", | ||
"use_netrc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you confirm that Bazel 1.2.0 provides these functions, and if not, what is the oldest version of Bazel that does? I'd like to make sure we're not breaking compatibility here. If these were added recently, it would be better to copy them here.
|
||
if "HOME" in ctx.os.environ and not ctx.os.name.startswith("windows"): | ||
netrcfile = "%s/.netrc" % (ctx.os.environ["HOME"]) | ||
if ctx.execute(["test", "-f", netrcfile]).return_code == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use test
here but ctx.path(netrcfile).exists
below? Seems like the latter should work in both cases.
def _get_auth(ctx, urls): | ||
"""Given the list of URLs obtain the correct auth dict.""" | ||
if ctx.attr.netrc: | ||
netrc = read_netrc(ctx, ctx.attr.netrc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid repetition, please separate the code that decides which file to read from the calls to read_netrc
and use_netrc
.
netrc = read_netrc(ctx, ctx.attr.netrc) | ||
return use_netrc(netrc, urls, ctx.attr.auth_patterns) | ||
|
||
if "HOME" in ctx.os.environ and not ctx.os.name.startswith("windows"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before looking for a .netrc
file in the home directory, check the NETRC
environment variable.
return use_netrc(netrc, urls, ctx.attr.auth_patterns) | ||
|
||
if "USERPROFILE" in ctx.os.environ and ctx.os.name.startswith("windows"): | ||
netrcfile = "%s/.netrc" % (ctx.os.environ["USERPROFILE"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows, it should be _netrc
, not .netrc
.
@@ -214,6 +241,8 @@ go_repository = repository_rule( | |||
"strip_prefix": attr.string(), | |||
"type": attr.string(), | |||
"sha256": attr.string(), | |||
"netrc": attr.string(), | |||
"auth_patterns": attr.string_dict(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add documentation to go_repository.bzl
for these attributes.
load("@io_bazel_rules_go//go/private:common.bzl", "env_execute", "executable_extension") | ||
load("@bazel_gazelle//internal:go_repository_cache.bzl", "read_cache_env") | ||
|
||
# We can't disable timeouts on Bazel, but we can set them to large values. | ||
_GO_REPOSITORY_TIMEOUT = 86400 | ||
|
||
def _get_auth(ctx, urls): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect folks will want to have just one netrc
file shared by all rules, so let's add these attributes to go_repository_config
as well and make sure they can be read by go_repository
if they're set there.
ctx.download_and_extract( | ||
url = ctx.attr.urls, | ||
sha256 = ctx.attr.sha256, | ||
stripPrefix = ctx.attr.strip_prefix, | ||
type = ctx.attr.type, | ||
auth = auth, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this will only work for HTTP mode. It also needs to work for module mode though (the ctx.attr.version
branch).
I think that just means making sure NETRC
is set to the correct file in env_keys
, but please verify.
Does seem to work with Bazel 3.3.1 on github private repos like this go_repository(
name = "...",
auth_patterns = {
"api.github.com": "Bearer <password>",
},
importpath = "github.com/org/repo",
strip_prefix = "org-repo-commit_here",
type = "zip", # zipball -> tarball and zip -> tar should also work
urls = ["https://api.github.com/repos/org/repo/zipball/commit_here"],
) plus ~/.netrc
|
Superseded by #1090 |
Initial PR to handle netrc had support for auth_patterns bazelbuild#848 But final version didn't bazelbuild#1090 auth_patterns can still be needed, for example for private github repositories Can be used like this ```python go_repository( name = "something", auth_patterns = { "api.github.com": "Bearer <password>", }, importpath = "github.com/someorg/something", strip_prefix = "something-...", type = "zip", urls = ["https://api.github.com/repos/someorg/something/zipball/..."], ) ``` Copying parameter documentation from bazel code
Initial PR to handle netrc had support for auth_patterns #848 But final version didn't #1090 auth_patterns can still be needed, for example for private github repositories Can be used like this ```python go_repository( name = "something", auth_patterns = { "api.github.com": "Bearer <password>", }, importpath = "github.com/someorg/something", strip_prefix = "something-...", type = "zip", urls = ["https://api.github.com/repos/someorg/something/zipball/..."], ) ```
What type of PR is this?
Feature
What package or component does this PR mostly affect?
go_repository
What does this PR do? Why is it needed?
This PR adds the
netrc
andauth_patterns
attributes to thego_repository
rule.This allows
go_repository
to be used with downloading servers that require login credentials.This also makes
go_repository
more closely resemble the interface ofhttp_archive
.Which issues(s) does this PR fix?
Fixes #847
Other notes for review
This patch was largely the result of directly pirating from the source code of
http_archive
. It may not be the most beautiful code ever.This patch has been tested on my local machine with a custom http server that requires login. I successfully re-used the same netrc file for
go_repository
and curl (with --netrc-file).If more tests are needed, please let me know.