Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: better document --shared build #14158

Closed
refack opened this issue Jul 10, 2017 · 19 comments
Closed

doc: better document --shared build #14158

refack opened this issue Jul 10, 2017 · 19 comments
Assignees
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. test Issues and PRs related to the tests.

Comments

@refack
Copy link
Contributor

refack commented Jul 10, 2017

  • Version: *
  • Platform: *
  • Subsystem: build,doc

./configure --shared / vcbuild shared should be better documented.

  1. How to build
  2. How to use
  3. Support level
@refack refack added build Issues and PRs related to build files or the CI. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. labels Jul 10, 2017
@refack refack added the doc Issues and PRs related to the documentations. label Jul 10, 2017
@addaleax addaleax added the test Issues and PRs related to the tests. label Jul 10, 2017
@addaleax
Copy link
Member

If I may make the suggestion, we should also test it in some way if possible.

@refack
Copy link
Contributor Author

refack commented Jul 10, 2017

If I may make the suggestion, we should also test it in some way if possible.

If we decide to support, maybe it should be deprecated?

[edit]
I'm just playing devil's advocate, it think it's an important meta-feature. But without CI testing it has rotted...
@electron @nwjs @nodejs/electron-installer @nodejs/build

@gibfahn
Copy link
Member

gibfahn commented Jul 11, 2017

cc/ @mhdawson, @a-roberts (and maybe @sam-github) who were looking at this.

@mhdawson
Copy link
Member

I've had it on our internal backlog to push this forward, we've just not gotten started on it yet. My starting goals are:

  1. define set of tests to validate shared library works
  2. add tests to ci, ideally in the regression runs
  3. look at how we can better package/delivery, ideally as part of the binaries we already ship. Challenge on this front is not increasing the package size so we may need to look at moving to the node binary being a launcher that uses the shared library, this is what Java does.

@mhdawson
Copy link
Member

And I'm firmly on the side that we need to improve not remove. It is used by electron and we also have internal products that need it.

@mhdawson
Copy link
Member

The person(s) on our team who worked on this in the past have moved on to a different project. My next step is to schedule some handover with them but then it will still be a matter on finding time to move it forward.

@refack
Copy link
Contributor Author

refack commented Jul 13, 2017

add tests to ci, ideally in the regression runs

#13078 tried to get cctest to link against the shared lib. Could ideally become said launcher so we can run the JS suite against it.

@refack
Copy link
Contributor Author

refack commented Jul 13, 2017

#13078 tried to get cctest to link against the shared lib. Could ideally become said launcher so we can run the JS suite against it.

Alternatively create something like d8 that will both serve as a test harness and an embedding example.

@mhdawson
Copy link
Member

mhdawson commented Sep 6, 2017

@gibfahn can you post the launcher code that we already have for testing the shared library ? I've not had time to get back to this and it would at least be something we can discuss here in terms of moving forward on the testing.

@mhdawson
Copy link
Member

mhdawson commented Sep 6, 2017

Seein #15195 maybe we should add a job that simply compiles the shared library as a first step towards fully testing it.

@refack
Copy link
Contributor Author

refack commented Sep 12, 2017

IMHO a job that just compiles for linux+Windows+macOS would be a great start

@mhdawson
Copy link
Member

It should compile and build across all platforms so I don't think there is any reason to limit the target platforms.

@yhwang
Copy link
Member

yhwang commented Sep 19, 2017

start to trace the current --shared and --enable-static options in configure and agreed with @refack about this:

IMHO a job that just compiles for linux+Windows+macOS would be a great start

and maybe both --shared and --enable-static need to be verified

@yhwang
Copy link
Member

yhwang commented Sep 27, 2017

after studying the static/shared lib build process, my thinking is maybe we should build the static lib and use it to build the node executable. reason being are:

  • directly integrate the static lib build with node executable
  • node executable also use static lib and run test against node executable is also some sort of verification of static lib
  • no performance impact of the node executable when using static lib. because there is still some performance impact when using shared lib

any thoughts?

yhwang added a commit to yhwang/node that referenced this issue Mar 2, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: nodejs#14158
PR-URL: nodejs#17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 21, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: #14158
PR-URL: #17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 21, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: #14158
Backport-PR-URL: #19050
PR-URL: #17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 28, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: #14158
Backport-PR-URL: #19050
PR-URL: #17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 28, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: #14158
PR-URL: #17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 30, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: #14158
Backport-PR-URL: #19050
PR-URL: #17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 30, 2018
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: #14158
PR-URL: #17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@Trott
Copy link
Member

Trott commented Feb 17, 2019

What remains to be done here?

@jasnell
Copy link
Member

jasnell commented Jun 26, 2020

This has not been updated in over a year and it's not clear what remaining actions there are. Closing but it can be reopened and I'm putting it on the Futures project board so it does not get lost.

@mhdawson
Copy link
Member

mhdawson commented Jul 2, 2020

@jasnell I think you failed to close so I'll do that. I still think we need a better defined API/documentation but I've not been able to line up people to work on it so I'll close for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants