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 created this revision.Jun 2 2017, 7:33 PM
bhughes set the repository for this revision to rP FreeBSD ports repository.
bhughes updated this revision to Diff 29159.Jun 2 2017, 7:36 PM

Add template to the commit message

bhughes edited the summary of this revision. (Show Details)Jun 2 2017, 7:37 PM
bhughes set the repository for this revision to rP FreeBSD ports repository.
robak edited edge metadata.EditedJun 2 2017, 8:44 PM

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/'

robak added inline comments.Jun 2 2017, 8:50 PM
www/node/Makefile
10 ↗(On Diff #29159)

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

robak added a comment.Jun 6 2017, 11:18 AM

Any updates?

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)Jun 27 2017, 5:17 AM
bhughes updated this revision to Diff 30123.

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.

robak added a comment.Jun 27 2017, 7:21 AM

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

mat accepted this revision.Jun 27 2017, 7:50 AM

Looks fine by me.

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.Jun 28 2017, 8:48 AM
bhughes updated this revision to Diff 30164.

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
bhughes edited the summary of this revision. (Show Details)Jun 28 2017, 8:50 AM
robak accepted this revision.Jun 28 2017, 8:52 AM

LGTM.

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.