Skip to content

Commit

Permalink
allow targets to force building locally
Browse files Browse the repository at this point in the history
  • Loading branch information
peterebden committed Aug 7, 2019
1 parent 7547c60 commit 6938eb1
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 13 deletions.
2 changes: 1 addition & 1 deletion rules/builtins.build_defs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def build_rule(name:str, cmd:str|dict='', test_cmd:str|dict='', srcs:list|dict=N
test_timeout:int|str=0, pre_build:function=None, post_build:function=None, requires:list=None, provides:dict=None,
licences:list=CONFIG.DEFAULT_LICENCES, test_outputs:list=None, system_srcs:list=None, stamp:bool=False,
tag:str='', optional_outs:list=None, progress:bool=False, size:str=None, _urls:list=None,
internal_deps:list=None, pass_env:list=None):
internal_deps:list=None, pass_env:list=None, local:bool=False):
pass


Expand Down
10 changes: 8 additions & 2 deletions rules/misc_rules.build_defs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs
hashes:list=None, timeout:int=0, binary:bool=False, sandbox:bool=None,
needs_transitive_deps:bool=False, output_is_complete:bool=True, test_only:bool&testonly=False,
secrets:list|dict=None, requires:list=None, provides:dict=None, pre_build:function=None,
post_build:function=None, tools:list|dict=None, pass_env:list=None):
post_build:function=None, tools:list|dict=None, pass_env:list=None, local:bool=False):
"""A general build rule which allows the user to specify a command.
Args:
Expand Down Expand Up @@ -91,6 +91,8 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs
to dynamically create new rules based on the output of another.
pass_env: List of environment variables to be passed from outside. Any changes to them will
be recorded in this target's hash and will hence force it to rebuild.
local: Forces the rule to be built locally; when remote execution is enabled it will not
be sent remotely but executed on the local machine.
"""
if out and outs:
raise TypeError('Can\'t specify both "out" and "outs".')
Expand All @@ -117,14 +119,15 @@ def genrule(name:str, cmd:str|list|dict, srcs:list|dict=None, out:str=None, outs
provides = provides,
test_only = test_only,
pass_env = pass_env,
local = local,
)


def gentest(name:str, test_cmd:str|dict, labels:list&features&tags=None, cmd:str|dict=None, srcs:list|dict=None, outs:list=None,
deps:list=None, tools:list|dict=None, data:list=None, visibility:list=None, timeout:int=0,
needs_transitive_deps:bool=False, flaky:bool|int=0, secrets:list|dict=None, no_test_output:bool=False,
test_outputs:list=None, output_is_complete:bool=True, requires:list=None,
sandbox:bool=None, size:str=None):
sandbox:bool=None, size:str=None, local:bool=False):
"""A rule which creates a test with an arbitrary command.
The command must return zero on success and nonzero on failure. Test results are written
Expand Down Expand Up @@ -163,6 +166,8 @@ def gentest(name:str, test_cmd:str|dict, labels:list&features&tags=None, cmd:str
including networking, process, IPC, etc. Only has an effect on Linux.
If this is on by default then tests can opt out by setting this to False.
size (str): Test size (enormous, large, medium or small).
local: Forces the rule to be built locally; when remote execution is enabled it will not
be sent remotely but executed on the local machine.
"""
return build_rule(
name = name,
Expand All @@ -187,6 +192,7 @@ def gentest(name:str, test_cmd:str|dict, labels:list&features&tags=None, cmd:str
no_test_output = no_test_output,
test_outputs = test_outputs,
flaky = flaky,
local = local,
)


Expand Down
1 change: 1 addition & 0 deletions src/build/incrementality.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ func ruleHash(state *core.BuildState, target *core.BuildTarget, runtime bool) []
hashOptionalBool(h, target.IsFilegroup)
hashOptionalBool(h, target.IsHashFilegroup)
hashOptionalBool(h, target.IsRemoteFile)
hashOptionalBool(h, target.Local)
for _, require := range target.Requires {
h.Write([]byte(require))
}
Expand Down
2 changes: 1 addition & 1 deletion src/build/incrementality_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var KnownFields = map[string]bool{
"TestCommand": true,
"TestCommands": true,
"NeedsTransitiveDependencies": true,
"Local": true,
"OptionalOutputs": true,
"OutputIsComplete": true,
"Requires": true,
Expand All @@ -52,7 +53,6 @@ var KnownFields = map[string]bool{

// These only contribute to the runtime hash, not at build time.
"Data": true,
"Containerise": true,
"TestSandbox": true,
"ContainerSettings": true,

Expand Down
2 changes: 2 additions & 0 deletions src/core/build_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ type BuildTarget struct {
// If true, the rule is given an env var at build time that contains the hash of its
// transitive dependencies, which can be used to identify the output in a predictable way.
Stamp bool
// If true, the target must be run locally (i.e. is not compatible with remote execution).
Local bool
// Marks the target as a filegroup.
IsFilegroup bool `print:"false"`
// Marks the target as a hash_filegroup.
Expand Down
23 changes: 17 additions & 6 deletions src/core/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,27 @@ func (state *BuildState) AddPendingTest(label BuildLabel) {

// TaskQueues returns a set of channels to listen on for tasks of various types.
// This should only be called once per state (otherwise you will not get a full set of tasks).
func (state *BuildState) TaskQueues() (parses <-chan LabelPair, builds, tests <-chan BuildLabel) {
func (state *BuildState) TaskQueues() (parses <-chan LabelPair, builds, tests, remoteBuilds, remoteTests <-chan BuildLabel) {
p := make(chan LabelPair, 100)
b := make(chan BuildLabel, 100)
t := make(chan BuildLabel, 100)
go state.feedQueues(p, b, t)
return p, b, t
rb := make(chan BuildLabel, 100)
rt := make(chan BuildLabel, 100)
go state.feedQueues(p, b, t, rb, rt)
return p, b, t, rb, rt
}

// feedQueues feeds the build queues created in TaskQueues.
// We retain the internal priority queue since it is unbounded size which is pretty important
// for us not to deadlock.
func (state *BuildState) feedQueues(parses chan<- LabelPair, builds, tests chan<- BuildLabel) {
func (state *BuildState) feedQueues(parses chan<- LabelPair, builds, tests, remoteBuilds, remoteTests chan<- BuildLabel) {
anyRemote := state.Config.Remote.NumExecutors > 0
queue := func(label BuildLabel, local, remote chan<- BuildLabel) chan<- BuildLabel {
if anyRemote && !state.Graph.Target(label).Local {
return remote
}
return local
}
for {
t, _ := state.pendingTasks.Get(1)
task := t[0].(pendingTask)
Expand All @@ -254,6 +263,8 @@ func (state *BuildState) feedQueues(parses chan<- LabelPair, builds, tests chan<
close(parses)
close(builds)
close(tests)
close(remoteBuilds)
close(remoteTests)
if state.results != nil {
close(state.results)
}
Expand All @@ -265,10 +276,10 @@ func (state *BuildState) feedQueues(parses chan<- LabelPair, builds, tests chan<
parses <- LabelPair{Label: task.Label, Dependent: task.Dependent, ForSubinclude: task.Type == SubincludeParse}
case Build, SubincludeBuild:
atomic.AddInt64(&state.progress.numRunning, 1)
builds <- task.Label
queue(task.Label, builds, remoteBuilds) <- task.Label
case Test:
atomic.AddInt64(&state.progress.numRunning, 1)
tests <- task.Label
queue(task.Label, tests, remoteTests) <- task.Label
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/parse/asp/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func createTarget(s *scope, args []pyObject) *core.BuildTarget {
target.TestOnly = test || isTruthy(15)
target.ShowProgress = isTruthy(36)
target.IsRemoteFile = isTruthy(38)
target.Local = isTruthy(41)

var size *core.Size
if args[37] != None {
Expand Down
2 changes: 1 addition & 1 deletion src/parse/parse_step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func assertPendingBuilds(t *testing.T, state *core.BuildState, targets ...string
}

func getAllPending(state *core.BuildState) ([]string, []string) {
parses, builds, tests := state.TaskQueues()
parses, builds, tests, _, _ := state.TaskQueues()
state.Stop()
var pendingParses, pendingBuilds []string
for parses != nil || builds != nil || tests != nil {
Expand Down
4 changes: 2 additions & 2 deletions src/plz/plz.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func Run(targets, preTargets []core.BuildLabel, state *core.BuildState, config *
// Start looking for the initial targets to kick the build off
go findOriginalTasks(state, preTargets, targets, arch)

parses, builds, tests := state.TaskQueues()
parses, builds, tests, remoteBuilds, remoteTests := state.TaskQueues()

// Start up all the build workers
var wg sync.WaitGroup
Expand All @@ -51,7 +51,7 @@ func Run(targets, preTargets []core.BuildLabel, state *core.BuildState, config *
}
for i := 0; i < config.Remote.NumExecutors; i++ {
go func(tid int) {
doTasks(tid, state, nil, builds, tests, arch, true)
doTasks(tid, state, nil, remoteBuilds, remoteTests, arch, true)
wg.Done()
}(config.Please.NumThreads + i)
}
Expand Down

0 comments on commit 6938eb1

Please sign in to comment.