build: Makefile dependency regression #17043

refack · 2017-11-15T13:50:25Z

Version: master
Platform: non-Windows
Subsystem: build

Several recent build jobs are failing apparently because make is miss-calculating target dependencies and tries to build the test/addons too soon (before the node binary is linked) and thus fails.
Looking for help in tracking down a possible cause.

Example failing output: https://www.irccloud.com/pastebin/raw/FWBUXcqM
https://ci.nodejs.org/job/node-test-commit-linux/nodes=fedora24/14120/consoleFull

https://github.com/nodejs/node/commits/master/Makefile:
@refack - tools,build: allow build without remark-cli - e624503
@joyeecheung - tools: don't lint files that have not changed - eebcb48
@gibfahn - build: suppress lint-md output - 60d055e
@joyeecheung - build: make test-doc and lint addon docs - 390eda1
@danbev - build: make doc target quiet - 6f684d9
@addaleax @tniessen - build: ignore empty folders in test-addons-napi - 65d2067
@joyeecheung - build: run linter before running tests - 2875459
@refack - build: improve make clean - 9ab6481
@watilde - tools: add make lint-md-clean - a399881
@watilde - build: add lint-md-build - 212f4b9
@watilde - tools: add lint-md command in Makefile - f132954
@Trott - build: use doc-only instead of doc - d6ba14e
@Trott - test: fix flaky test-make-doc - c9d5be4
@gibfahn - test: update test-npm to use test-npm-package.js - 532d8b2
@evanlucas - build: add c++ coverage support on macOS - a078584
@joyeecheung - test: test make doc and verify toc - df5dc2d
@maclover7 @refack - test: move inspector tests to parallel/sequential - 978629c
@bnoordhuis @joyeecheung - build: lint benchmark addon - 411695e
@bnoordhuis @joyeecheung - build: use local node-gyp for benchmark addon - 4157342
@refack - build: restore mistakenly dropped suites - 88d05aa
@gr2m @mhdawson - build: ignore empty folders in test-addons - 7da45f8

The text was updated successfully, but these errors were encountered:

joyeecheung · 2017-11-15T15:11:22Z

Hmm..maybe we should put | in the rules of build-addons and build-addons-napi?

joyeecheung · 2017-11-15T16:37:31Z

Also, I think I have seen this on my mac(or my linux box) very occasionally, long before all the commits listed in the OP (if my memory serve me right, at least before 8.x came out), it's probably not a recent regression, but a low-probability race or something

bnoordhuis · 2017-11-15T18:25:59Z

@joyeecheung I fixed just such a dependency issue in 0ea4855. That was in May last year though, maybe too long ago?

joyeecheung · 2017-11-16T05:14:36Z

@bnoordhuis Yes, that could be it. From the commit message:

As an order-only prerequisite, it's sometimes skipped when it shouldn't be.

I think this probably does not happen in CI where most things start fresh...so my guess in #17043 (comment) could be wrong, this is a new regression.

joyeecheung · 2017-11-16T05:17:42Z

Uh, wait, no, I thought that commit landed this May, but it actually landed last May, and I am pretty sure I've seen this error this year, so back to my original guess..

joyeecheung · 2017-11-16T08:41:21Z

I looked into the logs again and I think this is not caused by Makefile miscalculating the dependencies and tries to build addons before linking node, because it usually errors out in the middle of the for-loop of build-addons-napi. If node is not linked properly then it should error out in the first iteration instead. I think something is removing the symlink or removing out/Release/node during the execution, this should be caused by a dependency miscalculation as well though, e.g. make tries to recompile when build-addons-napi is executing

joyeecheung · 2017-11-17T02:55:22Z

Maybe somewhat related: somehow the napi addons are not built but still proceeded to the JS testing phase on smart os

https://ci.nodejs.org/job/node-test-commit-smartos/nodes=smartos16-64/13133/console

See test output

not ok 1868 addons-napi/test_make_callback/test
  ---
  duration_ms: 0.430
  severity: fail
  stack: |-
    module.js:544
        throw err;
        ^
    
    Error: Cannot find module './build/Release/binding'
        at Function.Module._resolveFilename (module.js:542:15)
        at Function.Module._load (module.js:472:25)
        at Module.require (module.js:585:17)
        at require (internal/module.js:11:18)
        at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_make_callback/test.js:7:17)
        at Module._compile (module.js:641:30)
        at Object.Module._extensions..js (module.js:652:10)
        at Module.load (module.js:560:32)
        at tryModuleLoad (module.js:503:12)
        at Function.Module._load (module.js:495:3)
  ...
not ok 1869 addons-napi/test_string/test
  ---
  duration_ms: 0.429
  severity: fail
  stack: |-
    module.js:544
        throw err;
        ^
    
    Error: Cannot find module './build/Release/test_string'
        at Function.Module._resolveFilename (module.js:542:15)
        at Function.Module._load (module.js:472:25)
        at Module.require (module.js:585:17)
        at require (internal/module.js:11:18)
        at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_string/test.js:6:21)
        at Module._compile (module.js:641:30)
        at Object.Module._extensions..js (module.js:652:10)
        at Module.load (module.js:560:32)
        at tryModuleLoad (module.js:503:12)
        at Function.Module._load (module.js:495:3)
  ...
not ok 1870 addons-napi/test_make_callback_recurse/test
  ---
  duration_ms: 0.347
  severity: fail
  stack: |-
    module.js:544
        throw err;
        ^
    
    Error: Cannot find module './build/Release/binding'
        at Function.Module._resolveFilename (module.js:542:15)
        at Function.Module._load (module.js:472:25)
        at Module.require (module.js:585:17)
        at require (internal/module.js:11:18)
        at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_make_callback_recurse/test.js:6:17)
        at Module._compile (module.js:641:30)
        at Object.Module._extensions..js (module.js:652:10)
        at Module.load (module.js:560:32)
        at tryModuleLoad (module.js:503:12)
        at Function.Module._load (module.js:495:3)
...

The building phase:

See build trace

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/6_object_wrap/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/7_factory_wrap/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/8_passing_wrapped/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_array/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_async/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_buffer/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_constructor/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_conversions/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_dataview/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_env_sharing/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_error/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_exception/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_fatal/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_function/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_general/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_handle_scope/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_make_callback/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_make_callback_recurse/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_number/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_object/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_promise/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_properties/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_reference/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_string/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_symbol/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_typedarray/

Building addon /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos16-64/test/addons-napi/test_warning/
touch test/addons-napi/.buildstamp
/bin/sh[1]: ./node: ./node: cannot execute [Invalid argument]
/bin/sh[1]: ./node: ./node: cannot execute [Invalid argument]
/bin/sh[1]: ./node: ./node: cannot execute [Invalid argument]

joyeecheung · 2017-11-17T03:21:08Z

Another one on smart OS, this time the test/addons were missing builds, but judging from the log they were built before the tests started, I think those builds were somehow deleted before the test executor went to the addon tests.

https://ci.nodejs.org/job/node-test-commit-smartos/13181/nodes=smartos15-64/console

joyeecheung · 2017-11-17T03:44:40Z

Looks like the race condition documented in the $(NODE_EXE) target

# The -r/-L check stops it recreating the link if it is already in place,
# otherwise $(NODE_EXE) being a .PHONY target means it is always re-run.
# Without the check there is a race condition between the link being deleted
# and recreated which can break the addons build when running test-ci

is back

joyeecheung · 2017-11-19T03:42:46Z

This was automatically closed in #17048 but I doubt if enforcing the order of dependency fixes this issue given that smartos still fails in its CI run, would like to give it a bit more time to find out, #17115 would probably help showing if the symlink is broken halfway during the addon build, it could be a bug in the CI machine orchestration instead of a bug in the Makefile.

PR-URL: #17048 Fixes: #17043 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

maclover7 · 2017-12-25T02:29:08Z

@joyeecheung (or anyone else) has this error resurfaced after #17048 landed?

joyeecheung · 2017-12-25T12:48:55Z

@maclover7 No, although I think this is caused by the cluster orchestration instead of changes to the build file. I think this can be closed now.

refack added build Issues and PRs related to build files or the CI. regression Issues related to regressions. labels Nov 15, 2017

refack added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Nov 15, 2017

joyeecheung mentioned this issue Nov 15, 2017

build: enforce order of dependency when building addons #17048

Closed

4 tasks

joyeecheung mentioned this issue Nov 17, 2017

tools: try installing js-yaml only once #16661

Closed

4 tasks

This was referenced Nov 17, 2017

build: do not build doc in source tarball #17100

Closed

build: refactor addon targets #17115

Closed

addaleax closed this as completed in c5b8e16 Nov 18, 2017

joyeecheung reopened this Nov 19, 2017

joyeecheung mentioned this issue Dec 6, 2017

src, test: node internals' postmortem metadata #14901

Closed

2 tasks

MylesBorins pushed a commit that referenced this issue Dec 12, 2017

build: enforce order of dependency when building addons

035a24e

PR-URL: #17048 Fixes: #17043 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

gibfahn pushed a commit that referenced this issue Dec 19, 2017

build: enforce order of dependency when building addons

f0fb1d4

PR-URL: #17048 Fixes: #17043 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

gibfahn pushed a commit that referenced this issue Dec 20, 2017

build: enforce order of dependency when building addons

3036b36

PR-URL: #17048 Fixes: #17043 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

joyeecheung closed this as completed Dec 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Makefile dependency regression #17043

build: Makefile dependency regression #17043

refack commented Nov 15, 2017

joyeecheung commented Nov 15, 2017

joyeecheung commented Nov 15, 2017 •

edited

Loading

bnoordhuis commented Nov 15, 2017

joyeecheung commented Nov 16, 2017 •

edited

Loading

joyeecheung commented Nov 16, 2017 •

edited

Loading

joyeecheung commented Nov 16, 2017 •

edited

Loading

joyeecheung commented Nov 17, 2017 •

edited

Loading

joyeecheung commented Nov 17, 2017 •

edited

Loading

joyeecheung commented Nov 17, 2017

joyeecheung commented Nov 19, 2017

maclover7 commented Dec 25, 2017

joyeecheung commented Dec 25, 2017

build: Makefile dependency regression #17043

build: Makefile dependency regression #17043

Comments

refack commented Nov 15, 2017

joyeecheung commented Nov 15, 2017

joyeecheung commented Nov 15, 2017 • edited Loading

bnoordhuis commented Nov 15, 2017

joyeecheung commented Nov 16, 2017 • edited Loading

joyeecheung commented Nov 16, 2017 • edited Loading

joyeecheung commented Nov 16, 2017 • edited Loading

joyeecheung commented Nov 17, 2017 • edited Loading

joyeecheung commented Nov 17, 2017 • edited Loading

joyeecheung commented Nov 17, 2017

joyeecheung commented Nov 19, 2017

maclover7 commented Dec 25, 2017

joyeecheung commented Dec 25, 2017

joyeecheung commented Nov 15, 2017 •

edited

Loading

joyeecheung commented Nov 16, 2017 •

edited

Loading

joyeecheung commented Nov 16, 2017 •

edited

Loading

joyeecheung commented Nov 16, 2017 •

edited

Loading

joyeecheung commented Nov 17, 2017 •

edited

Loading

joyeecheung commented Nov 17, 2017 •

edited

Loading