Skip to content

Commit

Permalink
Don't use local home dir for remote (#737)
Browse files Browse the repository at this point in the history
  • Loading branch information
peterebden authored Sep 19, 2019
1 parent 7dcf4fd commit 22bcc36
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
12 changes: 9 additions & 3 deletions src/core/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/peterebden/gcfg"

"github.com/thought-machine/please/src/cli"
"github.com/thought-machine/please/src/fs"
)

// OsArch is the os/arch pair, like linux_amd64 etc.
Expand Down Expand Up @@ -171,6 +172,8 @@ func ReadConfigFiles(filenames []string, profiles []string) (*Configuration, err
}
}

config.HomeDir = os.Getenv("HOME")

// We can only verify options by reflection (we need struct tags) so run them quickly through this.
return config, config.ApplyOverrides(map[string]string{
"python.testrunner": config.Python.TestRunner,
Expand Down Expand Up @@ -249,6 +252,7 @@ func DefaultConfiguration() *Configuration {
config.Cache.RPCMaxMsgSize.UnmarshalFlag("200MiB")
config.Test.Timeout = cli.Duration(10 * time.Minute)
config.Display.SystemStats = true
config.Remote.HomeDir = "~"
config.Go.GoTool = "go"
config.Go.CgoCCTool = "gcc"
config.Go.BuildIDTool = "go_buildid_replacer"
Expand Down Expand Up @@ -390,6 +394,7 @@ type Configuration struct {
Name string `help:"A name for this worker instance. This is attached to artifacts uploaded to remote storage." example:"agent-001"`
DisplayURL string `help:"A URL to browse the remote server with (e.g. using buildbarn-browser). Only used when printing hashes."`
ReadOnly bool `help:"If true, prevents this client from writing to the remote storage. Is overridden if being used for execution."`
HomeDir string `help:"The home directory on the build machine."`
} `help:"Settings related to remote execution & caching using the Google remote execution APIs. This section is still experimental and subject to change."`
Size map[string]*Size `help:"Named sizes of targets; these are the definitions of what can be passed to the 'size' argument."`
Cover struct {
Expand Down Expand Up @@ -484,6 +489,8 @@ type Configuration struct {
Compatibility bool `help:"Activates limited Bazel compatibility mode. When this is active several rule arguments are available under different names (e.g. compiler_flags -> copts etc), the WORKSPACE file is interpreted, Makefile-style replacements like $< and $@ are made in genrule commands, etc.\nNote that Skylark is not generally supported and many aspects of compatibility are fairly superficial; it's unlikely this will work for complex setups of either tool." var:"BAZEL_COMPATIBILITY"`
} `help:"Bazel is an open-sourced version of Google's internal build tool. Please draws a lot of inspiration from the original tool although the two have now diverged in various ways.\nNonetheless, if you've used Bazel, you will likely find Please familiar."`

// HomeDir is not a config setting but is used to construct the path.
HomeDir string
// buildEnvStored is a cached form of BuildEnv.
buildEnvStored *storedBuildEnv
}
Expand Down Expand Up @@ -564,13 +571,12 @@ func (config *Configuration) getBuildEnv(includePath bool) []string {
pair := strings.Replace(strings.ToUpper(k), "-", "_", -1) + "=" + v
env = append(env, pair)
}

// from the user's environment based on the PassEnv config keyword
for _, k := range config.Build.PassEnv {
if v, isSet := os.LookupEnv(k); isSet {
if k == "PATH" {
// plz's install location always needs to be on the path.
v = ExpandHomePath(config.Please.Location) + ":" + v
v = fs.ExpandHomePathTo(config.Please.Location, config.HomeDir) + ":" + v
includePath = false // skip this in a bit
}
env = append(env, k+"="+v)
Expand All @@ -581,7 +587,7 @@ func (config *Configuration) getBuildEnv(includePath bool) []string {
// but really external environment variables shouldn't affect this.
// The only concession is that ~ is expanded as the user's home directory
// in PATH entries.
env = append(env, "PATH="+ExpandHomePath(strings.Join(append([]string{config.Please.Location}, config.Build.Path...), ":")))
env = append(env, "PATH="+fs.ExpandHomePathTo(strings.Join(append([]string{config.Please.Location}, config.Build.Path...), ":"), config.HomeDir))
}

sort.Strings(env)
Expand Down
7 changes: 6 additions & 1 deletion src/fs/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ var homeRex = regexp.MustCompile("(?:^|:)(~(?:[/:]|$))")

// ExpandHomePath expands all prefixes of ~ without a user specifier to $HOME.
func ExpandHomePath(path string) string {
return ExpandHomePathTo(path, home)
}

// ExpandHomePathTo expands all prefixes of ~ without a user specifier to the given string.
func ExpandHomePathTo(path, to string) string {
return homeRex.ReplaceAllStringFunc(path, func(subpath string) string {
return strings.Replace(subpath, "~", home, -1)
return strings.Replace(subpath, "~", to, -1)
})
}
3 changes: 3 additions & 0 deletions src/remote/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ func (c *Client) CheckInitialised() error {
// init is passed to the sync.Once to do the actual initialisation.
func (c *Client) init() {
c.err = func() error {
// Create a copy of the state where we can modify the config
c.state = c.state.ForConfig()
c.state.Config.HomeDir = c.state.Config.Remote.HomeDir
// TODO(peterebden): We may need to add the ability to have multiple URLs which we
// would then query for capabilities to discover which is which.
// TODO(peterebden): Add support for TLS.
Expand Down

0 comments on commit 22bcc36

Please sign in to comment.