neocturne commented 7 years ago

Version: master, v4.4.5 (I think all supported versions are affected)
Platform: Arch Linux (x86_64), Debian 8, probably at least all Linux?
Subsystem: Build

Trying to build nodejs fails with the message

execvp: printf: Argument list too long

when the path to build directory is too long. We've hit this issue in OpenWrt/LEDE, but googling has turned up a lot of other people with the same issue.

I've found this patch to circumvent the issue in the OpenEmbedded patchwork: https://patchwork.openembedded.org/patch/72811/

Unfortunately, there is no further discussion and I have no idea why it is marked as superseded. I don't know the gyp code at all, so I can't tell if the patch is okay like this, but unconditionally using the filename ar-file-list seems a bit weird to me...

bnoordhuis commented 7 years ago

The patch looks okay to me. Maybe not great but it's not terrible either. We would have rejected it in the past due to our 'upstream first' policy but in the case of gyp that has been relaxed somewhat since upstream has largely abandoned it, effectively making us the maintainers.

You should file a pull request if you want to see the patch merged. I can't guarantee it actually gets merged but we will at least consider it, it won't be rejected outright.

I wonder though, what does ulimit -s print on the system where you hit that issue? The size of the argument list is a function of the stack size. Increase the stack size enough and the problem should go away.

neocturne commented 7 years ago

ulimit -s is 8192 on my system, I don't think larger values are common. As it is execvp that fails, I think it rather hits ARG_MAX, which is 2MB on Linux and can't be changed.

Should I open a PR against the node repo, node-gyp, or both?

bnoordhuis commented 7 years ago

I think it rather hits ARG_MAX, which is 2MB on Linux and can't be changed.

That's only with really old kernels, though. On non-ancient kernels it's capped at 1/4th of the stack size.

$ ulimit -s
8192
$ strace -s10 -fe execve out/Release/node -e 'for (var a = [], i = 0; i < 16; ++i) a.push("x".repeat(0x1FFFF)); child_process.spawnSync("/bin/echo", a, {stdio:"ignore"})' 2>&1 | grep bin/echo
[pid 26094] execve("/bin/echo", ["/bin/echo", "xxxxxxxxxx"..., "xxxxxxxxxx"..., "xxxxxxxxxx"..., "xxxxxxxxxx"..., ...], [/* 35 vars */])
  = -1 E2BIG (Argument list too long)
$ ulimit -s 32768
$ strace -s10 -fe execve out/Release/node -e 'for (var a = [], i = 0; i < 16; ++i) a.push("x".repeat(0x1FFFF)); child_process.spawnSync("/bin/echo", a, {stdio:"ignore"})' 2>&1 | grep bin/echo
[pid 26104] execve("/bin/echo", ["/bin/echo", "xxxxxxxxxx"..., "xxxxxxxxxx"..., "xxxxxxxxxx"..., "xxxxxxxxxx"..., ...], [/* 35 vars */])
  = 0

The length of a single argument is restricted to 128 kB but that shouldn't be a problem because you're never going to have a path that long.

Should I open a PR against the node repo, node-gyp, or both?

node

neocturne commented 7 years ago

Unfortunately, the patch I found doesn't work after all. I'll send PR as soon as I've found a working solution.

armandciejak commented 7 years ago

I've reworked the patch from OpenEmbedded for my own needs and it works fine with version 4.4.3. It is available here: https://github.com/riedonetworks/meta-nodejs/commits/dora/recipes/nodejs/files/0001-fix-execvp-printf-argument-list-too-long.patch

Trott commented 7 years ago

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

brianjmurrell commented 7 years ago

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed.

But is it fixed? "inactive" could just mean that nobody with the ability to do anything about the issue is willing to do anything. That doesn't mean it's actually fixed though.

Trott commented 7 years ago

But is it fixed?

No, it is not fixed.

"inactive" could just mean that nobody with the ability to do anything about the issue is willing to do anything.

In this case, I think it means exactly that along with:

The bug might reasonably be considered less a Node.js bug and more of a build system setup and/or OS tuning issue.
While it's possible to fix, the costs involved (such as maintenance of extra code) may not be worth the benefit of fixing a sorta kinda not-a-bug issue.

That doesn't mean it's actually fixed though.

That's correct. Believe me, if I could have closed this with "Fixed!" instead of "This issue has been inactive," I totally would have done that. I promise! 😆

Mostly what this needs is someone with the interest and the time to write a fix that will not be a maintenance burden or code smell. I suspect if you or anyone else were inclined to give it a try, we can find an experienced contributor on the project (perhaps @bnoordhuis?) who would be willing to answer questions that arise, review code after its written, and generally provide reasonable amounts of assistance.

Since obviously this is still causing problems for some small-but-very-real number of users, I'll re-open it and add a help wanted label.

bnoordhuis commented 7 years ago

9141 shows fixing this in GYP is not exactly trivial. A one-liner that sets`ulimit -s` to some high value is probably more palatable.

That said, if you hit this issue while building node.js, you'll probably hit it sooner or later with another project too.

Trott commented 7 years ago

@bnoordhuis Perhaps we can consider this a documentation issue and add a note to a troubleshooting guide (or even right in BUILDING.md) explaining what to do when it arises?

neocturne commented 7 years ago

Note that setting ulimit -s had no effect on my system. The patch I provided in #9141 was the only way I could make it work.

NeatBoar commented 6 years ago

Met the same problem today. Trying to build lede_17.01.4. Get the error:

make[4]: Entering directory '/data/openwrt_3/lede-sdk-17.01.4-brcm2708-bcm2709_gcc-5.4.0_musl-1.1.16_eabi.Linux-x86_64/build_dir/target-arm_cortex-a7+neon-vfpv4_musl-1.1.16_eabi/host/node-v4.4.5' make -C out BUILDTYPE=Release V=s make[5]: Entering directory '/data/openwrt_3/lede-sdk-17.01.4-brcm2708-bcm2709_gcc-5.4.0_musl-1.1.16_eabi.Linux-x86_64/build_dir/target-arm_cortex-a7+neon-vfpv4_musl-1.1.16_eabi/host/node-v4.4.5/out' make[5]: execvp: printf: Argument list too long deps/openssl/openssl.target.mk:862: recipe for target '/data/openwrt_3/lede-sdk-17.01.4-brcm2708-bcm2709_gcc-5.4.0_musl-1.1.16_eabi.Linux-x86_64/build_dir/target-arm_cortex-a7+neon-vfpv4_musl-1.1.16_eabi/host/node-v4.4.5/out/Release/obj.target/deps/openssl/libopenssl.a' failed make[5]: [/data/openwrt_3/lede-sdk-17.01.4-brcm2708-bcm2709_gcc-5.4.0_musl-1.1.16_eabi.Linux-x86_64/build_dir/target-arm_cortex-a7+neon-vfpv4_musl-1.1.16_eabi/host/node-v4.4.5/out/Release/obj.target/deps/openssl/libopenssl.a] Error 127 make[5]: Leaving directory '/data/openwrt_3/lede-sdk-17.01.4-brcm2708-bcm2709_gcc-5.4.0_musl-1.1.16_eabi.Linux-x86_64/build_dir/target-arm_cortex-a7+neon-vfpv4_musl-1.1.16_eabi/host/node-v4.4.5/out' Makefile:45: recipe for target 'node' failed make[4]: [node] Error 2

I dont need node-v4.4.5 package, but I dont know where to disable it. I unkecked all .js from menuconfig.

Changed root directory from: /data/openwrt_3/lede-sdk-17.01.4-brcm2708-bcm2709_gcc-5.4.0_musl-1.1.16_eabi.Linux-x86_64/ to /data/openwrt_3/lede/

And that solved my problem :)

jasnell commented 4 years ago

Should this issue remain open? As pointed out, there's really not much we can do in Node.js for this

bnoordhuis commented 4 years ago

A pull request to https://github.com/nodejs/gyp-next is the way forward for anyone who wants to pick this up. I'll go ahead and close this out.

leveneg commented 3 years ago

Hit this issue today and couldn't get one of the patch hunks to apply cleanly. After some formatting changes I got things going with node v14. Thought I'd share:

diff --git a/tools/gyp/pylib/gyp/generator/make.py b/tools/gyp/pylib/gyp/generator/make.py
index d163ae31..2ce1c301 100644
--- a/tools/gyp/pylib/gyp/generator/make.py
+++ b/tools/gyp/pylib/gyp/generator/make.py
@@ -155,6 +155,31 @@ cmd_alink_thin = rm -f $@ && $(AR.$(TOOLSET)) crsT $@ $(filter %.o,$^)
 quiet_cmd_link = LINK($(TOOLSET)) $@
 cmd_link = $(LINK.$(TOOLSET)) -o $@ $(GYP_LDFLAGS) $(LDFLAGS.$(TOOLSET)) -Wl,--start-group $(LD_INPUTS) $(LIBS) -Wl,--end-group

+# Note: this does not handle spaces in paths
+define xargs
+   $(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
+endef
+
+define write-to-file
+   @: >$(1)
+$(call xargs,@printf "%s\\n" >>$(1),$(2))
+endef
+
+OBJ_FILE_LIST := ar-file-list
+
+define create_archive
+        rm -f $(1) $(1).$(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crs $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
+define create_thin_archive
+        rm -f $(1) $(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crsT $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
 # We support two kinds of shared objects (.so):
 # 1) shared_library, which is just bundling together many dependent libraries
 # into a link line.
@@ -199,6 +224,31 @@ cmd_alink = rm -f $@ && $(AR.$(TOOLSET)) crs $@ $(filter %.o,$^)
 quiet_cmd_alink_thin = AR($(TOOLSET)) $@
 cmd_alink_thin = rm -f $@ && $(AR.$(TOOLSET)) crsT $@ $(filter %.o,$^)

+# Note: this does not handle spaces in paths
+define xargs
+   $(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
+endef
+
+define write-to-file
+   @: >$(1)
+$(call xargs,@printf "%s\\n" >>$(1),$(2))
+endef
+
+OBJ_FILE_LIST := ar-file-list
+
+define create_archive
+        rm -f $(1) $(1).$(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crs $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
+define create_thin_archive
+        rm -f $(1) $(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crsT $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
 # Due to circular dependencies between libraries :(, we wrap the
 # special "figure out circular dependencies" flags around the entire
 # input list during linking.
@@ -1766,14 +1816,28 @@ $(obj).$(TOOLSET)/$(TARGET)/%%.o: $(obj)/%%%s FORCE_DO_CMD
                 self.flavor not in ("mac", "openbsd", "netbsd", "win")
                 and not self.is_standalone_static_library
             ):
-                self.WriteDoCmd(
+                if self.flavor in ('linux', 'android'):
+                  self.WriteMakeRule(
                     [self.output_binary],
                     link_deps,
-                    "alink_thin",
-                    part_of_all,
-                    postbuilds=postbuilds,
-                )
+                    actions = ['$(call create_thin_archive,$@,$^)']
+                  )
+                else:
+                  self.WriteDoCmd(
+                      [self.output_binary],
+                      link_deps,
+                      "alink_thin",
+                      part_of_all,
+                      postbuilds=postbuilds,
+                  )
             else:
+              if self.flavor in ('linux', 'android'):
+                self.WriteMakeRule(
+                    [self.output_binary],
+                    link_deps,
+                    actions = ['$(call create_archive,$@,$^)']
+                )
+              else:
                 self.WriteDoCmd(
                     [self.output_binary],
                     link_deps,

p3x-robot commented 2 years ago

Hit this issue today and couldn't get one of the patch hunks to apply cleanly. After some formatting changes I got things going with node v14. Thought I'd share:

diff --git a/tools/gyp/pylib/gyp/generator/make.py b/tools/gyp/pylib/gyp/generator/make.py
index d163ae31..2ce1c301 100644
--- a/tools/gyp/pylib/gyp/generator/make.py
+++ b/tools/gyp/pylib/gyp/generator/make.py
@@ -155,6 +155,31 @@ cmd_alink_thin = rm -f $@ && $(AR.$(TOOLSET)) crsT $@ $(filter %.o,$^)
 quiet_cmd_link = LINK($(TOOLSET)) $@
 cmd_link = $(LINK.$(TOOLSET)) -o $@ $(GYP_LDFLAGS) $(LDFLAGS.$(TOOLSET)) -Wl,--start-group $(LD_INPUTS) $(LIBS) -Wl,--end-group

+# Note: this does not handle spaces in paths
+define xargs
+ $(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
+endef
+
+define write-to-file
+ @: >$(1)
+$(call xargs,@printf "%s\\n" >>$(1),$(2))
+endef
+
+OBJ_FILE_LIST := ar-file-list
+
+define create_archive
+        rm -f $(1) $(1).$(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crs $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
+define create_thin_archive
+        rm -f $(1) $(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crsT $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
 # We support two kinds of shared objects (.so):
 # 1) shared_library, which is just bundling together many dependent libraries
 # into a link line.
@@ -199,6 +224,31 @@ cmd_alink = rm -f $@ && $(AR.$(TOOLSET)) crs $@ $(filter %.o,$^)
 quiet_cmd_alink_thin = AR($(TOOLSET)) $@
 cmd_alink_thin = rm -f $@ && $(AR.$(TOOLSET)) crsT $@ $(filter %.o,$^)

+# Note: this does not handle spaces in paths
+define xargs
+ $(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
+endef
+
+define write-to-file
+ @: >$(1)
+$(call xargs,@printf "%s\\n" >>$(1),$(2))
+endef
+
+OBJ_FILE_LIST := ar-file-list
+
+define create_archive
+        rm -f $(1) $(1).$(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crs $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
+define create_thin_archive
+        rm -f $(1) $(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crsT $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
 # Due to circular dependencies between libraries :(, we wrap the
 # special "figure out circular dependencies" flags around the entire
 # input list during linking.
@@ -1766,14 +1816,28 @@ $(obj).$(TOOLSET)/$(TARGET)/%%.o: $(obj)/%%%s FORCE_DO_CMD
                 self.flavor not in ("mac", "openbsd", "netbsd", "win")
                 and not self.is_standalone_static_library
             ):
-                self.WriteDoCmd(
+                if self.flavor in ('linux', 'android'):
+                  self.WriteMakeRule(
                     [self.output_binary],
                     link_deps,
-                    "alink_thin",
-                    part_of_all,
-                    postbuilds=postbuilds,
-                )
+                    actions = ['$(call create_thin_archive,$@,$^)']
+                  )
+                else:
+                  self.WriteDoCmd(
+                      [self.output_binary],
+                      link_deps,
+                      "alink_thin",
+                      part_of_all,
+                      postbuilds=postbuilds,
+                  )
             else:
+              if self.flavor in ('linux', 'android'):
+                self.WriteMakeRule(
+                    [self.output_binary],
+                    link_deps,
+                    actions = ['$(call create_archive,$@,$^)']
+                )
+              else:
                 self.WriteDoCmd(
                     [self.output_binary],
                     link_deps,

this patch is awesome! the build works with 16 jobs. thanks!

p3x-robot commented 2 years ago

with this patch, shortening build 1:30 hours to 10 minutes

p3x-robot commented 2 years ago

@nxhack suggests, that i should instead of patching the build, just increase the ulimit -n 65535

build is runinng now. will let you if this modification is works.

nxhack commented 2 years ago

@p3x-robot

@nxhack suggests, that i should instead of patching the build, just increase the ulimit -n 65535

This is a different issue from what is being discussed in this thread.

p3x-robot commented 2 years ago

why is it different?

nxhack commented 2 years ago

@p3x-robot

As I pointed out elsewhere in my discussion with you, you need to separate the two events.

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

The need to increase file descriptors is only relevant for the latest binutils.

p3x-robot commented 2 years ago

gotcha. there are 2 issues here, one is the node build and the binutils... ok clear.

my binutils are lower. so that is not the problem for me.

binutils-common/stable,now 2.35.2-2 amd64 [installed,automatic]
binutils-x86-64-linux-gnu/stable,now 2.35.2-2 amd64 [installed,automatic]
binutils/stable,now 2.35.2-2 amd64 [installed]

jhgoebbert commented 1 year ago

Hit this issue today and couldn't get one of the patch hunks to apply cleanly. After some formatting changes I got things going with node v14. Thought I'd share:

diff --git a/tools/gyp/pylib/gyp/generator/make.py b/tools/gyp/pylib/gyp/generator/make.py
index d163ae31..2ce1c301 100644
--- a/tools/gyp/pylib/gyp/generator/make.py
+++ b/tools/gyp/pylib/gyp/generator/make.py
@@ -155,6 +155,31 @@ cmd_alink_thin = rm -f $@ && $(AR.$(TOOLSET)) crsT $@ $(filter %.o,$^)
 quiet_cmd_link = LINK($(TOOLSET)) $@
 cmd_link = $(LINK.$(TOOLSET)) -o $@ $(GYP_LDFLAGS) $(LDFLAGS.$(TOOLSET)) -Wl,--start-group $(LD_INPUTS) $(LIBS) -Wl,--end-group

+# Note: this does not handle spaces in paths
+define xargs
+   $(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
+endef
+
+define write-to-file
+   @: >$(1)
+$(call xargs,@printf "%s\\n" >>$(1),$(2))
+endef
+
+OBJ_FILE_LIST := ar-file-list
+
+define create_archive
+        rm -f $(1) $(1).$(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crs $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
+define create_thin_archive
+        rm -f $(1) $(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crsT $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
 # We support two kinds of shared objects (.so):
 # 1) shared_library, which is just bundling together many dependent libraries
 # into a link line.
@@ -199,6 +224,31 @@ cmd_alink = rm -f $@ && $(AR.$(TOOLSET)) crs $@ $(filter %.o,$^)
 quiet_cmd_alink_thin = AR($(TOOLSET)) $@
 cmd_alink_thin = rm -f $@ && $(AR.$(TOOLSET)) crsT $@ $(filter %.o,$^)

+# Note: this does not handle spaces in paths
+define xargs
+   $(1) $(word 1,$(2))
+$(if $(word 2,$(2)),$(call xargs,$(1),$(wordlist 2,$(words $(2)),$(2))))
+endef
+
+define write-to-file
+   @: >$(1)
+$(call xargs,@printf "%s\\n" >>$(1),$(2))
+endef
+
+OBJ_FILE_LIST := ar-file-list
+
+define create_archive
+        rm -f $(1) $(1).$(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crs $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
+define create_thin_archive
+        rm -f $(1) $(OBJ_FILE_LIST); mkdir -p `dirname $(1)`
+        $(call write-to-file,$(1).$(OBJ_FILE_LIST),$(filter %.o,$(2)))
+        $(AR.$(TOOLSET)) crsT $(1) @$(1).$(OBJ_FILE_LIST)
+endef
+
 # Due to circular dependencies between libraries :(, we wrap the
 # special "figure out circular dependencies" flags around the entire
 # input list during linking.
@@ -1766,14 +1816,28 @@ $(obj).$(TOOLSET)/$(TARGET)/%%.o: $(obj)/%%%s FORCE_DO_CMD
                 self.flavor not in ("mac", "openbsd", "netbsd", "win")
                 and not self.is_standalone_static_library
             ):
-                self.WriteDoCmd(
+                if self.flavor in ('linux', 'android'):
+                  self.WriteMakeRule(
                     [self.output_binary],
                     link_deps,
-                    "alink_thin",
-                    part_of_all,
-                    postbuilds=postbuilds,
-                )
+                    actions = ['$(call create_thin_archive,$@,$^)']
+                  )
+                else:
+                  self.WriteDoCmd(
+                      [self.output_binary],
+                      link_deps,
+                      "alink_thin",
+                      part_of_all,
+                      postbuilds=postbuilds,
+                  )
             else:
+              if self.flavor in ('linux', 'android'):
+                self.WriteMakeRule(
+                    [self.output_binary],
+                    link_deps,
+                    actions = ['$(call create_archive,$@,$^)']
+                )
+              else:
                 self.WriteDoCmd(
                     [self.output_binary],
                     link_deps,

this patch is awesome! the build works with 16 jobs. thanks!

Thank you. This fixes the issue "printf: Argument list too long" we have at build time with nodejs 16.x!

nodejs / node

Build fails when the path to the build root is too long #9137

9141 shows fixing this in GYP is not exactly trivial. A one-liner that sets`ulimit -s` to some high value is probably more palatable.

nodejs / node

Build fails when the path to the build root is too long #9137

9141 shows fixing this in GYP is not exactly trivial. A one-liner that setsulimit -s to some high value is probably more palatable.

9141 shows fixing this in GYP is not exactly trivial. A one-liner that sets`ulimit -s` to some high value is probably more palatable.