Bug 220983 - www/node: Incorrectly patching common.gypi
Summary: www/node: Incorrectly patching common.gypi
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Many People
Assignee: Bradley T. Hughes
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2017-07-24 23:28 UTC by Randy
Modified: 2021-03-08 16:34 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (bhughes)
koobs: merge-quarterly?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Randy 2017-07-24 23:28:17 UTC
The ports install for www/node is patching common.gypi which includes the following lines.

```
      ['OS=="freebsd"', {
        # Use this flag because on FreeBSD std::pairs copy constructor is non-trivial
        # https://lists.freebsd.org/pipermail/freebsd-toolchain/2016-March/002094.html
        'cflags': [ '-D_LIBCPP_TRIVIAL_PAIR_COPY_CTOR=1' ],
        'ldflags': [
          '-Wl,--export-dynamic',
        ],
```

Setting _LIBCPP_TRIVIAL_PAIR_COPY_CTOR=1 on FreeBSD 11 systems compling with clang will cause compile of node modules to fail with the following error:

```
gmake: Entering directory '/home/randy/Develop/tkg/node_modules/sharp/build'
  TOUCH Release/obj.target/libvips-cpp.stamp
  CXX(target) Release/obj.target/sharp/src/common.o
In file included from ../src/common.cc:15:
In file included from /usr/include/c++/v1/cstdlib:85:
/usr/include/c++/v1/__config:73:2: error: "_LIBCPP_TRIVIAL_PAIR_COPY_CTOR" is no longer supported.        use _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR instead
#error "_LIBCPP_TRIVIAL_PAIR_COPY_CTOR" is no longer supported. \
 ^
1 error generated.
gmake: *** [sharp.target.mk:151: Release/obj.target/sharp/src/common.o] Error 1
gmake: Leaving directory '/home/randy/Develop/tkg/node_modules/sharp/build'
gyp ERR! build error 
gyp ERR! stack Error: `gmake` failed with exit code: 2

```

The patch in files/patch-common.gypi needs to be removed at least for newer versions of FreeBSD.
Comment 1 Bradley T. Hughes freebsd_committer freebsd_triage 2017-08-02 14:06:40 UTC
Hi Randy,

The patch in www/node actually removes the unconditional _LIBCPP_TRIVIAL_PAIR_COPY_CTOR from common.gypi. This is to fix the build on newer versions of clang (such as 3.8.1 in 11.0-R and newer). However, this causes a build error on clang 3.4.1 (10.3-R), which is why www/node/Makefile will re-add the define when using clang < 3.8.

In other words, as far as I can see, the port does already does exactly what you are asking: remove the define for newer clang/FreeBSD versions, keep it for older ones.

Does that sound right to you?
Comment 2 Randy 2017-08-02 15:53:07 UTC
Bradley,

Thanks for looking at this.

This may just be a matter of node install and ~/.node-gyp getting out of sync. node-gyp seems to persist a local copy of common.gypi. I may also be confused in that I thought the patch files in ports were applied as reverse and read this patch as adding this define for all platforms. 

I will comment that the change might be more portable through compiler changes if we somehow detected compiler version used in npm installs in this common.gypi. If trying to coerce other node modules to build through changing of compiler used, this may not work.

But I agree that current state seems to be the most complete for current versions.

Thanks again for addressing this.
Comment 3 anton whalley 2017-09-28 08:45:31 UTC
Hi Bradley 
I've looked into this a little more the problem seems to be that the node.js binary is built with llvm_3.8 or at least it thinks it's build with 3.8 and the default on freebsd 11.1 is 4.0.

For the configuration of a native addon process.config is called
https://github.com/nodejs/node-gyp/blob/master/lib/configure.js#L116 

This config is then used to create build/config.gypi during the configuration for an addon. 

This is why builds are failing even though this patch has landed correctly.

process.config seems to be built into the binary verified by running
% strings /usr/local/bin/node | grep llvm_version
'llvm_version': '3.8'

I am not clear on how pkg provides it's prebuilt binaries but hopefully we can work this out. 

Anton
Comment 4 Bradley T. Hughes freebsd_committer freebsd_triage 2017-10-05 21:45:23 UTC
Hi Anton,

I hadn't realized that the common.gypi was actually compiled into the Node.js binary... this complicates things a bit. :/

I was considering a postinstall script that could maybe somehow overwrite the version detected at compile time. This is pretty easy to do from copying the relevant bits from the Node.js configure script:

---
#!/usr/bin/env python2

import os
import re
import shlex
import subprocess
import sys

def get_version_helper(cc, regexp):
  try:
    proc = subprocess.Popen(shlex.split(cc) + ['-v'], stdin=subprocess.PIPE,
                            stderr=subprocess.PIPE, stdout=subprocess.PIPE)
  except OSError:
    print('''Node.js configure error: No acceptable C compiler found!
        Please make sure you have a C compiler installed on your system and/or
        consider adjusting the CC environment variable if you installed
        it in a non-standard prefix.
        ''')
    sys.exit()

  match = re.search(regexp, proc.communicate()[1])

  if match:
    return match.group(2)
  else:
    return 0

def get_llvm_version(cc):
  return get_version_helper(
    cc, r"(^(?:FreeBSD )?clang version|based on LLVM) ([3-9]\.[0-9]+)")

CC = os.environ.get('CC', 'cc' if sys.platform == 'darwin' else 'gcc')
llvm_version = get_llvm_version(CC)
print("llvm_version=%s" % llvm_version)
---

The problem is... what do we do with it? Patching the binary doesn't seem right or feasible. Does this need to be patched/fixed in npm? node-gyp? I am really unsure. Part of me wonders if this could/should be solved by patching the node.h include file instead, and have it check for __FreeBSD, the __clang version, and the libc++ version so that it can #define _LIBCPP_TRIVIAL_PAIR_COPY_CTOR as needed there. This allows addons to be built with whichever compiler the user wants, and not just the compiler used to compiled Node.js.
Comment 5 Bradley T. Hughes freebsd_committer freebsd_triage 2017-10-21 11:46:09 UTC
I'm re-opening this, as it is a real issue.
Comment 6 Tsvetomir Tsonev 2017-12-11 09:13:01 UTC
Just wanted to highlight that the issue is blocking installation of popular packages such as node-sass and leveldown on stock FreeBSD 11.1 with binary packages.

With 11.0 now in EOL this is bound to turn into a real headache. 

Possible workarounds are to build node from ports or use "npm install xxx --llvm-version=4.0".
Comment 7 Max Khon freebsd_committer freebsd_triage 2018-01-26 16:46:47 UTC
See #222153 for the fix
Comment 8 Max Khon freebsd_committer freebsd_triage 2018-01-26 16:47:45 UTC
Bug 222153 contains the patch that fixes this
Comment 9 Bradley T. Hughes freebsd_committer freebsd_triage 2019-01-23 22:05:35 UTC
I am closing this, since the port no longer patches common.gypi, and essentially the issue has aged out.
Comment 10 andrea 2021-03-08 16:32:52 UTC
I just stumbled in the same problem with FreeBSD 12.2, CLang 10 and Node 10.23.3

I have a ReactJS application that includes node_modules/leveldown and depends on node-gyp-build

When compiling I was getting:

> leveldown@5.6.0 install /repository/StayWarehouse-React/node_modules/leveldown
> node-gyp-build

gmake: Entering directory '/repository/StayWarehouse-React/node_modules/leveldown/build'
  CXX(target) Release/obj.target/leveldb/deps/leveldb/leveldb-1.20/db/builder.o
In file included from ../deps/leveldb/leveldb-1.20/db/builder.cc:5:
In file included from ../deps/leveldb/leveldb-1.20/db/builder.h:8:
In file included from ../deps/leveldb/leveldb-1.20/include/leveldb/status.h:16:
In file included from /usr/include/c++/v1/string:503:
/usr/include/c++/v1/__config:122:2: error: "_LIBCPP_TRIVIAL_PAIR_COPY_CTOR" is no longer supported. use
      _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR instead
#error "_LIBCPP_TRIVIAL_PAIR_COPY_CTOR" is no longer supported. \
 ^
1 error generated.
gmake: *** [deps/leveldb/leveldb.target.mk:156: Release/obj.target/leveldb/deps/leveldb/leveldb-1.20/db/builder.o] Error 1
gmake: Leaving directory '/repository/StayWarehouse-React/node_modules/leveldown/build'
gyp ERR! build error
gyp ERR! stack Error: `gmake` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/repository/StayWarehouse-React/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:198:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:248:12)
gyp ERR! System FreeBSD 12.2-RELEASE
gyp ERR! command "/usr/local/bin/node" "/repository/StayWarehouse-React/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /repository/StayWarehouse-React/node_modules/leveldown
gyp ERR! node -v v10.23.3
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok


Getting in this thread I found out I had two different common.gypi:

diff ./usr/local/include/node/common.gypi ./root/.node-gyp/10.23.3/include/node/common.gypi
514a515,523
>         'conditions': [
>           ['"0" < llvm_version < "4.0"', {
>             # Use this flag because on FreeBSD std::pairs copy constructor is non-trivial.
>             # Doesn't apply to llvm 4.0 (FreeBSD 11.1) or later.
>             # Refs: https://lists.freebsd.org/pipermail/freebsd-toolchain/2016-March/002094.html
>             # Refs: https://svnweb.freebsd.org/ports/head/www/node/Makefile?revision=444555&view=markup
>             'cflags': [ '-D_LIBCPP_TRIVIAL_PAIR_COPY_CTOR=1' ],
>           }],
>         ],

I overwrote the second one with the first one to get rid of the offending condition and was able to compile node-gyp with no problems.

The VERY SAME application compiled for months without an issue on FreeBSD 12.1 (clang 8.0.1) and Node 10.22.1.
Comment 11 andrea 2021-03-08 16:34:39 UTC
(I guess the string comparison is getting confused by 10 coming before 4..)