Page MenuHomeFreeBSD

www/node: Update 7.10.0 -> 8.1.2
ClosedPublic

Authored by bhughes on Jun 2 2017, 7:33 PM.

Details

Summary

www/node: Update 7.10.0 -> 8.1.2

Bump to the latest major version of Node.js.

Remove files/patch-node.gyp, since this patch has been accepted
upstream. Remove
files/patch-deps_v8_src_base_atomicopsinternalsarm__gcc.h,
which patches a now non-existent file after a change in how V8
implements atomic operations. Conditionally add the
_LIBCPP_TRIVIAL_PAIR_COPY_CTOR define when using clang 3.x to
workaround a known problem in libc++.

Approved by: robak (mentor)

Test Plan

$ pwd
/usr/ports/www/node
$ portlint -C
WARN: Makefile: [0]: possible direct use of command "python" found. use ${PYTHON_CMD} instead.
WARN: Makefile: possible use of absolute pathname "/etc/make.conf".
0 fatal errors and 2 warnings found.

http://35.176.84.170/data/latest-per-pkg/node/8.1.2/

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bhughes set the repository for this revision to rP FreeBSD ports repository.

Add template to the commit message

bhughes set the repository for this revision to rP FreeBSD ports repository.

Can you provide links to your Poudriere build logs? Do so with every patch that requires build tests.
Regarding the commit message:

  • use 'www/node: update prev_version -> new_version' title format
  • skip parts like these, they're more justified for pkg message than for a commit message regarding port:

    'Bump to the latest major version of Node.js. This is a major release,

including many significant changes. Users are encouraged to read the
release announcement before upgrading:

https://nodejs.org/en/blog/release/v8.0.0/'

www/node/Makefile
10 ↗(On Diff #29159)

That should be either corrected to 8.x or simply removed (the version in COMMENT).

Can you provide links to your Poudriere build logs? Do so with every patch that requires build tests.

I will try. How persistent does the link need to be? I will host the log from my builder in EC2, which changes IP addresses fairly regularly (and doesn't have a permanent hostname).

Regarding the commit message:

  • use 'www/node: update prev_version -> new_version' title format

Will do.

  • skip parts like these, they're more justified for pkg message than for a commit message regarding port:

    'Bump to the latest major version of Node.js. This is a major release,

including many significant changes. Users are encouraged to read the
release announcement before upgrading:

https://nodejs.org/en/blog/release/v8.0.0/'

I was thinking about including an UPDATING entry, or do you think a pkg-message is better?

bhughes edited the summary of this revision. (Show Details)

Bump to latest upstream release, and add an UPDATING entry.

bhughes retitled this revision from www/node: Update to 8.0.0 to www/node: Update 7.10.0 -> 8.1.2.Jun 27 2017, 5:19 AM
bhughes edited the summary of this revision. (Show Details)
bhughes edited the test plan for this revision. (Show Details)
bhughes added a reviewer: mat.

As you can see from my poudriere logs, this does not build on 11.1-BETA3 or 12.0-CURRENT with clang 4.0. Removing the _LIBCPP_TRIVIAL_PAIR_COPY_CTOR=1 define fixes the build with clang 4.0, but then breaks the build on 10.3-R and 11.0-R using clang 3.x. I am trying to see if I can find a way to get it to build everywhere before considering adding a dependency on devel/clang40 on 10.3-R and 11.0-R.

Can you try with gcc to check if the problem is with clang or somewhere else?

This revision is now accepted and ready to land.Jun 27 2017, 7:50 AM

It does appear to be a clang problem:

https://bugs.llvm.org/show_bug.cgi?id=18249
https://bugs.llvm.org/show_bug.cgi?id=18350
https://bugs.llvm.org/show_bug.cgi?id=18853

I am currently testing a Makefile change to add the _LIBCPP_TRIVIAL_PAIR_COPY_CTOR conditionally if we are using clang < 4.0. I will update the patch if it works :)

bhughes edited edge metadata.

Updated patch, this now builds with the system compiler on all jails,
see http://35.176.84.170/data/latest-per-pkg/node/8.1.2/

This revision now requires review to proceed.Jun 28 2017, 8:48 AM
This revision is now accepted and ready to land.Jun 28 2017, 8:52 AM
This revision was automatically updated to reflect the committed changes.