Skip to content

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 25, 2017

As indicated by the FIXME comment, this macro guard is no longer needed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

As indicated by the FIXME comment, this macro guard is no longer needed.
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 25, 2017
@danbev
Copy link
Contributor Author

danbev commented Apr 25, 2017

@addaleax
Copy link
Member

LGTM

Landed in 5bb06e8

@addaleax addaleax closed this Apr 27, 2017
addaleax pushed a commit that referenced this pull request Apr 27, 2017
As indicated by the FIXME comment, this macro guard is no longer needed.

PR-URL: #12638
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@evanlucas
Copy link
Contributor

This is landing on v7.x-staging, but causes a compilation error.

Undefined symbols for architecture x86_64:
  "node::Abort()", referenced from:
      node::inspector::InspectorSocketServer::HandshakeCallback(node::inspector::InspectorSocket*, node::inspector::inspector_handshake_event, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) in inspector_socket_server.o
  "node::Assert(char const* const (*) [4])", referenced from:
      UtilTest_Malloc_Test::TestBody() in util.o
      UtilTest_Calloc_Test::TestBody() in util.o
      UtilTest_MaybeStackBuffer_Test::TestBody() in util.o
      node::MaybeStackBuffer<unsigned char, 1024ul>::SetLength(unsigned long) in util.o
      node::MaybeStackBuffer<unsigned char, 1024ul>::SetLengthAndZeroTerminate(unsigned long) in util.o
      node::MaybeStackBuffer<unsigned char, 1024ul>::AllocateSufficientStorage(unsigned long) in util.o
      node::MaybeStackBuffer<unsigned short, 1024ul>::SetLength(unsigned long) in util.o
      ...
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [/Volumes/code/forks/WORK-node/out/Release/cctest] Error 1
make[1]: *** Waiting for unfinished jobs....
rm fe2fa96b1035b05b2fe602777c9605ade7b05863.intermediate
make: *** [node] Error 2

Marking dont-land for now. Feel free to backport if it is needed. Thanks!

@addaleax
Copy link
Member

addaleax commented May 1, 2017

I think this just depends on the backport in #12664

@addaleax addaleax added lib / src Issues and PRs related to general changes in the lib or src directory. and removed dont-land-on-v7.x labels May 1, 2017
@jasnell jasnell mentioned this pull request May 11, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

@danbev should this land on v6.x? It cherry-picks cleanly, but I'm not sure whether it needs to depend on the other commits in #12664, some of which are semver-minor or semver-major.

@danbev danbev deleted the remove-gtest-macro-guard branch June 28, 2017 05:41
@MylesBorins
Copy link
Contributor

ping @danbev

danbev added a commit to danbev/node that referenced this pull request Aug 15, 2017
As indicated by the FIXME comment, this macro guard is no longer needed.

PR-URL: nodejs#12638
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
As indicated by the FIXME comment, this macro guard is no longer needed.

PR-URL: #12638
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
As indicated by the FIXME comment, this macro guard is no longer needed.

PR-URL: #12638
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants