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

Reached heap limit Allocation on webpack build #183

Closed
wants to merge 1 commit into from

Conversation

alebo-iX
Copy link

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
@maurice-audin
Copy link

+1

Build on CI is broken because of this since v2024.5.0

@BlackDex
Copy link
Collaborator

BlackDex commented Oct 3, 2024

Our CI works just fine: https://github.com/dani-garcia/bw_web_builds/actions/workflows/release.yml

Using the docker build also works just fine.
My understanding is that some systems need this because either there systems does not have a lot of memory available, or the node version is a bit older and has some compatibility issues with the system it is running on.

If this is within a CI, you can of course also just add that variable to your CI env's i would imagine.
It might have adverse effects maybe on systems where it runs fine without this option set.

@maurice-audin
Copy link

If this is within a CI, you can of course also just add that variable to your CI env's i would imagine. It might have adverse effects maybe on systems where it runs fine without this option set.

Environment is not used in the docker build

@BlackDex
Copy link
Collaborator

BlackDex commented Oct 3, 2024

My main issue is, that this setting could cause issues on systems which have less memory, where for some reason the build works just fine. Adding this by default might cause issues there.

I can think of other ways to solve this though, but that would mean that we would add something like a .env_build or something which we then load in the needed scripts. That way it is more flexible.

@maurice-audin
Copy link

A single line change would be in the Makefile:

$ git diff Makefile
--- a/Makefile
+++ b/Makefile
@@ -60,7 +60,7 @@ full: checkout patch-web-vault build tar
 .PHONY: full
 
 container:
- ${CONTAINER_CMD} build -t bw_web_vault .
+ ${CONTAINER_CMD} build ${CONTAINER_BUILD_ARGS} -t bw_web_vault .
 .PHONY: container
 
 container-extract: container

We could then set the variable CONTAINER_BUILD_ARGS to "--build-arg NODE_OPTIONS=--max-old-space-size=4096" in CI environment and behavior would stay the same with an empty variable.

@BlackDex
Copy link
Collaborator

BlackDex commented Oct 7, 2024

Using a build_env file is more robust I think.

BlackDex added a commit to BlackDex/bw_web_builds that referenced this pull request Nov 4, 2024
Sometimes you want to set specific environment variables during build.
For example, on some systems nodejs needs some extra options to be able to run correctly on low memory systems.
To make sure this will be loaded and works on both scripts and Dockerfile, you can now create a `.build_env` file.
This `.build_env` file should contain all the variables (including an export command) you want to expose during build time.

The template file has a nodejs example.

Closes dani-garcia#183

Signed-off-by: BlackDex <[email protected]>
@BlackDex BlackDex mentioned this pull request Nov 4, 2024
BlackDex added a commit to BlackDex/bw_web_builds that referenced this pull request Nov 4, 2024
Sometimes you want to set specific environment variables during build.
For example, on some systems nodejs needs some extra options to be able to run correctly on low memory systems.
To make sure this will be loaded and works on both scripts and Dockerfile, you can now create a `.build_env` file.
This `.build_env` file should contain all the variables (including an export command) you want to expose during build time.

The template file has a nodejs example.

Closes dani-garcia#183

Signed-off-by: BlackDex <[email protected]>
@BlackDex
Copy link
Collaborator

BlackDex commented Nov 4, 2024

@alebo-iX & @maurice-audin I have created a PR which should solve this issue in a more flexible way, instead of forcing the memory option on all builds, or add complex and error prone variables which might be empty or not, or contain globs etc...

See #184

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.

3 participants