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

test-torque.tq failing source tarball builds for v12.11.0 (V8 7.7) #29709

Closed
rvagg opened this issue Sep 26, 2019 · 10 comments
Closed

test-torque.tq failing source tarball builds for v12.11.0 (V8 7.7) #29709

rvagg opened this issue Sep 26, 2019 · 10 comments

Comments

@rvagg
Copy link
Member

rvagg commented Sep 26, 2019

EDIT: this is not x86 specific, see further comments below.

Holding up the unofficial-builds of v12.11.0 (we don't handle failures yet, so they're all blocked for now). Here's the failing build log for x86: https://unofficial-builds.nodejs.org/logs/201909252318-v12.11.0/x86.log

Key part:

make install DESTDIR=node-v12.11.0-linux-x86 V= PORTABLE=1
make -C out BUILDTYPE=Release V=
  TOUCH /home/node/node-v12.11.0/out/Release/obj.target/tools/v8_gypfiles/v8_version.stamp
make[2]: *** Norule to make target '../deps/v8/test/torque/test-torque.tq', needed by '7ea8302c51573d5268a52f6014edc10e884f440e.intermediate'.  Stop.

We don't officially support them but have plenty of users, so if we can address this it would be really great. It's V8-related and I don't know how to go about assessing whether this is easy to address.

@nodejs/v8 any help getting this addressed on master would be greatly appreciated.

@BridgeAR BridgeAR changed the title x86 builds broken since v12.11.0 (V8 7.8) x86 builds broken since v12.11.0 (V8 7.7) Sep 26, 2019
@mhart
Copy link
Contributor

mhart commented Sep 26, 2019

Possibly related: #25095

I think it's the tarball that's the problem

@rvagg
Copy link
Member Author

rvagg commented Sep 26, 2019

Actually yeah, I removed x86 from the unofficial-builds pipeline and the musl build broke with the same error: https://unofficial-builds.nodejs.org/logs/201909260213-v12.11.0/musl.log

So the title is wrong for this issue, it's not x86 specific.

@mhart
Copy link
Contributor

mhart commented Sep 26, 2019

Can reproduce by unpacking tarball, running configure, and then make (on Debian and Alpine):

docker run --rm gcc bash -c 'curl https://nodejs.org/dist/v12.11.0/node-v12.11.0.tar.xz | tar -xJ && cd node-v12.11.0 && ./configure && make -j4'

@rvagg rvagg changed the title x86 builds broken since v12.11.0 (V8 7.7) test-torque.tq failing source tarball builds for v12.11.0 (V8 7.7) Sep 26, 2019
@dylanaraps
Copy link

I can reproduce on a musl based system:

INFO: Using floating patch "tools/icu/patches/64/source/common/putil.cpp" from "tools/icu"
INFO: Using floating patch "tools/icu/patches/64/source/i18n/dtptngen.cpp" from "tools/icu"
INFO: configure completed successfully
make -C out BUILDTYPE=Release V=0
  touch /home/goldie/.cache/kiss/build-17145/nodejs/out/Release/obj.target/tools/v8_gypfiles/v8_version.stamp
make[1]: *** No rule to make target '../deps/v8/test/torque/test-torque.tq', needed by '0c23b8358f0f4da23b589e1b64ee728c8b90e303.intermediate'.  Stop.
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:101: node] Error 2

Very simple build options to reproduce:

./configure \
    --prefix=/usr

make

@richardlau
Copy link
Member

Part of the V8 7.7 update (#28918) added this:

"<(V8_ROOT)/test/torque/test-torque.tq",

Our source tarballs currently exclude everything in deps/v8/test:

node/Makefile

Line 1040 in d36b6f8

$(RM) -r $(TARNAME)/deps/v8/test

#29712 includes deps/v8/test/torque in the source tarball but still excludes the rest of deps/v8/test and should fix the build from the source tarball.

@targos
Copy link
Member

targos commented Sep 26, 2019

The V8 7.7 update didn't really add that line. It just made it explicit (the list was generated by a script before).

@richardlau
Copy link
Member

The V8 7.7 update didn't really add that line. It just made it explicit (the list was generated by a script before).

@targos It added it back in as we removed it previously in #25097.

@dylanaraps
Copy link

dylanaraps commented Sep 26, 2019

For those wanting a quick fix for the current tarball:

  • Grab deps/v8/test/torque/test-torque.tq from the repository.
  • Add it to the extracted tarball source in the same location.

~~While my build hasn't yet completed, it has been running for 20~ minutes now. It'd be great to know if more is needed to fix this present issue.~~ Build succeeded with above workaround.

@BridgeAR
Copy link
Member

I suggest we fix that issue soon and release a patch release right after.

ahwayakchih added a commit to ahwayakchih/docker-nodejs-app that referenced this issue Sep 26, 2019
Out of necessity (nodejs/node#29709),
just in case tarball is broken again in the future.
ahwayakchih added a commit to ahwayakchih/docker-nodejs-app that referenced this issue Sep 26, 2019
@mhart
Copy link
Contributor

mhart commented Sep 27, 2019

Confirmed that @dylanaraps' method of including deps/v8/test/torque/test-torque.tq alone is enough to get it to build. Here's a one-liner with curl to get it into your build:

  curl -sfSL https://github.com/nodejs/node/archive/${VERSION}.tar.gz | tar -xz --strip-components=1 -- node-12.11.0/deps/v8/test/torque/test-torque.tq

I used it successfully to build on Alpine: https://github.com/mhart/alpine-node/blob/12.11.0/Dockerfile#L35

@Trott Trott closed this as completed in f21818e Sep 28, 2019
targos pushed a commit that referenced this issue Oct 1, 2019
Builds from the source tarball were broken by the recent V8 upate
to 7.7 as a file needed to build torque wasn't included in the source
tarball as it resides in deps/v8/test.

PR-URL: #29712
Fixes: #29709
Refs: #28918
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
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 a pull request may close this issue.

6 participants