Skip to content
This repository has been archived by the owner on May 6, 2021. It is now read-only.

Add common implicit deps to Rules #78

Merged
merged 1 commit into from Nov 18, 2015
Merged

Add common implicit deps to Rules #78

merged 1 commit into from Nov 18, 2015

Conversation

danw
Copy link
Contributor

@danw danw commented Nov 18, 2015

For implicit dependencies that will be common to all users of a Rule,
add a new field 'CommandDeps' to the RuleParam. This is a list of
strings to be prepended to the implicit dependencies in each BuildParam.

This lets us have the dependencies declared next to where they are used,
instead of duplicated in areas that may be far apart.

I looked at passing this information down to ninja too, but it only
saves us a few percent of ninja file, and requires a modification to the
ninja file format.

@danw
Copy link
Contributor Author

danw commented Nov 18, 2015

Hmm, I'm actually seeing some failures with the latest patch. I'll reopen once I've fixed them.

@danw danw closed this Nov 18, 2015
For implicit dependencies that will be common to all users of a Rule,
add a new field 'CommandDeps' to the RuleParam. This is a list of
strings to be prepended to the implicit dependencies in each BuildParam.

This lets us have the dependencies declared next to where they are used,
instead of duplicated in areas that may be far apart.

I looked at passing this information down to ninja too, but it only
saves us a few percent of ninja file, and requires a modification to the
ninja file format.

Change-Id: Ifd910dee1506d4e32a76ed06206f853c4caec622
@danw
Copy link
Contributor Author

danw commented Nov 18, 2015

Fixed (variable scoping was wrong in live_tracker.go)

@colincross
Copy link
Contributor

LGTM

danw added a commit that referenced this pull request Nov 18, 2015
Add common implicit deps to Rules
@danw danw merged commit e600636 into google:master Nov 18, 2015
@danw danw deleted the ruledeps branch November 18, 2015 01:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants