Page MenuHomeFreeBSD

[PowerPC64] fix www/node build and runtime when using ELFv2 ABI
AcceptedPublic

Authored by alfredo.junior_eldorado.org.br on Sep 10 2019, 4:04 PM.

Details

Summary

This patch allows www/node to build and run on powerpc64. It fixes incorrect assumptions between ELFv1/ELFv2 and LE/BE logic.

Test Plan

Commands on server/node console:

root@alfredo-2:~/src/freebsd-ports/www/node # node
Welcome to Node.js v12.11.1.
Type ".help" for more information.
> var http = require('http');
undefined
> http.createServer(function (req, res) { res.writeHead(200, {'Content-Type': 'text/plain'});  res.end('Hello Node.js World!'); }).listen(8080);
Server {
  _events: [Object: null prototype] {
    request: [Function],
    connection: [Function: connectionListener]
  },
  _eventsCount: 2,
  _maxListeners: undefined,
  _connections: 0,
  _handle: TCP {
    reading: false,
    onconnection: [Function: onconnection],
    [Symbol(owner)]: [Circular]
  },
  _usingWorkers: false,
  _workers: [],
  _unref: false,
  allowHalfOpen: true,
  pauseOnConnect: false,
  httpAllowHalfOpen: false,
  timeout: 120000,
  keepAliveTimeout: 5000,
  maxHeadersCount: null,
  headersTimeout: 40000,
  _connectionKey: '6::::8080',
  [Symbol(IncomingMessage)]: [Function: IncomingMessage],
  [Symbol(ServerResponse)]: [Function: ServerResponse],
  [Symbol(asyncId)]: 209
}
>

From client:

# curl http://localhost:8080 -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.66.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: text/plain
< Date: Mon, 21 Oct 2019 21:43:27 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
* Connection #0 to host localhost left intact
Hello Node.js World!

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27342
Build 25593: arc lint + arc unit

Event Timeline

pkubaj requested changes to this revision.Sep 21 2019, 3:00 PM

It would be nice if you could apply files/patch-common.gypi only for elfv2, to have node available for elfv1.

This revision now requires changes to proceed.Sep 21 2019, 3:00 PM
pkubaj updated this revision to Diff 62397.Sep 21 2019, 7:03 PM
pkubaj edited the summary of this revision. (Show Details)

Remove -mminimal-toc only on 13.

To be committed after the switch to clang.

pkubaj accepted this revision.Sep 21 2019, 7:04 PM
This revision is now accepted and ready to land.Sep 21 2019, 7:04 PM

node isn't building with llvm90 anymore due to https://bugs.llvm.org/show_bug.cgi?id=43527, so patch is updated to use llvm-devel instead.

Note: this requires PPC_ABI variable with value set to "ELFv2" in order to build on powerpc64/elfv2

This revision now requires review to proceed.Oct 9 2019, 1:16 PM
pkubaj added inline comments.Oct 16 2019, 10:54 AM
www/node/files/extra-patch-common.gypi
9

cflags should have -mabi=elfv2 appended. Lack of it killed the build for me.

10

Same as above.

pkubaj added inline comments.Oct 16 2019, 12:23 PM
www/node/Makefile
79

extra-patch-common.gypi is valid only for elfv2 and elfv2 will be only 13 at the moment.

This means that we can drop EXTRA_PATCHES_13 and do directly:
EXTRA_PATCHES=${PATCHDIR}/extra-patch-common.gypi

81–84

Those are not necessary. With changes in this review, this port will build on ppc64 elfv2.

pkubaj requested changes to this revision.Oct 16 2019, 6:14 PM
This revision now requires changes to proceed.Oct 16 2019, 6:14 PM
alfredo.junior_eldorado.org.br marked an inline comment as done.Oct 17 2019, 11:37 PM
alfredo.junior_eldorado.org.br added inline comments.
www/node/Makefile
81–84

Let me double check this since this dependency was added due to a bug on llvm9 and the fix candidate is still under review.

www/node/files/extra-patch-common.gypi
9

-mabi=elfv2 should not be necessary since it's the default ABI in this contex

pkubaj added inline comments.Oct 18 2019, 7:24 AM
www/node/files/extra-patch-common.gypi
9

Without this, I got linking errors. Do you build the current version of www/node? I built www/node 12.10.0 before without adding ABI version, but 12.11.1 doesn't build without specifying ABI.

alfredo.junior_eldorado.org.br edited the summary of this revision. (Show Details)
alfredo.junior_eldorado.org.br edited the test plan for this revision. (Show Details)

Updated patches after tests with llvm90 as base compiler.

alfredo.junior_eldorado.org.br marked 4 inline comments as done.Oct 21 2019, 7:02 PM
alfredo.junior_eldorado.org.br added inline comments.
www/node/Makefile
81–84

it is still required, or I hit PPC CRT loop issue.

www/node/files/extra-patch-common.gypi
9

I couldn't reproduce it here being both base llvm and ports llvm-devel patched for ELFv2-default.
Apparently the compiler your're using isn't generating ELFv2 by default.

pkubaj accepted this revision.Oct 23 2019, 1:32 PM

Needs to be updated for 12.12.0:

1 out of 1 hunks failed--saving rejects to tools/v8_gypfiles/v8.gyp.rej
=> FreeBSD patch patch-tools_v8__gypfiles_v8.gyp failed to apply cleanly.
=> Patch(es)  patch-deps_openssl_config_archs_linux-elf_no-asm_openssl-cl.gypi patch-deps_openssl_config_archs_linux-elf_no-asm_openssl.gypi patch-deps_openssl_openssl-cl__no__asm.gypi patch-deps_openssl_openssl__no__asm.gypi patch-deps_v8_src_base_platform_platform-freebsd.cc patch-deps_v8_src_codegen_arm_cpu-arm.cc patch-deps_v8_src_codegen_ppc_constants-ppc.h patch-deps_v8_src_execution_simulator.h patch-deps_v8_src_libsampler_sampler.cc patch-node.gypi applied cleanly.

Other than that, fine with me.

This revision is now accepted and ready to land.Oct 23 2019, 1:32 PM

Node compiles just fine after removing files/patch-tools_v8__gypfiles_v8.gyp. Looks like it's now not necessary.

pkubaj requested changes to this revision.Tue, Oct 29, 12:25 PM
This revision now requires changes to proceed.Tue, Oct 29, 12:25 PM
alfredo.junior_eldorado.org.br marked an inline comment as done.
alfredo.junior_eldorado.org.br edited the summary of this revision. (Show Details)

updated patch for node-13.0

llvm-devel dependency removed since https://bugs.llvm.org/show_bug.cgi?id=43527 should be fixed in FreeBSD base by commit 354339.

pkubaj accepted this revision.Tue, Nov 5, 2:16 PM

OK for me.

This revision is now accepted and ready to land.Tue, Nov 5, 2:16 PM
www/node/Makefile
80

extra blank lines