emscripten: use upstream recommended llvm revision #63183
Conversation
BrewTestBot
added
nodejs
|
Anyone have any ideas on this High Sierra failure during the |
alebcay
force-pushed
the
emscripten-track-llvm
branch
2 times, most recently
from
3463732
to
f4421b4
Compare
jonchang
force-pushed
the
emscripten-track-llvm
branch
from
f4421b4
to
168f18b
Compare
|
Current failure on Big Sur: |
|
Quick question, in case you would know (no worries if not, I'll just have a look myself later on instead): Does emscripten really need such a fully-featured installation of llvm to work? The build docs only recommend building the following targets Their docs on packaging require the following binaries:
I don't think you need As it is the emscripten package takes up 2GB of space, and I imagine a lot of that is unused. Building more targets than you need to also increases the probability of build failures. (Not sure if that's the culprit here, but could be worth a look.) Ok, I guess that wasn't as quick as I thought. Sorry. |
alebcay
force-pushed
the
emscripten-track-llvm
branch
from
168f18b
to
ad16523
Compare
|
I think you raise a fair point. Seeing as the error encountered is while compiling the |
|
No worries. I know how difficult it can be trying to get llvm to build (I must've done so nearly a dozen times in the past week trying to implement an improvement to the llvm formula). Do you want more input on other things I think you can cut out? I understand if you'd rather not -- I'll just pursue cutting down the installation size in another PR when this one is merged. But I do think there's still some low-hanging fruit. Also, are you sure any of those runtimes are even necessary? My impression was they're not, and it seems dropping |
|
Well, I'll be darned. Looks like everything's working now. This one has been sitting around for a while, so I'd kindly ask that we address further improvements in a new PR. |
|
🤖 A scheduled task has triggered a merge. |
|
Sure, no problem. Leave it with me. |
The emscripten depends on a non-stable build of llvm, and is therefore packaged with with the required llvm build. This commit does the following: - removes unnecessary build targets in the llvm project - adds a test that the llvm dependencies were in fact built - declares a dependency on libffi [*] - makes llvm build commands consistent with llvm formula Until Homebrew#63183, the emscripten installation used nearly 2GB of storage space. Some improvements were made in that PR, and this commit is meant to follow through on that. [*] The arguments to cmake already previously referred to the libffi formula. It wasn't declared as a dependency, so the llvm install linked with the built-in libffi.
* emscripten: cleanup formula The emscripten depends on a non-stable build of llvm, and is therefore packaged with with the required llvm build. This commit does the following: - removes unnecessary build targets in the llvm project - adds a test that the llvm dependencies were in fact built - declares a dependency on libffi [*] - makes llvm build commands consistent with llvm formula Until #63183, the emscripten installation used nearly 2GB of storage space. Some improvements were made in that PR, and this commit is meant to follow through on that. [*] The arguments to cmake already previously referred to the libffi formula. It wasn't declared as a dependency, so the llvm install linked with the built-in libffi. * Fix build * Minimise llvm build size The added cmake flags are documented here: https://llvm.org/docs/BuildingADistribution.html#options-for-reducing-size * Remove llvm tests They keep failing for some reason, even if it appears llvm built properly. Not really sure what's going on. Closes #65799. Signed-off-by: Sean Molenaar <[email protected]> Signed-off-by: BrewTestBot <[email protected]>
- bumped emscripten version - changed llvm revision according to (see formula for details): - https://github.com/emscripten-core/emsdk/blob/master/emscripten-releases-tags.txt - https://chromium.googlesource.com/emscripten-releases/+/37fc7647c754ac9a28ad588c143b82286de0ef71/DEPS - use recommended binaryen revision - The same has been done with llvm. See: Homebrew#63183 - updated license to reflect bundled llvm and binaryen
- bumped emscripten version - changed llvm revision according to (see formula for details): - https://github.com/emscripten-core/emsdk/blob/master/emscripten-releases-tags.txt - https://chromium.googlesource.com/emscripten-releases/+/37fc7647c754ac9a28ad588c143b82286de0ef71/DEPS - use recommended binaryen revision - The same has been done with llvm. See: #63183 - updated license to reflect bundled llvm and binaryen Closes #66150. Signed-off-by: FX Coudert <[email protected]> Signed-off-by: BrewTestBot <[email protected]>
brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew test <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingbrew install <formula>)?See #63176, emscripten-ports/SDL2_mixer#2, emscripten-core/emscripten#12551. This should hopefully fix all of them.