Skip to content

Conversation

@staticfloat
Copy link
Member

Many users have expressed the frustration that setting USE_BINARYBUILDER=0 in Make.user doesn't take; this is because the initializations here should have been ?= (e.g. "set unless already set") instead of := (e.g. "set immediately"). This remedies that.

Many users have expressed the frustration that setting `USE_BINARYBUILDER=0` in `Make.user` doesn't take; this is because the initializations here should have been `?=` (e.g. "set unless already set") instead of `:=` (e.g. "set immediately").  This remedies that.
Co-Authored-By: Valentin Churavy <vchuravy@users.noreply.github.com>
$(foreach proj,$(BB_PROJECTS),$(if $(USE_BINARYBUILDER_$(proj)),,$(eval USE_BINARYBUILDER_$(proj) ?= $(USE_BINARYBUILDER))))

# Use the Assertions build
BINARYBUILDER_LLVM_ASSERTS := 0
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but shouldn't this just look at the LLVM_ASSERTIONS variable?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably? @vchuravy is that just a silly oversight on my part?

Copy link
Member

Choose a reason for hiding this comment

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

I originally had it split out to make switching more explicit. No strong feelings on my part.

We don't actually have an asserts build to download (yet)
@StefanKarpinski
Copy link
Member

I've thought this would be good to change for a long time. Would also be good to change any other variables that can be set in Make.user to configure the build process so that override isn't needed.

@staticfloat staticfloat merged commit 140d3ba into master May 16, 2019
@staticfloat
Copy link
Member Author

Will address the LLVM_ASSETS stuff in a future PR.

@StefanKarpinski StefanKarpinski deleted the sf/bb_makevar_gentleness branch May 16, 2019 20:43
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.

5 participants