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

fix nodejs build on v16 #1048

Closed
wants to merge 1 commit into from
Closed

fix nodejs build on v16 #1048

wants to merge 1 commit into from

Conversation

p3x-robot
Copy link
Contributor

No description provided.

@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

Hmmm. I'll try to test if there is a bug in this patch by changing the path to an insanely long one.

Please wait for a while.

@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

I have tested it and the existing patch builds fine even with unusually long paths, e.g., unusually long paths that cause "execvp: printf: Argument list too long".

Could you please clarify what your patch is intended to do?

@p3x-robot
Copy link
Contributor Author

i did not have a "execvp: printf: Argument list too long", i had a "/bin/sh: printf: Argument list too long" error in docker.

@p3x-robot
Copy link
Contributor Author

@p3x-robot
Copy link
Contributor Author

p3x-robot commented Oct 29, 2021

i have ulimit unlimited, but it still giving this error.

@p3x-robot
Copy link
Contributor Author

i experienced from yesterday:
#1046 (comment)

now with this latest argument too long patch is working as it is:
https://cdn.corifeus.com/openwrt/21.02.1/packages/arm_cortex-a9_vfpv3-d16/node/

@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

You need to distinguish between events.

  • Related to the version of binutils
  • Related to long path to build

This pull request is about a long path, which is not a problem at the moment.

The event related to binutils is that there are not enough file descriptors, so we need to use ulimit to increase the openfile descriptor.

ulimit -n 2048

https://github.com/patrikx3/openwrt-insomnia/blob/master/make-scripts/openwrt-insomnia-lib#L71

You probably need to write 'ulimit -n 2048' in this script, or there may be a docker limitation, so you should check the docker limitation.

@p3x-robot
Copy link
Contributor Author

it's ulimit is already unlimited. and sill giving this error.

@p3x-robot
Copy link
Contributor Author

@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

ulimit -n 2048

You'll need "-n" as well as "-s."

@p3x-robot
Copy link
Contributor Author

ulimit -n 2048

You'll need "-n" as well as "-s."

never mind...

@p3x-robot p3x-robot closed this Oct 29, 2021
@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

Stay calm and try to isolate the event.

Where is this error that you have just submitted?

"/bin/sh: printf: Argument list too long"

Is it possible to debug your script? If you can, try to find out which step it is coming from.

@p3x-robot
Copy link
Contributor Author

p3x-robot commented Oct 29, 2021

it is working any way, have been working non stop for 2 days to build this and finally it works. i am done with this build.
last time, i try without the patch and increase the --ulimit nofile=65536:65536 as you said. i hope that will work.

what do you debug my script? it is a simple make all... there is nothing to debug.

@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

what do you debug my script?

You'll find out :)

If a problem arises, dig deeper so that you can get to the problem. I'm a newbie to Docker. :)

@p3x-robot
Copy link
Contributor Author

what do you debug my script?

You'll find out :)

If a problem arises, dig deeper so that you can get to the problem. I'm a newbie to Docker. :)

nodejs/node#9137 (comment)

@nxhack
Copy link
Owner

nxhack commented Oct 29, 2021

nodejs/node#9137 (comment)

I am aware that this issue has already been resolved in this repository.

@p3x-robot
Copy link
Contributor Author

it is building right now. and you are right. it is not the ulimit -s but the ulimit -n giving this error.
i am patching in docker. it is building. it will take an hour to get the result.

@p3x-robot
Copy link
Contributor Author

i increased to --ulimit nofile=65535:65535

@p3x-robot p3x-robot reopened this Oct 29, 2021
@p3x-robot
Copy link
Contributor Author

i increased to --ulimit nofile=65535:65535

this is not working, i am done with this. that patch works as yesterday...

@nxhack
Copy link
Owner

nxhack commented Oct 31, 2021

SUMMARY

After careful review of this pull request, it is clear that only the following points have been changed.

 define xargs
-$(1) $(wordlist 1,1000,$(2))
-$(if $(word 1001,$(2)),$(call xargs,$(1),$(wordlist 1001,$(words $(2)),$(2))))))
+$(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
 endef

The build bug is the point that is fixed here. The build bug is fixed here, i.e., the argument is changed to one instead of 1000 units.

I agree that 1000 units is too large, considering what happened.
I've already completed the merge, with changes to improve stability, based on @riedonetworks' work on meta-nodejs.

https://github.com/riedonetworks/meta-nodejs/blob/dora/recipes/nodejs/files/0001-fix-execvp-printf-argument-list-too-long.patch

I'll close this pull request once it's done.

@nxhack nxhack closed this Oct 31, 2021
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.

2 participants