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.

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

Makefile: need better bootstrap regen detection #52477

Closed
maddyblue opened this issue Aug 6, 2020 · 2 comments
Closed

Makefile: need better bootstrap regen detection #52477

maddyblue opened this issue Aug 6, 2020 · 2 comments
Labels
A-build-system C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-dev-inf

Comments

@maddyblue
Copy link
Contributor

maddyblue commented Aug 6, 2020

The recent addition of protoc-gen-doc (#51803) caused people to have problems building:

./bin/protoc-gen-doc: program not found or is not executable

Running rm bin/.bootstrap fixes this. We need some better detection to regenerate .bootstrap when there are new programs added.

Epic CRDB-10447

Jira issue: CRDB-3944

@maddyblue maddyblue added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 6, 2020
@knz
Copy link
Contributor

knz commented Aug 6, 2020

The problem here is twofold:

  • the various utilities in bin/ do not depend on the .bootstrap target
  • the various uses of the utilities in bin/ do not depend on the executable in bin/ and thus not on .bootstrap.

So if we introduce a new utility and just change the action for .bootstrap, "nothing happen" and the uses of the new utility fail until the user explicitly refreshes .bootstrap.

IMHO it would be time-consuming and error-prone to establish all these dependencies at a fine grain. I have a better suggestion:

  • extract the rule that builds .bootstrap into a separate makefile bootstrap.mk
  • make the .bootstrap depend on boostrap.mk
  • make the various uses of utilities depend on .bootstrap

This way if we ever modify bootstrap.mk to add a new go install line, that will force a refresh of .bootstrap upon first use of a utility.

@jordanlewis
Copy link
Member

No longer relevant given bazel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-dev-inf
Projects
None yet
Development

No branches or pull requests

4 participants