-
Notifications
You must be signed in to change notification settings - Fork 23
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 .netrc support #82
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,17 +2,20 @@ package repo | |
|
||
import ( | ||
"crypto/sha256" | ||
"encoding/base64" | ||
"encoding/hex" | ||
"fmt" | ||
"hash" | ||
"io" | ||
"net/http" | ||
"net/url" | ||
"os" | ||
"os/user" | ||
"path" | ||
"path/filepath" | ||
"strings" | ||
|
||
"github.com/jdx/go-netrc" | ||
"github.com/rmohr/bazeldnf/pkg/api" | ||
"github.com/rmohr/bazeldnf/pkg/api/bazeldnf" | ||
log "github.com/sirupsen/logrus" | ||
|
@@ -229,6 +232,41 @@ func fileGet(filename string) (*http.Response, error) { | |
return resp, nil | ||
} | ||
|
||
var netrcCached *netrc.Netrc | ||
|
||
func getNetrc() (*netrc.Netrc, error) { | ||
if netrcCached != nil { | ||
return netrcCached, nil | ||
} | ||
usr, err := user.Current() | ||
if err != nil { | ||
return nil, fmt.Errorf("getting current user: %w", err) | ||
} | ||
n, err := netrc.Parse(filepath.Join(usr.HomeDir, ".netrc")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bazaldnf is consumed internally by the bazel rules. It seems like this would break hermeticity in that case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICT this part of bazeldnf runs in the repository rule / loading phase, which is "allowed" to poke at local files and perform network calls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moreso what I was getting at is that this dependency is opaque to everything that consumes bazeldnf itself. ie: how would you identify this as something that you build depends on? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bazel already defaults to reading ~/.netrc for the built in http_archive rules (see https://github.com/bazelbuild/bazel/blob/master/tools/build_defs/repo/http.bzl#L144 and https://github.com/bazelbuild/bazel/blob/master/tools/build_defs/repo/utils.bzl#L460 which show how it's implemented) without any additional configuration from the user. This is well established behaviour. Actually, I had another look through the code and I'll go back on my previous statement. I don't think this code path runs as part of the build at all:
In other words these changes only affect There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will only be used while resolving dependencies, all the rest of the time Bazel will use the rpm repo rule which under the hood uses the same mechanisms as http_archive. Making it use netrc as the rest of Bazel makes sense, it's similar to what other rules like |
||
if err == nil { | ||
netrcCached = n | ||
} | ||
return n, err | ||
} | ||
|
||
func httpGet(rawUrl string) (*http.Response, error) { | ||
url, err := url.Parse(rawUrl) | ||
if err != nil { | ||
return nil, fmt.Errorf("parsing %s as a URL: %w", rawUrl, err) | ||
} | ||
netrc, err := getNetrc() | ||
if err != nil { | ||
return nil, fmt.Errorf("getting netrc: %w", err) | ||
} | ||
m := netrc.Machine(url.Hostname()) | ||
req, err := http.NewRequest("GET", rawUrl, nil) | ||
if m != nil { | ||
auth := m.Get("login") + ":" + m.Get("password") | ||
req.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(auth))) | ||
} | ||
return http.DefaultClient.Do(req) | ||
} | ||
|
||
func (*getterImpl) Get(rawURL string) (*http.Response, error) { | ||
u, err := url.Parse(rawURL) | ||
if err != nil { | ||
|
@@ -237,7 +275,7 @@ func (*getterImpl) Get(rawURL string) (*http.Response, error) { | |
if u.Scheme == "file" { | ||
return fileGet(u.Path) | ||
} | ||
return http.Get(rawURL) | ||
return httpGet(rawURL) | ||
} | ||
|
||
func toHex(hasher hash.Hash) string { | ||
|
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.
Mind to rebase the branch and add some documentation so we can merge?
Could the path be configurable through some env variable? And then some test that does some url rewriting or something that requires no human interaction be created?