Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ tarball(
name = "deep-tarball",
srcs = [":docs"],
out = "deep-docs.tar.gz",
labels = ["hlink:plz-out/pkg"],
flatten = False,
labels = ["hlink:plz-out/pkg"],
)

go_binary(
Expand Down
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
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, local:bool=False, output_dirs:list=[], metadata=None,
exit_on_error:bool=CONFIG.EXIT_ON_ERROR):
exit_on_error:bool=CONFIG.EXIT_ON_ERROR, entry_points:dict={}):
pass


Expand Down
12 changes: 6 additions & 6 deletions rules/rules.go

Large diffs are not rendered by default.

12 changes: 7 additions & 5 deletions src/build/build_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ func prepareOnly(tid int, state *core.BuildState, target *core.BuildTarget) erro
// 3) Actually build the rule
// 4) Store result in the cache
func buildTarget(tid int, state *core.BuildState, target *core.BuildTarget, runRemotely bool) (err error) {
// TODO(jpoole): we've defined 4 steps that this function performs. We should be able to break it out into
// smaller functions
defer func() {
if r := recover(); r != nil {
if e, ok := r.(error); ok {
Expand Down Expand Up @@ -287,14 +285,13 @@ func buildTarget(tid int, state *core.BuildState, target *core.BuildTarget, runR
if err != nil {
return err
}
}
// The remote action will set the output directory outs here
if !runRemotely {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just checking this is deliberate? Not sure I see how it's related...?

Copy link
Copy Markdown
Contributor Author

@Tatskaari Tatskaari Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I just moved the addOutputDirectoriesToBuildOutput(target) into the else branch of the above if statement. It used to look like:

if runRemotely {
  ...
} else {
   ...
}

if !runRemotely {
   ... := addOutputDirectoriesToBuildOutput(target)
}

which was superfluous. The diff doesn't make it that clear though.


metadata.OutputDirOuts, err = addOutputDirectoriesToBuildOutput(target)
if err != nil {
return err
}
}

if target.PostBuildFunction != nil {
outs := target.Outputs()
if err := runPostBuildFunction(tid, state, target, string(metadata.Stdout), postBuildOutput); err != nil {
Expand Down Expand Up @@ -623,6 +620,11 @@ func moveOutputs(state *core.BuildState, target *core.BuildTarget) ([]string, bo
}
changed = changed || outputChanged
}
for ep, out := range target.EntryPoints {
if !fs.PathExists(filepath.Join(target.OutDir(), out)) {
return nil, true, fmt.Errorf("failed to produce output %v for entry point %v", out, ep)
}
}
if changed {
log.Debug("Outputs for %s have changed", target.Label)
} else {
Expand Down
4 changes: 4 additions & 0 deletions src/build/incrementality.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ func ruleHash(state *core.BuildState, target *core.BuildTarget, runtime bool) []
for _, o := range target.OutputDirectories {
h.Write([]byte(o))
}

for ep, out := range target.EntryPoints {
h.Write([]byte(ep + "=" + out))
}
return h.Sum(nil)
}

Expand Down
1 change: 1 addition & 0 deletions src/build/incrementality_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ var KnownFields = map[string]bool{
"Stamp": true,
"OutputDirectories": true,
"ExitOnError": true,
"EntryPoints": true,

// These only contribute to the runtime hash, not at build time.
"Data": true,
Expand Down
6 changes: 5 additions & 1 deletion src/core/build_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,11 @@ func initStampEnv() {

func toolPath(state *BuildState, tool BuildInput, abs bool) string {
if label := tool.Label(); label != nil {
path := state.Graph.TargetOrDie(*label).toolPath(abs)
entryPoint := ""
if o, ok := tool.(AnnotatedOutputLabel); ok {
entryPoint = o.Annotation
}
path := state.Graph.TargetOrDie(*label).toolPath(abs, entryPoint)
if !strings.Contains(path, "/") {
path = "./" + path
}
Expand Down
44 changes: 28 additions & 16 deletions src/core/build_input.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,41 +193,53 @@ func (label SystemPathLabel) String() string {
return label.Name
}

// NamedOutputLabel represents a reference to a subset of named outputs from a rule.
// The rule must have declared those as a named group.
type NamedOutputLabel struct {
// AnnotatedOutputLabel represents a build label with an annotation e.g. //foo:bar|baz where baz constitutes the
// annotation. This can be used to target a named output of this rule when depended on or an entry point when used in
// the context of tools.
type AnnotatedOutputLabel struct {
BuildLabel
Output string
Annotation string
}

// Paths returns a slice of paths to the files of this input.
func (label NamedOutputLabel) Paths(graph *BuildGraph) []string {
return addPathPrefix(graph.TargetOrDie(label.BuildLabel).NamedOutputs(label.Output), label.PackageName)
func (label AnnotatedOutputLabel) Paths(graph *BuildGraph) []string {
target := graph.TargetOrDie(label.BuildLabel)
if _, ok := target.EntryPoints[label.Annotation]; ok {
return label.BuildLabel.Paths(graph)
}
return addPathPrefix(target.NamedOutputs(label.Annotation), label.PackageName)
}

// FullPaths is like Paths but includes the leading plz-out/gen directory.
func (label NamedOutputLabel) FullPaths(graph *BuildGraph) []string {
func (label AnnotatedOutputLabel) FullPaths(graph *BuildGraph) []string {
target := graph.TargetOrDie(label.BuildLabel)
return addPathPrefix(target.NamedOutputs(label.Output), target.OutDir())
if _, ok := target.EntryPoints[label.Annotation]; ok {
return label.BuildLabel.FullPaths(graph)
}
return addPathPrefix(target.NamedOutputs(label.Annotation), target.OutDir())
}

// LocalPaths returns paths within the local package
func (label NamedOutputLabel) LocalPaths(graph *BuildGraph) []string {
return graph.TargetOrDie(label.BuildLabel).NamedOutputs(label.Output)
func (label AnnotatedOutputLabel) LocalPaths(graph *BuildGraph) []string {
target := graph.TargetOrDie(label.BuildLabel)
if _, ok := target.EntryPoints[label.Annotation]; ok {
return label.BuildLabel.LocalPaths(graph)
}
return target.NamedOutputs(label.Annotation)
}

// Label returns the build rule associated with this input. For a NamedOutputLabel it's always non-nil.
func (label NamedOutputLabel) Label() *BuildLabel {
// Label returns the build rule associated with this input. For a AnnotatedOutputLabel it's always non-nil.
func (label AnnotatedOutputLabel) Label() *BuildLabel {
return &label.BuildLabel
}

func (label NamedOutputLabel) nonOutputLabel() *BuildLabel {
func (label AnnotatedOutputLabel) nonOutputLabel() *BuildLabel {
return nil
}

// String returns a string representation of this input.
func (label NamedOutputLabel) String() string {
return label.BuildLabel.String() + "|" + label.Output
func (label AnnotatedOutputLabel) String() string {
return label.BuildLabel.String() + "|" + label.Annotation
}

// MustParseNamedOutputLabel attempts to parse a build output label. It's allowed to just be
Expand All @@ -236,7 +248,7 @@ func (label NamedOutputLabel) String() string {
func MustParseNamedOutputLabel(target string, pkg *Package) BuildInput {
if index := strings.IndexRune(target, '|'); index != -1 && index != len(target)-1 {
label := ParseBuildLabelContext(target[:index], pkg)
return NamedOutputLabel{BuildLabel: label, Output: target[index+1:]}
return AnnotatedOutputLabel{BuildLabel: label, Annotation: target[index+1:]}
}
return ParseBuildLabelContext(target, pkg)
}
Expand Down
14 changes: 7 additions & 7 deletions src/core/build_input_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,27 @@ import (
func TestParseNamedOutputLabel(t *testing.T) {
pkg := NewPackage("")
input := MustParseNamedOutputLabel("//src/core:target1|test", pkg)
label, ok := input.(NamedOutputLabel)
label, ok := input.(AnnotatedOutputLabel)
assert.True(t, ok)
assert.Equal(t, "src/core", label.PackageName)
assert.Equal(t, "target1", label.Name)
assert.Equal(t, "test", label.Output)
assert.Equal(t, "test", label.Annotation)
}

func TestParseNamedOutputLabelRelative(t *testing.T) {
pkg := NewPackage("src/core")
input := MustParseNamedOutputLabel(":target1|test", pkg)
label, ok := input.(NamedOutputLabel)
label, ok := input.(AnnotatedOutputLabel)
assert.True(t, ok)
assert.Equal(t, "src/core", label.PackageName)
assert.Equal(t, "target1", label.Name)
assert.Equal(t, "test", label.Output)
assert.Equal(t, "test", label.Annotation)
}

func TestParseNamedOutputLabelNoOutput(t *testing.T) {
pkg := NewPackage("")
input := MustParseNamedOutputLabel("//src/core:target1", pkg)
_, ok := input.(NamedOutputLabel)
_, ok := input.(AnnotatedOutputLabel)
assert.False(t, ok)
label, ok := input.(BuildLabel)
assert.True(t, ok)
Expand All @@ -47,7 +47,7 @@ func TestParseNamedOutputLabelEmptyOutput(t *testing.T) {
func TestParseNamedOutputLabelSubrepo(t *testing.T) {
pkg := NewPackage("")
input := MustParseNamedOutputLabel("@test_x86//src/core:target1", pkg)
_, ok := input.(NamedOutputLabel)
_, ok := input.(AnnotatedOutputLabel)
assert.False(t, ok)
label, ok := input.(BuildLabel)
assert.True(t, ok)
Expand All @@ -59,7 +59,7 @@ func TestParseNamedOutputLabelSubrepo(t *testing.T) {
func TestParseNamedOutputLabelRelativeSubrepo(t *testing.T) {
pkg := NewPackage("src/core")
input := MustParseNamedOutputLabel("@test_x86:target1", pkg)
_, ok := input.(NamedOutputLabel)
_, ok := input.(AnnotatedOutputLabel)
assert.False(t, ok)
label, ok := input.(BuildLabel)
assert.True(t, ok)
Expand Down
39 changes: 25 additions & 14 deletions src/core/build_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ type BuildTarget struct {
OutputDirectories []OutputDirectory `name:"output_dirs"`
// RuleMetadata is the metadata attached to this build rule. It can be accessed through the "get_rule_metadata" BIF.
RuleMetadata interface{} `name:"config"`
// EntryPoints represent named binaries within the rules output that can be targeted via //package:rule|entry_point_name
EntryPoints map[string]string `name:"entry_points"`
}

// BuildMetadata is temporary metadata that's stored around a build target - we don't
Expand Down Expand Up @@ -259,9 +261,6 @@ func (o OutputDirectory) Dir() string {
// individually i.e. out_dir/net/thoughtmachine/Main.java -> net/thoughtmachine/Main.java. If this is false then these
// files would be included as out_dir/net/thoughtmachine/Main.java -> net.
func (o OutputDirectory) ShouldAddFiles() bool {
// TODO(jpoole): consider if we should have full glob matching for the suffix so we can do stuff like **.java
// or *_test.go. This will prove difficult for rex where we only have the file names rather than the actual
// directory
return strings.HasSuffix(string(o), "/**")
}

Expand Down Expand Up @@ -566,10 +565,10 @@ func (target *BuildTarget) Outputs() []string {
ret = make([]string, 0, len(target.Sources))
// Filegroups just re-output their inputs.
for _, src := range target.Sources {
if namedLabel, ok := src.(NamedOutputLabel); ok {
if namedLabel, ok := src.(AnnotatedOutputLabel); ok {
// Bit of a hack, but this needs different treatment from either of the others.
for _, dep := range target.DependenciesFor(namedLabel.BuildLabel) {
ret = append(ret, dep.NamedOutputs(namedLabel.Output)...)
ret = append(ret, dep.NamedOutputs(namedLabel.Annotation)...)
}
} else if label := src.nonOutputLabel(); label == nil {
ret = append(ret, src.LocalPaths(nil)[0])
Expand Down Expand Up @@ -1350,17 +1349,29 @@ func (target *BuildTarget) IsTool(tool BuildLabel) bool {
}

// toolPath returns a path to this target when used as a tool.
func (target *BuildTarget) toolPath(abs bool) string {
outputs := target.Outputs()
ret := make([]string, len(outputs))
for i, o := range outputs {
if abs {
ret[i] = path.Join(RepoRoot, target.OutDir(), o)
} else {
ret[i] = path.Join(target.Label.PackageName, o)
func (target *BuildTarget) toolPath(abs bool, namedOutput string) string {
outToolPath := func(outputs ...string) string {
ret := make([]string, len(outputs))
for i, o := range outputs {
if abs {
ret[i] = path.Join(RepoRoot, target.OutDir(), o)
} else {
ret[i] = path.Join(target.Label.PackageName, o)
}
}
return strings.Join(ret, " ")
}

if namedOutput != "" {
if o, ok := target.EntryPoints[namedOutput]; ok {
return outToolPath(o)
}
if outs, ok := target.namedOutputs[namedOutput]; ok {
return outToolPath(outs...)
}
panic(fmt.Sprintf("%v has no named output or entry point %v", target.Label, namedOutput))
}
return strings.Join(ret, " ")
return outToolPath(target.Outputs()...)
}

// AddOutput adds a new output to the target if it's not already there.
Expand Down
16 changes: 14 additions & 2 deletions src/core/build_target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,20 @@ func TestToolPath(t *testing.T) {
wd, _ := os.Getwd()
RepoRoot = wd
root := wd + "/plz-out/gen/src/core"
assert.Equal(t, fmt.Sprintf("%s/file1.go %s/file2.go", root, root), target.toolPath(true))
assert.Equal(t, "src/core/file1.go src/core/file2.go", target.toolPath(false))
assert.Equal(t, fmt.Sprintf("%s/file1.go %s/file2.go", root, root), target.toolPath(true, ""))
assert.Equal(t, "src/core/file1.go src/core/file2.go", target.toolPath(false, ""))
}

func TestToolPathWithEntryPoint(t *testing.T) {
target := makeTarget1("//src/core:target1", "")
target.AddOutput("file1.go")
target.AddOutput("file2.go")
target.EntryPoints = map[string]string{"f1": "file1.go"}
wd, _ := os.Getwd()
RepoRoot = wd
root := wd + "/plz-out/gen/src/core"
assert.Equal(t, fmt.Sprintf("%s/file1.go", root), target.toolPath(true, "f1"))
assert.Equal(t, "src/core/file1.go", target.toolPath(false, "f1"))
}

func TestDependencies(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions src/parse/asp/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,9 @@ func addOut(s *scope, args []pyObject) pyObject {
target.AddOutput(name)
s.pkg.MustRegisterOutput(name, target)
} else {
_, ok := target.EntryPoints[name]
s.NAssert(ok, "Named outputs can't have the same name as entry points")

target.AddNamedOutput(name, out)
s.pkg.MustRegisterOutput(out, target)
}
Expand Down
19 changes: 18 additions & 1 deletion src/parse/asp/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const (
outDirsBuildRuleArgIdx
configBuildRuleArgIdx
exitOnErrorArgIdx
entryPointsArgIdx
)

// createTarget creates a new build target as part of build_rule().
Expand Down Expand Up @@ -99,7 +100,6 @@ func createTarget(s *scope, args []pyObject) *core.BuildTarget {
target.Local = isTruthy(localBuildRuleArgIdx)
target.ExitOnError = isTruthy(exitOnErrorArgIdx)
target.RuleMetadata = args[configBuildRuleArgIdx]

for _, o := range asStringList(s, args[outDirsBuildRuleArgIdx], "output_dirs") {
target.AddOutputDirectory(o)
}
Expand Down Expand Up @@ -223,6 +223,7 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
addStrings(s, "visibility", args[visibilityBuildRuleArgIdx], func(str string) {
t.Visibility = append(t.Visibility, parseVisibility(s, str))
})
addEntryPoints(s, args[entryPointsArgIdx], t)
addMaybeNamedSecret(s, "secrets", args[secretsBuildRuleArgIdx], t.AddSecret, t.AddNamedSecret, t, true)
addProvides(s, "provides", args[providesBuildRuleArgIdx], t)
if f := callbackFunction(s, "pre_build", args[preBuildBuildRuleArgIdx], 1, "argument"); f != nil {
Expand All @@ -233,6 +234,22 @@ func populateTarget(s *scope, t *core.BuildTarget, args []pyObject) {
}
}

// addEntryPoints adds entry points to a target
func addEntryPoints(s *scope, arg pyObject, target *core.BuildTarget) {
entryPointsPy, ok := asDict(arg)
s.Assert(ok, "entry_points must be a dict")

entryPoints := make(map[string]string, len(entryPointsPy))
for name, entryPointPy := range entryPointsPy {
entryPoint, ok := entryPointPy.(pyString)
s.Assert(ok, "Values of entry_points must be strings, found %v at key %v", entryPointPy.Type(), name)
s.Assert(target.NamedOutputs(entryPoint.String()) == nil, "Entry points can't have the same name as a named output")
entryPoints[name] = string(entryPoint)
}

target.EntryPoints = entryPoints
}

// addMaybeNamed adds inputs to a target, possibly in named groups.
func addMaybeNamed(s *scope, name string, obj pyObject, anon func(core.BuildInput), named func(string, core.BuildInput), systemAllowed, tool bool) {
if obj == nil {
Expand Down
4 changes: 2 additions & 2 deletions src/parse/parse_step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ func addDeps(graph *core.BuildGraph, pkg *core.Package) {

func assertPendingParses(t *testing.T, state *core.BuildState, targets ...string) {
parses, _ := getAllPending(state)
assert.EqualValues(t, targets, parses)
assert.ElementsMatch(t, targets, parses)
}

func assertPendingBuilds(t *testing.T, state *core.BuildState, targets ...string) {
_, builds := getAllPending(state)
assert.EqualValues(t, targets, builds)
assert.ElementsMatch(t, targets, builds)
}

func getAllPending(state *core.BuildState) ([]string, []string) {
Expand Down
Loading