Skip to content

Implement entry points for tools#1243

Merged
Tatskaari merged 10 commits into
thought-machine:masterfrom
Tatskaari:entry-point-tools
Sep 30, 2020
Merged

Implement entry points for tools#1243
Tatskaari merged 10 commits into
thought-machine:masterfrom
Tatskaari:entry-point-tools

Conversation

@Tatskaari
Copy link
Copy Markdown
Contributor

@Tatskaari Tatskaari commented Sep 24, 2020

I think this is about ready to be merged. There're a few things that will have to follow:

  • Validate that the rule generated the entry points as output
  • Make plz run entry point aware

@sagikazarmark
Copy link
Copy Markdown
Contributor

I'm trying to understand what this does, but I'm completely lost TBH.

@Tatskaari
Copy link
Copy Markdown
Contributor Author

Tatskaari commented Sep 25, 2020

I'm trying to understand what this does, but I'm completely lost TBH.

I'm trying to make it so you can define a file within a rule as the "entry point" to that rule when you run it.

Say you have the compiled go SDK that looks like:

$GOROOT/
- bin/
   - fmt
   - go
- pkg/...

This will allow you to add

entry_points = {
   "go": "bin/go",
   "fmt": "bin/fmt",
}

to your rule and then add:

tools = {
  "GO": "//tools:go_sdk|go",
  "LINT": "//tools:go_sdk|fmt",
}

This will make the whole of //tools:go_sdk a dependency of your rule but set $TOOL_{GO/FMT} to the correct value.

@sagikazarmark
Copy link
Copy Markdown
Contributor

Interesting, thanks for the explanation!

Comment thread src/build/build_step.go
}
}
// 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.

Comment thread src/core/build_target.go Outdated
// 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
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.

add the name:"entry_points" tag, that will make plz query print show it nicely

Comment thread src/remote/action.go Outdated
// ActionResult. Servers do not necessarily verify this but we need to make sure they are
// complete for future requests.
func (c *Client) verifyActionResult(target *core.BuildTarget, command *pb.Command, actionDigest *pb.Digest, ar *pb.ActionResult, verifyOutputs, isTest bool) error {
func (c *Client) verifyActionResult(target *core.BuildTarget, command *pb.Command, actionDigest *pb.Digest, ar *pb.ActionResult, verifyRemoteBlobsExists, isTest bool) error {
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.

nit: verifyRemoteBlobsExist

Comment thread test/build_defs/BUILD Outdated
)

filegroup(
name = "free_bsd_config",
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.

nit: freebsd_config

@Tatskaari Tatskaari force-pushed the master branch 3 times, most recently from 5e24a0d to e5a2b6d Compare September 30, 2020 10:33
@Tatskaari Tatskaari merged commit a01f61b into thought-machine:master Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants