Skip to content

MacOS build fails #33401

Description

@shackijj

MacOS builds fail on CI

Examples:
https://github.com/nodejs/node/pull/33395/checks?check_run_id=673915168
https://github.com/nodejs/node/pull/33396/checks?check_run_id=673650394
https://github.com/nodejs/node/pull/33399/checks?check_run_id=674009996

make[2]: *** [/Users/runner/runners/2.169.1/work/node/node/out/Release/obj.target/libnode/src/api/environment.o] Error 1
In file included from ../src/async_wrap.cc:23:
In file included from ../src/async_wrap-inl.h:29:
../src/node_internals.h:115:17: error: 'Reallocate' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
  virtual void* Reallocate(void* data, size_t old_size, size_t size);
                ^
../deps/v8/include/v8.h:5064:19: note: overridden virtual function is here
    virtual void* Reallocate(void* data, size_t old_length, size_t new_length);
                  ^
1 error generated.
make[2]: *** [/Users/runner/runners/2.169.1/work/node/node/out/Release/obj.target/libnode/src/async_wrap.o] Error 1
rm 901e15e3bfa09076435ef2c6529c32d9b1b08dee.intermediate 4932d3b9aac55583abc688388403c323f0ca30f4.intermediate
make[1]: *** [node] Error 2
make: *** [build-ci] Error 2

The failure might be caused by this commit fcc183c. @richardlau is it OK that builds fail?

Activity

  1. richardlau commented on May 14, 2020

    @richardlau
    Member

    The failure is because after fcc183c warnings are being treated as errors for non-deps code. The non-deps code was warning free at the beginning of the week -- unfortunately the V8 8.3 update landing has introduced new warnings, such as the one above (unfortunate in the sense that the actions/CI runs for the V8 update happened before we switched on the --errors-on-warn flag, the idea being that having the flag on would help prevent PRs landing that introduced new warnings).

  2. added
    buildIssues and PRs related to Node.js builds or CI infrastructure.
    macosIssues and PRs related to the macOS platform.
    on May 14, 2020
  3. shackijj commented on May 14, 2020

    @shackijj
    ContributorAuthor

    Can we do something apart from disabling --errors-on-warn for macOS and reverting V8 8.3 update?

  4. richardlau commented on May 14, 2020

    @richardlau
    Member

    This might be fixed by #32716 (which landed on the V8 canary branch)? cc @gengjiawen

  5. shackijj commented on May 14, 2020

    @shackijj
    ContributorAuthor

    In order to land, a Pull Request needs to be reviewed and approved by at least two Node.js Collaborators (one Collaborator approval is enough if the pull request has been open for more than 7 days) and pass a CI (Continuous Integration) test run.

    The quote above is from this guide. Is landing of PRs an automated process? If yes, can we somehow improve it as to avoid merging commits that don't pass CI test run?

  6. gengjiawen commented on May 14, 2020

    @gengjiawen
    Member

    This might be fixed by #32716 (which landed on the V8 canary branch)? cc @gengjiawen

    Yeap, need to cherrypick to master.

  7. gengjiawen commented on May 14, 2020

    @gengjiawen
    Member

    If yes, can we somehow improve it as to avoid merging commits that don't pass CI test run.

    It passed the CI, just not rebased to the latest. We always land commit with full
    CI pass.

  8. gengjiawen commented on May 14, 2020

    @gengjiawen
    Member

    This might be fixed by #32716 (which landed on the V8 canary branch)? cc @gengjiawen

    Yeap, need to cherrypick to master.

    PR: #33402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    buildIssues and PRs related to Node.js builds or CI infrastructure.macosIssues and PRs related to the macOS platform.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions