Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Makefile: fix broken master build #22236

Merged
merged 2 commits into from
Jan 31, 2018
Merged

Makefile: fix broken master build #22236

merged 2 commits into from
Jan 31, 2018

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Jan 31, 2018

See individual commits for details.

@benesch
Makefile: define CPP_PROTOS[_CCL]_TARGET before its use
9ca1b66 
libroach and libroachccl were entirely failing to express a dependency
on CPP_PROTOS_TARGET and CPP_PROTOS_CPP_TARGET, as neither of those
variables were defined at their point of use. Hoist those variables
higher in the Makefile.

This will actually fix the failing builds that cockroachdb#22231 attempted to
address.

Release note: None
@benesch
Makefile: fix cpp proto cleanup
66c05c8 
The former find invocations were only operating inside ./pkg, but the
CPP protos are generated into c-deps/protos and c-deps/protoccl. Since
they have dedicated directories, just nuke those directories instead of
mucking around with find.

Release note: None
@benesch benesch requested a review from a team January 31, 2018 00:24
knz
knz approved these changes Jan 31, 2018
View reviewed changes
Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

Thank you!
Although I still wonder why we prefer := to = in vars.

@benesch
Copy link
Contributor Author

benesch commented Jan 31, 2018

Thanks, @knz! For posterity, from Slack:

Stuff that uses $(shell) or other expensive functions needs to be careful to use := to avoid repeated invocations, which is I think where the ban on = originated.

@benesch benesch merged commit 0789070 into cockroachdb:master Jan 31, 2018
@benesch benesch deleted the make-undefined branch January 31, 2018 00:56
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knz knz knz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benesch @cockroach-teamcity @knz